Wrong pixel format/output when converting video to yuv444p*

[viley]

Converting a video input to any yuv444p pixel format seems broken right now. Visually it looks very wrong (purples instead of blacks), in VLC, Potplayer and FFplay.
FFmpeg's scaler (or auto_scaler) says it's writing the requested pixel format, but using FFprobe on the result, it finds a "gbrp" pixel format.

This happens for video sources regardless of input format, and doesn't seem to depend on the target encoder (tried both libx264 and libx265) or container (tried avi, mp4, and matroska).

Simplest way to reproduce, i.e. with the attached TESTCASE.mkv:

ffmpeg -i TESTCASE.mkv -pix_fmt yuv444p -c:v libx264 -crf 10 TESTCASE-yuv.mkv

Full debug log attached in a file, but if I add -loglevel 42, I see the auto_scaler converting to the correct format apparently:

[graph 0 input from stream 0:0 @ 0000000004e8cd00] w:420 h:212 pixfmt:gbrp tb:1/1000 fr:25/1 sar:1/1
[auto_scaler_0 @ 0000000004e92c00] w:iw h:ih flags:'bicubic' interl:0
[format @ 0000000004e8e0c0] auto-inserting filter 'auto_scaler_0' between the filter 'Parsed_null_0' and the filter 'format'
[auto_scaler_0 @ 0000000004e92c00] w:420 h:212 fmt:gbrp sar:1/1 -> w:420 h:212 fmt:yuv444p sar:1/1 flags:0x4

However, FFprobe detects gbrp:

Input #0, matroska,webm, from 'TESTCASE-yuv.mkv':
  Metadata:
    ENCODER         : Lavf58.68.100
  Duration: 00:00:04.00, start: 0.000000, bitrate: 17 kb/s
  Stream #0:0: Video: h264 (High 4:4:4 Predictive), gbrp(tv, gbr/unknown/unknown, progressive), 420x212 [SAR 1:1 DAR 105:53], 25 fps, 25 tbr, 1k tbn, 50 tbc (de
fault)
    Metadata:
      ENCODER         : Lavc58.126.100 libx264
      DURATION        : 00:00:04.000000000

More notes:

• As mentioned, any yuv444p* pixel format will show the issue. If I use yuv444p10le, FFprobe will report gbrp10le, etc.

• Trying to explicitly insert a scaler does not fix the issue, for instance with -vf "scale=iw:ih:flags=neighbor+full_chroma_inp:in_range=full:out_range=full:out_color_matrix=bt709"

• This doesn't happen with yuv420p* or yuv422p* formats, only the 4:4:4 ones.

• This also does not happen if the source is a looped .png, for instance: ffmpeg -loop 1 -i testcase.png -frames:v 100 -pix_fmt yuv444p -c:v libx264 -crf 0 out.mkv - in this case the output looks correct and FFprobe shows "yuv444p(tv, progressive)".

Version: Using gyan.dev's Windows build (git-master, 2021-02-28).

