Multiperiod MPD with VTT subtitles doesn't add the presentationTimeOffset

[vickyvxr]

Have you read the FAQ and checked for duplicate issues:
Yes
What version of Shaka Player are you using:
2.2.7
Can you reproduce the issue with our latest release version:
Yes
Can you reproduce the issue with the latest code from master:
Yes
Are you using the demo app or your own custom app:
Demo app and custom app
If custom app, can you reproduce the issue using our demo app:
Yes
What browser and OS are you using:
Chrome - Windows 10
What are the manifest and license server URIs:
(you can send the URIs to shaka-player-issues@google.com instead, but please use GitHub and the template for the rest)

What did you do?
Create a multiperiod MPD with VTT subtitles with presentationTimeOffsets in segmentTemplates

What did you expect to happen?
The subtitles are displayed in the correct timecode with the presentationTimeOffset added/substracted.

What actually happened?
The subtitles are always displayed in the same timecode. See example below.

We have found an issue with presentationTimeOffsets and VTT subtitles in multiperiod MPDs,
as far as I can understand, presentationOffsets aren't taken in account in subtitles.
(#595) when segmentTimeline are presents, but for us, it is necessary to move the VTT files
timestamps using the presentationTimeOffset of each period.
These are our adaptation sets for subtitle tracks:

I will explain this issue with an example, imagine that we have 3 periods:
A [0-60] -> Subtitle "A" in 30, presentationTimeOffset=0
B [60-120] -> Subtitle "B" in 90, presentationTimeOffset=60
C [120-180] -> Subtitle "C" in 150, presentationTimeOffset=90

In each period we have a subtitle track with the corresponding presentationTimeOffsets.
With the current implementation, in each period the subtitles offset are reset, so in each period,
will always appear the "A" subtitle in seconds, 30,90,150.

If we pass the presentationTimeOffset to the subtitle parser, please check the pull request:

// In multiperiod, the presentationTimeOffset is necessary to move the Vtt
// subtitles to the correct time position
if (time.presentationTimeOffset != 0) {
offset = time.segmentStart - time.presentationTimeOffset;
}

We will see the subtitles in each period, "A" in 30, "B" in 90 and "C" in 150,
and if we move the periods:
B A C
The subtitles will be also moved and visible in each period. "B" in 30, "A" in 90 and "C" in 150.

This is my first contribution in github and I don't have much experience with shaka. Could you please check if this is the best solution?

Please check the pull request.

[TheModMaker]

Can you post a playable URL to some content that reproduces your problem?

[joeyparrish]

Presentation time offsets are already taken into account via TextEngine. If you would, please send a URL so that we can reproduce the issue, and then we will be better able to understand the problem and review your PR.

If you don't want to post the URL publicly for any reason, you can share with the team privately by emailing the link to shaka-player-issues@google.com.

Thanks!

[vickyvxr]

Good morning,
You can use these two MPDs generated using your sample media in your shaka player demo APP.
These MPDs have 3 periods of 20 seconds with presentationTimeOffsets in audio/video/subtitle tracks.
If you try these files in the demo with the latest version you can see that the subtitles are reset in each period, using the PR version the subtitles are maintained and synchronized with the audio track in all the periods.

• 1 period: https://www.dropbox.com/s/zeq1fmo5vout6an/1period.mpd?dl=0
  Subtitles are maintained during timeline
• 3 periods ABC: https://www.dropbox.com/s/ke8x1scauthawau/3periodsABC.mpd?dl=0
• 3 periods BCA: https://www.dropbox.com/s/cpxc5x0xhlywyo6/3periodsBAC.mpd?dl=0

Thank you very much for your support :)

[joeyparrish]

Sorry for the long delay.

One of your manifests had a typo, but it was easy enough to fix. Those MPDs using our own test segments are very helpful. Thanks!

We have reproduced the issue, but I suspect this use-case might conflict with another. I still have some digging to do.

[joeyparrish]

This seems to be specific to SegmentTemplate+duration. I'm fairly sure the bug is in the DASH parser, but the fix is more challenging than I expected.

[joeyparrish]

Several issues came up here.

First, in your "ABC" test manifest, the text segments had a duration of 60 inside a period of duration 20. This is an edge case we didn't handle correctly in the DASH parser, and it caused failures in StreamingEngine.

Second, the DASH parser didn't apply presentationTimeOffset correctly in the case of SegmentTemplate+duration.

Third, the "BAC" test manifest had text segments with a duration of only 20 seconds, even though the media segment was actually 60. This meant that after applying an offset of -20 or -40 seconds, everything went a little crazy. I fixed the manifest for this.

Here are the corrected versions of all three manifests, which I now have working locally:

https://storage.googleapis.com/shaka-demo-assets/_bugs/1164/1period.mpd
https://storage.googleapis.com/shaka-demo-assets/_bugs/1164/3periodsABC.mpd
https://storage.googleapis.com/shaka-demo-assets/_bugs/1164/3periodsBAC.mpd

[vickyvxr]

I'm sorry about all the typos in the manifests, thank you very much for the corrected versions.
And I will check the bug correction to learn the best way to solve it!

Thanks for all!

[joeyparrish]

The fix is in the master branch now. Please let us know if you have any questions!

[joeyparrish]

The fix has been cherry-picked for v2.2.9.