fix(recording): prefer playable browser capture codecs#275
Closed
meiiie wants to merge 2 commits intowebadderallorg:mainfrom
Closed
fix(recording): prefer playable browser capture codecs#275meiiie wants to merge 2 commits intowebadderallorg:mainfrom
meiiie wants to merge 2 commits intowebadderallorg:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughNew MIME-type selection logic was extracted into Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as "useScreenRecorder\n(hook)"
participant Selector as "selectRecordingMimeType\n(utility)"
participant RecorderAPI as "MediaRecorder.isTypeSupported\n(recorder probe)"
participant Player as "HTMLVideoElement.canPlayType\n(playback probe)"
Hook->>Selector: request mimeType
Selector->>RecorderAPI: probe supported types
RecorderAPI-->>Selector: supported list
Selector->>Player: probe playback for candidates
Player-->>Selector: canPlayType results
Selector-->>Hook: selected mimeType | undefined
Hook->>MediaRecorder: new MediaRecorder(stream, { ...mimeType? })
note right of MediaRecorder: recording starts with chosen mimeType
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 2
🤖 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/hooks/recordingMimeType.test.ts`:
- Around line 24-36: The test currently makes vp9 both supported and playable so
it doesn't verify skipping an unplayable higher-preference codec; change the
mocked probes passed to selectRecordingMimeType so the first supported
preference (e.g., "video/webm;codecs=av1") is supported by isTypeSupported but
returns an empty string from canPlayType (unplayable), while a lower-preference
codec like "video/webm;codecs=vp9" returns "probably"; update the
isTypeSupported and canPlayType lambdas accordingly and keep the assertion
expect(mimeType).toBe("video/webm;codecs=vp9").
In `@src/hooks/recordingMimeType.ts`:
- Around line 25-30: The hook currently falls back to
FALLBACK_RECORDING_MIME_TYPE even when RECORDING_MIME_TYPE_PREFERENCES are all
rejected by isTypeSupported(), which can pass a rejected mimeType to new
MediaRecorder; change the hook so it returns undefined when no preference is
actually supported (i.e., return playableType ?? supportedTypes[0] ?? undefined)
instead of FALLBACK_RECORDING_MIME_TYPE, and update callers in
useScreenRecorder.ts where new MediaRecorder(..., { mimeType }) is constructed
to only include mimeType in MediaRecorderOptions when the returned value is
defined (or throw a controlled error before constructing the recorder if you
prefer explicit failure).
🪄 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
Run ID: 22fd8bea-0fb1-45d6-9ab3-458edecfd765
📒 Files selected for processing (3)
src/hooks/recordingMimeType.test.tssrc/hooks/recordingMimeType.tssrc/hooks/useScreenRecorder.ts
Comment on lines
+24
to
+36
| it("skips recorder-only codecs when playback support is missing", () => { | ||
| const mimeType = selectRecordingMimeType({ | ||
| isTypeSupported: (type) => | ||
| [ | ||
| "video/webm;codecs=av1", | ||
| "video/webm;codecs=h264", | ||
| "video/webm;codecs=vp9", | ||
| ].includes(type), | ||
| canPlayType: (type) => (type === "video/webm;codecs=vp9" ? "probably" : ""), | ||
| }); | ||
|
|
||
| expect(mimeType).toBe("video/webm;codecs=vp9"); | ||
| }); |
Contributor
There was a problem hiding this comment.
Make this test actually exercise skipping an unplayable higher-preference codec.
As written, vp9 is both supported and playable, so a broken implementation that ignores playback probing would still pass. Make the first supported preference unplayable and assert that selection falls through.
🧪 Proposed test adjustment
it("skips recorder-only codecs when playback support is missing", () => {
const mimeType = selectRecordingMimeType({
isTypeSupported: (type) =>
[
- "video/webm;codecs=av1",
- "video/webm;codecs=h264",
"video/webm;codecs=vp9",
+ "video/webm;codecs=vp8",
].includes(type),
- canPlayType: (type) => (type === "video/webm;codecs=vp9" ? "probably" : ""),
+ canPlayType: (type) => (type === "video/webm;codecs=vp8" ? "probably" : ""),
});
- expect(mimeType).toBe("video/webm;codecs=vp9");
+ expect(mimeType).toBe("video/webm;codecs=vp8");
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it("skips recorder-only codecs when playback support is missing", () => { | |
| const mimeType = selectRecordingMimeType({ | |
| isTypeSupported: (type) => | |
| [ | |
| "video/webm;codecs=av1", | |
| "video/webm;codecs=h264", | |
| "video/webm;codecs=vp9", | |
| ].includes(type), | |
| canPlayType: (type) => (type === "video/webm;codecs=vp9" ? "probably" : ""), | |
| }); | |
| expect(mimeType).toBe("video/webm;codecs=vp9"); | |
| }); | |
| it("skips recorder-only codecs when playback support is missing", () => { | |
| const mimeType = selectRecordingMimeType({ | |
| isTypeSupported: (type) => | |
| [ | |
| "video/webm;codecs=vp9", | |
| "video/webm;codecs=vp8", | |
| ].includes(type), | |
| canPlayType: (type) => (type === "video/webm;codecs=vp8" ? "probably" : ""), | |
| }); | |
| expect(mimeType).toBe("video/webm;codecs=vp8"); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/recordingMimeType.test.ts` around lines 24 - 36, The test currently
makes vp9 both supported and playable so it doesn't verify skipping an
unplayable higher-preference codec; change the mocked probes passed to
selectRecordingMimeType so the first supported preference (e.g.,
"video/webm;codecs=av1") is supported by isTypeSupported but returns an empty
string from canPlayType (unplayable), while a lower-preference codec like
"video/webm;codecs=vp9" returns "probably"; update the isTypeSupported and
canPlayType lambdas accordingly and keep the assertion
expect(mimeType).toBe("video/webm;codecs=vp9").
webadderall
added a commit
that referenced
this pull request
Apr 19, 2026
Verifies the selected recording MIME type can also be decoded by the in-app <video> element via canPlayType(), so that codecs like av1 — which MediaRecorder.isTypeSupported() reports as supported for encoding but are not decodable on fresh Windows installs without the AV1 Video Extension — are not chosen. Also passes undefined (not an unsupported fallback string) into new MediaRecorder() when no preferred type is supported, letting the browser pick a default instead of throwing NotSupportedError. Replaces the previous 'as ProcessedDesktopSource' cast with a properly typed LINUX_PORTAL_SOURCE constant. Co-authored-by: meiiie <meiiie@users.noreply.github.com>
Collaborator
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
2 participants
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
Fix browser-based recording sessions that save successfully but fail to open in the editor with "Failed to load video".
Motivation
The browser capture path could prefer MIME types that
MediaRecorderaccepts but the in-app player cannot reliably decode, and it could still pass an explicitly unsupported fallback MIME type intonew MediaRecorder(...)when probing found no supported preference.Type of Change
Related Issue(s)
No tracked issue was linked for this bug.
Screenshots / Video
N/A for this bugfix-only change.
Testing Guide
npx vitest --run src/hooks/recordingMimeType.test.ts src/hooks/useScreenRecorder.test.ts.npx tsc --noEmit.Checklist