[FFmpeg-devel] [PATCH 2/2] fftools/ffprobe: 240M matrix is not the same as BT.601

[Marton Balint]

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