Tagging as a regression, since it looks like this was broken at some point between 2020-11-19 (last build that didn't show the issue) and 2021-01-12.

[viley]

test case input

[viley]

test case output

[viley]

debug log for mentioned command line

[viley]

Also happens with BtbN's build from today (N-101352-g0e645b98c6 2021-03-01 12:33)

[Balling]

Matrix coefficients: Identity

Seriously?
I agree it is a bug and the behavior has changed, i.e. Identity matrix was not written and now it is.

For now use

ffmpeg -i TESTCASE.mkv -pixel_format yuv444p -color_primaries 1 -color_trc 1 -colorspace 1 TESTCASE-yuv1.mkv

You can add -color_range too. Also yuv420p and yuv422p are "broken" too.

Identity (and YCgCo) are very different matrices, as the order of operations are different (MatrixCoefficients 0 (Identity) or 8 (YCgCo) not only use different equations for the full-to-limited range adjustment but also perform the full-to-limited range adjustment before applying the transfer matrix). Maybe that is why. Dunno. FFmpeg tries to preserve such flags, maybe they overpresrved it.

BTW, please note that if you will ever use bmp, it is broken. See: ​https://video.stackexchange.com/questions/19944/ffmpeg-bmp-to-yuv-x264-color-shift

you must add -vf format=rgb24 for it, that is #8056.

[Carl Eugen Hoyos]

Regression since 7369595c556516b90bf3acdf85043dc11add7d89

[Carl Eugen Hoyos]

Replying to Balling:

Please absolutely stop changing the status to open if there is still information missing!

[viley]

Replying to Balling:

For now use

ffmpeg -i TESTCASE.mkv -pixel_format yuv444p -color_primaries 1 -color_trc 1 -colorspace 1 TESTCASE-yuv1.mkv

You can add -color_range too. Also yuv420p and yuv422p are "broken" too.

Thanks for the info and the workaround. Above works perfectly when scaling with out_color_matrix=bt709 and adding -color_range 2 (which was my intended use case).

[Balling]

BTW, I checked and the output is lossless except for one byte that signals color matrix.

[jeeb]

Alright, this seems to be an issue in the scale filter not overriding color metadata properly in case of a pixel format conversion :) Thus the issue itself is much older, but it just never popped up until my change started passing the information further.

If I apply the following patch I can see that the scale filter is happily doing the flagging after copying props from the input frame.

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 58eee96744..8377be2e39 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -647,6 +647,22 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s
                          out,out_stride);
 }
 
+static void log_avframe(AVFilterContext *ctx, const char *prefix, AVFrame *frame)
+{
+    if (!frame)
+        return;
+
+    av_log(ctx, AV_LOG_VERBOSE,
+           "%s picture info: w=%d , h=%d, pix_fmt=%s (range=%s, primaries=%s, transfer=%s, cspace=%s, cloc=%s)\n",
+           prefix, frame->width, frame->height,
+           av_get_pix_fmt_name(frame->format),
+           av_color_range_name(frame->color_range),
+           av_color_primaries_name(frame->color_primaries),
+           av_color_transfer_name(frame->color_trc),
+           av_color_space_name(frame->colorspace),
+           av_chroma_location_name(frame->chroma_location));
+}
+
 static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out)
 {
     AVFilterContext *ctx = link->dst;
@@ -806,6 +822,9 @@ scale:
         scale_slice(link, out, in, scale->sws, 0, link->h, 1, 0);
     }
 
+    log_avframe(link->dst, "Input", in);
+    log_avframe(link->dst, "Output", out);
+
     av_frame_free(&in);
     return 0;
 }

[auto_scaler_0 @ 00000179d2dc2200] Input picture info: w=420 , h=212, pix_fmt=gbrp (range=pc, primaries=unknown, transfer=unknown, cspace=gbr, cloc=left)
[auto_scaler_0 @ 00000179d2dc2200] Output picture info: w=420 , h=212, pix_fmt=yuv444p (range=tv, primaries=unknown, transfer=unknown, cspace=gbr, cloc=left)

Of course, the color space should no longer be "gbr" at that point.

[Balling]

Replying to JEEB:

Alright, this seems to be an issue in the scale filter not overriding color metadata properly in case of a pixel format conversion :) Thus the issue itself is much older, but it just never popped up until my change started passing the information further.

Or maybe because by default it uses libx264 and not -c:v libx264rgb?

[jeeb]

Replying to Balling:

Replying to JEEB:

Alright, this seems to be an issue in the scale filter not overriding color metadata properly in case of a pixel format conversion :) Thus the issue itself is much older, but it just never popped up until my change started passing the information further.

Or maybe because by default it uses libx264 and not -c:v libx264rgb?

I am not sure what that is a reference to? The encoder selection (and thus which pix_fmt is selected by default as the output of the video filter chain) is outside the scope of both:

1. my changes to ffmpeg.c (to actually pass on information provided by the filter chain onwards), as well as
2. the issue identified in the scale video filter in this ticket.

Or would you say that f.ex. RGB→YCbCr conversion should still preserve the color space value of gbr in the output AVFrame? :) (which is only there due to there being a av_frame_copy_props call, which copies all the metadata from the input AVFrame - which is not filtered afterwards depending on whether it was just a scale job, or if the pix_fmt was changed, etc etc)

