fix(export): preserve duration with short video-stream metadata - #433
Conversation
📝 WalkthroughWalkthroughThe PR refines video duration calculation by introducing mismatch detection in ChangesEffective Video Stream Duration Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/mediaTiming.ts`:
- Around line 94-95: The conditional that decides whether to return safeDuration
uses a strict greater-than check (gapSeconds > largeMismatchThresholdSeconds)
which excludes the exact boundary; change the operator to greater-than-or-equal
(gapSeconds >= largeMismatchThresholdSeconds) in the same conditional so that
when gapSeconds equals largeMismatchThresholdSeconds the function returns
safeDuration rather than safeStreamDuration, updating the expression that
references gapSeconds and largeMismatchThresholdSeconds accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a09274a-0bc7-4448-91cb-589e12a88438
📒 Files selected for processing (4)
src/lib/exporter/modernVideoExporter.tssrc/lib/exporter/videoExporter.tssrc/lib/mediaTiming.test.tssrc/lib/mediaTiming.ts
| if (gapSeconds > largeMismatchThresholdSeconds) { | ||
| return safeDuration; |
There was a problem hiding this comment.
Treat the mismatch cutoff as inclusive to avoid exact-threshold truncation.
At the exact boundary (gapSeconds === largeMismatchThresholdSeconds), the current > still prefers safeStreamDuration. That can keep a truncating result in edge cases.
Proposed fix
- if (gapSeconds > largeMismatchThresholdSeconds) {
+ if (gapSeconds >= largeMismatchThresholdSeconds) {
return safeDuration;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/mediaTiming.ts` around lines 94 - 95, The conditional that decides
whether to return safeDuration uses a strict greater-than check (gapSeconds >
largeMismatchThresholdSeconds) which excludes the exact boundary; change the
operator to greater-than-or-equal (gapSeconds >= largeMismatchThresholdSeconds)
in the same conditional so that when gapSeconds equals
largeMismatchThresholdSeconds the function returns safeDuration rather than
safeStreamDuration, updating the expression that references gapSeconds and
largeMismatchThresholdSeconds accordingly.
…ration-mismatch fix(export): preserve duration with short video-stream metadata
…ration-mismatch fix(export): preserve duration with short video-stream metadata
Description
Fix export duration planning when the source media reports a much shorter video-stream duration than the overall container duration.
The exporter should not cut a source that presents as
60soverall down to40sonly because the video stream metadata is shorter. This keeps the existing stream-duration behavior for small metadata differences, but falls back to the container duration when the mismatch is large enough to indicate an export-truncation risk.Motivation
Issue #400 reports Windows system-audio recordings where the raw recording is full length, but the editor export is much shorter, for example raw
60sand exported MP440s.The local repro pass found a matching exporter-side metadata shape: an MP4 with container duration
60s, video stream duration40s, and audio stream duration60s. Recordly's browser demuxer reports the same60/40/60split, and the existing duration helper would plan the export from the40svideo stream duration.Type of Change
Related Issue(s)
Changes Made
60scontainer /40svideo-stream duration mismatchScope Note
This only targets editor/export duration planning for large container-vs-video-stream metadata mismatches. It does not change native Windows capture, WGC finalization, audio capture, frame scheduling, or the separate native GPU export work in #410.
Testing Guide
Checklist
Local checks run:
npm exec -- biome check src/lib/mediaTiming.ts src/lib/mediaTiming.test.ts src/lib/exporter/modernVideoExporter.ts src/lib/exporter/videoExporter.tsnpm test -- src/lib/mediaTiming.test.tsnpm test -- src/lib/exporter/streamingDecoder.test.tsnpm exec tsc -- --noEmitMetadata repro:
60s, video stream duration40s, audio stream duration60sweb-demuxerreports the same60/40/60split in ChromiumgetEffectiveVideoStreamDurationSeconds({ duration: 60, streamDuration: 40 })returned4060Native capture check:
7.57s58.71s, loopback WAV around45.11s, app-equivalent mux padded audio back to matching duration around58.71s60s/ export-40ssplit locally, so this PR should stay draft until a failing raw file or current build retest confirms [Bug]: Exported video duration is much shorter than actual recording time #400Summary by CodeRabbit
Bug Fixes
Tests