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

Add some Media Capabilities API tests #49140

Merged

Conversation

chrisn
Copy link
Contributor

@chrisn chrisn commented Nov 13, 2024

Per spec draft at https://www.w3.org/TR/2024/WD-media-capabilities-20241007/

  • Add tests for 'does not imply a codec'
  • Add tests for 'valid MIME type string'
  • Add tests for 'single media codec'

PTAL @markafoltz

Per spec draft at https://www.w3.org/TR/2024/WD-media-capabilities-20241007/

- Test for 'does not imply a codec'
- Test for 'valid MIME type string'
- Test for 'single media codec'
Copy link
Member

@markafoltz markafoltz left a comment

Choose a reason for hiding this comment

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

Looks good with a few minor fixups.

Presumably we will want similar tests for encodingInfo()?

media-capabilities/decodingInfo.any.js Outdated Show resolved Hide resolved
media-capabilities/decodingInfo.any.js Outdated Show resolved Hide resolved
media-capabilities/decodingInfo.any.js Outdated Show resolved Hide resolved
media-capabilities/decodingInfo.any.js Outdated Show resolved Hide resolved
@markafoltz
Copy link
Member

I requested merge rights in #49250 so I may be able to merge this myself 👏

@chrisn
Copy link
Contributor Author

chrisn commented Nov 19, 2024

Thanks. I'll update the tests per your suggestions and add some similar tests for encodingInfo.

@chrisn
Copy link
Contributor Author

chrisn commented Nov 19, 2024

I added encodingInfo tests, fixed a minor issue in the decodingInfo tests I added (with the framerate option), as well for your suggested edits.

Copy link
Member

@markafoltz markafoltz left a comment

Choose a reason for hiding this comment

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

Looks good with one small fix to look at

@@ -180,10 +233,10 @@ promise_test(t => {
width: 800,
height: 600,
bitrate: 3000,
framerate: '24000/1001',
framerate: 24,
Copy link
Member

Choose a reason for hiding this comment

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

Was this change intentional? The test suggests the framerate needs to be 'x/y'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, no, I just fixed that.

Copy link
Member

@markafoltz markafoltz left a comment

Choose a reason for hiding this comment

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

LGTM!

@markafoltz markafoltz merged commit d3d219e into web-platform-tests:master Dec 4, 2024
19 checks passed
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.

4 participants