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.

Sign up for GitHub

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

Jump to bottom

ttml play issue #1028

Closed
kuznetcoff777 opened this issue Sep 19, 2017 · 8 comments
Closed

ttml play issue #1028

kuznetcoff777 opened this issue Sep 19, 2017 · 8 comments
Labels
component: captions/subtitles The issue involves captions or subtitles component: TTML The issue involves TTML subtitles specifically status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone
v2.3.0

Comments

@kuznetcoff777
Copy link

Have you read the FAQ and checked for duplicate issues:yes

What version of Shaka Player are you using: latest

Can you reproduce the issue with our latest release version:demo

Can you reproduce the issue with the latest code from master: demo

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

If custom app, can you reproduce the issue using our demo app:

What browser and OS are you using: any

What are the manifest and license server URIs:
*(you can send the URIs to [email protected] instead, but please use GitHub and the template for the rest)*sent to mail

What did you do? Opened stream and waited for subtitles

What did you expect to happen? Subtitles in player

What actually happened? No subs appeared, but in http://dashif.org/reference/players/javascript/v2.5.0/samples/dash-if-reference-player/index.html it works fine. I suppose there is some trouble with ttml itslef, but how to workaround this problem? We need to lauch stream on your player with subtitles.
@theodab
Copy link
Contributor

theodab commented Sep 19, 2017

Looking over the caption mp4s, I think see the problem.
Our mp4 ttml parser assumes that an mp4 will only have a single mdat with ttml inside. These samples, however, have two mdats containing ttml each. Therefore, we end up ignoring the first mdat, and only using the second one... and the second mdat has only a single ttml cue in it, and that cue has a duration of 0.
This should be fairly simple to fix. Thanks for pointing it out.

@theodab theodab added type: bug Something isn't working correctly and removed needs triage labels Sep 19, 2017
@kuznetcoff777
Copy link
Author

kuznetcoff777 commented Sep 20, 2017 •
edited
Loading

Second mdat is seems to be invalid, i attempted to cut it, but no luck, error 3000 was faced.
Is there any way to make a hot fix on 2.2 version by ourself? We cannot wait for 2.3 version...

@theodab
Copy link
Contributor

theodab commented Sep 20, 2017

Well, I got the change out now, so you can just pull from master.

@theodab theodab added this to the v2.0-beta milestone Sep 20, 2017
@theodab
Copy link
Contributor

theodab commented Sep 20, 2017

On reflection I'm not entirely sure if this is a bug, or if it's bad content, but I can't find a spec for ttml in mp4 so I'll leave the bug label for now.
This... bug (?) should be present in 2.0, but this change will only work after the ttml parser refactoring in 2.1.

@theodab theodab modified the milestones: v2.0-beta, v2.1.0 Sep 20, 2017
@kuznetcoff777
Copy link
Author

Maybe label is enhancement?
Thanks a lot, it works now.

joeyparrish pushed a commit that referenced this issue Sep 26, 2017
@theodab @joeyparrish
Modified mp4 ttml parser to allow multiple MDATs.
d439cdb 
Beforehand, if we were parsing a ttml subtitles stored in an mp4, we did
not account for the possibility that there would be multiple mdat boxes,
each with its own ttml data. In such a case, we would only use the
payload from the final mdat.
I don't know if having multiple mdat like that is really in the spec, but
at least one user has had this problem, and it's a straightforward change
to make that doesn't really slow down the text parser at all (since we
parse every mdat anyway).

Closes #1028

Change-Id: Id782695548cb92f7aa7278843695b670d8c15ff0
@joeyparrish joeyparrish added the type: enhancement New feature or request label Sep 26, 2017
@joeyparrish joeyparrish modified the milestones: v2.1.0, v2.3.0 Sep 26, 2017
@joeyparrish joeyparrish removed the type: bug Something isn't working correctly label Sep 26, 2017
@joeyparrish
Copy link
Member

The fix has been cherry-picked to v2.2.x and will be released in v2.2.2.

@theodab theodab added the component: TTML The issue involves TTML subtitles specifically label Oct 26, 2017
@kuznetcoff777
Copy link
Author

@theodab @joeyparrish
Guys, have not you destroyed fix in 2.3.0? We checked new version and it seems to be does not work, if button cc is pressed, then captions are not downloaded

@kuznetcoff777 kuznetcoff777 mentioned this issue Dec 25, 2017
Closed
@theodab
Copy link
Contributor

theodab commented Jan 2, 2018

I tried it out in the demo. Your example asset doesn't work in the v2.3.0 demo for non-caption-related reasons, but in the nightly build demo it works fine for me. Have you tried out the nightly?

@theodab theodab added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Jan 11, 2018
@joeyparrish joeyparrish removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Feb 12, 2018
@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@joeyparrish joeyparrish added the component: captions/subtitles The issue involves captions or subtitles label May 19, 2020
@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.
Assignees
No one assigned
Labels
component: captions/subtitles The issue involves captions or subtitles component: TTML The issue involves TTML subtitles specifically status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Milestone
v2.3.0
Development

No branches or pull requests
4 participants
@joeyparrish @shaka-bot @kuznetcoff777 @theodab