Add bFrameAdjustment configuration option to address PTS/DTS bug

[baconz]

I know you guys have been working on gap jumping as a workaround for the PTS/DTS bug, which should go a long way for many use cases. We have our own gap jumping implementation, but it has some limitations. Specifically: transitions to new periods are not smooth, and content with a correctly set base_media_decode_time (DTS) will not start because it ends up with a negative presentation start time.

This approach fixes the underlying problem, but requires uniform and predictable frame rates and bframe counts.

I think it is a relatively unobtrusive solution that goes a long way towards achieving smooth multi-period playback for most use cases.

Take a look and let me know what you think. If it is something you'd consider I can clean it up and fix tests.

---

WIP: pending discussion, will add/fix tests

This change introduces a configuration option that adjusts the timestampOffset and appendWindowEnd to address the PTS/DTS bug in Chrome. It assumes that the number of bframes is known, and constant. It also assumes that the manifest provides an accurate framerate.

Full explanation at: https://github.com/baconz/shaka-player/blob/bframe-adjustment-for-chrome/externs/shaka/player.js#L474

[joeyparrish]

@baconz, this is very interesting. I'm glad that this is a workable approach for you, but I would like to defer consideration of this PR until we are done with #555. At that point, if our solution does not meet your needs, we can reconsider this PR or something based on it. In the mean time, I'm glad you are not stuck waiting on us!

[baconz]

Sounds good @joeyparrish. Let me know if/when you'd like to discuss this further.

[joeyparrish]

@baconz, we recently landed gap jumping in #555. Is this PR still applicable? Or does gap jumping solve the same problem this PR is meant to solve?

[baconz]

Hey Joey, I'm out this week, but will take a look next Monday when I'm back.

…

On Mon, Apr 24, 2017, 16:15 Joey Parrish ***@***.***> wrote: @baconz <https://github.com/baconz>, we recently landed gap jumping in #555 <#555>. Is this PR still applicable? Or does gap jumping solve the same problem this PR is meant to solve? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#731 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABElxlPt66zzIIjp6j8h1qD8z_hkHncRks5rzVdGgaJpZM4Mkobw> .

[baconz]

@joeyparrish unfortunately #555 doesn't solve the whole picture for us. There are two problems:

(1) Chrome sees negative start times because we use PTS as the the PTO, and Chrome uses the DTS as the timestamp of the first sample.
(2) Chrome sees gaps because Chrome sees periods as shorter than they should be.

#555 solves (2), but not (1).

With (1) Shaka will throw a 3016 and you'll get an error along these lines in media-internals:

video frame with PTS 0us has negative DTS -66734us after applying timestampOffset, handling any discontinuity, and filtering against append window

Here's a diagram that represents the problem for a single segment:

   PTO
   PTS
   6006
      |

 |
 0
DTS

Here the PTO would be set to 6006, and Chrome would see the first timestamp as -6006.

This PR allows you to mitigate this problem by adjusting the PTO based on the maximum number of concurrent b frames in the underlying content.

[joeyparrish]

Okay, I see now.

I understand the need for this, but I am still hesitant because this feels like a lot of code to work around a Chrome bug. I don't know when, but this will get fixed in Chrome at some point.

How would a user of this feature employ it only on Chrome, and only on affected versions of Chrome?

[baconz]

I agree that it is a lot of code to work around this bug, but given that the bug has been open 3 years, we're not holding out hope that it will be fixed. To us it seemed cleaner to push the workaround as close as possible to the client instead of branching off somewhere further up the video pipeline.

We make the decision on whether or not to enable the flag when we call player.configure() based on out-of-band browser information (I think we use bowser).

If we really wanted to be fancy, we could embed a very simple fragment with a single frame and a known PTS. When we initialize the player, we could pass it to a test SourceBuffer and read back the buffered range to find out if the browser has this bug.

If we wanted to be really fancy, I think we could detect the number of consecutive b-frames by using the num_ref_frames parameter in the SPS, but that would require parsing the init.mp4 and parsing the SPS, which seems like a lot of code.

[joeyparrish]

@baconz, I've been informed by the Chrome team that they are actively working on this and hope to land something soon.

Since you have this workaround implemented in your fork, there's no urgency to merge it, correct? If so, I propose we defer for a couple more weeks to see what Chrome team can accomplish in terms of a fix in that time.

[baconz]

@joeyparrish sounds good, it would be awesome if they push a fix!

[joeyparrish]

@baconz, I know this has dragged on for quite some time, but would you be willing to test another round of Chrome fixes with regard to this issue?

In particular, you should use Chrome beta (63) with the MseBufferByPts feature turned on via the command-line:

google-chrome-beta --user-data-dir=/tmp/user-data --enable-features=MseBufferByPts

I only recently became aware of the fact that Chrome team is experimenting with PTS-based buffering behind this flag, to avoid accidentally breaking any media sites that may only work with the buggy behavior they are trying to fix.

[joeyparrish]

Alternately, a test case or demo page that showcases the problem would be allow us to go directly to Chrome team with your specific use case, and allow them to iterate more quickly toward fixing the issues that are affecting you.

[joeyparrish]

I believe the Chrome DTS/PTS bug has now been fixed, so I'm closing this PR. If you still have issues with the latest stable release of Chrome, please file a new issue/PR. Thanks!