Merge pull request #233 from siddharthvaddem/codex/issue-230
fix: avoid false early decode failures
This commit is contained in:
@@ -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");
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
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,
|
||||
lastDecodedFrameSec: 5.33,
|
||||
requiredEndSec: 6.498,
|
||||
streamDurationSec: 5.33,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("fails when decode stops far before the required end", () => {
|
||||
expect(
|
||||
shouldFailDecodeEndedEarly({
|
||||
cancelled: false,
|
||||
lastDecodedFrameSec: 5.33,
|
||||
requiredEndSec: 10,
|
||||
streamDurationSec: 5.33,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("fails when no frame could be decoded for a non-empty timeline", () => {
|
||||
expect(
|
||||
shouldFailDecodeEndedEarly({
|
||||
cancelled: false,
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -12,6 +12,55 @@ export interface DecodedVideoInfo {
|
||||
audioCodec?: string;
|
||||
}
|
||||
|
||||
type EarlyDecodeEndCheck = {
|
||||
cancelled: boolean;
|
||||
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,
|
||||
lastDecodedFrameSec,
|
||||
requiredEndSec,
|
||||
streamDurationSec,
|
||||
}: EarlyDecodeEndCheck): boolean {
|
||||
if (cancelled || requiredEndSec <= 0) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (lastDecodedFrameSec === null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const decodeGapSec = requiredEndSec - lastDecodedFrameSec;
|
||||
if (decodeGapSec <= EARLY_DECODE_END_THRESHOLD_SEC) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (typeof streamDurationSec !== "number" || !Number.isFinite(streamDurationSec)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
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. */
|
||||
type OnFrameCallback = (
|
||||
frame: VideoFrame,
|
||||
@@ -366,12 +415,17 @@ 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,
|
||||
lastDecodedFrameSec,
|
||||
requiredEndSec,
|
||||
streamDurationSec: this.metadata.streamDuration,
|
||||
})
|
||||
) {
|
||||
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).`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user