State the audio duration instead of inferring it - #294
Merged
Merged
Conversation
The narrated master shipped with no audio stream at all, and before that with 2.3 seconds of audio against five seconds of picture. Both came from the same wrong assumption about -shortest: in ffmpeg 4.x it keys off INPUT durations, so pairing it with apad — whose output is infinite by design — does not trim the padded stream back to the video. It either left the original speech length alone or dropped the stream. The output duration is now stated outright with -t. That is not a workaround for -shortest; it is the thing actually being asserted, which is that picture and sound are the same length. Verified against real ffmpeg rather than by reading the argument list. The pipeline test now encodes a 2.3s tone over 150 frames and asserts the muxed result has an audio stream whose duration is within one AAC frame of five seconds. Both previous failures — the short track and the missing track — pass an arguments-only check, which is why neither was caught before production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan48 finding(s) HIGH/CRITICAL: 2 | MEDIUM: 31 | LOW: 15
Snippets are redacted; ThreatCrush never prints matched credential material. |
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.
The narrated master shipped with no audio stream at all, and before that with 2.3 seconds of audio against five seconds of picture. Both came from the same wrong assumption about
-shortest.In ffmpeg 4.x,
-shortestkeys off input durations. Pairing it withapad— whose output is infinite by design — does not trim the padded stream back to the video. It either left the original speech length alone or dropped the stream entirely.The output duration is now stated outright with
-t. That isn't a workaround for-shortest; it's the thing actually being asserted — that picture and sound are the same length.Verified against real ffmpeg this time
The pipeline test now encodes a 2.3s tone over 150 frames and asserts the muxed result has an audio stream whose duration is within one AAC frame of five seconds.
Both previous failures pass an arguments-only check — a short track and a missing track both look fine if you only inspect the flag list — which is exactly why neither was caught before production. I'd been verifying the wrong thing.
Context
Narration itself is confirmed working end to end in production: 91 KB of audio, script "Crypto Payments, No Custody. Get started free at coinpayportal.com.", generated from the campaign's approved copy. The same revision produced all three animated banners at exact IAB sizes (300×250 / 728×90 / 320×50) and 146 KB / 121 KB / 41 KB, inside the 150 KB budget, plus correct
avc1.64001FHLS codecs. This was the last thing between that and a finished narrated revision.2582 passed / 1 failed (pre-existing tracker-geo). Typechecks clean.