LibWeb+LibMedia: Reject audio⇔video codec mismatches in decodingInfo() - #10888
LibWeb+LibMedia: Reject audio⇔video codec mismatches in decodingInfo()#10888sideshowbarker wants to merge 1 commit into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe PR adds RFC 6381 codec-string mapping for supported audio and video codecs. Media capability validation now strictly validates MIME strings, codec parameters, codec counts, and media/codec type matches. MIME-sniffing code gains string validation. WPT coverage and expected results are updated for the new validation behavior. Sequence Diagram(s)sequenceDiagram
participant Navigator
participant MediaCapabilities
participant MimeSniff
participant CodecID
Navigator->>MediaCapabilities: decodingInfo(configuration)
MediaCapabilities->>MimeSniff: is_valid_mime_type_string(content_type)
MimeSniff-->>MediaCapabilities: MIME validity
MediaCapabilities->>CodecID: codec_id_from_rfc6381_codec_string(codec_string)
CodecID-->>MediaCapabilities: codec ID or CodecID::Unknown
MediaCapabilities-->>Navigator: MediaCapabilitiesInfo
Possibly related issues
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Libraries/LibWeb/MediaCapabilitiesAPI/MediaCapabilities.cpp`:
- Around line 90-100: Update the codec validation in is_able_to_decode_media()
so an unrecognized codec ID is rejected or otherwise reported unsupported
instead of reaching the supported result. Reuse the existing
codec_id_from_rfc6381_codec_string/track_type_from_codec_id flow to detect
unknown codecs, while preserving the audio/video track-type mismatch checks.
In `@Tests/LibWeb/Text/input/wpt-import/media-capabilities/decodingInfo.any.js`:
- Around line 377-386: Update the test using videoConfigurationWithDynamicRange
to create an independent clone before overriding contentType, rather than
reusing the mutated fixture. Ensure this media-source decodingInfo test retains
the intended HDR codec configuration and remains order-independent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 37c24086-ee1b-4eec-a99a-d8687e91ee95
📒 Files selected for processing (7)
Libraries/LibMedia/CodecID.hLibraries/LibWeb/MediaCapabilitiesAPI/MediaCapabilities.cppLibraries/LibWeb/MediaCapabilitiesAPI/MediaCapabilities.hLibraries/LibWeb/MimeSniff/MimeType.cppLibraries/LibWeb/MimeSniff/MimeType.hTests/LibWeb/Text/expected/wpt-import/media-capabilities/decodingInfo.any.txtTests/LibWeb/Text/input/wpt-import/media-capabilities/decodingInfo.any.js
💤 Files with no reviewable changes (1)
- Libraries/LibWeb/MediaCapabilitiesAPI/MediaCapabilities.h
868d146 to
218cbf6
Compare
5c69e03 to
9886057
Compare
This aligns our mediaCapabilities.decodingInfo() implementation with WPT expectations to reject an audio codec where a video codec is expected, and reject a video codec where an audio codec is expected. The spec (see w3c/media-capabilities#261) doesn’t explicitly state that requirement — but Gecko and WebKit both also reject such codec mismatches (and pass the relevant WPT subtests). This also includes a track_type_from_codec_id() bug fix — to correctly classify Theora as a video codec, rather than incorrectly as audio. Fixes LadybirdBrowser#10696
9886057 to
ec1f836
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This aligns our
mediaCapabilities.decodingInfo()implementation with WPT expectations to reject an audio codec where a video codec is expected, and reject a video codec where an audio codec is expected. The spec (see w3c/media-capabilities#261) doesn’t explicitly state that requirement — but Gecko and WebKit both also reject such codec mismatches (and pass the relevant WPT subtests).This also includes a
track_type_from_codec_id()bug fix — to correctly classify Theora as a video codec, rather than incorrectly as audio. Fixes #10696.