From d5ab86ea44baea5348b2b911aad6eb773187285a Mon Sep 17 00:00:00 2001 From: Mahdy Arief Date: Sat, 28 Mar 2026 06:09:50 +0700 Subject: [PATCH] fix(export, native, ui): resolve critical data integrity and resource management bugs in PR #122 - Implement comprehensive solo/mute export pipeline in audioEncoder.ts - Fix duplicate caption IDs by adding chunk-specific indices in handlers.ts - Prioritize native monitor selection by bounds in monitor_utils.cpp - Fix alignment for blur annotations extending beyond canvas boundaries - Resolve timeline selection desync and sidebar visibility for blur annotations - Fix memory leaks in audio waveform generation with AudioContext cleanup --- electron/ipc/handlers.ts | 11 +++-- .../native/wgc-capture/src/monitor_utils.cpp | 27 +----------- .../video-editor/AudioSettingsPanel.tsx | 6 ++- src/components/video-editor/VideoEditor.tsx | 3 +- .../video-editor/projectPersistence.ts | 14 +++--- src/lib/exporter/annotationRenderer.ts | 18 ++++---- src/lib/exporter/audioEncoder.ts | 44 ++++++++++++++++--- src/lib/exporter/videoExporter.ts | 5 +++ src/utils/audioWaveform.ts | 8 +++- 9 files changed, 82 insertions(+), 54 deletions(-) diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index f871ddfd..ee2b4a05 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1607,7 +1607,9 @@ async function generateAutoCaptionsFromVideo( let jsonEnabled = true const updateChunkProgress = (progress: number) => { if (totalDurationMs > 0) { - const totalProgress = (offsetMs / totalDurationMs * 100) + (progress / (totalDurationMs / CHUNK_SIZE_MS)); + const totalRangeMs = totalDurationMs > 0 ? totalDurationMs : 1; + const rangeOffsetMs = offsetMs - startTimeMs; + const totalProgress = (rangeOffsetMs / totalRangeMs * 100) + (progress / (totalRangeMs / CHUNK_SIZE_MS)); safeSend(webContents, 'auto-caption-progress', { progress: Math.min(99, totalProgress) }) } else { safeSend(webContents, 'auto-caption-progress', { progress }) @@ -1634,15 +1636,16 @@ async function generateAutoCaptionsFromVideo( // Adjust timings and deduplicate const adjustedCues = cues - .map(cue => ({ + .map((cue, idx) => ({ ...cue, + id: `caption-${offsetMs}-${idx}`, startMs: cue.startMs + offsetMs, endMs: cue.endMs + offsetMs })) // Only keep cues that START within this chunk's main window (prevent overlap duplicates) // Except for the very last chunk where we take everything .filter(cue => { - const isLastChunk = totalDurationMs > 0 && (offsetMs + CHUNK_SIZE_MS >= totalDurationMs); + const isLastChunk = offsetMs + CHUNK_SIZE_MS >= endTimeMs; if (isLastChunk) return true; return cue.startMs < offsetMs + CHUNK_SIZE_MS; }); @@ -1659,7 +1662,7 @@ async function generateAutoCaptionsFromVideo( break; } - if (totalDurationMs > 0 && offsetMs + CHUNK_SIZE_MS >= totalDurationMs) { + if (offsetMs + CHUNK_SIZE_MS >= endTimeMs) { break; } diff --git a/electron/native/wgc-capture/src/monitor_utils.cpp b/electron/native/wgc-capture/src/monitor_utils.cpp index a6dbe0c4..c4fe0739 100644 --- a/electron/native/wgc-capture/src/monitor_utils.cpp +++ b/electron/native/wgc-capture/src/monitor_utils.cpp @@ -46,32 +46,7 @@ HMONITOR findMonitorByDisplayId(int64_t displayId) { } } - // Try matching lower 32-bits (HMONITOR handles on Windows are often 32-bit values zero-extended or sign-extended to 64-bit) - for (const auto& m : monitors) { - if ((static_cast(reinterpret_cast(m.handle)) & 0xFFFFFFFF) == (displayId & 0xFFFFFFFF)) { - std::cerr << "WARNING: Found monitor via 32-bit partial match. Target: " << displayId - << ", Found: " << static_cast(reinterpret_cast(m.handle)) << std::endl; - return m.handle; - } - } - - // If we only have one monitor and we couldn't match by ID, just use the only one we have. - // This is a safe fallback for the most common case (single monitor setups) where Electron - // and native Windows might report different IDs for the same display. - if (monitors.size() == 1) { - std::cerr << "WARNING: Found only one monitor, using it as fallback for displayId " << displayId - << " (Handle: " << reinterpret_cast(monitors[0].handle) << ")" << std::endl; - return monitors[0].handle; - } - - // Debug: Print available monitors if not found - std::cerr << "ERROR: Monitor matching failed for displayId " << displayId << ". Enumerated " << monitors.size() << " monitors:" << std::endl; - for (const auto& m : monitors) { - std::cerr << " - Handle: " << reinterpret_cast(m.handle) - << " (device: " << wstringToString(m.deviceName) - << ", bounds: " << m.x << "," << m.y << " " << m.width << "x" << m.height << ")" << std::endl; - } - + // No exact ID match found. Return nullptr to allow main.cpp to try coordinate-based matching. return nullptr; } diff --git a/src/components/video-editor/AudioSettingsPanel.tsx b/src/components/video-editor/AudioSettingsPanel.tsx index 07fe5931..da1b7c06 100644 --- a/src/components/video-editor/AudioSettingsPanel.tsx +++ b/src/components/video-editor/AudioSettingsPanel.tsx @@ -34,9 +34,13 @@ export function AudioSettingsPanel({ const [waveform, setWaveform] = useState(null); useEffect(() => { + let active = true; if (audio.audioPath) { - generateWaveform(audio.audioPath, 120).then(setWaveform); + generateWaveform(audio.audioPath, 120).then(result => { + if (active) setWaveform(result); + }); } + return () => { active = false; }; }, [audio.audioPath]); const clipDurationMs = audio.endMs - audio.startMs; diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index cf271ac0..7faa2756 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -3838,6 +3838,7 @@ export default function VideoEditor() { aspectRatio={aspectRatio} onAspectRatioChange={setAspectRatio} selectedAnnotationId={selectedAnnotationId} + annotationRegions={annotationRegions} onSeek={(time) => videoPlaybackRef.current?.seek(time)} autoCaptions={autoCaptions} onAutoCaptionsChange={setAutoCaptions} @@ -3878,7 +3879,7 @@ export default function VideoEditor() { onAudioFadeOutMsChange={handleAudioFadeOutMsChange} onAudioDelete={handleAudioDelete} selectedCaptionId={selectedCaptionId} - onSelectCaption={setSelectedCaptionId} + onSelectCaption={handleSelectCaption} timeSelection={timeSelection} isMasterSelected={isMasterSelected} masterAudioVolume={masterAudioVolume} diff --git a/src/components/video-editor/projectPersistence.ts b/src/components/video-editor/projectPersistence.ts index 35325bc2..1cf10469 100644 --- a/src/components/video-editor/projectPersistence.ts +++ b/src/components/video-editor/projectPersistence.ts @@ -341,7 +341,10 @@ export function normalizeProjectEditor(editor: Partial): Pro id: region.id, startMs, endMs, - type: region.type === "image" || region.type === "figure" ? region.type : "text", + type: + region.type === "image" || region.type === "figure" || region.type === "blur" + ? region.type + : "text", content: typeof region.content === "string" ? region.content : "", textContent: typeof region.textContent === "string" ? region.textContent : undefined, imageContent: typeof region.imageContent === "string" ? region.imageContent : undefined, @@ -384,10 +387,11 @@ export function normalizeProjectEditor(editor: Partial): Pro zIndex: isFiniteNumber(region.zIndex) ? region.zIndex : index + 1, figureData: region.figureData ? { - ...DEFAULT_FIGURE_DATA, - ...region.figureData, - } + ...DEFAULT_FIGURE_DATA, + ...region.figureData, + } : undefined, + blurIntensity: isFiniteNumber(region.blurIntensity) ? region.blurIntensity : undefined, }; }) : []; @@ -408,7 +412,7 @@ export function normalizeProjectEditor(editor: Partial): Pro startMs, endMs, audioPath: typeof region.audioPath === "string" ? region.audioPath : "", - volume: isFiniteNumber(region.volume) ? clamp(region.volume, 0, 1) : 1, + volume: isFiniteNumber(region.volume) ? clamp(region.volume, 0, 2) : 1, muted: typeof region.muted === "boolean" ? region.muted : false, soloed: typeof region.soloed === "boolean" ? region.soloed : false, fadeInMs: isFiniteNumber(region.fadeInMs) ? clamp(region.fadeInMs, 0, 10000) : 0, diff --git a/src/lib/exporter/annotationRenderer.ts b/src/lib/exporter/annotationRenderer.ts index 0dfad151..a9a73ff1 100644 --- a/src/lib/exporter/annotationRenderer.ts +++ b/src/lib/exporter/annotationRenderer.ts @@ -307,25 +307,25 @@ function renderBlur( // Determine bounds and ensure they are within canvas to avoid ImageData errors const srcX = Math.max(0, x); const srcY = Math.max(0, y); - const srcW = Math.min(width, ctx.canvas.width - srcX); - const srcH = Math.min(height, ctx.canvas.height - srcY); + const srcWidth = Math.min(x + width, ctx.canvas.width) - srcX; + const srcHeight = Math.min(y + height, ctx.canvas.height) - srcY; - if (srcW <= 0 || srcH <= 0) { + if (srcWidth <= 0 || srcHeight <= 0) { ctx.restore(); return; } - // Capture the current canvas region - const imageData = ctx.getImageData(srcX, srcY, srcW, srcH); + // Capture the current canvas region precisely for this intersection + const imageData = ctx.getImageData(srcX, srcY, srcWidth, srcHeight); let offscreen: HTMLCanvasElement | OffscreenCanvas; if (typeof document !== 'undefined') { offscreen = document.createElement('canvas'); } else { - offscreen = new OffscreenCanvas(srcW, srcH); + offscreen = new OffscreenCanvas(srcWidth, srcHeight); } - offscreen.width = srcW; - offscreen.height = srcH; + offscreen.width = srcWidth; + offscreen.height = srcHeight; const offCtx = offscreen.getContext('2d') as CanvasRenderingContext2D; offCtx.putImageData(imageData, 0, 0); @@ -340,7 +340,7 @@ function renderBlur( } ctx.clip(); - // Apply the blur filter and draw the captured region back + // Apply the blur filter and draw the captured region back at its source position ctx.filter = `blur(${intensity}px)`; ctx.drawImage(offscreen as any, srcX, srcY); diff --git a/src/lib/exporter/audioEncoder.ts b/src/lib/exporter/audioEncoder.ts index 975a99e0..c617cf66 100644 --- a/src/lib/exporter/audioEncoder.ts +++ b/src/lib/exporter/audioEncoder.ts @@ -28,6 +28,7 @@ export class AudioProcessor { masterAudioVolume = 1, audioTrackVolume = 1, masterAudioMuted = false, + masterAudioSoloed = false, ): Promise { const sortedTrims = trimRegions ? [...trimRegions].sort((a, b) => a.startMs - b.startMs) : [] const sortedSpeedRegions = speedRegions @@ -39,8 +40,15 @@ export class AudioProcessor { ? [...audioRegions].sort((a, b) => a.startMs - b.startMs) : [] - // When audio regions or speed edits are present, use AudioContext mixing path. - if (sortedSpeedRegions.length > 0 || sortedAudioRegions.length > 0) { + // When audio regions, speed edits, or global volume/mute settings are present, use AudioContext mixing path. + if ( + sortedSpeedRegions.length > 0 || + sortedAudioRegions.length > 0 || + masterAudioVolume !== 1 || + audioTrackVolume !== 1 || + masterAudioMuted || + masterAudioSoloed + ) { const renderedAudioBlob = await this.renderMixedTimelineAudio( videoUrl, sortedTrims, @@ -49,6 +57,7 @@ export class AudioProcessor { masterAudioVolume, audioTrackVolume, masterAudioMuted, + masterAudioSoloed, ) if (!this.cancelled) { await this.muxRenderedAudioBlob(renderedAudioBlob, muxer) @@ -288,6 +297,7 @@ export class AudioProcessor { masterAudioVolume: number, audioTrackVolume: number, masterAudioMuted: boolean, + masterAudioSoloed: boolean, ): Promise { const mediaSource = await resolveMediaElementSource(videoUrl) const media = document.createElement('audio') @@ -311,9 +321,11 @@ export class AudioProcessor { const audioContext = new AudioContext() const destinationNode = audioContext.createMediaStreamDestination() - // Connect original video audio + // Connect original video audio with its own gain node const sourceNode = audioContext.createMediaElementSource(media) - sourceNode.connect(destinationNode) + const masterGainNode = audioContext.createGain() + sourceNode.connect(masterGainNode) + masterGainNode.connect(destinationNode) // Prepare external audio region elements const audioRegionElements: { @@ -412,6 +424,18 @@ export class AudioProcessor { } } + // Check for Solo - if any track is soloed, everything else is muted + const anyTrackSoloed = masterAudioSoloed || audioRegionElements.some(e => e.region.soloed); + + // Update Master Video Audio Track Volume + let masterTargetVolume = masterAudioMuted ? 0 : audioTrackVolume; + if (anyTrackSoloed && !masterAudioSoloed) { + masterTargetVolume = 0; + } + // Factor in the overall Master output volume + masterTargetVolume *= masterAudioVolume; + masterGainNode.gain.setTargetAtTime(masterTargetVolume, audioContext.currentTime, 0.015); + // Sync external audio regions with the video timeline position for (const entry of audioRegionElements) { const { media: audioEl, region, gainNode } = entry @@ -429,9 +453,15 @@ export class AudioProcessor { } fadeMultiplier = Math.max(0, Math.min(1, fadeMultiplier)); - // Apply total volume including global settings - const totalVolume = masterAudioMuted ? 0 : region.volume * audioTrackVolume * masterAudioVolume * fadeMultiplier; - gainNode.gain.setTargetAtTime(totalVolume, audioContext.currentTime, 0.015); + // Respect Solo and Mute + let regionTargetVolume = (region.muted || masterAudioMuted) ? 0 : region.volume; + if (anyTrackSoloed && !region.soloed) { + regionTargetVolume = 0; + } + // Also factor in global master volume + regionTargetVolume *= masterAudioVolume * fadeMultiplier; + + gainNode.gain.setTargetAtTime(regionTargetVolume, audioContext.currentTime, 0.015); if (audioEl.paused) { audioEl.currentTime = audioOffset diff --git a/src/lib/exporter/videoExporter.ts b/src/lib/exporter/videoExporter.ts index 307e90fe..fa4a40be 100644 --- a/src/lib/exporter/videoExporter.ts +++ b/src/lib/exporter/videoExporter.ts @@ -59,6 +59,10 @@ interface VideoExporterConfig extends ExportConfig { previewWidth?: number; previewHeight?: number; onProgress?: (progress: ExportProgress) => void; + masterAudioVolume: number; + masterAudioMuted: boolean; + masterAudioSoloed: boolean; + audioTrackVolume: number; } export class VideoExporter { @@ -210,6 +214,7 @@ export class VideoExporter { this.config.masterAudioVolume, this.config.audioTrackVolume, this.config.masterAudioMuted, + this.config.masterAudioSoloed, ), "audio processing", ); diff --git a/src/utils/audioWaveform.ts b/src/utils/audioWaveform.ts index 7db89bac..017faac5 100644 --- a/src/utils/audioWaveform.ts +++ b/src/utils/audioWaveform.ts @@ -13,12 +13,12 @@ export async function generateWaveform(audioPath: string, samples = 200): Promis return waveformCache.get(cacheKey)!; } + const audioContext = new (window.AudioContext || (window as any).webkitAudioContext)(); try { const response = await fetch(toFileUrl(audioPath)); const arrayBuffer = await response.arrayBuffer(); // Use an offline audio context to decode the data - const audioContext = new (window.AudioContext || (window as any).webkitAudioContext)(); const audioBuffer = await audioContext.decodeAudioData(arrayBuffer); const channelData = audioBuffer.getChannelData(0); // Use the first channel @@ -40,5 +40,11 @@ export async function generateWaveform(audioPath: string, samples = 200): Promis } catch (error) { console.error('Failed to generate waveform:', error); return new Array(samples).fill(0); + } finally { + try { + await audioContext.close(); + } catch (e) { + // ignore close errors + } } }