-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Shaka stops display webvtt subtitles when webvtt cue-time is greater than 2^33 (~26h30m43s) #6320
Comments
Confirmed repro. Thanks for the report! The stream URLs make this much easier to work on.
I believe you are correct. I suppressed the rollover logic in the debug console by setting I tried the HLS+TS clip, but it hangs at the rollover point with errors about bad timestamps in the media. There might be an issue with our transmuxer here. |
The HLS+TS clip plays in v4.4.0 and v4.5.0, but not in v4.6.0, v4.7.0, or v4.7.11 (latest). In the versions where the HLS+TS doesn't hang at the rollover point, we still need the Probably the HLS+TS hang is caused by a transmuxer change introduced between v4.5.0 and v4.6.0. I'll narrow that down further. |
No bug fix releases were made to v4.5.x before v4.6.0 came out. So the culprit for the media hanging must be one of the changes in v4.6.0: https://github.com/shaka-project/shaka-player/releases/tag/v4.6.0 |
The transmuxer bug that impacts the HLS+TS stream begins with ae423b4, PR #5846. @avelad, I would love your input on this when you are available. I can work on the VTT rollover, though. Making the rollover logic conditional is easy enough, but the condition is whether or not we're playing TS. Plumbing that knowledge into the parser may be ugly. But I'll figure it out. |
@chihchianglu-google, you can work around this issue with the following configuration: player.configure('manifest.hls.sequenceMode', false); This can cause issues with certain HLS content if the timestamps in the media are wrong, but I suspect the streams you are creating will be just fine. Does this work for you as a workaround for now? Can you test this, and if it is robust enough for you, can you recommend this to your customers? @avelad, related to this issue, we should discuss:
Thanks! |
Lowering priority to P2 based on the configuration workaround, assuming acceptance of that workaround by the OP |
@joeyparrish , the config seems not working for me. Here's the player link. |
I think it would be the best given the number of issues there are with the native TS.
The problem is that we do not have pes-rollover implemented in the tsparser and tstransmuxer, we should implement it. I can work on it in 3 weeks, if that's okay with you. |
@chihchianglu-google, you are right. I was mistaken. I must have made a mistake while testing yesterday. Bumping back to P1. @avelad wrote:
@chihchianglu-google, the TS parser fix will have to wait until @avelad is available to work on it. Thanks for your understanding! |
I did some more digging and found that the timestamp logic was working in v4.2.0 - v4.2.2, but not in v4.2.3 or v4.3.0. This was the change responsible AFAICT: 3b9af2e, PR #4586, which mentions internal bug b/253104251 in the regression test. If I remove the rollover logic entirely, that regression test will fail. It's not clear to me yet how to proceed. I don't have access to the original streams that led to that change, only the regression test that simulates them. |
Just my 2 cents. |
This moves VTT sequence mode offset calculations into a method. It also makes all X-TIMESTAMP-MAP usage dependent on HLS specifically, rather than sequence mode, simplifying the conditions. Sequence mode is typically only used with HLS, and X-TIMESTAMP-MAP is explicitly only for HLS. So excluding X-TIMESTAMP-MAP for DASH made sense. This required updating some tests to explicitly set both the manifest type and sequence mode flag. This does *not* change the offset calculations themselves. Changes will be made in follow-up PRs. Issue shaka-project#6320
This moves VTT sequence mode offset calculations into a method. It also makes all X-TIMESTAMP-MAP usage dependent on HLS specifically, rather than sequence mode, simplifying the conditions. Sequence mode is typically only used with HLS, and X-TIMESTAMP-MAP is explicitly only for HLS. So excluding X-TIMESTAMP-MAP for DASH makes sense, instead of conflating HLS and sequence mode. This required updating some tests to explicitly set both the manifest type and sequence mode flag. This does *not* change the offset calculations themselves. Changes will be made in follow-up PRs. Issue #6320
You're not wrong, but it's more complicated than that for many reasons.
We have HLS live streams in the demo app, but none of them have subtitles. So I don't have any real-world examples at hand to compare the formatting and timestamps of different live streaming solutions to vet any changes I make. I'm looking more closely at the HLS spec so I can try to adjust the logic based on that. |
Related to #6334 Related to #6320 (comment) Also reverts #6045 since now it is not necessary
This moves VTT sequence mode offset calculations into a method. It also makes all X-TIMESTAMP-MAP usage dependent on HLS specifically, rather than sequence mode, simplifying the conditions. Sequence mode is typically only used with HLS, and X-TIMESTAMP-MAP is explicitly only for HLS. So excluding X-TIMESTAMP-MAP for DASH makes sense, instead of conflating HLS and sequence mode. This required updating some tests to explicitly set both the manifest type and sequence mode flag. This does *not* change the offset calculations themselves. Changes will be made in follow-up PRs. Issue #6320
Related to #6334 Related to #6320 (comment) Also reverts #6045 since now it is not necessary
This moves VTT sequence mode offset calculations into a method. It also makes all X-TIMESTAMP-MAP usage dependent on HLS specifically, rather than sequence mode, simplifying the conditions. Sequence mode is typically only used with HLS, and X-TIMESTAMP-MAP is explicitly only for HLS. So excluding X-TIMESTAMP-MAP for DASH makes sense, instead of conflating HLS and sequence mode. This required updating some tests to explicitly set both the manifest type and sequence mode flag. This does *not* change the offset calculations themselves. Changes will be made in follow-up PRs. Issue #6320
Related to #6334 Related to #6320 (comment) Also reverts #6045 since now it is not necessary
Have you read the FAQ and checked for duplicate open issues?
Yes.
If the problem is related to FairPlay, have you read the tutorial?
Not related to FairPlay.
What version of Shaka Player are you using?
v4.7.11 (uncompiled)
Btw, I tried almost all the versions since v3, and none of them works.
Can you reproduce the issue with our latest release version?
Yes, v4.7.11 is the latest build / version as of 3/4/2024.
Can you reproduce the issue with the latest code from
main
?Yes, I tried it on nightly build page (which should be the main).
Are you using the demo app or your own custom app?
I try it on demo page and nightly build page
If custom app, can you reproduce the issue using our demo app?
Not custom app.
What browser and OS are you using?
Chrome on MacOS 14.3.1 (23D60)
For embedded devices (smart TVs, etc.), what model and firmware version are you using?
Not on embedded devices.
What are the manifest and license server URIs?
HLS + FMP4: link
HLS + TS: link
DASH + FMP4: link
What configuration are you using? What is the output of
player.getConfiguration()
?Simply paste the manifest links on shaka player demo page w/o changing any configuration.
What did you do?
Paste the HLS manifest links to shaka player demo page, and starting from 56~58 seconds, you will start seeing subtitles not showing (safari native player, hls.js, and bitmovin are all playing w/o issues).
What did you expect to happen?
Subtitles keeps showing throughout the whole stream w/o issues.
What actually happened?
The issue happens when webvtt cue-time is greater than 2^33 (roughly 26h30m43s), which is the mpegts wraparound time.
Currently, Shaka always applies this logic that tries to calculate and apply the offset if cue-time is greater than 2^33(roughly 26h30m43s), and it's not right, because in FMP4, there's no timestamp wraparound.
With this logic, the playback seeking time is wrong, thus no subtitles display after that point.
To make sure my assumption is correct:
I created a hacked live stream to reproduce the issue, by starting the stream with timestamp that's ~60 seconds smaller than 2^33, and after ~60 seconds, shaka fails to show subtitles while safari native player, hls.js, and bitmovin are all playing w/o issues. You should see the same behavior from the manifests I provided above.
DASH shouldn't run into the same issue because the shaka code I highlighted above only applies to HLS, and if you check the DASH manifest I provided above, it's indeed playing w/o issues.
Let me know if you need anything further from me, thanks.
The text was updated successfully, but these errors were encountered: