feat: wire up voice enhance UI, fix denoise stereo bug - #9
Conversation
- add Enhance tab (upload+denoise a file) and per-clip Voice Enhance section (denoise a timeline video clip's own audio in place) - fix denoiseAudio() parsing raw WAV response as JSON - fix noisereduce channel-axis mismatch (soundfile gives (frames, channels), noisereduce expects (channels, frames)) causing 500 on stereo audio
There was a problem hiding this comment.
Summary
Reviewed — found 5 issues across 3 files. The PR wires up the voice enhance UI in a new EnhanceView tab and a EnhanceAudioSection in video properties, plus fixes a stereo denoising bug in the Python backend. The stereo fix is correct and the denoiseAudio return-type fix from DenoiseResult to Blob resolves a latent broken API call. Architecture conformance is good. All findings are low-severity — three object URL memory leaks, one dead type, and one stale callback closure.
Findings
apps/web/src/components/editor/panels/assets/views/enhance.tsx
Object URL memory leaks —
URL.createObjectURL()is called on lines 43 and 57 without correspondingURL.revokeObjectURL(). Additionally,handlePickFile(line 28) and the Remove button handler (line 100) nullifyresultUrlstate without revoking the old blob URL first. Blob URLs accumulate in browser memory until page navigation.getAudioDurationleaks Audio element — Lines 183–188 create a newAudioelement per call but never callremove()or setsrc = ""after the duration is resolved. The blob URL passed to it is also never revoked.handlePickFilestale closure — The empty dependency array[]on line 31 means the callback can't access the currentresultUrlvalue to revoke its blob URL. A ref should be used to track the active blob URL.
apps/web/src/components/editor/panels/properties/video-properties.tsx
- Object URL memory leak in
EnhanceAudioSection— Line 95 creates a blob URL viaURL.createObjectURL(denoisedBlob)that is stored as a timeline element'ssourceUrlbut is never revoked when the element is removed or the function is re-invoked.
apps/web/src/types/ai.ts
- Dead
DenoiseResulttype — Lines 95–98 defineDenoiseResultwhich has no remaining consumers afterdenoiseAudiowas changed to returnBlob. The adjacentDenoiseRequestinterface (lines 91–93) is also unused.
Verdict
getAudioDuration cleanup gap — should be addressed before merging to avoid memory accumulation in long SPA editing sessions.
| try { | ||
| const blob = await aiClient.denoiseAudio(file, strength); | ||
| setResultBlob(blob); | ||
| setResultUrl(URL.createObjectURL(blob)); |
There was a problem hiding this comment.
Object URL memory leak:URL.createObjectURL(blob) creates a new blob URL on every enhance call, but the previous blob URL (if any) is never revoked via URL.revokeObjectURL(). In a long SPA editing session, this accumulates memory that's only freed on page navigation.
Fix: Before calling setResultUrl(URL.createObjectURL(blob)), revoke the current resultUrl if it exists:
if(resultUrl)URL.revokeObjectURL(resultUrl);This requires either adding resultUrl to the useCallback deps (also fixing the stale-closure issue noted on line 28) or using a ref.
| const handleAddToTimeline = useCallback(async () => { | ||
| if (!resultBlob) return; | ||
| const outFile = new File([resultBlob], `enhanced_${Date.now()}.wav`, { type: "audio/wav" }); | ||
| const audioUrl = URL.createObjectURL(outFile); |
There was a problem hiding this comment.
Object URL memory leak:URL.createObjectURL(outFile) creates a blob URL stored as the timeline element's sourceUrl. If the user removes the element from the timeline, this blob URL is never revoked. Consider tracking timeline-inserted blob URLs and revoking them when the corresponding element is deleted.
| const handlePickFile = useCallback((f: File) => { | ||
| setFile(f); | ||
| setResultUrl(null); |
There was a problem hiding this comment.
Stale closure + missing revoke:handlePickFile has an empty dependency array [], so it can't access the currentresultUrl to call URL.revokeObjectURL() before nullifying it. When the user picks a new file, the previous enhanced preview's blob URL is abandoned without cleanup.
Fix: Use a useRef<string | null> to track the active blob URL so it's always reachable from this callback regardless of the closure.
constresultUrlRef=useRef<string|null>(null);// In handlePickFile:if(resultUrlRef.current)URL.revokeObjectURL(resultUrlRef.current);resultUrlRef.current=null;setResultUrl(null);// After creating URL in handleEnhance:resultUrlRef.current=url;setResultUrl(url);| className="text-[10px] text-destructive hover:text-destructive/80 shrink-0 ml-2" | ||
| onClick={() => { | ||
| setFile(null); | ||
| setResultUrl(null); |
There was a problem hiding this comment.
Object URL memory leak: The Remove button handler sets setResultUrl(null) without first revoking the old blob URL via URL.revokeObjectURL(). Same stale-closure issue as handlePickFile — the inline handler can't access the current resultUrl to revoke it. Use the same ref-based approach suggested for handlePickFile on line 28.
| ); | ||
| } | ||
| function getAudioDuration(url: string): Promise<number> { |
There was a problem hiding this comment.
Audio element leak:getAudioDuration creates a new Audio instance on every call but never releases it. On both loadedmetadata and error events, the audio element and its blob URL resource should be cleaned up:
functiongetAudioDuration(url: string): Promise<number>{returnnewPromise((resolve)=>{constaudio=newAudio(url);constcleanup=()=>{audio.removeEventListener("loadedmetadata",onLoaded);audio.removeEventListener("error",onError);audio.src="";audio.remove();if(url.startsWith("blob:"))URL.revokeObjectURL(url);};constonLoaded=()=>{constd=audio.duration;cleanup();resolve(d);};constonError=()=>{cleanup();resolve(5);};audio.addEventListener("loadedmetadata",onLoaded);audio.addEventListener("error",onError);});}| type: "audio/wav", | ||
| }); | ||
| const denoisedBlob = await aiClient.denoiseAudio(file, strength); | ||
| const audioUrl = URL.createObjectURL(denoisedBlob); |
There was a problem hiding this comment.
Object URL memory leak:URL.createObjectURL(denoisedBlob) creates a blob URL that's stored in the timeline element's sourceUrl but never revoked. If the user removes the audio track or re-runs enhance (which would replace the previous blob URL), memory accumulates.
Risk AssessmentThis PR adds a new Voice Enhance UI feature across two components and fixes a stereo denoising bug in the Python backend. The diff is focused (359 lines, 5 files), the fix is targeted and correct, and the new UI is additive with no changes to existing code paths except the Risk score: 2/10 (threshold: 3) — within the configured risk threshold Concerns
Verdict✅ Safe to ship. All findings are low-severity and the core logic (stereo fix, API wiring) is correct. Rollback is trivial — revert the PR with no data or migration impact. |
There was a problem hiding this comment.
Reviewed — the stereo fix is correct and the voice enhance UI wiring is solid. Five low-severity findings (object URL memory leaks, stale closure, dead type) noted inline but none are blockers. Risk score 2/10, within the configured threshold of 3. ✅ Approved.
Voice Enhance was backend-only (
/api/audio/denoise) with no UI, and had a bug that would've broken it if called anyway.denoiseAudio()parsing a raw WAV response as JSON