fix: patch WebM Duration header on streamed recordings (stacks on #617)#1
Closed
neurot1cal wants to merge 1 commit into
Closed
Conversation
The streaming-chunks-to-disk change in siddharthvaddem#617 skipped fixWebmDuration because the renderer no longer holds the blob. As a result, streamed recordings save with no Duration EBML element — the editor's seek bar, timeline, and any scrub/trim that needs a known duration all break. ffprobe reports duration=N/A for files saved by the previous behaviour. Patch on disk in the main process after WriteStream.end() resolves, using the existing @fix-webm-duration parser API to rewrite the Info segment's Duration element. Reuses the existing dependency. Best-effort: a failure logs and returns rather than blocking the editor from opening — the file is still playable, just with broken controls if patching fails. Verified locally on a 90-second streamed recording: before: duration=N/A, bit_rate=N/A, editor reads 0:03/0:00 after: duration=90.862, bit_rate=10201352, editor reads 0:00/1:30, timeline populated, zoom blocks land on real timestamps Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
|
Closing — folding this into a diff on the upstream PR (siddharthvaddem#617) instead of a separate PR, so everything stays on the original thread. Branch stays on my fork if it's easier to pull from. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on siddharthvaddem#617. Resolves the duration-metadata regression I flagged in my review (finding siddharthvaddem#2).
Problem
After siddharthvaddem#617, streamed
.webmfiles save successfully but with no Duration EBML element.ffprobereportsduration=N/Aandbit_rate=N/A. The editor opens the file but every control that needs a timeline breaks: seek bar reads0:03 / 0:00, the timeline panel says "No Video Loaded", scrub/trim/zoom blocks have no time basis.This was the user-felt failure mode for the long recordings siddharthvaddem#617 set out to save: pre-PR they got nothing; post-PR they get an uneditable file.
Fix
Patch the Duration header on disk in the main process after
WriteStream.end()resolves. The renderer can no longer runfixWebmDuration(blob, durationMs)because the blob no longer exists, but the same library exposes a parser-level API (fixParsedWebmDuration(WebmFile, durationMs)) that operates on aUint8Array— which works fine in Node.Reads the whole file into a main-process Buffer, patches, writes back. Same memory footprint as the pre-PR renderer path, just on the side without V8's heap cap. Adds ~1–5 s to stop time depending on file size.
Verified locally
duration=N/A,bit_rate=N/A, editor reads0:03 / 0:00duration=90.862000,bit_rate=10201352, editor reads0:00 / 1:30, timeline populated, zoom blocks land on real timestampsnpx tsc --noEmit— 0 errorsnpm test— 18 files, 151/151 passing (same as baseline)Best-effort by design
If
fixParsedWebmDurationreturnsfalse(no Info section, or already valid) or any I/O fails, we log and return without throwing. The file is still playable — decoders walk frames sequentially regardless of the Duration field — so a patch failure is a degraded UX, not a broken recording. Surfacing the failure to the user is a follow-up.Possible alternatives if size becomes an issue
-c copyremux. App already ships ffmpeg per the fix: stream recording chunks to disk to support recordings longer than 10 minutes siddharthvaddem/openscreen#617 review. Slower (~10–20s on a long recording) and doubles disk I/O, but zero new code surface.I went with the simplest variant for the first cut. Happy to pivot if the wall-time on stop matters in practice.