To fix the scale filter bug there needs to be some logic added to not pass on input metadata as-is in case of a conversion that would nullify it, such as conversion from YCbCr to RGB or vice versa.

[Balling]

Replying to JEEB:

I am not sure what that is a reference to?

Because there are two h.264 wrappers around one 264 encoder, nobody cared about that bug is what I meant. rgb will not touch Identity matrix. And not rgb will always remove it.

Also BTW, what that means is that there is no FATE for RGB to YCbCr that looks into matrix. Wow. See, it all passed: ​https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201027183056.17993-5-jeebjp@gmail.com/

[jeeb]

Replying to Balling:

Replying to JEEB:

I am not sure what that is a reference to?

Because there are two h.264 wrappers around one 264 encoder, nobody cared about that bug is what I meant. rgb will not touch Identity matrix. And not rgb will always remove it.

I see it a bit differently. Nobody cared about this bug before as the information from the filter chain was not utilized. :) Thus you had to once set the metadata in the filter chain, and once for the encoder - separately. Also most likely other filters would then only look at pix_fmt to see if it's RGB or not, not the colorspace field.

Also it would work if the input would not set any color space fields, which explains why f.ex. Ut Video from RGB to YCbCr would work, or just pure piped raw RGB. In this case the H.264 decoder sets the colorspace field, and that metadata is passed through the scale filter.

Also BTW, what that means is that there is no FATE for RGB to YCbCr that looks into matrix. Wow. See, it all passed: ​https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201027183056.17993-5-jeebjp@gmail.com/

Yup. We have tests most likely to verify that the conversion from RGB to YCbCr works correctly (data wise), but nothing checks that if the input AVFrame has color metadata set, that the AVFrame metadata made after such a conversion. That would basically be something to add after the issue is fixed, I 100% agree.

[Balling]

Replying to viley:

Above works perfectly when scaling with out_color_matrix=bt709 and adding -color_range 2 (which was my intended use case).

You will also have to add -vf scale=out_range=pc, not only -color_range pc, since swscale defaults to limited range bt.601 (zscale default to source range).

[Balling]

So, Jan, are you going to fix it?

[jeeb]

I did make an attempt (​https://github.com/jeeb/ffmpeg/commits/swscale_check_colorspace_changed) some time ago, but alas with that quick test I couldn't flag the right spots on where swscale had changed the color space. In theory having swscale tell you when it changed the content color space would be the best way since that way people utilizing swscale wouldn't have to duplicate that logic onto their own API usage (outside of vf_scale). Alternatively we'd just have AVFrame interfaces for swscale, but I don't think that would happen any time soon.

Thus it seems like the most realistic short term thing is a change towards vf_scale where you have a color space checker function that checks if f.ex. the gbr identify matrix is set and your output pix_fmt is not RGB (among other cases). Then the color space information could be reset accordingly (since currently the problem is that av_frame_copy_props does indeed copy all properties from the source AVFrame to the output AVFrame, and nothing touches the colorspace value in the structure).

On IRC there were ideas like just stopping the exposure of the identity matrix since effectively the RGB pix_fmts already provide that information, but I was not sure if that was the best solution.

[Balling]

I see you are trying to get rid off libx264rgb. Was I right and this is the way to fix this issue? ​https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210702112513.36348-4-jeebjp@gmail.com/

[jeeb]

No, that is a separate track :) . libx264 is just the only encoder with this sort of separation, and the original bug requested 4:2:0 to be the default. I think the separation never really did that, just split off RGB from 4:4:4 (so for RGB input you got 4:4:4 instead of RGB by default). So as such as far as compatibility is concerned, it never seemed to match the requested capability. And it caught my attention since I ended up fixing the configure checks related to it :P .

The real fix will either come with a working version of that swscale check (I did poke people about it, but didn't get any response), or when elenril's AVFrame based API is merged into swscale, which will come with the switch of vf_scale to that API.

After all, the problem right now is just that when a conversion is done (RGB is converted to YCbCr), the given output AVFrame field is not reset accordingly. You can see an example of that in the avfilter/vf_scale: reset color properties if a conversion was done commit in my branch.

edit:
What I mean is that by adding RGB to libx264 you will of course get RGB by default instead of 4:4:4 YCbCr as the default pixel format, and thus the bug will not be visible. But not being visible is different from the issue actually being fixed.

As soon as you try to encode any non-RGB from such "RGB flagged with that colorspace value", you will mistakenly get that value still in the converted output AVFrame as vf_scale or swscale does not currently clear it. Be it YCbCr 4:2:0, 4:2:2 or 4:4:4, for example.

[jeeb]

I could not come with a better idea for now - and since the initial version of the swscale AVFrame API will not have any sanity checks - so posted an ​initial fix for this that cleans up this one case of invalid metadata in case vf_scale is pushing out non-RGB, but the copied props contain the RGB/XYZ identity matrix.

[Balling]

this one case of invalid metadata

There are other cases??

Why you do not want to reset all ​https://github.com/jeeb/ffmpeg/commit/09abc1ea76d87d73ad2ec4a331139baeb618f8fb#diff-dc711ef6bb905f1c3a99a4552c12f72c2020e1fc6240ed5f3d9d5867081d4ca4R743?

In particular it is very important that chroma siting (which is different for JPEG, avc and HDR in HEVC) will be correctly set in VUI in avc/hevc bitsream. P.S. I do not know whether it is being done even now, for example convertion from jpeg 4:2:0 to avc 4:2:0 should change chroma siting from center to left not just tag it, but does it happen?

Ideally we should set all parameters: range, matrix and primaries/trc and that will immediatelly write nclx atom in mp4.

[jeeb]

Replying to Balling:

this one case of invalid metadata

There are other cases??

I generally don't expect any code to be perfect. So there may or may not be issues. It's a way of wording. So far it's this one that has popped up.

Why you do not want to reset all ​https://github.com/jeeb/ffmpeg/commit/09abc1ea76d87d73ad2ec4a331139baeb618f8fb#diff-dc711ef6bb905f1c3a99a4552c12f72c2020e1fc6240ed5f3d9d5867081d4ca4R743?

Because nobody helped me when I asked regarding swscale and where the needed locations were. Also it was disliked since it throws the baby out together with the bathwater. So the simple invalidation it is. If the AVFrame is flagged as RGB, but pix_fmt is not RGB, then off that value goes.

I expect that more logic that actually follows what swscale itself is doing (whatever it is doing) will be added as the swscale AVFrame API gets merged.

In particular it is very important that chroma siting (which is different for JPEG, avc and HDR in HEVC) will be correctly set in VUI in avc/hevc bitsream. P.S. I do not know whether it is being done even now, for example convertion from jpeg 4:2:0 to avc 4:2:0 should change chroma siting from center to left not just tag it, but does it happen?

I don't think there's any other filter than maybe zscale which will do chroma_location adjustment for you in libavfilter (and even in that case you probably have to go through 4:4:4). So much more realistic is just the case of passing that value on from the input, so it is flagged in the output correctly.

That said, you can just look at the result of:

git grep -E "chroma_sample_location|chroma_location" -- libavcodec/

You can see that the cli tools and pretty much all encoders actually do not touch that value at all :P . H.264/HEVC decoders and the dav1d wrapper do expose it, though. Which is why players have access to the value in these cases.

I actually noticed this the other day when reviewing an SVT-AV1 patch that touched other metadata values, but not chroma location.

[Balling]

You also said FATE tests should be done :)
You are wrong that it is only zscale. It is not:

-vf 'scale=out_color_matrix=bt709:out_h_chr_pos=0:out_v_chr_pos=128'

The position is specified in the luma grid / 256, so for a 4x4 grid in 4:2:0, 0:0 is center of the first pixel, 256:256 the last, and 0:128 is in the first column, between both rows - i.e. mpeg2 position.

HEVC type 2 should be 0:0 from what I can tell (pixfmt.h calls this AVCHROMA_LOC_TOPLEFT).

Oh, and also AVC center-left default should not be written.

As for how to tag the file:

ffmpeg -i file.mp4 -c copy -bsf:v hevc_metadata=chroma_sample_loc_type=2 file1.mp4
Obviously 0, 1, 2, 3, 4, 5 are possible but also -1 can be applied to not touch it. 2 is for BT.2100 in hevc unless it is profile 5 Dolby Vision that uses 0 :)

[Balling]

You now (after 9dd410c80416197188337e3b7e1600be41d2ea64) set matrix unspecified in VUI, which is very bad! Check with ffmpeg -i TESTCASE-yuvnew.mp4 -c copy -bsf:v trace_headers -f null -

this file:
ffmpeg -i rec.mkv -vf scale=out_color_matrix=bt709:flags=accurate_rnd,format=yuv420p -c:v libx264 -crf 10 -color_primaries 1 -color_trc 1 TESTCASE-yuvnew.mp4 where rec.mkv is rgb from #9374.

What is worse, not only the problem here is not really fixed, but also the opposite problem when you do ffmpeg -i .\TESTCASE-yuvnewer.mp4 -vf scale=flags=accurate_rnd -c:v libx264rgb -crf 10 -color_primaries 1 -color_trc 1 rgbsupper.mp4 from YCbCr with -colorspace 0 happens (produced with ffmpeg -i rec.mkv -vf scale=out_color_matrix=bt709:flags=accurate_rnd,format=yuv420p -c:v libx264 -crf 10 -color_primaries 1 -color_trc 1 -colorspace 1 TESTCASE-yuvnewer.mp4). It does not happen when -colorspace 1 is not done. Oogh.

[jeeb]

1) Nobody stops you testing these patches before they land since it's not like they're hidden somewhere (and also with the hindsight of going back to this point of the message after writing the rest of it, you could have seen this issue that you are now complaining about was there even before).

2) Can you please be less rambly and all over the place? It's really hard to understand what and where since the steps and expected and received results are all over the place. For the record, vf_scale never set the matrix coefficients, so unless out->format as a pix_fmt didn't for some reason had a mis-matching RGB flag I do not know how the flying f-word I could have made this any worse than it was. I just stopped a completely incorrect configuration from going forwards.

Thus, step by step trying to decrypt your message:

ffmpeg -v verbose -i ffmpeg_samples\ticket_9374\rec.mkv -vf scale=out_color_matrix=bt709:flags=accurate_rnd,format=yuv420p -c:v libx264 -crf 10 -color_primaries 1 -color_trc 1 TESTCASE-yuvnew.mp4

leads to a result that I'd expect, which seems completely fine on playback.

Stream #0:0[0x1](und): Video: h264 (High), 1 reference frame (avc1 / 0x31637661), yuv420p(tv, unknown/bt709/bt709, left), 1920x1200, 952 kb/s, 25 fps, 25 tbr, 12800 tbn (default)

The matrix coefficients are not set to BT.709 or anything else specific because the filter never did that. One needs to go look into the swscale code to actually verify and add code to set the value according to swscale's behavior. And I've touched swscale enough during the past few months to not want to touch that thing for a while, so excuse me. The zscale filter has options for all these three values which is why if they're configured you then get them passed through and received from zimg's configuration. So no, I've not made this specific conversion any worse. At all.

Alright, so let's go to the next step where you forcibly set the matrix coefficients through an AVOption for the encoder.

ffmpeg -v verbose -i ffmpeg_samples\ticket_9374\rec.mkv -vf scale=out_color_matrix=bt709:flags=accurate_rnd,format=yuv420p -c:v libx264 -crf 10 -color_primaries 1 -color_trc 1 -colorspace 1 TESTCASE-yuvnewer.mp4

That results in:

Stream #0:0[0x1](und): Video: h264 (High), 1 reference frame (avc1 / 0x31637661), yuv420p(tv, bt709, left), 1920x1200, 952 kb/s, 25 fps, 25 tbr, 12800 tbn (default)

Apparently when all values match the separate logging is skipped. So that says that all the metadata you've set is correctly passed on. Still everything good.

Finally, the example where you convert YCbCr to RGB, and then forcibly set the output encoder's primaries and transfer to BT.709.

