fix(export): normalize WebCodecs decoder codec strings#675
Open
bernardopg wants to merge 2 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR consolidates video decoder configuration logic into a shared module. It extracts codec normalization and fallback handling from two decoder implementations into a reusable ChangesVideo decoder configuration extraction
Sequence Diagram sequenceDiagram
participant Caller as Decoder callsite
participant ConfigHelper as configureVideoDecoder
participant VideoDecoder as VideoDecoder API
Caller->>ConfigHelper: configureVideoDecoder(decoder, decoderConfig)
ConfigHelper->>ConfigHelper: normalizeVideoDecoderConfig(config)
ConfigHelper->>ConfigHelper: build candidate list (prefer-software?, H.264 fallback)
loop candidates
ConfigHelper->>VideoDecoder: isConfigSupported(candidate)
alt supported
ConfigHelper->>VideoDecoder: decoder.configure(candidate)
VideoDecoder-->>ConfigHelper: configured
ConfigHelper-->>Caller: return chosen candidate
else
ConfigHelper-->>ConfigHelper: try next
end
end
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lib/exporter/videoDecoderConfig.ts`:
- Around line 14-20: The prefersSoftwareDecode helper currently only matches the
exact "vp9" string; update it to recognize full VP9 codec identifiers (e.g.
"vp09.00.10.08") by testing the normalizedCodec against a prefix/regex for VP9
(e.g. /^vp09?/ or startsWith("vp9")/startsWith("vp09")), so it returns true for
both "vp9" and "vp09..." forms; apply the same change to the other occurrence
referenced around lines 27-29 and keep the existing AV1 checks unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d042dc4-b4d5-4d41-8f48-7f7cb7105358
📒 Files selected for processing (4)
src/lib/exporter/forwardFrameSource.tssrc/lib/exporter/streamingDecoder.tssrc/lib/exporter/videoDecoderConfig.test.tssrc/lib/exporter/videoDecoderConfig.ts
Author
|
Addressed the review feedback in
Validation:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a shared WebCodecs decoder configuration helper used by both the main streaming export decoder and the forward frame source.
The helper normalizes bare codec names returned by
web-demuxer:av01to a complete AV1 codec stringvp08tovp8vp09tovp9h264and bareavc1toavc1.640033It checks each candidate with
VideoDecoder.isConfigSupported()before configuration and falls back toavc1.640033when a malformed or unsupported AVC codec string is reported.Motivation
H.264 recordings in WebM containers can produce codec strings such as
h264,avc1, oravc1.2420032. Passing these directly to WebCodecs can fail withVideoDecoder error: Unknown or ambiguous codec name, preventing MP4 export.Type of Change
Related Issue(s)
Closes #674
Screenshots / Video
Not applicable; this changes decoder configuration and has unit-test coverage.
Testing Guide
npm test -- src/lib/exporter/videoDecoderConfig.test.ts src/lib/exporter/streamingDecoder.test.tsnpx biome check src/lib/exporter/videoDecoderConfig.ts src/lib/exporter/videoDecoderConfig.test.ts src/lib/exporter/streamingDecoder.ts src/lib/exporter/forwardFrameSource.tsFocused result: 17 tests passed and all changed files pass Biome.
Full-suite result: 778 of 779 tests passed. The remaining Windows helper-path test requires a staged native Windows binary and fails on the Linux checkout. The repository-wide TypeScript and Biome checks also report existing diagnostics in files outside this change.
Checklist
Summary by CodeRabbit
Refactor
Tests