refactor(web): add runtime audio encoder capability negotiation for mp4 exports#757
Conversation
…p4 exports Introduce a dedicated utility that probes browser support for AAC/WebCodecs audio settings instead of assuming a single hardcoded configuration (`mp4a.40.2`, 192 kbps, stereo, 44.1 kHz). This file should centralize support detection and fallback selection so ChromeOS/Chrome variants that reject the current config can still export with audio. Affected files: audio-codec-support.ts Signed-off-by: ChinhLee <76194645+chinhkrb113@users.noreply.github.com>
|
@chinhkrb113 is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA new utility module was added that exports a function to dynamically discover supported MP4 audio encoder configurations. The function iterates through candidate AAC codec configurations and uses the Web Audio API's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/lib/export/audio-codec-support.ts (2)
1-26: Prefersatisfiesover explicit literal type annotation (Line 1).
const CANDIDATE_CONFIGS: AudioEncoderConfig[] = ...adds a redundant annotation on a literal-initialized variable. Usesatisfiesto keep structural validation while preserving better literal inference.♻️ Proposed change
-const CANDIDATE_CONFIGS: AudioEncoderConfig[] = [ +const CANDIDATE_CONFIGS = [ { codec: "mp4a.40.2", sampleRate: 44100, numberOfChannels: 2, bitrate: 192000, }, { codec: "mp4a.40.2", sampleRate: 44100, numberOfChannels: 2, bitrate: 128000, }, { codec: "mp4a.40.2", sampleRate: 44100, numberOfChannels: 1, bitrate: 128000, }, { codec: "mp4a.40.2", sampleRate: 48000, numberOfChannels: 2, bitrate: 128000, }, -]; +] satisfies AudioEncoderConfig[];As per coding guidelines, "Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/export/audio-codec-support.ts` around lines 1 - 26, The CANDIDATE_CONFIGS constant is annotated with an explicit type (AudioEncoderConfig[]) which is redundant for a literal-initialized value; change the declaration to use the TypeScript `satisfies` operator so the array still validates against AudioEncoderConfig but preserves better literal type inference (replace the explicit type annotation on CANDIDATE_CONFIGS with a `satisfies AudioEncoderConfig[]` form while keeping the same array contents).
39-40: Remove redundantcontinueincatch(Line 40).
continueis unnecessary here because the loop naturally proceeds to the next iteration after thecatchblock ends.🧹 Proposed simplification
} catch { - continue; + // try next candidate }As per coding guidelines, "Don't use unnecessary continue statements".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/export/audio-codec-support.ts` around lines 39 - 40, Remove the redundant "continue" inside the catch block of the loop that iterates over codecs (the for/while loop containing the try { ... } catch { continue; } pattern) in audio-codec-support.ts; simply delete the "continue;" so the catch block is empty (or only handles logging if needed) and allow the loop to naturally proceed to the next iteration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/lib/export/audio-codec-support.ts`:
- Around line 28-45: Replace the hardcoded AAC probe in the MP4 export flow with
the new helper: call resolveSupportedMp4AudioConfig() from the export logic (the
place currently calling AudioEncoder.isConfigSupported with codec "mp4a.40.2"
and sampleRate/numberOfChannels) and use the returned AudioEncoderConfig (or
null) instead of the inline probe; if it returns null, proceed with the existing
fallback/no-audio path. Update the code path that currently expects { supported
} from AudioEncoder.isConfigSupported to await resolveSupportedMp4AudioConfig()
and branch on its result so codec negotiation is centralized.
---
Nitpick comments:
In `@apps/web/src/lib/export/audio-codec-support.ts`:
- Around line 1-26: The CANDIDATE_CONFIGS constant is annotated with an explicit
type (AudioEncoderConfig[]) which is redundant for a literal-initialized value;
change the declaration to use the TypeScript `satisfies` operator so the array
still validates against AudioEncoderConfig but preserves better literal type
inference (replace the explicit type annotation on CANDIDATE_CONFIGS with a
`satisfies AudioEncoderConfig[]` form while keeping the same array contents).
- Around line 39-40: Remove the redundant "continue" inside the catch block of
the loop that iterates over codecs (the for/while loop containing the try { ...
} catch { continue; } pattern) in audio-codec-support.ts; simply delete the
"continue;" so the catch block is empty (or only handles logging if needed) and
allow the loop to naturally proceed to the next iteration.
🪄 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: 3cf63b8a-3467-440d-ab4f-fdb76f444a08
📒 Files selected for processing (1)
apps/web/src/lib/export/audio-codec-support.ts
| export async function resolveSupportedMp4AudioConfig(): Promise<AudioEncoderConfig | null> { | ||
| if (typeof AudioEncoder === "undefined") { | ||
| return null; | ||
| } | ||
|
|
||
| for (const config of CANDIDATE_CONFIGS) { | ||
| try { | ||
| const support = await AudioEncoder.isConfigSupported(config); | ||
| if (support.supported) { | ||
| return config; | ||
| } | ||
| } catch { | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify helper adoption and detect remaining hardcoded probing in exporter flow.
set -euo pipefail
echo "== locate new helper usages =="
rg -nP --type=ts '\bresolveSupportedMp4AudioConfig\s*\(' -C2
echo
echo "== locate direct AudioEncoder probing still in renderer/export paths =="
rg -nP --type=ts '\bAudioEncoder\.isConfigSupported\s*\(' -C3 apps/web/src/services/renderer/scene-exporter.ts apps/web/src/lib/export/audio-codec-support.ts || true
echo
echo "Expected:"
echo "1) scene-exporter path references resolveSupportedMp4AudioConfig()."
echo "2) hardcoded AAC probe logic is removed or delegated to the utility."Repository: OpenCut-app/OpenCut
Length of output: 1930
Integrate the new helper into the MP4 export path.
The resolveSupportedMp4AudioConfig() function is defined but never called. scene-exporter.ts (lines 107–110) still contains the hardcoded AAC probe logic that should delegate to this utility:
const { supported } = await AudioEncoder.isConfigSupported({
codec: "mp4a.40.2",
sampleRate: this.audioBuffer.sampleRate,
numberOfChannels: this.audioBuffer.numberOfChannels,Replace this with resolveSupportedMp4AudioConfig() to centralize codec negotiation and achieve the PR's runtime-fallback objective.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/export/audio-codec-support.ts` around lines 28 - 45, Replace
the hardcoded AAC probe in the MP4 export flow with the new helper: call
resolveSupportedMp4AudioConfig() from the export logic (the place currently
calling AudioEncoder.isConfigSupported with codec "mp4a.40.2" and
sampleRate/numberOfChannels) and use the returned AudioEncoderConfig (or null)
instead of the inline probe; if it returns null, proceed with the existing
fallback/no-audio path. Update the code path that currently expects { supported
} from AudioEncoder.isConfigSupported to await resolveSupportedMp4AudioConfig()
and branch on its result so codec negotiation is centralized.
1 participant
Code Quality
Problem
Introduce a dedicated utility that probes browser support for AAC/WebCodecs audio settings instead of assuming a single hardcoded configuration (
mp4a.40.2, 192 kbps, stereo, 44.1 kHz). This file should centralize support detection and fallback selection so ChromeOS/Chrome variants that reject the current config can still export with audio.Severity:
highFile:
apps/web/src/lib/export/audio-codec-support.tsSolution
Implement a
resolveSupportedMp4AudioConfig()function that:Changes
apps/web/src/lib/export/audio-codec-support.ts(new)We are not currently accepting PRs except for critical bugs.
If this is a bug fix:
If this is a feature:
This PR will be closed. Please open an issue to discuss first.Contributed by Lê Thành Chỉnh
Code is a tool. Mindset is the real value.
Closes #718
Summary by CodeRabbit