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

TTML: wrong attribute "imagetype", camel case expected. #908

Closed
Canta opened this issue Mar 5, 2021 · 7 comments · Fixed by #909
Closed

TTML: wrong attribute "imagetype", camel case expected. #908

Canta opened this issue Mar 5, 2021 · 7 comments · Fixed by #909
Assignees
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@Canta
Copy link
Contributor

Canta commented Mar 5, 2021

System info

Operating System: Ubuntu 20.04
Shaka Packager Version: f0a52cb

Issue and steps to reproduce the problem

While testing some DVBSUB-to-TTML conversions with shaka packager, I could see at playback time the following message using DASH reference player:
Error : SMPTE 2052-1:2013 defines the attribute name as "imageType" and does not define "imagetype"

Checking out some text segment/chunk, I can see the attrubute (note the lowercase t):

$ cat text_5.mp4 
(...)
  <smpte:image imagetype="PNG" encoding="Base64" xml:id="img_3">
(...)

Packager Command:

packager \
'in=file4.ts,stream=0,init_segment=1080p_init.mp4,segment_template=1080p_$Number$.m4s' \
'in=file4.ts,stream=1,skip_encryption=1,init_segment=audio_init.mp4,segment_template=audio_$Number$.m4s,language=POL' \
'in=file4.ts,stream=2,skip_encryption=1,format=ttml+mp4,init_segment=text_init.mp4,segment_template=text_$Number$.mp4,language=ENG' \
--mpd_output test.mpd

What is the expected result?:
The same, but with that attribute in camel case format, as expected by the spec (which I didn't read: I'm trusting the player here).

What happens instead?
The error quoted above.

Looks like an easy thing to fix.

@kqyang
Copy link
Contributor

kqyang commented Mar 5, 2021

Thanks for the bug report. Would you like to submit a PR to fix it?

@TheModMaker FYI.

@kqyang kqyang added the type: bug Something isn't working correctly label Mar 5, 2021
@Canta
Copy link
Contributor Author

Canta commented Mar 5, 2021

Yes, no problem. I'm struggling since about an hour ago to have my github fork updated so I could send a pull request. As soon as I fix that, will push the quick fix for this.

@kqyang
Copy link
Contributor

kqyang commented Mar 5, 2021

Great. Thanks.

@Canta
Copy link
Contributor Author

Canta commented Mar 8, 2021

Careful: this seems to also affect shaka player.

I was doing some tests today, and shaka player doesn't show the TTML images after the change in this ticket.
Take a look at this:
https://github.com/google/shaka-player/blob/23fe7128494d0678d6fd8e3bc951f303907ec5cf/lib/text/ttml_text_parser.js#L574

I believe the right way of handling this is changing both packager and player, as otherwise legit TTMLs would not play correctly on shaka player, and shaka packager generated files would not play correctly on other TTML-compatible players.

@Canta
Copy link
Contributor Author

Canta commented Mar 8, 2021

Here's the spec mentioned in the first comment: https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=7291854
Section 5.7.3 (page 12) describes the smtpe:image tag with an imageType attribute (note the capital T).

@kqyang
Copy link
Contributor

kqyang commented Mar 8, 2021

@joeyparrish @TheModMaker FYI.

@Canta You may want to file a bug to Shaka Player and discuss the player issue there.

@joeyparrish
Copy link
Member

We will fix Shaka Player to accept both versions, so existing Packager-generated content with the lowercase attribute name will still function. Thanks!

kqyang pushed a commit that referenced this issue Mar 9, 2021
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label May 8, 2021
@shaka-project shaka-project locked and limited conversation to collaborators May 8, 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

Successfully merging a pull request may close this issue.

4 participants