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

EBU subtitles regression in 4.1.3 - subtitles lost: not added to region #4680

Closed
david-hm-morgan opened this issue Nov 10, 2022 · 3 comments · Fixed by #4733
Closed

EBU subtitles regression in 4.1.3 - subtitles lost: not added to region #4680

david-hm-morgan opened this issue Nov 10, 2022 · 3 comments · Fixed by #4733
Labels
component: captions/subtitles The issue involves captions or subtitles component: TTML The issue involves TTML subtitles specifically priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@david-hm-morgan
Copy link
Contributor

david-hm-morgan commented Nov 10, 2022

Have you read the FAQ and checked for duplicate open issues?
Yes

What link can we use to reproduce this?
https://shaka-player-demo.appspot.com/demo/#audiolang=en;textlang=en;uilang=en;asset=https://otvplayer.nagra.com/vod/dash/clear/bbc/bbc_ebu-tt-d_subtitle_fragmented/dash/ondemand/elephants_dream/1/client_manifest-all-minus-1080p.mpd;panel=CUSTOM%20CONTENT;build=uncompiled

What version of Shaka Player are you using?
v4.3.0-uncompiled
and all the way back to v4.1.3-uncompiled

What browser and OS are you using?
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/107.0.0.0 Safari/537.36

What did you do?

  1. Play this stream with captions enabled
  2. Observe the captions at 2:52 to 2:57

What did you expect to happen?
See the captions as follows:
You have to learn to listen.
then
This is not some game.

i.e. from the following TTML snippet.

...
            <p begin="00:02:52.00" end="00:02:54.80" xml:id="sub23">
                <span style="s1">You have to learn to listen.</span>
            </p>
            <p begin="00:02:55.84" end="00:02:57.64" xml:id="sub24">
                <span style="s1">This is not some game.</span>
            </p>
...

What actually happened?

Neither of these were shown, nothing was in this time range.

Note: this can only be reprod by playing the stream through this range uninterrupted, if you pause in that region, they can be shown without issue.

Note: I believe this was introduced in #4412

Note the coincidental relationship with #4631 on the same stream and same release.

@joeyparrish
Copy link
Member

I see you're using v4.1.3. Please try the latest release in that branch (v4.1.7) and overall (v4.2.4 as of now, though v4.3.0 will be out later today).

(We ask similar questions in the issue template, but you seem to have deleted them. Filling out the issue template saves a lot of time for everyone involved, especially across many timezones.)

If you're still seeing the issue in those more recent versions, that would be useful to know. Thanks!

@david-hm-morgan
Copy link
Contributor Author

I see you're using v4.1.3. Please try the latest release in that branch (v4.1.7) and overall (v4.2.4 as of now, though v4.3.0 will be out later today).

(We ask similar questions in the issue template, but you seem to have deleted them. Filling out the issue template saves a lot of time for everyone involved, especially across many timezones.)

If you're still seeing the issue in those more recent versions, that would be useful to know. Thanks!

Sorry, I cloned the similar issue rather than started afresh from the template and lost some stuff. Refreshed now with the template.

Confirmed still present in 4.3.0.

@david-hm-morgan
Copy link
Contributor Author

After some time of debugging this, some clues emerging.
Just before the cue (say N) that is not shown, the region element is uprooted when we remove the cue N-1.
Then it is not reinstated when N or N+1 is handled.
Only reinstated with N+2.

@avelad avelad added type: bug Something isn't working correctly component: TTML The issue involves TTML subtitles specifically component: captions/subtitles The issue involves captions or subtitles priority: P1 Big impact or workaround impractical; resolve before feature release labels Nov 14, 2022
@github-actions github-actions bot added this to the v4.4 milestone Nov 14, 2022
joeyparrish added a commit that referenced this issue Dec 7, 2022
Reinstate previously removed region elements when next caption finds it
is not there: Detect the absence and ensure `updateDOM` is true so its
reinstated and the next caption can be shown.

Closes #4680

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
joeyparrish added a commit that referenced this issue Dec 8, 2022
Reinstate previously removed region elements when next caption finds it
is not there: Detect the absence and ensure `updateDOM` is true so its
reinstated and the next caption can be shown.

Closes #4680

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
joeyparrish added a commit that referenced this issue Dec 8, 2022
Reinstate previously removed region elements when next caption finds it
is not there: Detect the absence and ensure `updateDOM` is true so its
reinstated and the next caption can be shown.

Closes #4680

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
joeyparrish added a commit that referenced this issue Dec 8, 2022
Reinstate previously removed region elements when next caption finds it
is not there: Detect the absence and ensure `updateDOM` is true so its
reinstated and the next caption can be shown.

Closes #4680

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
joeyparrish added a commit that referenced this issue Dec 8, 2022
Reinstate previously removed region elements when next caption finds it
is not there: Detect the absence and ensure `updateDOM` is true so its
reinstated and the next caption can be shown.

Closes #4680

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
joeyparrish added a commit that referenced this issue Dec 8, 2022
Reinstate previously removed region elements when next caption finds it
is not there: Detect the absence and ensure `updateDOM` is true so its
reinstated and the next caption can be shown.

Closes #4680

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Feb 5, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: captions/subtitles The issue involves captions or subtitles component: TTML The issue involves TTML subtitles specifically priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
3 participants