From de18a2f46faee737acde293640fc2ac68248112d Mon Sep 17 00:00:00 2001 From: Siddharth Date: Tue, 17 Mar 2026 19:30:47 -0700 Subject: [PATCH 1/4] fix: avoid false early decode failures --- src/lib/exporter/streamingDecoder.test.ts | 40 ++++++++++++++++++++ src/lib/exporter/streamingDecoder.ts | 46 +++++++++++++++++++++-- 2 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 src/lib/exporter/streamingDecoder.test.ts diff --git a/src/lib/exporter/streamingDecoder.test.ts b/src/lib/exporter/streamingDecoder.test.ts new file mode 100644 index 00000000..18b06727 --- /dev/null +++ b/src/lib/exporter/streamingDecoder.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from "vitest"; +import { shouldFailDecodeEndedEarly } from "./streamingDecoder"; + +describe("shouldFailDecodeEndedEarly", () => { + it("does not fail once every segment has been satisfied", () => { + expect( + shouldFailDecodeEndedEarly({ + cancelled: false, + segmentsLength: 1, + completedSegments: 1, + lastDecodedFrameSec: 5.33, + requiredEndSec: 6.498, + }), + ).toBe(false); + }); + + it("fails when decode stops before the remaining segments can be covered", () => { + expect( + shouldFailDecodeEndedEarly({ + cancelled: false, + segmentsLength: 2, + completedSegments: 1, + lastDecodedFrameSec: 5.33, + requiredEndSec: 6.498, + }), + ).toBe(true); + }); + + it("fails when no frame could be decoded for a non-empty timeline", () => { + expect( + shouldFailDecodeEndedEarly({ + cancelled: false, + segmentsLength: 1, + completedSegments: 0, + lastDecodedFrameSec: null, + requiredEndSec: 1, + }), + ).toBe(true); + }); +}); diff --git a/src/lib/exporter/streamingDecoder.ts b/src/lib/exporter/streamingDecoder.ts index 7ebd78dc..b4d75493 100644 --- a/src/lib/exporter/streamingDecoder.ts +++ b/src/lib/exporter/streamingDecoder.ts @@ -12,6 +12,38 @@ export interface DecodedVideoInfo { audioCodec?: string; } +type EarlyDecodeEndCheck = { + cancelled: boolean; + segmentsLength: number; + completedSegments: number; + lastDecodedFrameSec: number | null; + requiredEndSec: number; +}; + +export function shouldFailDecodeEndedEarly({ + cancelled, + segmentsLength, + completedSegments, + lastDecodedFrameSec, + requiredEndSec, +}: EarlyDecodeEndCheck): boolean { + if (cancelled || segmentsLength === 0) { + return false; + } + + // If we already satisfied every segment, the exporter has successfully + // filled any short metadata tail using the last decoded frame. + if (completedSegments >= segmentsLength) { + return false; + } + + if (lastDecodedFrameSec === null) { + return true; + } + + return requiredEndSec - lastDecodedFrameSec > 1; +} + /** Caller must close the VideoFrame after use. */ type OnFrameCallback = ( frame: VideoFrame, @@ -366,12 +398,18 @@ export class StreamingVideoDecoder { const requiredEndSec = segments.length > 0 ? segments[segments.length - 1].endSec : 0; if ( - !this.cancelled && - lastDecodedFrameSec !== null && - requiredEndSec - lastDecodedFrameSec > 1 + shouldFailDecodeEndedEarly({ + cancelled: this.cancelled, + segmentsLength: segments.length, + completedSegments: segmentIdx, + lastDecodedFrameSec, + requiredEndSec, + }) ) { + const decodedAtLabel = + lastDecodedFrameSec === null ? "no decoded frame" : `${lastDecodedFrameSec.toFixed(3)}s`; throw new Error( - `Video decode ended early at ${lastDecodedFrameSec.toFixed(3)}s (needed ${requiredEndSec.toFixed(3)}s).`, + `Video decode ended early at ${decodedAtLabel} (needed ${requiredEndSec.toFixed(3)}s).`, ); } } From 1680ef9b77573a074d5168088de220ab79c86ef7 Mon Sep 17 00:00:00 2001 From: Siddharth Date: Tue, 17 Mar 2026 19:46:56 -0700 Subject: [PATCH 2/4] fix: guard exported file paths in export flow --- src/components/video-editor/VideoEditor.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index 1b6de7e4..0f8b93e9 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -1040,7 +1040,7 @@ export default function VideoEditor() { if (saveResult.canceled) { toast.info("Export canceled"); - } else if (saveResult.success) { + } else if (saveResult.success && saveResult.path) { handleExportSaved("GIF", saveResult.path); } else { setExportError(saveResult.message || "Failed to save GIF"); @@ -1167,7 +1167,7 @@ export default function VideoEditor() { if (saveResult.canceled) { toast.info("Export canceled"); - } else if (saveResult.success) { + } else if (saveResult.success && saveResult.path) { handleExportSaved("Video", saveResult.path); } else { setExportError(saveResult.message || "Failed to save video"); From 7e65d52847bb138f101e3731e6f363c3a3b058fc Mon Sep 17 00:00:00 2001 From: Siddharth Date: Tue, 17 Mar 2026 20:07:15 -0700 Subject: [PATCH 3/4] fix --- src/lib/exporter/streamingDecoder.test.ts | 23 ++++++++----- src/lib/exporter/streamingDecoder.ts | 40 ++++++++++++++++------- tests/e2e/gif-export.spec.ts | 4 +-- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/src/lib/exporter/streamingDecoder.test.ts b/src/lib/exporter/streamingDecoder.test.ts index 18b06727..1969c84d 100644 --- a/src/lib/exporter/streamingDecoder.test.ts +++ b/src/lib/exporter/streamingDecoder.test.ts @@ -6,22 +6,20 @@ describe("shouldFailDecodeEndedEarly", () => { expect( shouldFailDecodeEndedEarly({ cancelled: false, - segmentsLength: 1, - completedSegments: 1, lastDecodedFrameSec: 5.33, requiredEndSec: 6.498, + streamDurationSec: 5.33, }), ).toBe(false); }); - it("fails when decode stops before the remaining segments can be covered", () => { + it("fails when decode stops far before the required end", () => { expect( shouldFailDecodeEndedEarly({ cancelled: false, - segmentsLength: 2, - completedSegments: 1, lastDecodedFrameSec: 5.33, - requiredEndSec: 6.498, + requiredEndSec: 10, + streamDurationSec: 5.33, }), ).toBe(true); }); @@ -30,11 +28,20 @@ describe("shouldFailDecodeEndedEarly", () => { expect( shouldFailDecodeEndedEarly({ cancelled: false, - segmentsLength: 1, - completedSegments: 0, lastDecodedFrameSec: null, requiredEndSec: 1, }), ).toBe(true); }); + + it("fails when the decoder has not reached the reported stream end", () => { + expect( + shouldFailDecodeEndedEarly({ + cancelled: false, + lastDecodedFrameSec: 4.9, + requiredEndSec: 6.498, + streamDurationSec: 5.33, + }), + ).toBe(true); + }); }); diff --git a/src/lib/exporter/streamingDecoder.ts b/src/lib/exporter/streamingDecoder.ts index b4d75493..d994b662 100644 --- a/src/lib/exporter/streamingDecoder.ts +++ b/src/lib/exporter/streamingDecoder.ts @@ -14,34 +14,51 @@ export interface DecodedVideoInfo { type EarlyDecodeEndCheck = { cancelled: boolean; - segmentsLength: number; - completedSegments: number; lastDecodedFrameSec: number | null; requiredEndSec: number; + streamDurationSec?: number; }; +const EARLY_DECODE_END_THRESHOLD_SEC = 1; +const METADATA_TAIL_TOLERANCE_SEC = 1.5; +const STREAM_DURATION_MATCH_TOLERANCE_SEC = 0.25; + export function shouldFailDecodeEndedEarly({ cancelled, - segmentsLength, - completedSegments, lastDecodedFrameSec, requiredEndSec, + streamDurationSec, }: EarlyDecodeEndCheck): boolean { - if (cancelled || segmentsLength === 0) { + if (cancelled || requiredEndSec <= 0) { return false; } - // If we already satisfied every segment, the exporter has successfully - // filled any short metadata tail using the last decoded frame. - if (completedSegments >= segmentsLength) { + if (lastDecodedFrameSec === null) { + return true; + } + + const decodeGapSec = requiredEndSec - lastDecodedFrameSec; + if (decodeGapSec <= EARLY_DECODE_END_THRESHOLD_SEC) { return false; } - if (lastDecodedFrameSec === null) { + if (typeof streamDurationSec !== "number" || !Number.isFinite(streamDurationSec)) { return true; } - return requiredEndSec - lastDecodedFrameSec > 1; + const metadataTailSec = requiredEndSec - streamDurationSec; + const decodedNearStreamEnd = + Math.abs(lastDecodedFrameSec - streamDurationSec) <= STREAM_DURATION_MATCH_TOLERANCE_SEC; + + if ( + decodedNearStreamEnd && + metadataTailSec > 0 && + metadataTailSec <= METADATA_TAIL_TOLERANCE_SEC + ) { + return false; + } + + return true; } /** Caller must close the VideoFrame after use. */ @@ -400,10 +417,9 @@ export class StreamingVideoDecoder { if ( shouldFailDecodeEndedEarly({ cancelled: this.cancelled, - segmentsLength: segments.length, - completedSegments: segmentIdx, lastDecodedFrameSec, requiredEndSec, + streamDurationSec: this.metadata.streamDuration, }) ) { const decodedAtLabel = diff --git a/tests/e2e/gif-export.spec.ts b/tests/e2e/gif-export.spec.ts index b18fd443..a8515462 100644 --- a/tests/e2e/gif-export.spec.ts +++ b/tests/e2e/gif-export.spec.ts @@ -87,8 +87,8 @@ test("exports a GIF from a loaded video", async () => { await editorWindow.getByTestId("testId-gif-format-button").click(); await editorWindow.getByTestId("testId-export-button").click(); - // ── 6. Wait for the toast to say exported successfully - await expect(editorWindow.getByText(`GIF exported successfully to pending`)).toBeVisible({ + // ── 6. Wait for the success toast. + await expect(editorWindow.getByText("GIF exported successfully")).toBeVisible({ timeout: 90_000, }); From 69f1b4d20f3ba25dff7512d8de4aa3d6ad9830b9 Mon Sep 17 00:00:00 2001 From: Siddharth Date: Tue, 17 Mar 2026 20:25:34 -0700 Subject: [PATCH 4/4] fix 231 --- src/components/video-editor/VideoEditor.tsx | 28 +++++++++++++++-- src/lib/exporter/gifExporter.test.ts | 19 +++++++++++ src/lib/exporter/gifExporter.ts | 35 +++++++++++++++------ 3 files changed, 69 insertions(+), 13 deletions(-) create mode 100644 src/lib/exporter/gifExporter.test.ts diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index 1b6de7e4..cbf9b29b 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -1040,7 +1040,7 @@ export default function VideoEditor() { if (saveResult.canceled) { toast.info("Export canceled"); - } else if (saveResult.success) { + } else if (saveResult.success && saveResult.path) { handleExportSaved("GIF", saveResult.path); } else { setExportError(saveResult.message || "Failed to save GIF"); @@ -1167,7 +1167,7 @@ export default function VideoEditor() { if (saveResult.canceled) { toast.info("Export canceled"); - } else if (saveResult.success) { + } else if (saveResult.success && saveResult.path) { handleExportSaved("Video", saveResult.path); } else { setExportError(saveResult.message || "Failed to save video"); @@ -1232,11 +1232,16 @@ export default function VideoEditor() { // Build export settings from current state const sourceWidth = video.videoWidth || 1920; const sourceHeight = video.videoHeight || 1080; + const aspectRatioValue = + aspectRatio === "native" + ? getNativeAspectRatioValue(sourceWidth, sourceHeight, cropRegion) + : getAspectRatioValue(aspectRatio); const gifDimensions = calculateOutputDimensions( sourceWidth, sourceHeight, gifSizePreset, GIF_SIZE_PRESETS, + aspectRatioValue, ); const settings: ExportSettings = { @@ -1260,7 +1265,17 @@ export default function VideoEditor() { // Start export immediately handleExport(settings); - }, [videoPath, exportFormat, exportQuality, gifFrameRate, gifLoop, gifSizePreset, handleExport]); + }, [ + videoPath, + exportFormat, + exportQuality, + gifFrameRate, + gifLoop, + gifSizePreset, + aspectRatio, + cropRegion, + handleExport, + ]); const handleCancelExport = useCallback(() => { if (exporterRef.current) { @@ -1475,6 +1490,13 @@ export default function VideoEditor() { videoPlaybackRef.current?.video?.videoHeight || 1080, gifSizePreset, GIF_SIZE_PRESETS, + aspectRatio === "native" + ? getNativeAspectRatioValue( + videoPlaybackRef.current?.video?.videoWidth || 1920, + videoPlaybackRef.current?.video?.videoHeight || 1080, + cropRegion, + ) + : getAspectRatioValue(aspectRatio), )} onExport={handleOpenExportDialog} selectedAnnotationId={selectedAnnotationId} diff --git a/src/lib/exporter/gifExporter.test.ts b/src/lib/exporter/gifExporter.test.ts new file mode 100644 index 00000000..1ad16717 --- /dev/null +++ b/src/lib/exporter/gifExporter.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; +import { calculateOutputDimensions } from "./gifExporter"; +import { GIF_SIZE_PRESETS } from "./types"; + +describe("calculateOutputDimensions", () => { + it("uses the selected aspect ratio for scaled GIF exports", () => { + expect(calculateOutputDimensions(1080, 1920, "medium", GIF_SIZE_PRESETS, 16 / 9)).toEqual({ + width: 1280, + height: 720, + }); + }); + + it("fits original-size GIF exports within the source bounds at the selected aspect ratio", () => { + expect(calculateOutputDimensions(1080, 1920, "original", GIF_SIZE_PRESETS, 16 / 9)).toEqual({ + width: 1080, + height: 606, + }); + }); +}); diff --git a/src/lib/exporter/gifExporter.ts b/src/lib/exporter/gifExporter.ts index af49ce25..b9067567 100644 --- a/src/lib/exporter/gifExporter.ts +++ b/src/lib/exporter/gifExporter.ts @@ -58,24 +58,39 @@ export function calculateOutputDimensions( sourceHeight: number, sizePreset: GifSizePreset, sizePresets: typeof GIF_SIZE_PRESETS, + targetAspectRatio = sourceWidth / sourceHeight, ): { width: number; height: number } { const preset = sizePresets[sizePreset]; const maxHeight = preset.maxHeight; + const aspectRatio = + Number.isFinite(targetAspectRatio) && targetAspectRatio > 0 + ? targetAspectRatio + : sourceWidth / sourceHeight; + + const toEven = (value: number) => { + const evenValue = Math.max(2, Math.floor(value / 2) * 2); + return evenValue; + }; + + if (sizePreset === "original") { + const sourceAspect = sourceWidth / sourceHeight; + if (aspectRatio >= sourceAspect) { + const width = toEven(sourceWidth); + const height = toEven(width / aspectRatio); + return { width, height }; + } - // If original is smaller than max height or preset is 'original', use source dimensions - if (sourceHeight <= maxHeight || sizePreset === "original") { - return { width: sourceWidth, height: sourceHeight }; + const height = toEven(sourceHeight); + const width = toEven(height * aspectRatio); + return { width, height }; } - // Calculate scaled dimensions preserving aspect ratio - const aspectRatio = sourceWidth / sourceHeight; - const newHeight = maxHeight; - const newWidth = Math.round(newHeight * aspectRatio); + const targetHeight = maxHeight; + const targetWidth = Math.round(targetHeight * aspectRatio); - // Ensure dimensions are even (required for some encoders) return { - width: newWidth % 2 === 0 ? newWidth : newWidth + 1, - height: newHeight % 2 === 0 ? newHeight : newHeight + 1, + width: toEven(targetWidth), + height: toEven(targetHeight), }; }