fix: handle av1 VideoDecoder errors#334
fix: handle av1 VideoDecoder errors#334siddharthvaddem merged 2 commits intosiddharthvaddem:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded logic to parse AV1CodecConfigurationRecord bytes into a full WebCodecs AV1 codec string and updated StreamingVideoDecoder.decodeAll() to replace bare Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21361d9bf8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/exporter/streamingDecoder.ts (1)
33-49: Add basic AV1 field sanity checks before building codec stringConsider falling back when parsed values are out of AV1-valid ranges (e.g.,
profile > 2orlevel > 23) to avoid emitting impossible codec IDs from malformed/non-av1C extradata.Suggested hardening
const profile = (bytes[1] >> 5) & 0x07; const level = bytes[1] & 0x1f; + if (profile > 2 || level > 23) return fallback;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/exporter/streamingDecoder.ts` around lines 33 - 49, The code builds an av01 codec string from parsed bytes but lacks sanity checks; add validation of the parsed fields (profile, level, tier, highBitdepth/twelveBit -> bitdepth) before composing the string: ensure profile is <= 2, level is within AV1 levels (0–23), tier is 0 or 1, and bitdepth is one of 8/10/12; if any check fails, return a safe fallback (e.g., a fixed safe codec id or empty string) instead of emitting impossible values. Locate the logic around the variables profile, level, tier, highBitdepth, twelveBit, and bitdepth in the function that returns the `av01.${profile}...` string and implement the described guard checks and fallback return.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/exporter/streamingDecoder.ts`:
- Around line 19-21: The current bytes extraction drops ArrayBufferView offsets;
update the bytes construction to preserve view offsets by checking if
description is an ArrayBuffer (create a Uint8Array over the whole buffer) or an
ArrayBufferView (use new Uint8Array(description.buffer, description.byteOffset,
description.byteLength)); adjust the variable bytes in streamingDecoder.ts (the
code that currently creates bytes from description) to use this view-aware logic
so parsing uses the correct slice and byte positions.
---
Nitpick comments:
In `@src/lib/exporter/streamingDecoder.ts`:
- Around line 33-49: The code builds an av01 codec string from parsed bytes but
lacks sanity checks; add validation of the parsed fields (profile, level, tier,
highBitdepth/twelveBit -> bitdepth) before composing the string: ensure profile
is <= 2, level is within AV1 levels (0–23), tier is 0 or 1, and bitdepth is one
of 8/10/12; if any check fails, return a safe fallback (e.g., a fixed safe codec
id or empty string) instead of emitting impossible values. Locate the logic
around the variables profile, level, tier, highBitdepth, twelveBit, and bitdepth
in the function that returns the `av01.${profile}...` string and implement the
described guard checks and fallback return.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e44705d4-d79b-4f46-9017-28a73c2c7d06
📒 Files selected for processing (1)
src/lib/exporter/streamingDecoder.ts
Description
Fix AV1 VideoDecoder error during export when recordings use AV1 in WebM containers (the default on Linux). When
web-demuxerreturns a bare "av01" codec string, we now parse theAV1CodecConfigurationRecordfrom the extradata to build the full WebCodecs-compatible codec string (e.g. "av01.0.09M.08").Motivation
My export kept crashing 😭. Chrome/Electron's
MediaRecorderwrites AV1 WebM files with anAV1CodecConfigurationRecordwhose version field is 127 (0xFFfirst byte) instead of the spec-mandated 1 (0x81). Theweb-demuxerlibrary's WASM-side parser (set_av1_codec_string) strictly checksversion != 1and bails early, returning a bare "av01" string rather than the full parametrized form. VideoDecoder.configure() then rejects it with "Unknown or ambiguous codec name", making all AV1 exports fail.extradata: FF 09 0C 00 ↓ web-demuxer: version = 0xFF & 0x7F = 127 ≠ 1 → early return → "av01" ↓ VideoDecoder.configure({ codec: "av01" }) → "Unknown or ambiguous codec name"When
getDecoderConfig()returns a bare "av01", we now parse the extradata ourselves, only requiring the marker bit to be set rather than a strict version check. For the example above this produces "av01.0.09M.08" (Main profile, level 3.1, Main tier, 8-bit). Falls back to "av01.0.01M.08" if extradata is missing or too short.Type of Change
Related Issue(s)
None. Discovered while packaging for NixOS.
Screenshots / Video
N/A: no UI changes. The fix is in the export pipeline (
streamingDecoder.ts).Testing
Before fix: Export fails with "VideoDecoder Error: Unknown or ambiguous codec name"
After fix: Export completes successfully
You can verify your recording is AV1/WebM with:
ffprobe -v warning -select_streams v:0 -show_entries stream=codec_name ~/.config/openscreen/recordings/<your-recording>.webmIf you see
Unknown version 127 of AV1CodecConfigurationRecord found!in the warnings, your file is affected by this bug.Checklist
Summary by CodeRabbit