Declare the codecs the renditions actually have - #280
Merged
Merged
Conversation
The multivariant playlist hardcoded CODECS="avc1.640028,mp4a.40.2" for every
rendition. Both halves were wrong, and I only found it by fetching a shipped
playlist and probing the file it points at.
The level: shipped renditions probe as H.264 High @ 3.1, which is avc1.64001F.
0x28 is level 4.0. A player reads CODECS to choose a decoder configuration
before it fetches a segment, so the declaration is a promise about the
bitstream, and this one was not kept.
The audio: every ad rendered so far is silent and carries a video stream only,
yet each rendition advertised an AAC-LC track. An audio codec named in CODECS
is an audio track the player expects; a strict one (Safari/AVPlayer) can
allocate a decoder and wait for media that never arrives.
Both are now derived from ffprobe on the encoded rendition rather than written
down in advance: avcCodecString maps the reported profile name back to its
profile_idc and formats avc1.PPCCLL, and mp4a.40.2 is appended only when the
file actually has an audio stream. The recorded codecs on the hls asset row
match what the playlist declares, instead of a third hardcoded copy.
The pipeline test now asserts against the real encode that every declared codec
matches /^avc1\\.[0-9A-F]{6}$/ and that no silent rendition mentions mp4a.
Already-rendered revisions keep their incorrect playlist. Nothing reads it yet
— there is no decision endpoint and no property serving HLS — and re-rendering
them means bumping RENDERER_VERSION, which by design invalidates every cached
encode. Worth doing before anything streams these, not worth seven hours of
re-encoding today.
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
1 participant
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.
Found by fetching a shipped playlist from production storage and probing the file it points at. The multivariant playlist hardcoded
CODECS="avc1.640028,mp4a.40.2"for every rendition. Both halves were wrong.The level
Shipped renditions probe as H.264 High @ level 3.1 —
avc1.64001F.0x28is level 4.0.A player reads
CODECSto choose a decoder configuration before it fetches a segment, so the declaration is a promise about the bitstream. This one wasn't kept.The audio
Every ad rendered so far is silent and carries a video stream only:
…yet each rendition advertised an AAC-LC track. An audio codec named in
CODECSis an audio track the player expects; a strict one (Safari/AVPlayer) can allocate a decoder and wait for media that never arrives. This is precisely the kind of thing that works in Chrome and fails on an Apple TV.The fix
Both values are now derived from
ffprobeon the encoded rendition rather than written down in advance:avcCodecString()maps the reported profile name back to itsprofile_idcand formatsavc1.PPCCLLmp4a.40.2is appended only when the file actually has an audio streamcodecsrecorded on thehlsasset row now mirrors what the playlist declares, instead of being a third hardcoded copyVerification
The end-to-end pipeline test (real ffmpeg, real encode) now asserts that every declared codec matches
/^avc1\.[0-9A-F]{6}$/and that no silent rendition mentionsmp4a. Unit tests cover the profile→idc mapping and the audio-present/absent split.Repo-wide: 2551 passed / 1 failed — the pre-existing
tracker-geofailure (mmdb not installed locally). Root and worker typechecks clean.Already-rendered revisions
They keep their incorrect playlist. Nothing reads it yet — there's no decision endpoint and no property serving HLS — and re-rendering them means bumping
RENDERER_VERSION, which by design invalidates every cached encode and would re-run all 177. Worth doing before anything actually streams these; not worth seven hours of re-encoding today. Flagging rather than deciding unilaterally.