Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix dropped frames caused by AVC min/max PTS not matching first/last PTS #2876

Merged
merged 6 commits into from Jul 13, 2020
Merged

Fix dropped frames caused by AVC min/max PTS not matching first/last PTS #2876

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7efc406
Fix dropped frames caused by AVC min/max PTS not matching first/last PTS
Jul 9, 2020
8708ba1
Do not drop audio frames when switching qualities since the overlap d…
Jul 10, 2020
177034f
Do not chang starting DTS when PTS < DTS is found
Jul 10, 2020
05ce6a7
Reset demuxer when backtracking
Jul 11, 2020
052feab
Clamp DTS after timecode hole/overlap check and Safari frame duration…
Jul 13, 2020
b1d0964
Adjust timecode hole tolerance to account for 59.94 and 29.97 framera…
Jul 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/controller/stream-controller.js
View file
Open in desktop
Expand Up @@ -1029,6 +1029,10 @@ class StreamController extends BaseStreamController {
this.nextLoadPosition = data.startPTS;
this.state = State.IDLE;
this.fragPrevious = frag;
if (this.demuxer) {
this.demuxer.destroy();
this.demuxer = null;
}
this.tick();
return;
}
Expand Down
79 changes: 42 additions & 37 deletions src/remux/mp4-remuxer.js
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@ import MP4 from './mp4-generator';
import Event from '../events';
import { ErrorTypes, ErrorDetails } from '../errors';

import { toMsFromMpegTsClock, toMpegTsClockFromTimescale, toTimescaleFromScale } from '../utils/timescale-conversion';
import { toMsFromMpegTsClock, toMpegTsClockFromTimescale } from '../utils/timescale-conversion';

import { logger } from '../utils/logger';

Expand Down Expand Up @@ -144,7 +144,7 @@ class MP4Remuxer {
};
if (computePTSDTS) {
// remember first PTS of this demuxing context. for audio, PTS = DTS
initPTS = initDTS = audioSamples[0].pts - audioTrack.inputTimeScale * timeOffset;
initPTS = initDTS = audioSamples[0].pts - Math.round(audioTrack.inputTimeScale * timeOffset);
}
}

