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

Unavailable external subtitles prohibit playback #474

Closed
BucherTomas opened this issue Aug 3, 2016 · 16 comments
Closed

Unavailable external subtitles prohibit playback #474

BucherTomas opened this issue Aug 3, 2016 · 16 comments
Assignees
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@BucherTomas
Copy link
Contributor

Modify the URL for external subtitles in demo\assets.js for asset titled 'Tears Of Steel (external text)' so that the file becomes unreachable. Attempting to play this asset will now result in player throwing an http 404 error, requesting the subtitles file indefinitely and not starting playback.

Since subtitles usually have the least importance, the player should be able to start playback without external subtitles even if there are possible http errors present. The fact that the playback does not start at all in this case has a negative impact on user experience.

@joeyparrish joeyparrish self-assigned this Aug 3, 2016
@joeyparrish
Copy link
Member

I definitely see what you mean. It's not a good experience for playback not to start.

Design-wise, the trouble is that we treat all content types generically. Video, audio, and text are all streamed exactly the same way, including buffering. So when text is missing (404), we are stuck buffering, just like if audio or video was missing.

At a higher level, if subtitles fail to load for a piece of content, that's something an integrator needs to know early in their development process. It shouldn't happen, and silently ignoring it leads to a bad experience for users who need text, such as non-native speakers and the deaf. So I believe that we should not silently ignore text failures.

I believe the negative impact on UX caused by failure to start playback is the fault of the application developer for giving the library an inaccessible or incorrect URL. It's not at all obvious to the library that a given stream in a piece of content could be considered "unimportant" and allowed to silently fail. And I don't think contentType == "text" is a good signal for that.

To sum up, we treat text as important because it is necessary for non-native speakers and the deaf. We believe that application developers should feed the library good data, and that dispatching errors is the correct way to respond to being fed bad data.

For now, I'm marking this "working as intended", but we can continue to discuss alternatives if you like.

@joeyparrish joeyparrish added the status: working as intended The behavior is intended; this is not a bug label Aug 3, 2016
@BucherTomas
Copy link
Contributor Author

I understand the reasoning behind this. I am also aware that captioning is becoming mandatory in some countries. Nevertheless, I would point out that usability of accompanying text always boils down to the project the text is used in.

While I agree with you about situations where the text transcripts audio track for deaf or hearing-impaired people, you can also use subtitles for example for descriptive texts similar to text overlays in Youtube player. Their purpose could be just to describe an object on screen in a how-to video without audio commentary, especially thanks to the screen positioning capability of webvtt format. Their presence here is not necessary, they could just provide additional detail which, if missing, wouldn't have fatal consequences for understanding the video. Playback without subtitles is definitely preferred in this scenario over not starting at all.

Developers in similar (granted, less than common) projects now would need to check for the external text file availability first, and if unreachable, omit it from data being fed to Shaka.

@MachalovaZuzana
Copy link

Nowadays, in our company, we are discussing the similar issue. I must agree with Tomas. Based on my own experience with customers and their end-users, it would be better solution to ignore unavailable external subtitles and allow to play video. In case, that the first experience of end-user with playing the content will be so bad, the end-user will never return back.

@joeyparrish
Copy link
Member

@MachalovaZuzana, @BucherTomas, I understand what you're saying, and I would like to find a solution for you. To help me design a solution that meets everyone's needs, can you explain how/why external subs would be unavailable? How does this situation arise?

@BucherTomas
Copy link
Contributor Author

The subtitle files are delivered from other servers than the actual video content in our own environment. This means that for whatever reason, be it server failure, connectivity problems, etc., the subtitles could become unavailable despite video content still being usable.

While the chance of this occurring is very small, it is still realistic enough that we would prefer the video content to remain playable should this happen.

@joeyparrish
Copy link
Member

@BucherTomas, how would you feel about making this behavior configurable, with the default being to behave as we do today? To ignore text failures, you might do something like:

player.configure({
  streaming: {
    ignoreTextStreamFailures: true,
  }
});

@BucherTomas
Copy link
Contributor Author

Yes, that would of course work. We would enable this option in our player as default.

@joeyparrish joeyparrish added type: enhancement New feature or request and removed status: working as intended The behavior is intended; this is not a bug labels Aug 15, 2016
@joeyparrish joeyparrish added this to the v2.0.0 milestone Aug 15, 2016
@joeyparrish joeyparrish assigned ismena and unassigned joeyparrish Aug 15, 2016
@joeyparrish
Copy link
Member

Great, then we'll add that option for you. Thanks for discussing!

@BucherTomas
Copy link
Contributor Author

Thank you for listening!

@BucherTomas
Copy link
Contributor Author

This commit is now perfect for our scenario with single subtitles, thank you for that.

You might take a look at scenario with multiple subtitles, where an attempt to switch from an unavailable text file to one that is available now ends up throwing exception
"Assertion failed: switch: expected mediaState to exist" in streaming_engine.js, line 417.
Playback continues uninterrupted, so that is good, but chosen subtitles do not load.

To repro (tested with master at commit 40f8817):

  1. Play Tears of Steel (external text) asset from Azure Media Services group and prevent downloading English subtitles.
  2. Switch to French subtitles, or alternatively set Preferred text language as 'fr' before loading the stream.

@ismena
Copy link
Contributor

ismena commented Aug 26, 2016

@BucherTomas glad single subtitles scenario works for you now. We'll look into multiple one.

@ismena ismena reopened this Aug 26, 2016
@ismena ismena added type: bug Something isn't working correctly and removed type: enhancement New feature or request labels Aug 26, 2016
@ismena
Copy link
Contributor

ismena commented Aug 26, 2016

@BucherTomas thanks for spotting the problem, it should be good now. Please let us now if you have any questions or issues.

@BucherTomas
Copy link
Contributor Author

@ismena thank you, now it is functional from viewer's point of view, although the previously mentioned exception still occurs in browser console when switching from unavailable to available subtitles.

@joeyparrish
Copy link
Member

@BucherTomas, can you clarify what the text is on the message you're seeing? Also, is it an exception, an error log, or a warning?

@ismena
Copy link
Contributor

ismena commented Aug 29, 2016

@BucherTomas thanks again for providing feedback. My understanding is, you meant the "Assertion failed: switch: expected mediaState to exist" message that you were seeing in the browser console when switching from unavailable subtitles to available ones. This shouldn't happen anymore. Please let me know if my understanding is incorrect.

shaka-bot pushed a commit that referenced this issue Aug 29, 2016
When switching from an unavailable to an available text stream we
are adding a text media state. This happens after switch method
checks for it and the assertion fails.
This is a change to return from the switch method before the
assertion gets checked.

Issue #474

Change-Id: I66fd4cfc7b61ea0acb2a7539cc5afaba4e17044e
@BucherTomas
Copy link
Contributor Author

Yes, this was the message. I wrote the exact wording in one of my earlier messages in this thread and just referred to it in my last post.
My reasoning for getting rid of the message was that if somebody audits the page the player is on from technical standpoint, these kinds of messages do not look favorable to the owner of the site.

Everything is perfect now, the message is gone. Thanks again.

@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@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