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.

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

Improvement of segment timeline $Time$ accuracy #702

Conversation

TobbeEdgeware
Copy link
Contributor

This PR solves #690.

With the following SegmentTimeline snippet

  <SegmentTemplate timescale="10000000"  media="t$Time$.mp4">
  <SegmentTimeline>
  <S t="5369174623790086" d="40000000/>
  <S d="40000000" />
  </SegmentTimeline>

the second segment erroneously gets the wrong timestamp

5369174663790087 because it is calculated as

 (5369174623790086 / 10000000 + 40000000 / 10000000) * 10000000 = 5369174663790087

and results in an HTTP 404 error when requesting the segment using the $Time$.mp4 segment.

By avoiding, the division by 10000000 a more accurate calculation can be done:

5369174623790086 + 40000000 = 5369174663790086

This is fixed in the PR by storing the unscaled values unscaledStart and unscaledPresentationOffset and using them for generating the timeReplacement for the segment request URL.

A lot of other calculations use times scaled to seconds as before, but since they don't need to be fully accurate, no change is needed.

Unfortunately, we haven't found a good way to add a unit or functional test for this PR, but no existing tests are broken. The fix solves the issue we have seen and makes our streams play in shaka player as they do in dash.js. We don't have reliable public URL, though.

The current code essentially does this

    timeReplacement =
	(startTime / timescale + presentationTimeOffset) * timescale

When timescale is large enough (e.g. 10 MHz in order to support MS
Smooth Streaming as well), "startTime / timescale * timescale" may not
always be exactly "startTime" because of floating point precision,
which could produce incorrect segment URLs.

Keep startTime and presentationTimeOffset unchanged in the timeline
just to avoid the multiply/divide dance.
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@TobbeEdgeware
Copy link
Contributor Author

We will get back with a PR with another commit account, that is included in the CLA.

@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants