| Message ID | 20210526012603.12511-2-val.zapod.vz@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,1/2] pixfmt: fixed wrong fix of comment | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
patch subject has a type (ffplay v.s. ffprobe) On Wed, 26 May 2021, Valerii Zapodovnikov wrote: > --- > fftools/ffplay.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fftools/ffplay.c b/fftools/ffplay.c > index 0be1d90bf9..53bd9362fa 100644 > --- a/fftools/ffplay.c > +++ b/fftools/ffplay.c > @@ -963,12 +963,12 @@ static void set_sdl_yuv_conversion_mode(AVFrame *frame) > if (frame && (frame->format == AV_PIX_FMT_YUV420P || frame->format == AV_PIX_FMT_YUYV422 || frame->format == AV_PIX_FMT_UYVY422)) { > if (frame->color_range == AVCOL_RANGE_JPEG) > mode = SDL_YUV_CONVERSION_JPEG; > - else if (frame->colorspace == AVCOL_SPC_BT709) > + else if (frame->colorspace == AVCOL_SPC_BT709) /* FIXME: sometimes it selects this even for BT.601 matrix, see issue 8862 */ I'd rather remove this comment, what is "sometimes"? The ticket is a bit confusing, and if you add that ffplay - by default - uses the automagic color space conversion of the scale filter, which prefers to keep the colorspace in YUV even if it loses precision, so the rendering of YUV444 happens in YUV420 instead of RGB which makes the screenshots attached less comparable because slightly different color (if there is) is not the most significant difference... And if the colors are wrong after YUV444->YUV420, then it is an swscale issue probably anyway, not fixable within ffplay... > mode = SDL_YUV_CONVERSION_BT709; > - else if (frame->colorspace == AVCOL_SPC_BT470BG || frame->colorspace == AVCOL_SPC_SMPTE170M || frame->colorspace == AVCOL_SPC_SMPTE240M) > + else if (frame->colorspace == AVCOL_SPC_BT470BG || frame->colorspace == AVCOL_SPC_SMPTE170M) > mode = SDL_YUV_CONVERSION_BT601; > } > - SDL_SetYUVConversionMode(mode); > + SDL_SetYUVConversionMode(mode); /* FIXME: no support for linear transfer */ > #endif > } > Otherwise looks good. Thanks, Marton
I am sorry, what? It converts to 420 always? Wow? But you are right. This is a problem that colors are wrong after YUV444 - > YUV420 -> RGB. It selects BT.709 matrix even if BT.601 matrix is requested, this can be fixed by "ffplay -vf scale=in_color_matrix=auto,format=gbrp" or just by using mpv. Maybe I need to further test VUI vs -movflags +write_colr vs what my samples use.
On Sun, 6 Jun 2021, Valerii Zapodovnikov wrote: > I am sorry, what? It converts to 420 always? Wow? But you are right. Yes, because ffplay uses the default SDL renderer (may it be opengl/directx or anything else), and ffplay queries the hardware acccelerated pixel formats supported by that renderer (which usually is YUV420 and RGB, and SDL has a very limited list of pixel formats in the first place). -loglevel verbose shows you the texture format ffplay is using. > is a problem that colors are wrong after YUV444 - > YUV420 -> RGB. It > selects BT.709 matrix even if BT.601 matrix is requested, YUV444 -> YUV420 happens in swscale. YUV420 -> RGB happens in SDL using hardware acceleration. If swscale passes the orignial color matrix info properly to the YUV420 frame, then I am not sure where the bug can be, because ffplay directly checks AVFrame->color_range for setting the SDL conversion mode. Regards, Marton > this can be fixed > by "ffplay -vf scale=in_color_matrix=auto,format=gbrp" or just by using > mpv. Maybe I need to further test VUI vs -movflags +write_colr vs what my > samples use.
AVFrame->color_range? But even if I were using full range file (I do not) that would force BT.601 full range matrix (that is JPEG matrix) and I have the opposite here, BT.709 is forced. I suppose the problem can be checked with simple printf of frame->colorspace with that sample that uses BT.601. This one: https://trac.ffmpeg.org/attachment/ticket/9167/2.mp4 It should be wrong at that point. Or use breakpoint in debugger. :) Also, BTW, Nvidia drivers windows 10, what is matrix used for 4K YCbCr formats by default? Is it already BT.709? Because xvYCC can be both BT.601 and BT.709 (but it is only limited range, even if full is set by mistake, see latest "ANSI/CTA-861-H") and sYCC with selectable quantisation uses BT.601 even on 4K! But those are so called digital formats, they should be further forced.
So I am resending still with that comment and a typo fixed to "ffplay". I also found this: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20180501194013.9552-8-onemda@gmail.com/ It way be nice to apply that too, but then again my problem is not that.
diff --git a/fftools/ffplay.c b/fftools/ffplay.c index 0be1d90bf9..53bd9362fa 100644 --- a/fftools/ffplay.c +++ b/fftools/ffplay.c @@ -963,12 +963,12 @@ static void set_sdl_yuv_conversion_mode(AVFrame *frame) if (frame && (frame->format == AV_PIX_FMT_YUV420P || frame->format == AV_PIX_FMT_YUYV422 || frame->format == AV_PIX_FMT_UYVY422)) { if (frame->color_range == AVCOL_RANGE_JPEG) mode = SDL_YUV_CONVERSION_JPEG; - else if (frame->colorspace == AVCOL_SPC_BT709) + else if (frame->colorspace == AVCOL_SPC_BT709) /* FIXME: sometimes it selects this even for BT.601 matrix, see issue 8862 */ mode = SDL_YUV_CONVERSION_BT709; - else if (frame->colorspace == AVCOL_SPC_BT470BG || frame->colorspace == AVCOL_SPC_SMPTE170M || frame->colorspace == AVCOL_SPC_SMPTE240M) + else if (frame->colorspace == AVCOL_SPC_BT470BG || frame->colorspace == AVCOL_SPC_SMPTE170M) mode = SDL_YUV_CONVERSION_BT601; } - SDL_SetYUVConversionMode(mode); + SDL_SetYUVConversionMode(mode); /* FIXME: no support for linear transfer */ #endif }