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

robwalch · 2020-07-09T03:19:51Z

This PR will...

Fix segment duration calculation when first and last PTS do not match min and max (fixes HLS.js dropping frames after discontinuities #2873)
Fix rounding errors in initPTS calculation:
Actual 😞 90000 * 16.4 === 1475999.9999999998
Expected 90000 * 16.4 === 1476000
Limit how frequently the position of video frames are adjusted based on estimated next DTS, maintaining composition time when possible
1. Clamp DTS to 0 after timecode hole/overlap check and Safari frame duration calculation (prevents incorrect nextDTS delta adjustment from being applied)
2. Adjust timecode hole tolerance to account for 59.94 and 29.97 frame rate variance (prevents adjustment from being applied with 0-2 dts sample duration variance)
When backtracking because of dropped video frames, reset the demuxer so that we don't use next DTS based on dropped frames
Do not drop audio frames based on overlap when samples are not from the same level or a discontinuity. The estimated next PTS is often wrong and causes gaps by dropping audio that should be appended

Why is this Pull Request needed?

Incorrect segment duration calculation causes frame drops at discontinuities, and other playlist alignment and buffer errors. Also prevent some of the sample timecode hacks from doing more harm than good.

Resolves issues:

#2873

Checklist

changes have been done against master branch, and PR does not conflict
new unit / functional tests have been added (whenever applicable)

#2873

robwalch · 2020-07-09T03:29:19Z

src/remux/mp4-remuxer.js

-    // 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);


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.

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?

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.

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).

src/remux/mp4-remuxer.js

itsjamie · 2020-07-10T03:16:57Z

src/remux/mp4-remuxer.js

-        logger.log(`Video: PTS/DTS adjusted: ${toMsFromMpegTsClock(firstPTS, true)}/${toMsFromMpegTsClock(firstDTS, true)}, delta: ${toMsFromMpegTsClock(delta, true)} ms`);
+        minPTS -= delta;
+        if (foundHole) {
+          logger.warn(`AVC: ${toMsFromMpegTsClock(delta, true)} ms hole between fragments detected,filling it`);


Dropping the current PTS / DTS in these log message would be nice for peopel trying to fix bad content.

I changed these from logs to warnings because I believe most of the time these adjustments are being made in error. These hacks are certainly needed in some cases, but should not be for well formed streams.

@itsjamie that information is available in the following log message

logger.log(`Video: PTS/DTS adjusted: ${toMsFromMpegTsClock(minPTS, true)}/${toMsFromMpegTsClock(firstDTS, true)}, delta: ${toMsFromMpegTsClock(delta, true)} ms`);

…etection is not based on what was previously appeneded

… calculation

…te variance

robwalch · 2020-07-13T16:23:15Z

src/remux/mp4-remuxer.js

    // 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.
    if (isSafari) {
      mp4SampleDuration = Math.round((lastDTS - firstDTS) / (inputSamples.length - 1));
    }

+    // Clamp first DTS to 0 so that we're still aligning on initPTS,
+    // and not passing negative values to MP4.traf. This will change initial frame compositionTimeOffset!
+    firstDTS = Math.max(firstDTS, 0);


This will keep initial DTS appearing as it was before, and apply the same change to initial frame CTS as before. The difference is now we avoid modifying both initial PTS and mp4SampleDuration in the code above.

robwalch · 2020-07-13T16:24:57Z

src/remux/mp4-remuxer.js

-          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 > 2;


This check prevents us from adjusting timecode in our first BBB test stream with a 59.94 and 29.97 frame rates.

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

7efc406

#2873

robwalch requested review from itsjamie and tjenkinson July 9, 2020 03:20

robwalch commented Jul 9, 2020

View reviewed changes

robwalch mentioned this pull request Jul 9, 2020

HLS.js dropping frames after discontinuities #2873

Closed

robwalch added this to Top priorities in Release Planning and Backlog via automation Jul 9, 2020

robwalch moved this from Top priorities to v0.14.1 in Release Planning and Backlog Jul 9, 2020

robwalch added this to the 0.14.1 milestone Jul 9, 2020

itsjamie reviewed Jul 9, 2020

View reviewed changes

src/remux/mp4-remuxer.js Show resolved Hide resolved

robwalch commented Jul 9, 2020

View reviewed changes

src/remux/mp4-remuxer.js Show resolved Hide resolved

robwalch commented Jul 9, 2020

View reviewed changes

src/remux/mp4-remuxer.js Outdated Show resolved Hide resolved

itsjamie reviewed Jul 10, 2020

View reviewed changes

src/remux/mp4-remuxer.js Outdated Show resolved Hide resolved

itsjamie reviewed Jul 10, 2020

View reviewed changes

Rob Walch added 3 commits July 10, 2020 22:46

Do not drop audio frames when switching qualities since the overlap d…

8708ba1

…etection is not based on what was previously appeneded

Do not chang starting DTS when PTS < DTS is found

177034f

Reset demuxer when backtracking

05ce6a7

robwalch force-pushed the bugfix/fix-dropped-frames branch from d090481 to 05ce6a7 Compare July 11, 2020 03:17

itsjamie previously approved these changes Jul 13, 2020

View reviewed changes

Rob Walch added 2 commits July 13, 2020 12:14

Clamp DTS after timecode hole/overlap check and Safari frame duration…

052feab

… calculation

Adjust timecode hole tolerance to account for 59.94 and 29.97 framera…

b1d0964

…te variance

robwalch dismissed itsjamie’s stale review via b1d0964 July 13, 2020 16:16

robwalch commented Jul 13, 2020

View reviewed changes

robwalch merged commit 45d5a92 into master Jul 13, 2020

robwalch deleted the bugfix/fix-dropped-frames branch July 13, 2020 18:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

robwalch commented Jul 9, 2020 •

edited

robwalch Jul 9, 2020 •

edited

itsjamie Jul 10, 2020

robwalch Jul 10, 2020 •

edited

robwalch Jul 10, 2020

itsjamie Jul 10, 2020 •

edited

robwalch Jul 10, 2020 •

edited

robwalch Jul 12, 2020

robwalch Jul 13, 2020

robwalch Jul 13, 2020

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

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

Conversation

robwalch commented Jul 9, 2020 • edited

This PR will...

Why is this Pull Request needed?

Resolves issues:

Checklist

robwalch Jul 9, 2020 • edited

Choose a reason for hiding this comment

itsjamie Jul 10, 2020

Choose a reason for hiding this comment

robwalch Jul 10, 2020 • edited

Choose a reason for hiding this comment

robwalch Jul 10, 2020

Choose a reason for hiding this comment

itsjamie Jul 10, 2020 • edited

Choose a reason for hiding this comment

robwalch Jul 10, 2020 • edited

Choose a reason for hiding this comment

robwalch Jul 12, 2020

Choose a reason for hiding this comment

robwalch Jul 13, 2020

Choose a reason for hiding this comment

robwalch Jul 13, 2020

Choose a reason for hiding this comment

robwalch commented Jul 9, 2020 •

edited

robwalch Jul 9, 2020 •

edited

robwalch Jul 10, 2020 •

edited

itsjamie Jul 10, 2020 •

edited

robwalch Jul 10, 2020 •

edited