Avoid floating point arithmetic when calculating time replacement

[ghost]

The current code essentially does this

timeReplacement = (startTime / timescale + ...) * 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 unchanged in the timeline just to avoid the multiply/divide dance.

@TobbeEdgeware and @PhamCongDuc this is SHIELD-1235. The code is not exactly pretty, but I want to make sure I get the logic right before worrying about where to save "unscaledStart"

[PhamCongDuc]

I add var item = { in here, rebuild Shaka player with your code and it works fine. I test without your code, the issue appears again. Thanks @duynguyen-ew . How can we propose your change to apply in Shaka player repository.

[lithammer]

Yes, the var part is important. Otherwise you'll be creating a global variable.

[TobbeEdgeware]

I think this looks nice, except that there is a potential issue with floating point accuracy from the presentationTimeOffset as well (we use zero, but for multiperiod, we may need other values).
It would be good to check if that part can be improved as well (an complication is that presentationTimeOffset can be defined on different levels with different timescales).

To get the code accepted, the best is to

1. create an issue on github describing the problem
2. make sure that current tests go through, and ideally add another test for this
3. submit a pull request for it.

I can help out with creating the issue. It might help since I've relationship with the main developer since before.

[TobbeEdgeware]

I looked a bit more at the file lib/dash/mpd_utils.js and the createTimeline function uses integer arithmetic to add presentationTime to the timestamp. However, the parseSegmentInfo sends back a scaled presentationTimeOffset. It might work to add a presentationTimeOffsetUnscaled to the return object and then use that together with the patch.

[ghost]

Thanks @TobbeEdgeware . @PhamCongDuc I'll leave it to you to handle the rest with help from @TobbeEdgeware. You should also make sure that it does fix the problems for TVB or we're just wasting time fixing something else.

[TobbeEdgeware]

I've made a shaka player issue shaka-project#690 and we'll wait for response on whether they are open to pull request.

If they are positive, we may need to improve the fix to include presentationTimeOffset and possibly tests. I'll be back to you @duynguyen-ew in that case.

@PhamCongDuc On the TVB side, it would be good if you use your own patched version for the moment. In any case, it will take some time until this is included in a shaka-player release.

[PhamCongDuc]

Thanks @duynguyen-ew , @TobbeEdgeware . I ran Skaka auto tests, nothing failed

[TobbeEdgeware]

We got an answer to google#690 that they are positive to a patch.

I think one should check that all of "startTime", "endTime", and "presentationTimeOffset" are handled exactly, and not just "startTime".

Will you make one from Vietnam (with an github account Edgeware email address)?

[PhamCongDuc]

Yes. I'm doing that (checking presentationTimeOffset) and will check endTime. I sent you a message on Slack.

[ghost]

Closing as #2 covers it too