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

setTextTrackVisibility bug #1938

Closed
bcupac opened this issue May 13, 2019 · 12 comments
Closed

setTextTrackVisibility bug #1938

bcupac opened this issue May 13, 2019 · 12 comments
Assignees
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@bcupac
Copy link

bcupac commented May 13, 2019

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

What version of Shaka Player are you using?
master branch

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?
custom

If custom app, can you reproduce the issue using our demo app?
n/a

What browser and OS are you using?
Chrome, MacOS

For embedded devices (smart TVs, etc.), what model and firmware version are you using?

What are the manifest and license server URIs?

What did you do?

There is embedded english CC and other English CC is externally added:

player.addTextTrack(closedCaptionsUrl, 'eng', 'caption', 'text/vtt')
					.then(track => {
						player.selectTextTrack(track);
						player.setTextTrackVisibility(false);
					});

then after the CC toggle button is clicked, the below triggers:

const textTrackVisible = !player.isTextTrackVisible();
				player.setTextTrackVisibility(textTrackVisible);

What did you expect to happen?
it should show external CC.

What actually happened?
it did not show it. But after the toggle CC is pressed 3 times, it will show CC aka after

player.setTextTrackVisibility(true) then
player.setTextTrackVisibility(false) then
player.setTextTrackVisibility(true)

On the other hand. If I start with this:

player.addTextTrack(closedCaptionsUrl, 'eng', 'caption', 'text/vtt')
					.then(track => {
						player.selectTextTrack(track);
						player.setTextTrackVisibility(undefined);
					});

then i get a warning 'text visibility has fallen out of sync' but the button works as it should and shows CC on first click.

@ismena
Copy link
Contributor

ismena commented May 13, 2019

Hey @bcupac thanks for reporting!
Could you provide a manifest and text for us to reproduce this?
Feel free to send them privately to [email protected] if you're concerned about posting publicly.

@ismena ismena added the status: waiting on response Waiting on a response from the reporter(s) of the issue label May 13, 2019
@bcupac
Copy link
Author

bcupac commented May 14, 2019

can't share the manifest or sub, will find a way to either recreate or further debug, thanks!:)

@shaka-bot shaka-bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label May 14, 2019
@michellezhuogg michellezhuogg added the status: waiting on response Waiting on a response from the reporter(s) of the issue label May 30, 2019
@joeyparrish joeyparrish added type: bug Something isn't working correctly and removed needs triage status: waiting on response Waiting on a response from the reporter(s) of the issue labels Jun 5, 2019
@joeyparrish
Copy link
Member

I can reproduce with our demo and the content in it. Steps:

  1. Load "Big Buck Bunny: The Dark Truths", which has no subtitles.
  2. In the debugger, add subtitles from "Angel One": shakaDemoMain.player_.addTextTrack('https://storage.googleapis.com/shaka-demo-assets/angel-one/s-en.webvtt', 'en', 'captions', 'text/vtt')
  3. Use the UI to enable subtitles. Nothing happens.
  4. Disable and re-enable subtitles. They are finally shown.

@shaka-bot shaka-bot added this to the v2.6 milestone Jun 5, 2019
@joeyparrish joeyparrish self-assigned this Jun 5, 2019
@joeyparrish
Copy link
Member

After addTextTrack() is called, there is one text track, which is marked "active". A network request for the subs has been made. The text displayer has a list of cues. isTextTrackVisible() on the player agrees with the internal state of the text displayer.

After selecting English from the UI, isTextTrackVisible() and the internal state of the text displayer both show that text should be visible, but it's not.

@joeyparrish
Copy link
Member

Ah, because "cues" is empty after enabling the display of text.

@joeyparrish
Copy link
Member

setTextTrackVisibility calls StreamingEngine's loadNewTextStream, which clears the text buffer through MediaSourceEngine.

@joeyparrish
Copy link
Member

It's unclear which is the fault: either that loadNewTextStream doesn't detect that it's the same stream and ignore it, or that the buffer doesn't refill after being cleared. I tend to think the former, since we'd prefer not to fetch anything twice if it's already in buffer.

