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

Fix #3421 - accept non-compliant imagetype attribute name in SMPTE-TT image #3425

Merged
merged 1 commit into from
Oct 8, 2020

Conversation

davemevans
Copy link
Contributor

Turns out #3332 broke one of the test assets (https://livesim.dashif.org/dash/vod/testpic_2s/img_subs.mpd) so was easy to verify the fix.

@dsilhavy dsilhavy added this to the 3.2.0 milestone Oct 8, 2020
@dsilhavy dsilhavy merged commit f09050b into Dash-Industry-Forum:development Oct 8, 2020
@nigelmegitt
Copy link
Contributor

As a reference player should dash.js be supporting invalid input data like this? SMPTE 2052-1:2013 defines the attribute name as imageType and does not define imagetype at all. XML attribute names are case sensitive. Seems like the correct fix for this breaking a test asset is to fix the test asset, not make the code support something that's not in the specification.

Of course for use not as a reference player, this kind of forgiving behaviour is probably what folk generally want, so the right option here depends on the target requirements for the player.

@dsilhavy
Copy link
Collaborator

dsilhavy commented Oct 8, 2020

I think this is a general question, and it depends on the situation. dash.js is used in production by various companies so we should design the player as robust as possible. Throughout the code we already have some fixes for non standard compliant content.

In this case the fix was simple and has no negative influence on any other part of the code. In addition, it does not add complexity. So I think it is fine to add this. We might consider giving a warning in the console so developers know that their content is not standard compliant.

@davemevans davemevans deleted the 3421 branch October 8, 2020 12:36
@davemevans
Copy link
Contributor Author

davemevans commented Oct 8, 2020

This isn't just a test asset issue: there is production content in the wild that uses the incorrect case for this attribute - see the original ticket (#3421). FWIW, the player erroneously supported only imagetype until August, and no one noticed for about five years ...

A warning would be a sensible addition, as would fixing the test asset 😉

@TobbeEdgeware
Copy link

I've updated the test asset https://livesim.dashif.org/dash/vod/testpic_2s/img_subs.mpd with the correct spelling, as well as some other variants on the same site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants