incorrect YCoCg to RGB conversion in swscale

[Connor Worley]

Per ​https://trac.ffmpeg.org/wiki/colorspace

The term "YUV" is ambiguous and often used wrongly, including the definition of pixel formats in FFmpeg. A more accurate term for how color is stored in digital video would be ​YCbCr.

When converting AV_PIX_FMT_YUV* to RGB in swscale, a YCbCr matrix is selected for AVCOL_SPC_YCOCG, and incorrect color is produced.

This happens here: ​https://git.videolan.org/?p=ffmpeg.git;a=blob;f=libswscale/yuv2rgb.c;h=0a84b662f9c15a3ee3b4e482ed91212e3f99ff43;hb=HEAD#l64

To fix this, AVCOL_SPC_YCOCG should not use a YCbCr conversion matrix. The YCoCg conversion matrix can be found at ​https://en.wikipedia.org/wiki/YCoCg

[Balling]

YCbCr matrix is selected for AVCOL_SPC_YCOCG, and incorrect color is produced.

We do not support YCoCg. Use mpv instead.

A more accurate term for how color is stored in digital video would be ​YCbCr.

Not that simple. YUV refers to pixel format used that is called AV_PIX_FMT_YUV*. The correct name is indeed AV_PIX_FMT_YUV*, not AV_PIX_FMT_YCBCR*. On the other hand the matrices used here ​https://github.com/FFmpeg/FFmpeg/blob/fa469545ba0782a9245474ecca873aa8984f506e/libavutil/pixfmt.h#L608

are indeed YCbCr, we do not support BT.407 analog TV's YUV.

[Connor Worley]

We do not support YCoCg. Use mpv instead.

I suggest adding some user-facing warning when AVCOL_SPC_YCOCG is used in places where it is intentionally unsupported.

[Balling]

We do. ​https://github.com/FFmpeg/FFmpeg/blob/fa469545ba0782a9245474ecca873aa8984f506e/libavfilter/vf_scale.c#L775

[Balling]

I have YCgCo samples. You do not have to attach them

[Connor Worley]

That is not true of the swscale code path. The attached file does not display a warning:

ffplay started on 2024-01-31 at 16:01:17
Report written to "ffplay-20240131-160117.log"
Log level: 48
Command line:
ffplay /Users/cworley/Downloads/dice_ycg6.mov -report -autoexit
ffplay version 6.1.1 Copyright (c) 2003-2023 the FFmpeg developers
  built with Apple clang version 15.0.0 (clang-1500.1.0.2.5)
  configuration: --prefix=/opt/homebrew/Cellar/ffmpeg/6.1.1_2 --enable-shared --enable-pthreads --enable-version3 --cc=clang --host-cflags= --host-ldflags='-Wl,-ld_classic' --enable-ffplay --enable-gnutls --enable-gpl --enable-libaom --enable-libaribb24 --enable-libbluray --enable-libdav1d --enable-libharfbuzz --enable-libjxl --enable-libmp3lame --enable-libopus --enable-librav1e --enable-librist --enable-librubberband --enable-libsnappy --enable-libsrt --enable-libssh --enable-libsvtav1 --enable-libtesseract --enable-libtheora --enable-libvidstab --enable-libvmaf --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxml2 --enable-libxvid --enable-lzma --enable-libfontconfig --enable-libfreetype --enable-frei0r --enable-libass --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libopenjpeg --enable-libopenvino --enable-libspeex --enable-libsoxr --enable-libzmq --enable-libzimg --disable-libjack --disable-indev=jack --enable-videotoolbox --enable-audiotoo  libavutil      58. 29.100 / 58. 29.100
  libavcodec     60. 31.102 / 60. 31.102
  libavformat    60. 16.100 / 60. 16.100
  libavdevice    60.  3.100 / 60.  3.100
  libavfilter     9. 12.100 /  9. 12.100
  libswscale      7.  5.100 /  7.  5.100
  libswresample   4. 12.100 /  4. 12.100
  libpostproc    57.  3.100 / 57.  3.100
Initialized metal renderer.
[AVFormatContext @ 0x151b35420] Opening '/Users/cworley/Downloads/dice_ycg6.mov' for reading
[file @ 0x600002e98c60] Setting default whitelist 'file,crypto,data'
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x151b35420] Format mov,mp4,m4a,3gp,3g2,mj2 probed with size=2048 and score=100
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x151b35420] ISO: File Type Major Brand: qt  
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x151b35420] Unknown dref type 0x206c7275 size 12
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x151b35420] Processing st: 0, edit list 0 - media time: 0, duration: 522
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x151b35420] Before avformat_find_stream_info() pos: 103588 bytes read:33443 seeks:1 nb_streams:1
[dxv @ 0x14200bc00] YOCOCG6 compression with YCG6 texture (version 3.0)
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x151b35420] All info found
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x151b35420] After avformat_find_stream_info() pos: 102913 bytes read:136320 seeks:2 frames:1
Input #0, mov,mp4,m4a,3gp,3g2,mj2, from '/Users/cworley/Downloads/dice_ycg6.mov':
  Metadata:
    major_brand     : qt  
    minor_version   : 512
    compatible_brands: qt  
    encoder         : Resolume
  Duration: 00:00:00.03, start: 0.000000, bitrate: 24861 kb/s
  Stream #0:0[0x1], 1, 1/15360: Video: dxv (DXD3 / 0x33445844), yuv420p(ycgco/unknown/unknown), 800x600, 24690 kb/s, 30 fps, 30 tbr, 15360 tbn (default)
    Metadata:
      handler_name    : VideoHandler
      vendor_id       : FFMP
detected 12 logical cores
[dxv @ 0x14200bfb0] YOCOCG6 compression with YCG6 texture (version 3.0)
Video frame changed from size:0x0 format:none serial:-1 to size:800x600 format:yuv420p serial:1
[ffplay_buffer @ 0x600002b94420] Setting 'video_size' to value '800x600'
[ffplay_buffer @ 0x600002b94420] Setting 'pix_fmt' to value '0'
[ffplay_buffer @ 0x600002b94420] Setting 'time_base' to value '1/15360'
[ffplay_buffer @ 0x600002b94420] Setting 'pixel_aspect' to value '0/1'
[ffplay_buffer @ 0x600002b94420] Setting 'frame_rate' to value '30/1'
[ffplay_buffer @ 0x600002b94420] w:800 h:600 pixfmt:yuv420p tb:1/15360 fr:30/1 sar:0/1
[AVFilterGraph @ 0x600003899a40] query_formats: 2 queried, 1 merged, 0 already done, 0 delayed
Created 800x600 texture with SDL_PIXELFORMAT_IYUV.
   0.01 M-V:  0.000 fd=   0 aq=    0KB vq=    0KB sq=    0B f=0/0   
[AVIOContext @ 0x151a1fb20] Statistics: 136320 bytes read, 2 seeks

[Balling]

See my comment in ​https://trac.ffmpeg.org/ticket/9132#comment:2

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).

There is no point in implementing that, Chrome/Chromium implemented it and so did Nvidia. See ​https://github.com/mpv-player/mpv/issues/4340

Mpv has much better code for this that just works. Ffplay uses SDL, that cannot support it, see #8862

[Balling]

Oh, I suppose you do not get it how to enable it in MPV. So the software decoder does not support YCgCo, of course. Use:

mpv.com --target-colorspace-hint=yes -vo=gpu-next -gpu-api=d3d11 -hwdec=d3d11va dice_ycg6.mov

That is for Windows.

[Connor Worley]

I am aware the colorspace is broken in swscale and not in mpv. If the difficulties in fixing it in swscale are insurmountable, I believe it would be better to add a warning or deprecate and remove it rather than leaving it silently broken.

[Balling]

The warning does work, just not everywhere. Why I dunno, known bug. Try this file (please download it, ffplay the url will not work) ​https://web.archive.org/web/20210329043315/http://files.1f0.de/samples/bunny_444_YCgCo.mkv

[Connor Worley]

The vf_scale filter you linked is automatically added to the filtergraph depending on the input format: ​https://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavfilter/avfiltergraph.c;h=e4f59f56e2b102372178c693d8439059565d207f;hb=HEAD#l467 -- this is not always necessary, so it does not always happen.

Both this filtergraph behavior and the vf_scale filter itself are unrelated to the lack of a warning in swscale.

[Connor Worley]

