-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
presentationTimeOffset in SegmentBase is interpreted in seconds instead of time units #1099
Comments
Just realized that a similar issue has just been filed: #1098 |
A brief review of DASH parser from v1 makes me think we didn't have this bug in v1. In v2 (all versions), we seem to parse SegmentTemplate and SegmentTimeline correctly and divide by timescale, but not SegmentBase. So the damage is limited to SegmentBase's presentationTimeOffset, but the bug seems to be present in every single v2 release, all the way back to v2.0.0-beta. |
In DASH SegmentBase, we were not dividing presentationTimeOffset by the timescale. In all other instances of presentationTimeOffset in DASH, we were handling it correctly. This bug was present in all v2 releases until now. The solution is not only to divide by timescale, but to rename all internal uses of presentationTimeOffset to either unscaled or scaled, to differentiate between those in timescale units and those in seconds. I believe inconsistent naming and units were a contributing factor to the creation of the bug. Closes #1099 Change-Id: Id561f8eb1f5bc011c606e1925c12f0d8183fd51a
In DASH SegmentBase, we were not dividing presentationTimeOffset by the timescale. In all other instances of presentationTimeOffset in DASH, we were handling it correctly. This bug was present in all v2 releases until now. The solution is not only to divide by timescale, but to rename all internal uses of presentationTimeOffset to either unscaled or scaled, to differentiate between those in timescale units and those in seconds. I believe inconsistent naming and units were a contributing factor to the creation of the bug. Closes #1099 Minimal cherry-pick of the original fix from v2.2.x
The fix has been cherry-picked for v2.2.5 and v2.1.9. |
Have you read the FAQ and checked for duplicate issues: YES
What version of Shaka Player are you using: Latest
Can you reproduce the issue with our latest release version: YES
Can you reproduce the issue with the latest code from
master
: YESAre you using the demo app or your own custom app: Demo
What browser and OS are you using: Chrome on Linux
What are the manifest and license server URIs:
https://storage.googleapis.com/wvtemp/shaka/bourne/bourne_audio_pto_20.mpd
https://storage.googleapis.com/wvtemp/shaka/bourne/bourne_audio_pto_4410.mpd
What did you do?
Play the above manifests in Shaka demo player.
What did you expect to happen?
https://storage.googleapis.com/wvtemp/shaka/bourne/bourne_audio_pto_20.mpd
The audio should start playing from 20/44100 in media timeline. However, it starts from 20 seconds instead.
https://storage.googleapis.com/wvtemp/shaka/bourne/bourne_audio_pto_4410.mpd
The audio should start playing from 4410/44100 = 0.1 seconds in media timeline. However, the player returns "segment_index.js:240 The last segment should not end before the end of the Period." instead.
Here is the relevant clause of presentationTimeOffset in the spec:
The text was updated successfully, but these errors were encountered: