fix(engine): recover audio duration via full packet scan when the container header has none - #3889
Open
miga-heygen wants to merge 1 commit into
Open
fix(engine): recover audio duration via full packet scan when the container header has none#3889miga-heygen wants to merge 1 commit into
miga-heygen wants to merge 1 commit into
Conversation
…r header has none extractAudioMetadata() previously trusted only ffprobe's container-header duration (format.duration). A container whose encoder never got a chance to seek back and patch that field in — e.g. audio piped to a non-seekable destination, which is exactly what a browser MediaRecorder-style capture does — comes back with no duration at format OR stream level, not merely zero. With no fallback, the render pipeline treated the track as zero-length and silently dropped it: no error, no warning, just missing audio in the output. The function already had one precedent for recovering a bad duration (an AAC-LC-specific packet-count recomputation), but it only applies to that one codec/profile combination. This generalizes the idea to any codec: when duration is still unrecoverable after the existing logic, prefer the already-probed stream-level duration if usable, otherwise scan every packet's timestamp to EOF and take the last one's pts_time + duration_time. Only the true last parsed line is trusted, mirroring the existing final-video-frame-timestamp prober a few hundred lines above in the same file (same bounded-output settings, same reasoning): the output-retention limit that bounds a long probe's cost keeps only the tail of ffprobe's output, which can slice through the middle of an early line, but never through the last one. Taking the last line only (not a max across all lines) avoids a truncation artifact ever masquerading as the answer. Adds regression coverage: successful recovery from a full scan, preferring the cheap stream-level duration when present, graceful no-op when the scan finds nothing or the probe itself fails, tolerating codecs that omit per-packet duration, correct behavior when a truncated leading line contains a bogus large timestamp, and confirming the new fallback stays out of the way when the existing codec-specific refinement already succeeded. Co-Authored-By: Miguel Angel <miguel.sierra@heygen.com>
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.
Summary
extractAudioMetadata()previously trusted only ffprobe's container-header duration (format.duration) when probing an audio file. A container whose encoder never got a chance to seek back and patch that field in — e.g. audio piped to a non-seekable destination, which is exactly what a browserMediaRecorder-style capture does — comes back with no duration at format OR stream level, not merely zero. With no fallback, the render pipeline treated the track as zero-length and silently dropped it from the composition: no error, no warning, just missing audio in the output.The function already had one precedent for recovering an untrustworthy duration — an AAC-LC-specific packet-count recomputation — but it's gated to that one codec/profile combination. This generalizes the idea to any codec: when duration is still unrecoverable after the existing logic, prefer the already-probed stream-level duration if it's usable, otherwise scan every packet's timestamp to EOF and take the last packet's
pts_time + duration_time.Only the true last parsed line is trusted (not a max across every line), mirroring the existing final-video-frame-timestamp prober already in the same file, which uses the identical output-retention settings for the identical reason: the retention limit that bounds a long probe's cost keeps only the tail of ffprobe's output, and that can slice through the middle of an early line — but never through the true last one. An earlier version of this fix took the max across all parsed lines instead, which a caught-in-review adversarial pass showed could let a truncated, numerically-bogus early line silently win over the correct answer.
Test plan
duration_time, a truncated-leading-line case proving a bogus large timestamp is correctly ignored in favor of the true last line, and confirming the new fallback is skipped when the existing codec-specific refinement already recovered a duration.libopus-in-WebM encode to a non-seekable destination (ffmpeg ... -f webm - > out.webm) and confirmed viaffprobethat neither format- nor stream-level duration is present, while a full packet scan correctly recovers the true duration from packet timestamps.node --test/vitest runon the touched test file: 118/122 pass (4 pre-existing, unrelated failures traced to an unresolved Git LFS pointer for a fixture PNG in this environment — reproduced identically on a clean checkout of the base branch before this change).audioMixer,chunkEncoder,htmlCompiler,audioPadTrimtest suites): all pass, no regressions.tsc --noEmit,oxlint,oxfmt, andfallow auditagainst the base branch: clean.