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

Regression: Subtitles are not changed when they're invisible #1293

Closed
yairans opened this issue Feb 11, 2018 · 4 comments

Comments

Projects
None yet
5 participants
@yairans
Copy link

commented Feb 11, 2018

NOTE: For bugs, if you delete this template, we will send it again and ask you to fill it out.

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

What version of Shaka Player are you using?: 2.3.2

Can you reproduce the issue with our latest release version?: yes

Are you using the demo app or your own custom app?: demo

What browser and OS are you using?: Chrome 64/macOS High Sierra

What are the manifest and license server URIs?:
https://storage.googleapis.com/shaka-demo-assets/angel-one/dash.mpd

What did you do?
Load
Turn on the CC button
Verify the text language is English
Turn off the CC button
Switch to another text language (French for example)
Turn on the CC button
What did you expect to happen?
The captions should be in French
What actually happened?
The captions are still in English

@yairans yairans changed the title Regression: Subtitles are not changed when them invisible Regression: Subtitles are not changed when they're invisible Feb 11, 2018

@theodab theodab added the bug label Feb 12, 2018

@theodab

This comment has been minimized.

Copy link
Member

commented Feb 12, 2018

Reproduced. I'll look into it, thanks for the report.

@theodab theodab self-assigned this Feb 12, 2018

@chambs

This comment has been minimized.

Copy link

commented Feb 15, 2018

Seems to work fine when rolling back a change at player.js#onChooseStreams_ at the return statement.
Not sure what kind of problems this can bring though. From my tests so far, no side effects were found.

  // Don't fire a tracks-changed event since we aren't inside the new Period
  // yet.
  // Don't initialize with text stream if the caption is not visible.
  // if (this.isTextTrackVisible()) {
  //   return { variant: chosenVariant, text: chosenTextStream };
  // } else {
  //   return { variant: chosenVariant, text: null };
  // }
  return { variant: chosenVariant, text: chosenTextStream };
@theodab

This comment has been minimized.

Copy link
Member

commented Feb 15, 2018

I have a CL out for this. The problem was that we are trying to unload the text stream when captions are disabled, but we forgot to actually unload the text data from MediaSource. In that asset the entire video's captions are a single segment, so it never loads new text data.

@shaka-bot shaka-bot closed this in 79a215b Feb 15, 2018

@joeyparrish

This comment has been minimized.

Copy link
Member

commented Feb 21, 2018

Cherry-picked for v2.3.3.

joeyparrish added a commit that referenced this issue Feb 21, 2018

Clear old text stream when loading new text stream
In our previous fixes for issue #1058, we made it so that we
remove the text engine when not displaying captions.
However, we did not clear MediaSource's buffered text. Thus, when
loading a new text stream after re-enabling the captions, the old
buffered text would still be displayed. Text tracks, in VOD content,
can potentially be a single file for the entire video. Thus, when
changing text languages while closed captions were disabled, the
actual captions did not change.

This modifies streaming engine to clear buffered text, to fix that
problem. It also updates unit tests.

Closes #1293

Change-Id: Iddf2591101286474862b52579d4cf6b464e450fb

@joeyparrish joeyparrish added this to the v2.4.0 milestone Mar 1, 2018

@shaka-bot shaka-bot added the archived label Apr 16, 2018

@google google locked and limited conversation to collaborators Apr 16, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.