I misspoke -- the sample I attached circumvents the swscale path entirely. Still, the warning should not be coupled to avfilter. Another warning should be added in the non-swscale path.

[Connor Worley]

Summarizing what I believe is wrong:

• In ffplay, going through vf_scale should cause an error after 45e09a3041 (vf_scale: use colorspace negotiation API), but ffplay does not set the colorspace parameter on the buffersrc filter so some default conversion is chosen.
• In ffplay, automatic conversion can erroneously happen without going through vf_scale because the buffersink filter accepts YCoCg as a mergeable colorspace.
• swscale silently returns nonsense coefficients for YCoCg

I would like for these things to work, but if they can't, I would much rather they error than produce incorrect output.

[Balling]

the sample I attached circumvents the swscale path entirely

Yeah, I got that. :) DXV Version 3 is interesting.

[Niklas Haas]

Replying to Connor Worley:

Summarizing what I believe is wrong:

• In ffplay, going through vf_scale should cause an error after 45e09a3041 (vf_scale: use colorspace negotiation API), but ffplay does not set the colorspace parameter on the buffersrc filter so some default conversion is chosen.
• In ffplay, automatic conversion can erroneously happen without going through vf_scale because the buffersink filter accepts YCoCg as a mergeable colorspace.
• swscale silently returns nonsense coefficients for YCoCg

I would like for these things to work, but if they can't, I would much rather they error than produce incorrect output.

This sounds like a reasonable assessment. Definitely, ffplay should only advertise supported colorspaces that it can actually output.

I actually have a patch on the ML for fixing the first point: ​https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240110090547.69151-1-ffmpeg@haasn.xyz/

There were some comments on the ML but no real opposal or approval. I'll merge it after a 24h grace period.

[Balling]

Check dcc7263b0e7935b6864e43465cfeca423bdf9e77 though I cannot see anything changing, hm.

[Niklas Haas]

The change is in this scenario:

$ ./ffplay -v verbose -vf format=color_spaces=bt709 dice_ycg6.mov
[ffplay_buffer @ 0x7f1458003200] w:800 h:600 pixfmt:yuv420p tb:1/15360 fr:30/1 sar:0/1 csp:ycgco range:unknown
[auto_scale_0 @ 0x7f1458004dc0] w:iw h:ih flags: interl:0
[Parsed_format_0 @ 0x7f1458003c80] auto-inserting filter 'auto_scale_0' between the filter 'ffplay_buffer' and the filter 'Parsed_format_0'
Impossible to convert between the formats supported by the filter 'ffplay_buffer' and the filter 'auto_scale_0'

It now correctly errors out.

Or:

$ ./ffplay -v verbose -vf format=rgb24 dice_ycg6.mov
[Parsed_format_0 @ 0x7f3288003c00] auto-inserting filter 'auto_scale_0' between the filter 'ffplay_buffer' and the filter 'Parsed_format_0'
Impossible to convert between the formats supported by the filter 'ffplay_buffer' and the filter 'auto_scale_0'

I will submit a patch for the second point, which would just require making ffplay request only formats it actually supports.

[Niklas Haas]

I merged a patch for the second point to master, which means that ffplay dice_ycg6.mov now simply gives you:

Input #0, mov,mp4,m4a,3gp,3g2,mj2, from 'dice_ycg6.mov':=0/0

Metadata:

major_brand : qt
minor_version : 512
compatible_brands: qt
encoder : Resolume

Duration: 00:00:00.03, start: 0.000000, bitrate: 24861 kb/s
Stream #0:0[0x1]: Video: dxv (DXD3 / 0x33445844), yuv420p(ycgco/unknown/unknown), 800x600, 24690 kb/s, 30 fps, 30 tbr, 15360 tbn (default)

Metadata:

handler_name : VideoHandler
vendor_id : FFMP

Impossible to convert between the formats supported by the filter 'ffplay_buffer' and the filter 'auto_scale_0'

and exits out. This is, at least, preferable to displaying a broken image *by default*. If you know what you're doing, you can still use vf_setparams to override the colorspace explicitly... is what I *would* say, but vf_setparams doesn't use the YUV negotiation API yet. Let me fix that.

[Niklas Haas]

Fixed vf_setparams, so now the only thing that can be done to improve the status quo would be to actually implement YCgCo in libswscale.