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

Move 'valid codec with spaces' to validButUnsupportedConfigs configs #48870

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

Djuffin
Copy link
Contributor

@Djuffin Djuffin commented Oct 29, 2024

UA is not expected to support codec strings with whitespaces around it, even though it's a valid config.

Originally when this test was introduced by 10ae579 , it tested that spaces around codec string don't make it invalid. Later on this 3939cbf change started to check that ' vp09.00.10.08 ' is a correct way to refer to a VP9 codec and VideoDecoder.configure() should be succeed. The spec does not support this point of view and I see no reason to allow it.

UA is not expected to support codec strings with whitespaces around it, even though it's a valid config.
Copy link
Contributor

@padenot padenot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that, we misread the spec.

@padenot
Copy link
Contributor

padenot commented Nov 18, 2024

@Djuffin Djuffin merged commit 30ee69b into web-platform-tests:master Nov 18, 2024
19 checks passed
moz-wptsync-bot pushed a commit that referenced this pull request Dec 23, 2024
Previously, codec strings containing spaces were trimmed, and
VideoDecoder would report them as "supported". However, per changes
introduced in PR #48870 [1], such codec strings, while "valid" in terms
of syntax, should be considered "unsupported" by the VideoDecoder.

Given that codec strings with spaces should be marked as "unsupported",
it's unnecessary to parse these codec strings before checking if they
are supported video codecs. The underlying checking method reports codec
strings containing spaces as "unsupported", naturally aligning with the
expected behavior.

[1] #48870

Differential Revision: https://phabricator.services.mozilla.com/D231716

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1931827
gecko-commit: 375d118ff39a50118a2fa26b3892d81da6f327a8
gecko-reviewers: media-playback-reviewers, alwu
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 24, 2024
… spaces r=media-playback-reviewers,alwu

Previously, codec strings containing spaces were trimmed, and
VideoDecoder would report them as "supported". However, per changes
introduced in PR #48870 [1], such codec strings, while "valid" in terms
of syntax, should be considered "unsupported" by the VideoDecoder.

Given that codec strings with spaces should be marked as "unsupported",
it's unnecessary to parse these codec strings before checking if they
are supported video codecs. The underlying checking method reports codec
strings containing spaces as "unsupported", naturally aligning with the
expected behavior.

[1] web-platform-tests/wpt#48870

Differential Revision: https://phabricator.services.mozilla.com/D231716
moz-wptsync-bot pushed a commit that referenced this pull request Dec 24, 2024
Previously, codec strings containing spaces were trimmed, and
VideoDecoder would report them as "supported". However, per changes
introduced in PR #48870 [1], such codec strings, while "valid" in terms
of syntax, should be considered "unsupported" by the VideoDecoder.

Given that codec strings with spaces should be marked as "unsupported",
it's unnecessary to parse these codec strings before checking if they
are supported video codecs. The underlying checking method reports codec
strings containing spaces as "unsupported", naturally aligning with the
expected behavior.

[1] #48870

Differential Revision: https://phabricator.services.mozilla.com/D231716

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1931827
gecko-commit: 375d118ff39a50118a2fa26b3892d81da6f327a8
gecko-reviewers: media-playback-reviewers, alwu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants