Repository navigation
feat(cli): doctor names the tested ffmpeg build and says loudness is only comparable on it - #6566
Merged
Merged
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 1 nit · head 583471f8
CI is green. The pin check leaves out the image that produces the regression baselines, so the PR's "a pin bump cannot leave the doctor text stale" holds for only three of the four 8.0.1 pins. Details inline.
— Genesis
… pin test covers the test image
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.
What
hyperframes doctornow says which ffmpeg build is the tested one (8.0.1, what CI, the render images and the regression baselines run). On any other build the FFmpeg row adds "loudness numbers are only comparable on 8.0.1"; on the tested build it says "(tested build)". No warning incheck.Why
The render closes a loop on the encoded AAC: it measures the true peak, then re-encodes with a corrective gain so the peak stays under -1 dBFS. That gain follows the encoder build's own peak. The same fixture gets -5.3 dB of correction on ffmpeg 7.0.2 and -4.6 dB on 8.0.1, which is a 0.7 dB loudness gap (-7.0 vs -6.3 LUFS in a normal local render). The mix graph,
amixand a plain AAC encode are identical across the two builds, so this is not a bug to fix in the engine; the honest answer is to say which build the numbers are tied to.Single owner
TESTED_FFMPEG_VERSIONindoctor.tsis the one string the CLI uses. A test fails if the CI install action,Dockerfile.renderor the Cloud Run Dockerfile stop naming that version, so a pin bump cannot leave the doctor text stale.Why under 100 lines
One constant, one pure function, one call site, and a test that ties the constant to the three pins; nothing else carries the note.
Tests
doctor.test.ts: the tested and other-build wording, and the pin check (fails when the constant is bumped alone). Ran 3 times green on a Linux box.