Skip to content

Fix FFV1 writing: BGR0 instead of a format the encoder rejects#71

Merged
kmatzen merged 1 commit into
mainfrom
fix/ffv1-pixel-format
Jul 19, 2026
Merged

Fix FFV1 writing: BGR0 instead of a format the encoder rejects#71
kmatzen merged 1 commit into
mainfrom
fix/ffv1-pixel-format

Conversation

@kmatzen

@kmatzen kmatzen commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Fixes #70.

VideoWriter::open() forced FFV1 to AV_PIX_FMT_GBRP. The FFV1 encoder does not accept it:

[ffv1] Specified pixel format gbrp is not supported by the ffv1 encoder.
gbrp     -> FAIL: Invalid argument
yuv444p  -> OK
yuv420p  -> OK

(direct avcodec_open2 probe against libavcodec 62 — the encoder advertises gbrp9le and wider, but not plain 8-bit gbrp)

avcodec_open2 therefore always failed and FFV1 output never worked at all.

BGR0 keeps the stated intent — stay in RGB, no YUV conversion, no chroma subsampling — and matches the BGR24 cv::Mat input channel-for-channel.

How this stayed hidden

The two tests covering FFV1 called SKIP() when open() failed:

if (!writer.open(path, AV_CODEC_ID_FFV1, 320, 240, {30, 1})) {
    SKIP("FFV1 codec not available or pixel format not supported");
}

The message guesses at a missing codec. FFV1 is built into ffmpeg and was present the whole time; the format we handed it was wrong. Combined with ctest reporting "100% tests passed" regardless of skips (#66), a green build hid a completely dead code path.

Both are now REQUIRE. A present-but-unusable encoder is a bug, not an environment quirk.

Stronger assertion

The round-trip test allowed a tolerance of 2, with a comment attributing it to GBRP round-tripping through the scaler — a path that never actually ran. BGR0 never leaves RGB, so the round trip is bit-exact, and the test now checks equality:

CHECK(pixel[0] == 42);   // B
CHECK(pixel[1] == 84);   // G
CHECK(pixel[2] == 168);  // R

Passes.

Result

Suite goes from 37 passed / 2 skipped to 39 passed / 0 skipped — 1129 assertions, no skips.

I found this by investigating why those two tests skipped rather than working around them, while implementing the skip guard for #66. The skip check itself is a follow-up PR; it would have failed CI while this bug was live, which is the correct behaviour but needed this fixed first.

🤖 Generated with Claude Code

VideoWriter::open() forced FFV1 to AV_PIX_FMT_GBRP, which the FFV1
encoder does not accept:

  [ffv1] Specified pixel format gbrp is not supported by the ffv1 encoder
  gbrp    -> FAIL: Invalid argument
  yuv444p -> OK

(direct avcodec_open2 probe against libavcodec 62; the encoder advertises
gbrp9le and wider, but not plain 8-bit gbrp)

So avcodec_open2 always failed and FFV1 output never worked at all.

BGR0 keeps the stated intent -- stay in RGB, no YUV conversion, no chroma
subsampling -- and matches the BGR24 cv::Mat input channel-for-channel.

The two tests covering FFV1 called SKIP() when open() failed, guessing at
a missing codec. FFV1 is built into ffmpeg and was present the whole time;
it was the pixel format we handed it that was wrong. Combined with ctest
reporting "100% tests passed" regardless of skips, a green build hid a
completely broken path. Both are now REQUIRE: a present-but-unusable
encoder is a bug, not an environment quirk.

The round-trip check allowed a tolerance of 2, justified by the GBRP path
that never ran. BGR0 never leaves RGB, so it is bit-exact and the test now
asserts equality.

Suite goes from 37 passed / 2 skipped to 39 passed / 0 skipped.

Fixes #70

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kmatzen
kmatzen merged commit 199fc65 into main Jul 19, 2026
3 checks passed
@kmatzen
kmatzen deleted the fix/ffv1-pixel-format branch July 19, 2026 00:08
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.

FFV1 writing is broken: forces a pixel format the encoder rejects

1 participant