[FFmpeg-devel] [PATCH 1/3] lavfi/vf_psnr: add warning when color ranges differ

[Niklas Haas]

On Sat, 01 Apr 2023 10:47:35 -0700 Chema Gonzalez <chemag at gmail.com> wrote:
> The PSNR filter uses the pixel values without considering
> the color ranges. This is incorrect. Patch adds a warning
> so at least the user knows it.
> 
> Let's see an example:
> 
> (1) Let's get a simple black pixel/white pixel image.
> ```
> $ echo -n -e "\x00\x00\x00\xff\xff\xff" > /tmp/foo.rgb24
> ```
> 
> (2) From this image, let's distill full and limited range y4m
> copies.
> 
> ```
> $ ffmpeg -y -f rawvideo -video_size 2x1 -pix_fmt rgb24 -i /tmp/foo.rgb24 -vf scale="out_range=full" -pix_fmt yuv420p /tmp/foo.full.y4m
> $ xxd /tmp/foo.full.y4m
> 00000000: 5955 5634 4d50 4547 3220 5732 2048 3120  YUV4MPEG2 W2 H1
> 00000010: 4632 353a 3120 4970 2041 303a 3020 4334  F25:1 Ip A0:0 C4
> 00000020: 3230 6a70 6567 2058 5953 4353 533d 3432  20jpeg XYSCSS=42
> 00000030: 304a 5045 4720 5843 4f4c 4f52 5241 4e47  0JPEG XCOLORRANG
> 00000040: 453d 4655 4c4c 0a46 5241 4d45 0a00 ff80  E=FULL.FRAME....
> 00000050: 80                                       .
> ```
> 
> and
> 
> ```
> $ ffmpeg -y -f rawvideo -video_size 2x1 -pix_fmt rgb24 -i /tmp/foo.rgb24 -vf scale="out_range=limited" -pix_fmt yuv420p /tmp/foo.limited.y4m
> $ xxd /tmp/foo.limited.y4m
> 00000000: 5955 5634 4d50 4547 3220 5732 2048 3120  YUV4MPEG2 W2 H1
> 00000010: 4632 353a 3120 4970 2041 303a 3020 4334  F25:1 Ip A0:0 C4
> 00000020: 3230 6a70 6567 2058 5953 4353 533d 3432  20jpeg XYSCSS=42
> 00000030: 304a 5045 4720 5843 4f4c 4f52 5241 4e47  0JPEG XCOLORRANG
> 00000040: 453d 4c49 4d49 5445 440a 4652 414d 450a  E=LIMITED.FRAME.
> 00000050: 10eb 8080                                ....
> ```
> 
> Note that the 2x images are the same (both have 1x pixel at the
> darkest black, and one at the brightest white). Only difference
> is the range.
> 
> (3) Let's calculate the PSNR score:
> ```
> $ ./ffmpeg -filter_threads 1 -filter_complex_threads 1 -i /tmp/foo.full.y4m -i /tmp/foo.limited.y4m -lavfi "psnr" -f null -
> ...
> [Parsed_psnr_0 @ 0x2f5dac0] PSNR y:22.972065 u:inf v:inf average:25.982365 min:25.982365 max:25.982365
> ```
> 
> As we are comparing an image with itself, we expect "y:inf" as the
> luma PSNR. Issue here is that the PSNR filter just uses the pixel
> values, ignoring the color ranges.
> 
> A possible solution would be to have the filter do the conversion.
> 
> Proposed solution is to add a warning.
> 
> ```
> $ ./ffmpeg -filter_threads 1 -filter_complex_threads 1 -i /tmp/foo.full.y4m -i /tmp/foo.limited.y4m -lavfi "psnr" -f null -
> ...
> [Parsed_psnr_0 @ 0x2f5dac0] master and reference frames use different color ranges (pc != tv)
> ...
> [Parsed_psnr_0 @ 0x2f5dac0] PSNR y:22.972065 u:inf v:inf average:25.982365 min:25.982365 max:25.982365
> ```
> 
> Tested:
> 
> Ran fate.
> ```
> $ make fate -j
> ...
> TEST    seek-lavf-ppmpipe
> TEST    seek-lavf-pgmpipe
> TEST    seek-lavf-mxf_opatom
> ```
> ---
>  libavfilter/vf_psnr.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libavfilter/vf_psnr.c b/libavfilter/vf_psnr.c
> index 15cde7e8c8..058a8932f4 100644
> --- a/libavfilter/vf_psnr.c
> +++ b/libavfilter/vf_psnr.c
> @@ -188,6 +188,13 @@ static int do_psnr(FFFrameSync *fs)
>          td.planeheight[c] = s->planeheight[c];
>      }
>  
> +    if (master->color_range != ref->color_range) {
> +        av_log(ctx, AV_LOG_WARNING, "master and reference "
> +               "frames use different color ranges (%s != %s)\n",
> +               av_color_range_name(master->color_range),
> +               av_color_range_name(ref->color_range));
> +    }
> +
>      ff_filter_execute(ctx, compute_images_mse, &td, NULL,
>                        FFMIN(s->planeheight[1], s->nb_threads));
>  
> -- 
> 2.39.2
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

LGTM.

The warning will probably be removed once filter graph color range negotiation
exists.