Expand All @@ -163,8 +163,9 @@ class MP4Remuxer {
}
};
if (computePTSDTS) {
initPTS = Math.min(initPTS, videoSamples[0].pts - inputTimeScale * timeOffset);
initDTS = Math.min(initDTS, videoSamples[0].dts - inputTimeScale * timeOffset);
const startPTS = Math.round(inputTimeScale * timeOffset);
initPTS = Math.min(initPTS, videoSamples[0].pts - startPTS);
initDTS = Math.min(initDTS, videoSamples[0].dts - startPTS);
this.observer.trigger(Event.INIT_PTS_FOUND, { initPTS });
}
} else if (computePTSDTS && tracks.audio) {
Expand All @@ -189,10 +190,10 @@ class MP4Remuxer {
let mp4SampleDuration;
let mdat;
let moof;
let firstPTS;
let firstDTS;
let lastPTS;
let lastDTS;
let minPTS = Number.MAX_SAFE_INTEGER;
let maxPTS = -Number.MAX_SAFE_INTEGER;
itsjamie marked this conversation as resolved.
Show resolved Hide resolved
const timeScale = track.timescale;
const inputSamples = track.samples;
const outputSamples = [];
Expand Down Expand Up @@ -232,6 +233,9 @@ class MP4Remuxer {
inputSamples.forEach(function (sample) {
sample.pts = ptsNormalize(sample.pts - initPTS, nextAvcDts);
sample.dts = ptsNormalize(sample.dts - initPTS, nextAvcDts);

minPTS = Math.min(sample.pts, minPTS);
maxPTS = Math.max(sample.pts, maxPTS);
robwalch marked this conversation as resolved.
Show resolved Hide resolved
});

// sort video samples by DTS then PTS then demux id order
Expand All @@ -246,41 +250,34 @@ class MP4Remuxer {
if (PTSDTSshift < 0) {
logger.warn(`PTS < DTS detected in video samples, shifting DTS by ${toMsFromMpegTsClock(PTSDTSshift, true)} ms to overcome this issue`);
for (let i = 0; i < inputSamples.length; i++) {
inputSamples[i].dts += PTSDTSshift;
inputSamples[i].dts = Math.max(0, inputSamples[i].dts + PTSDTSshift);
}
}

// compute first DTS and last DTS, normalize them against reference value
let sample = inputSamples[0];
firstDTS = Math.max(sample.dts, 0);
firstPTS = Math.max(sample.pts, 0);
Copy link
Collaborator Author

@robwalch robwalch Jul 9, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clamping to zero can cause timecode to be modified in the following block (delta = firstDTS - nextAvcDts...if (delta)) which can then prevent compositionTimeOffset from being calculated properly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the stream should come in DTS order.

I forget the exact look of them, but I know the whole thing with audio priming can lead to negative PTS times to get audio to not have any pops at the start of streams, something that popped into my head as something to raise here, where previously those would have been clamped to start at 0, thinking if there is any behaviour later that might change?

Copy link
Collaborator Author

@robwalch robwalch Jul 10, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I see if that ptsNormalize is subtracting initPTS from these timecodes. So any stream with CTS > 0 on the first frame will produce negative DTS values that get clamped here. Do we want to set those DTS values to 0 and as a result change the CTS 0 on those frames?

The change below is that const delta = firstDTS - nextAvcDts will report a delta where it might not have. Moving the DTS then causes this "fix" to kick in, which then also moves PTS of the first frame, which then causes the duration of the segment to be modified, which then changes the entire timeline.

We want to stop doing that because it results in bugs like #2873 which this fixes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

audio priming is a concern as it can produce gaps at the start of streams. But that's happening regardless of these changes. I'll look at more streams on the JW platform as we've had problems with audio priming shifting PTS in some levels with different encoding profiles than others (where some mezzanine levels have different mp4 edts than others).

// Get first/last DTS
firstDTS = inputSamples[0].dts;
lastDTS = inputSamples[inputSamples.length - 1].dts;

// check timestamp continuity accross consecutive fragments (this is to remove inter-fragment gap/hole)
let delta = firstDTS - nextAvcDts;
// check timestamp continuity across consecutive fragments (this is to remove inter-fragment gap/hole)
const delta = firstDTS - nextAvcDts;
// if fragment are contiguous, detect hole/overlapping between fragments
if (contiguous) {
if (delta) {
if (delta > 1) {
logger.log(`AVC: ${toMsFromMpegTsClock(delta, true)} ms hole between fragments detected,filling it`);
} else if (delta < -1) {
logger.log(`AVC: ${toMsFromMpegTsClock(-delta, true)} ms overlapping between fragments detected`);
const foundHole = delta >= 1;
const foundOverlap = delta < -1;
if (foundHole || foundOverlap) {
if (foundHole) {
logger.warn(`AVC: ${toMsFromMpegTsClock(delta, true)} ms hole between fragments detected, filling it`);
} else {
logger.warn(`AVC: ${toMsFromMpegTsClock(-delta, true)} ms overlapping between fragments detected`);
}

// remove hole/gap : set DTS to next expected DTS
firstDTS = nextAvcDts;
minPTS -= delta;
inputSamples[0].dts = firstDTS;
// offset PTS as well, ensure that PTS is smaller or equal than new DTS
firstPTS = Math.max(firstPTS - delta, nextAvcDts);
inputSamples[0].pts = firstPTS;
logger.log(`Video: PTS/DTS adjusted: ${toMsFromMpegTsClock(firstPTS, true)}/${toMsFromMpegTsClock(firstDTS, true)}, delta: ${toMsFromMpegTsClock(delta, true)} ms`);
inputSamples[0].pts = minPTS;
logger.log(`Video: PTS/DTS adjusted: ${toMsFromMpegTsClock(minPTS, true)}/${toMsFromMpegTsClock(firstDTS, true)}, delta: ${toMsFromMpegTsClock(delta, true)} ms`);
}
}

// compute lastPTS/lastDTS
sample = inputSamples[inputSamples.length - 1];
lastDTS = Math.max(sample.dts, 0);
lastPTS = Math.max(sample.pts, 0, lastDTS);

// on Safari let's signal the same sample duration for all samples
// sample duration (as expected by trun MP4 boxes), should be the delta between sample DTS
// set this constant duration as being the avg delta between consecutive DTS.
Expand Down Expand Up @@ -357,7 +354,7 @@ class MP4Remuxer {
// the duration of the last frame to minimize any potential gap between segments.
let maxBufferHole = config.maxBufferHole,
gapTolerance = Math.floor(maxBufferHole * timeScale),
deltaToFrameEnd = (audioTrackLength ? firstPTS + audioTrackLength * timeScale : this.nextAudioPts) - avcSample.pts;
deltaToFrameEnd = (audioTrackLength ? minPTS + audioTrackLength * timeScale : this.nextAudioPts) - avcSample.pts;
if (deltaToFrameEnd > gapTolerance) {
// We subtract lastFrameDuration from deltaToFrameEnd to try to prevent any video
// frame overlap. maxBufferHole should be >> lastFrameDuration anyway.
Expand Down Expand Up @@ -414,8 +411,8 @@ class MP4Remuxer {
let data = {
data1: moof,
data2: mdat,
startPTS: firstPTS / timeScale,
endPTS: (lastPTS + mp4SampleDuration) / timeScale,
startPTS: minPTS / timeScale,
endPTS: (maxPTS + mp4SampleDuration) / timeScale,
startDTS: firstDTS / timeScale,
endDTS: this.nextAvcDts / timeScale,
type: 'video',
Expand Down Expand Up @@ -505,9 +502,17 @@ class MP4Remuxer {

// If we're overlapping by more than a duration, drop this sample
if (delta <= -maxAudioFramesDrift * inputSampleDuration) {
logger.warn(`Dropping 1 audio frame @ ${toMsFromMpegTsClock(nextPts, true)} ms due to ${toMsFromMpegTsClock(delta, true)} ms overlap.`);
inputSamples.splice(i, 1);
// Don't touch nextPtsNorm or i
if (contiguous) {
logger.warn(`Dropping 1 audio frame @ ${toMsFromMpegTsClock(nextPts, true) / 1000}s due to ${toMsFromMpegTsClock(delta, true)} ms overlap.`);
inputSamples.splice(i, 1);
// Don't touch nextPtsNorm or i
} else {
// When changing qualities we can't trust that audio has been appended up to nextAudioPts
// Warn about the overlap but do not drop samples as that can introduce buffer gaps
logger.warn(`Audio frame @ ${toMsFromMpegTsClock(pts, true) / 1000}s overlaps nextAudioPts by ${toMsFromMpegTsClock(delta, true)} ms.`);
nextPts = pts + inputSampleDuration;
i++;
}
} // eslint-disable-line brace-style

// Insert missing frames if:
Expand All @@ -516,7 +521,7 @@ class MP4Remuxer {
// 3: currentTime (aka nextPtsNorm) is not 0
else if (delta >= maxAudioFramesDrift * inputSampleDuration && delta < MAX_SILENT_FRAME_DURATION_90KHZ && nextPts) {
let missing = Math.round(delta / inputSampleDuration);
logger.warn(`Injecting ${missing} audio frames @ ${toMsFromMpegTsClock(nextPts, true)} ms due to ${toMsFromMpegTsClock(delta, true)} ms gap.`);
logger.warn(`Injecting ${missing} audio frames @ ${toMsFromMpegTsClock(nextPts, true) / 1000}s due to ${toMsFromMpegTsClock(delta, true)} ms gap.`);
for (let j = 0; j < missing; j++) {
let newStamp = Math.max(nextPts, 0);
fillFrame = AAC.getSilentFrame(track.manifestCodec || track.codec, track.channelCount);
Expand Down Expand Up @@ -560,7 +565,7 @@ class MP4Remuxer {
// logger.log(`Audio/PTS:${toMsFromMpegTsClock(pts, true)}`);
// if not first sample

if (lastPTS !== undefined) {
if (lastPTS !== undefined && mp4Sample) {
mp4Sample.duration = Math.round((pts - lastPTS) / scaleFactor);
} else {
let delta = pts - nextAudioPts;
Expand Down