ffmpeg -v verbose -i TESTCASE-yuvnewer.mp4 -vf scale=flags=accurate_rnd -c:v libx264rgb -crf 10 -color_primaries 1 -color_trc 1 rgbsupper.mp4

Encoder configuration looks good to me (pix_fmt is RGB):

 Stream #0:0(und): Video: h264 (avc1 / 0x31637661), rgb24(pc, bt709, progressive), 1920x1200, q=2-31, 25 fps, 12800 tbn (default)

vf_scale's internal conversion also seems Just Fine

[Parsed_scale_0 @ 000002009cc31700] w:1920 h:1200 fmt:yuv420p sar:0/1 -> w:1920 h:1200 fmt:rgb24 sar:0/1 flags:0x40000

But yes, it seems like for some reason when the encoded output is marked as full range4:4:4 YCbCr.

Stream #0:0[0x1](und): Video: h264 (High 4:4:4 Predictive), 1 reference frame (avc1 / 0x31637661), yuvj444p(pc, bt709, left), 1920x1200, 2230 kb/s, 25 fps, 25 tbr, 12800 tbn (default)

I am not sure, though, why you think this is the fault of my change and why you have started pointing the finger at me.

As I noted, vf_scale never set the matrix coefficients to anything. It just copied whatever metadata the input AVFrame might have had, and passed it on. I have now stopped that in a single case which is clearly wrong to fix a single problematic case.

And if you don't trust me, just revert that commit I added and see what you get out of that last YCbCr->RGB conversion of yours. I just tested it here locally and my changes have nothing to do with it. You have successfully hit a separate bug somewhere along the range, most likely another case of where vf_scale should not be passing the input AVFrame's metadata out as-is. These bugs have been there since at least 7e350379f87e7f74420b4813170fe808e2313911 , and the only reason why things hit my commit from late 2020 is because I for the first time in the history of ffmpeg.c actually plugged the input pipe into the output pipe, so people see all these bugs that always were there but were not visible in ffmpeg.c (But would have confused the heck out of other libavfilter users).

[Balling]

you could have seen this issue that you are now complaining about was there even before

I think I did analyse it right, if source file has no BT.709 matrix libx264rgb sets identity still after your patch, if BT.709 matrix is there libx264rgb does not set Identity, unspecified is used after your patch. While for ycbcr matrix is not needed, unspecified can work just as good, for RGB identity is needed and unspecified will lead to defaulting to ycbcr (as happened with "marked as full range4:4:4 YCbCr"). I came to this idea to test back from ycbcr back to rgb today, in the morning. So... I could not have applied patchwork's patch to test this. Sorry.

Anyway, the problem is that I thought that your patch while resetting the matrix to unspecified will still in the end apply some matrix to VUI. I was wrong. Sorry about that too. You said so yourself: "Then the color space information could be reset accordingly (since currently the problem is that av_frame_copy_props does indeed copy all properties from the source AVFrame to the output AVFrame, and nothing touches the colorspace value in the structure)."

P.S. BTW, I did not suggest that YCbCr --> RGB issue is related to your patch. Will recheck again.

[Balling]

Okay, interesting moment here is that RGB jpegs produce chroma 1 (JPEG's center) with x265 yuv420p which I think cannot be true.

It looks like mpv does properly support hevc chroma 1 type and type 0 as I checked by doing -c copy.

RGB matrix from YCbCr is indeed working now. Unspecified of course still needs to be fixed from RGB to YCBCR. swscale defaults to BT.601 so may be there would have been more point to write BT.601 instead of unspecified. Unfortunately, since YCbCr matrix is not one (there is 601, 709, smpte 240M and 2020 matrices and that is not talking about Chromaticity-derived non-constant luminance system, that is type 12, where you get matrix from looking into what primaries are set too) and the value is not propogatings in swscale, it has to be set by hand still even now. Oogh.

BTW, we need to start testing XYZ too. How does one produce xyz with zscale?

[Balling]

Can we close this? After all the other remaining issue here is described here. #9167. That is to you Paul.

[Balling]

Duplicate of #9167.