Skip to content

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
mainfrom
fix/webm-audio-zero-duration-eof-scan
Open

fix(engine): recover audio duration via full packet scan when the container header has none#3889
miga-heygen wants to merge 1 commit into
mainfrom
fix/webm-audio-zero-duration-eof-scan

Conversation

@miga-heygen

Copy link
Copy Markdown
Contributor

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 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 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

  • New regression suite covering: successful recovery from a full packet scan, preferring the cheap stream-level duration when present (skips the scan entirely), graceful no-op (stays at 0, never throws) when the scan finds nothing or the probe itself fails, tolerating codecs/containers that omit per-packet 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.
  • Verified the recovery mechanism against a real repro: piped a libopus-in-WebM encode to a non-seekable destination (ffmpeg ... -f webm - > out.webm) and confirmed via ffprobe that neither format- nor stream-level duration is present, while a full packet scan correctly recovers the true duration from packet timestamps.
  • node --test/vitest run on 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).
  • Downstream consumers (audioMixer, chunkEncoder, htmlCompiler, audioPadTrim test suites): all pass, no regressions.
  • tsc --noEmit, oxlint, oxfmt, and fallow audit against the base branch: clean.

…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant