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

Changes from 1 commit
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
38 changes: 18 additions & 20 deletions src/remux/mp4-remuxer.js
View file
Open in desktop
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 @@ -250,10 +254,9 @@ class MP4Remuxer {
}
}

// 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;
Expand All @@ -270,17 +273,12 @@ class MP4Remuxer {
firstDTS = nextAvcDts;
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`);
minPTS = Math.max(minPTS - delta, nextAvcDts);
inputSamples[0].pts = minPTS;
robwalch marked this conversation as resolved.
Show resolved Hide resolved
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 +355,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 +412,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