Declare colour tags on the Y4M input so tagging the output stops re-matrixing pixels - #105
Open
StuartCameronCode wants to merge 1 commit into
Open
StuartCameronCode wants to merge 1 commit into
StuartCameronCode wants to merge 1 commit into
Conversation
…atrixing pixels The worker re-stamped the source's colour tags as encoder output flags only. With FFmpeg 9.0.1 that is not a relabelling: the Y4M pipe is untagged (csp:unknown), the output demands bt709, so the graph auto-inserts a scaler that treats "unknown" as BT.601 and converts every pixel 601->709. Every encode from a tagged source was colour-shifted (flat 10-bit frame 565/236/756 came back 546/259/741; the encode differed from its own preview by 6.1/255). Output-only -color_primaries/-color_trc also never reached the file. Declare the same tags on the pipe input (between -f yuv4mpegpipe and -i -) as well as on the output, so there is nothing to convert. Input options rather than setparams because a Custom FFmpeg Argument -vf replaces the whole -vf chain. The output flags stay: input-only let a full-range source be negotiated down to limited. build_ffmpeg_args and build_ffmpeg_args_for_test updated together; the misleading "metadata fix, not a pixel one" comment is corrected. Tests: Rust unit tests for input/output agreement and argument placement (plus a source-level check on the real builder); a heavy Flutter test that encodes the tagged pal-sd-25.mov fixture and asserts bit-identical samples vs the untagged encode and the source (FFV1), vs untagged for ProRes 4444's pinned pix_fmt, all four tags written, and a match with the preview. All fail with the input declaration removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GLXdGLfPwgYjW1AkonGqN
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.
Bug
The worker re-stamps the source's colour tags as encoder output flags. The Y4M pipe from vspipe carries no colour info, so FFmpeg 9.0 negotiated
csp:unknown → csp:bt709in the filter graph and auto-inserted a scaler that treated "unknown" as BT.601 — every encode from a tagged source was re-matrixed 601→709 (e.g. Y/U/V 565/236/756 → 546/259/741).-colorspacealone triggers it; rawvideo and Y4M inputs behave the same; ProRes 4444 with its pinned pix_fmt too.Also latent: output-only
-color_primaries/-color_trcnever reached the file (the encoder copies them from the untagged frames), so only 2 of 4 tags were written.Fix
ColorMetadatagainsto_ffmpeg_input_args()/to_ffmpeg_output_args()(the same list by construction).build_ffmpeg_argsandbuild_ffmpeg_args_for_testdeclare the tags between-f yuv4mpegpipeand-i -, and keep the output flags. Input and output agree, so nothing is converted, and all four tags are written.setparamsin-vf: a Custom FFmpeg Argument containing-vfreplaces the whole chain and would silently bring the shift back.ColorMetadataaccepts, and for x264, ProRes 4444, HuffYUV (-pix_fmt yuv422p) and hevc_videotoolbox.User-visible: encodes from tagged sources change pixels (to the correct ones).
Tests
ColorMetadataunit tests; arg-position test for the Y4M input declaration; a check onbuild_ffmpeg_args' own source (the real builder needs located deps).integration_colour_tag_pixels_test.darton taggedpal-sd-25.mov: FFV1 tagged encode bit-identical to untagged and to the source, all four tags present; encode matches preview (diff 0.000/255); ProRes 4444 tagged == untagged.cargo testgreen except the whisper test (add-on not installed);flutter test --exclude-tags heavygreen exceptvapoursynth_integration_test, which can't finddeps/from a worktree.Not verified: NVENC/QSV/AMF; Windows/Linux/macOS x64; a full-range source end to end (raw ffmpeg only).
Docs: CLAUDE.md colour-metadata rule updated; dated write-up in docs/ENGINEERING_NOTES.md.
🤖 Generated with Claude Code
https://claude.ai/code/session_014GLXdGLfPwgYjW1AkonGqN