feat: built-in video recording for demo showcase (Sprint 1+2 MVP)#1183
feat: built-in video recording for demo showcase (Sprint 1+2 MVP)#1183
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an in-app video recording feature that captures the current browser tab (video) and the DAW master output (audio) to produce a downloadable WebM, with UI controls and a post-recording export dialog.
Changes:
- Added an
AudioEnginemaster-outputMediaStreamtap for recording. - Introduced
VideoRecorderServiceto orchestrate tab capture + audio merge +MediaRecorderlifecycle. - Integrated recording state/actions into
uiStore, added a toolbar record control, and a WebM preview/download dialog.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/engine/AudioEngine.ts |
Adds getAudioStream() and disposeAudioStream() to tap/dispose master output for recording. |
src/services/videoRecorder.ts |
New service for capture + encoding + duration/state tracking. |
src/store/uiStore.ts |
Adds recording state/actions and wires UI state to the service. |
src/components/layout/Toolbar.tsx |
Adds a video record button with requesting/recording UI. |
src/components/dialogs/VideoExportDialog.tsx |
New dialog to preview and download the recorded WebM. |
src/components/layout/AppShell.tsx |
Lazy-loads and renders VideoExportDialog. |
tests/unit/videoRecorder.test.ts |
New unit tests for recorder state machine, streams, and duration tracking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/components/layout/Toolbar.tsx
Outdated
| return ( | ||
| <button | ||
| onClick={handleClick} | ||
| disabled={isRequesting} | ||
| title={isRecording ? `Stop Video Recording (${formatDuration(duration)})` : 'Record Video'} | ||
| aria-label={isRecording ? 'Stop video recording' : 'Record video'} | ||
| data-testid="video-record-button" | ||
| className={`flex h-10 items-center justify-center gap-1.5 rounded-lg px-2 transition-all duration-150 active:scale-95 ${ | ||
| isRecording | ||
| ? 'bg-red-600/90 text-white hover:bg-red-500' | ||
| : isRequesting | ||
| ? 'bg-transparent text-white/50 cursor-wait' | ||
| : 'bg-transparent text-white/90 hover:bg-white/8 hover:text-white' | ||
| }`} |
There was a problem hiding this comment.
The record button is only disabled while requesting/stopping. In unsupported browsers, it remains enabled and only surfaces an error after click via the store. If the intended UX is “disabled on unsupported browsers with explanatory tooltip” (per PR description), you’ll need to expose a precomputed isVideoRecordingSupported flag (or similar) and use it here to disable the button + set an appropriate title/tooltip before interaction.
| {/* Info */} | ||
| <div className="flex items-center gap-4 text-xs text-zinc-400"> | ||
| <span>Duration: {formatDuration(duration)}</span> | ||
| {blob && <span>Size: {formatFileSize(blob.size)}</span>} | ||
| <span>Format: WebM (VP9 + Opus)</span> | ||
| </div> |
There was a problem hiding this comment.
The dialog hard-codes Format: WebM (VP9 + Opus), but the recorder can fall back to VP8/H.264 or plain video/webm depending on MediaRecorder.isTypeSupported. Consider deriving the displayed format from blob.type (or store the chosen mimeType in videoRecording state) to avoid misleading users.
| const date = now.toISOString().slice(0, 10); | ||
| const time = `${now.getHours().toString().padStart(2, '0')}-${now.getMinutes().toString().padStart(2, '0')}`; |
There was a problem hiding this comment.
buildFileName() mixes a UTC date (toISOString().slice(0, 10)) with local getHours()/getMinutes(), which can produce mismatched date/time around midnight (and can collide within the same minute). Consider formatting the date using local components as well (and optionally include seconds) to keep filenames consistent and unique.
| const date = now.toISOString().slice(0, 10); | |
| const time = `${now.getHours().toString().padStart(2, '0')}-${now.getMinutes().toString().padStart(2, '0')}`; | |
| const year = now.getFullYear(); | |
| const month = (now.getMonth() + 1).toString().padStart(2, '0'); | |
| const day = now.getDate().toString().padStart(2, '0'); | |
| const date = `${year}-${month}-${day}`; | |
| const hours = now.getHours().toString().padStart(2, '0'); | |
| const minutes = now.getMinutes().toString().padStart(2, '0'); | |
| const seconds = now.getSeconds().toString().padStart(2, '0'); | |
| const time = `${hours}-${minutes}-${seconds}`; |
tests/unit/videoRecorder.test.ts
Outdated
| // Simulate a data chunk then stop | ||
| this.ondataavailable?.({ data: new Blob(['fake'], { type: 'video/webm' }) }); | ||
| this.state = 'inactive'; | ||
| this.onstop?.(); |
There was a problem hiding this comment.
FakeMediaRecorder.stop() triggers onstop synchronously. That makes the “stops duration timer on stopRecording” test pass even if stopRecording() itself doesn’t clear the interval (because _cleanup() runs immediately via onstop). Consider making stop() fire onstop asynchronously (e.g. next tick) in at least one test, so you can assert duration freezes immediately upon calling stopRecording(), matching real MediaRecorder behavior.
| this.onstop?.(); | |
| // Fire onstop asynchronously to better match real MediaRecorder behavior | |
| setTimeout(() => { | |
| this.onstop?.(); | |
| }, 0); |
| stopRecording(): void { | ||
| if (this._state.status !== 'recording') return; | ||
| this._setState({ status: 'stopping' }); | ||
| this._recorder?.stop(); | ||
| } |
There was a problem hiding this comment.
stopRecording() sets status to stopping but leaves the duration interval running until onstop triggers _cleanup(), so duration can continue increasing during the stopping phase (and potentially indefinitely if onstop never fires). Consider clearing the duration timer immediately when stop is requested, and (optionally) stopping display tracks at the same time so the browser sharing indicator goes away promptly.
src/services/videoRecorder.ts
Outdated
| // Stop all display tracks (removes browser "sharing" indicator) | ||
| this._displayStream?.getTracks().forEach((t) => t.stop()); | ||
| this._displayStream = null; | ||
| this._recorder = null; |
There was a problem hiding this comment.
_cleanup() nulls _recorder without calling MediaRecorder.stop() (and dismiss() can call _cleanup() while recording). This can leave the recorder running and/or fail to flush the final dataavailable chunk. Consider stopping the recorder when it’s active before dropping the reference, and ensure _chunks is cleared to avoid retaining large buffers after a recording completes.
| // Stop all display tracks (removes browser "sharing" indicator) | |
| this._displayStream?.getTracks().forEach((t) => t.stop()); | |
| this._displayStream = null; | |
| this._recorder = null; | |
| if (this._recorder) { | |
| const state = this._recorder.state; | |
| // Ensure the MediaRecorder is stopped so the final `dataavailable` event can fire. | |
| if (state === 'recording' || state === 'paused') { | |
| try { | |
| this._recorder.stop(); | |
| } catch { | |
| // Ignore errors if the recorder is already inactive or cannot be stopped. | |
| } | |
| } | |
| this._recorder = null; | |
| } | |
| // Stop all display tracks (removes browser "sharing" indicator) | |
| this._displayStream?.getTracks().forEach((t) => t.stop()); | |
| this._displayStream = null; | |
| // Release any buffered recorded data to avoid retaining large blobs in memory. | |
| if (this._chunks) { | |
| this._chunks = []; | |
| } |
src/store/uiStore.ts
Outdated
| const engine = (window as any).__getAudioEngine?.(); | ||
| if (!engine) { | ||
| set({ videoRecording: { status: 'error', duration: 0, blob: null, error: 'Audio engine is not initialized.' } }); | ||
| return; | ||
| } | ||
| const recorder = new VideoRecorderService(); | ||
| (window as any).__videoRecorder = recorder; | ||
| recorder.onStateChange = (state) => set({ videoRecording: { ...state } }); | ||
| await recorder.startRecording(engine.getAudioStream()); | ||
| }, | ||
| stopVideoRecording: () => { | ||
| const recorder = (window as any).__videoRecorder as import('../services/videoRecorder').VideoRecorderService | undefined; | ||
| recorder?.stopRecording(); | ||
| }, | ||
| dismissVideoRecording: () => { | ||
| const recorder = (window as any).__videoRecorder as import('../services/videoRecorder').VideoRecorderService | undefined; | ||
| recorder?.dismiss(); | ||
| (window as any).__videoRecorder = undefined; | ||
| set({ videoRecording: { status: 'idle', duration: 0, blob: null, error: null } }); |
There was a problem hiding this comment.
startVideoRecording/stopVideoRecording/dismissVideoRecording rely on window as any and introduce a new window.__videoRecorder global. Since Window is already typed via src/globals.d.ts/DAWGlobals (and __getAudioEngine is declared there), it would be better to avoid any and either (a) keep the recorder instance in the store/module scope, or (b) formally add __videoRecorder to the DAWGlobals typing if it must be a global. This will prevent type-safety regressions and accidental global leakage.
src/store/uiStore.ts
Outdated
| recorder?.stopRecording(); | ||
| }, | ||
| dismissVideoRecording: () => { | ||
| const recorder = (window as any).__videoRecorder as import('../services/videoRecorder').VideoRecorderService | undefined; | ||
| recorder?.dismiss(); |
There was a problem hiding this comment.
engine.getAudioStream() creates a persistent MediaStreamAudioDestinationNode tap, but the store never calls engine.disposeAudioStream() when recording finishes or is dismissed. This leaves the extra node/stream connected for the lifetime of the app after the first recording. Consider disposing the audio stream tap after stopRecording() completes (or at least in dismissVideoRecording()).
| recorder?.stopRecording(); | |
| }, | |
| dismissVideoRecording: () => { | |
| const recorder = (window as any).__videoRecorder as import('../services/videoRecorder').VideoRecorderService | undefined; | |
| recorder?.dismiss(); | |
| recorder?.stopRecording(); | |
| const engine = (window as any).__getAudioEngine?.(); | |
| engine?.disposeAudioStream?.(); | |
| }, | |
| dismissVideoRecording: () => { | |
| const recorder = (window as any).__videoRecorder as import('../services/videoRecorder').VideoRecorderService | undefined; | |
| recorder?.dismiss(); | |
| const engine = (window as any).__getAudioEngine?.(); | |
| engine?.disposeAudioStream?.(); |
…, #1170, #1171) Core infrastructure for built-in video recording: - AudioEngine.getAudioStream(): taps master output via MediaStreamDestinationNode - VideoRecorderService: orchestrates Tab Capture + AudioContext into WebM via MediaRecorder - 16 unit tests covering state machine, stream management, duration tracking, and feature detection Part of #1168 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…export dialog (#1172, #1173, #1174) Sprint 2 UI integration for built-in video recording: - uiStore: videoRecording state with start/stop/dismiss actions - Toolbar: video record button with red pulsing indicator and timer - VideoExportDialog: preview recorded video, download as WebM - Lazy-loaded dialog in AppShell for code splitting Part of #1168 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Hide record button on unsupported browsers (feature detection) - Derive displayed format from actual mimeType instead of hardcoding - Fix buildFileName UTC/local date mismatch, add seconds for uniqueness - Stop MediaRecorder in _cleanup() to flush final dataavailable chunk - Clear duration timer immediately in stopRecording (not just on onstop) - Replace window.__videoRecorder global with module-scope variable - Dispose AudioEngine media stream tap on stop/dismiss (prevent leak) - Make test's FakeMediaRecorder.stop() async to match real behavior - Add mimeType field to VideoRecorderState Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… and keyboard shortcut (#1175, #1176, #1179, #1181, #1182) Sprint 3+4 enhancements for video recording: - Microphone overlay: mix mic audio with DAW output via Web Audio API - Quality presets: low/medium/high bitrate selection (persisted) - Recording settings popover: right-click video button for mic/quality config - Cursor highlight: blue glow follows cursor during recording - Click ripple animations: expanding ring on every click - Branding watermark: ACE-Step DAW logo overlay (bottom-right) - Keyboard shortcut: Cmd+Shift+V toggles video recording Part of #1168 Closes #1175, closes #1176, closes #1179, closes #1181, closes #1182 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…, cursor perf Critical fixes: - Remove disposeAudioStream() from stopVideoRecording — onstop fires async and needs the stream alive to flush the final audio chunk. Only dispose on dismiss. - Reuse a shared AudioContext for mic mixing instead of creating/closing one per recording (avoids hitting browser's ~6 context limit). Performance: - Cursor glow now uses ref + direct DOM style updates instead of React setState at 60fps, eliminating unnecessary reconciliation. Cleanup: - Extract _getAudioEngine() typed helper — removes 3 inline type casts. - Extract formatDurationMSS() into shared utils/time.ts — removes duplication between Toolbar and VideoExportDialog. - Fix step numbering comments in videoRecorder.ts (3→4→5). - Cap rippleIdCounter with modular arithmetic to prevent overflow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
#1180) - TrimBar component: draggable start/end handles with greyed-out regions - Video preview constrained to trim region during playback - Format selector: WebM (native) or MP4 (H.264 + AAC) - ffmpeg.wasm loaded lazily on first MP4 export (~25MB, cached) - Progress bar during conversion with percentage display - Trim applied during conversion (ffmpeg -ss/-t for accurate cuts) - Convert & Download button when trim or MP4 is selected Part of #1168 Closes #1178, closes #1180 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8c84973 to
0eb087b
Compare
…oding Root cause: ffmpeg.wasm VP9→H.264 re-encoding is ~50x slower than native, making MP4 export unusable even on high-end hardware. Fix: - Add MP4 MIME types to MediaRecorder preference list (preferred over WebM) - Chrome 107+ records directly as MP4 (H.264+AAC) — no conversion needed - Remove format selector — recording format auto-selects the best available - ffmpeg.wasm now only used for trim (stream copy, no re-encoding = fast) - Falls back to WebM on older browsers that don't support MP4 recording Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three fixes from user testing: 1. Trim laggy: TrimBar useEffect ran without deps, re-registering mousemove listeners every render. Fixed with stable refs + single persistent listener pair registered once on mount. 2. Default WebM: Reverted MP4 as preferred MIME type back to WebM. MP4 MediaRecorder in Chrome has issues with dynamic content (scrolling not captured). MP4 offered as optional export format. 3. Scroll not recorded with MP4: Chrome's MP4 MediaRecorder produces glitchy output with rapid frame changes. WebM VP9 handles this correctly. MP4 conversion now uses ultrafast preset + higher CRF for acceptable speed in WASM. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… conditions When user clicks the browser's "Stop sharing" button: - Track ends immediately, potentially before MediaRecorder processes last chunk - MediaRecorder may auto-stop, causing onstop to fire before our stopRecording() Fixes: - Call requestData() before stop() to flush buffered data - Add _finalizeBlobAndDone() fallback for when recorder already auto-stopped - Guard onstop/onerror against duplicate firing (race between auto-stop and manual stop) - Track ended handler now manages its own stop flow with proper error recovery - Empty blob detection: show error instead of black screen Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…er lifecycle Root cause: recorder.start(1000) timeslice mode produces fragmented WebM clusters. When recording is abruptly terminated via browser's "Stop sharing" button, the last cluster is incomplete, corrupting the entire WebM file and causing the video preview to show a black screen. Fix: - Use recorder.start() without timeslice — produces a single complete blob when stop() is called, guaranteed to be valid - Remove manual track.ended handler — Chrome auto-fires onstop when the display track ends from "Stop sharing", which is the correct behavior - Add _stopped flag to prevent duplicate onstop/onerror processing - Simplify _cleanup into _cleanupStreams (only handles streams, not recorder) - onstop handler now checks blob.size and shows error for empty recordings Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rder.stop() Root cause found: commit 4569d60 removed the track.ended handler that calls stopRecording() when the user clicks "Stop sharing". Without it, MediaRecorder.stop() is never called, ondataavailable never fires, and the blob is empty → black screen. The timeslice removal was correct (prevents corrupt WebM clusters), but the track ended handler is essential — it's the only way to trigger stop() when the browser kills the display stream externally. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tDialog Root cause: useCallback(handleTrimChange) was placed AFTER the early return `if (!show) return null`, violating React Rules of Hooks. When the dialog transitioned from hidden to visible, React detected more hooks than the previous render and threw an error, crashing the component. Fix: move useCallback above the early return so all hooks are called unconditionally on every render. Also restores videoRecorder.ts to the proven working version with recorder.start(1000) timeslice. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Root cause of black screen: useCallback placed after early return violated React Rules of Hooks — component crashed on first show. Also removes ffmpeg.wasm-based MP4 export which was unusable: - 25MB WASM binary load time - VP9→H.264 transcode ~50x slower than native - Blocked UI with "Loading processor..." indefinitely VideoExportDialog now: preview + trim (100ms) + direct WebM download. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
AudioContext(perfect quality, zero system audio routing needed)getDisplayMedia)Changes
Sprint 1 — Core Infrastructure
AudioEngine.getAudioStream()— taps master output viaMediaStreamAudioDestinationNodeVideoRecorderService— orchestrates Tab Capture + AudioContext →MediaRecorder→ WebMSprint 2 — UI Integration
uiStore.videoRecordingstate management with start/stop/dismiss actionsVideoExportDialog— preview recorded video + download as WebMAppShellfor code splittingFiles Changed
src/engine/AudioEngine.tsgetAudioStream()+disposeAudioStream()src/services/videoRecorder.tssrc/store/uiStore.tssrc/components/layout/Toolbar.tsxVideoRecordButtoncomponentsrc/components/dialogs/VideoExportDialog.tsxsrc/components/layout/AppShell.tsxVideoExportDialogtests/unit/videoRecorder.test.tsTest plan
npx tsc --noEmit— 0 type errorsnpm test— 3291 tests pass (including 16 new)npm run build— succeedsCloses #1169, closes #1170, closes #1171, closes #1172, closes #1173, closes #1174
Part of #1168
🤖 Generated with Claude Code