@bcupac
Copy link
Author

bcupac commented Jun 14, 2019

So, when

addTextTrack

is called, it calls

 await this.streamingEngine_.loadNewTextStream(stream);

which runs this

 if ((this.textStreamSequenceId_ == currentSequenceId) &&
        !this.mediaStates_.has(ContentType.TEXT) &&
        !this.unloadingTextStream_) {
      const presentationTime = this.playerInterface_.getPresentationTime();
      const needPeriodIndex = this.findPeriodForTime_(presentationTime);

      const state = this.createMediaState_(stream,
          needPeriodIndex,
          /* resumeAt= */ 0);

      this.mediaStates_.set(ContentType.TEXT, state);
      this.scheduleUpdate_(state, 0);
    }

Now, when we select a text track, we again call

loadNewTextStream

but now :

 !this.mediaStates_.has(ContentType.TEXT)

is false, so the code block does not trigger.
Then after we turn off the subtitles

  this.streamingEngine_.unloadTextStream();

gets called and it triggers

 this.mediaStates_.delete(ContentType.TEXT);

so the next time we turn on the subtitles:

!this.mediaStates_.has(ContentType.TEXT)

is true so the if code block is executed and it starts working and displaying subtitles.

@ismena
Copy link
Contributor

ismena commented Jun 25, 2019

This should be working now, please feel free to reopen if you're still seeing the issue.

@joeyparrish joeyparrish assigned ismena and unassigned joeyparrish Jun 26, 2019
TheModMaker pushed a commit that referenced this issue Jul 3, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
We start streaming text once subtitles have been enabled.
With external text, we used to create a media state for it
as soon as text was added even if captions were disabled
and we did not actually stream, although no text was
being parsed and shown.
This lead to the following problem:
If external text was added with subtitles disabled, we
created the media state with no actual cues in it.
Once the subtitles were enabled, we saw that a media
state existed for text and assumed it had content.
So, we started streaming, but nothing was shown.
This change makes us only create the media state if
subtitles are enabled and defer creation otherwise.

Closes #1938.

Change-Id: Iee39c30fbd6b7f0abe7772dfb75cba561fcc9998
@michellezhuogg
Copy link
Contributor

michellezhuogg commented Jul 9, 2019 via email

@ismena
Copy link
Contributor

ismena commented Jul 9, 2019

Honestly, I'm not 100% sure about that console message. I've seen it show up in different situations, but it's never related to any root cause as far as I can tell ¯_(ツ)_/¯

This particular bug was for external text not showing up when it's enabled. I don't think the console message is related.

@michellezhuogg
Copy link
Contributor

michellezhuogg commented Jul 9, 2019 via email

shaka-bot pushed a commit that referenced this issue Jul 16, 2019
I promised Theodore to add a test for this.
A Lannister always pays his (technical)  debts.

Issue #1938

Change-Id: I9741b8c65a4a26b1c07ed1feb9082751dab49505
AnteWall pushed a commit to AnteWall/shaka-player that referenced this issue Jul 17, 2019
We start streaming text once subtitles have been enabled.
With external text, we used to create a media state for it
as soon as text was added even if captions were disabled
and we did not actually stream, although no text was
being parsed and shown.
This lead to the following problem:
If external text was added with subtitles disabled, we
created the media state with no actual cues in it.
Once the subtitles were enabled, we saw that a media
state existed for text and assumed it had content.
So, we started streaming, but nothing was shown.
This change makes us only create the media state if
subtitles are enabled and defer creation otherwise.

Closes shaka-project#1938.

Change-Id: Iee39c30fbd6b7f0abe7772dfb75cba561fcc9998
AnteWall pushed a commit to AnteWall/shaka-player that referenced this issue Jul 17, 2019
I promised Theodore to add a test for this.
A Lannister always pays his (technical)  debts.

Issue shaka-project#1938

Change-Id: I9741b8c65a4a26b1c07ed1feb9082751dab49505
@shaka-project shaka-project locked and limited conversation to collaborators Aug 24, 2019
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

5 participants