From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 7BC9A47827 for ; Sat, 23 Sep 2023 14:55:11 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 34B5868C933; Sat, 23 Sep 2023 17:55:08 +0300 (EEST) Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F3F4968C874 for ; Sat, 23 Sep 2023 17:55:01 +0300 (EEST) Received: from localhost (unknown [193.1.64.8]) by haasn.dev (Postfix) with ESMTPSA id DED5D42157; Sat, 23 Sep 2023 16:55:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1695480901; bh=8gN0Fv3k8osnOcU0FK+xOnJVFB527r9Zy9OGMaORS5E=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=CbplFsEy36RI67Qu7H2UC3FPGrO9OO0xzNc7dyN59bN9pv6IA1Z8J11izAccZyU/a NjskVpvwyi395ayurq0MkB+/db51KDtf4Tjilkz9Y4j+NEpPIOoKvg8DL+lOu9TZRP vhKhe3wk/PwtwoKHT9MA1TPQDEzsUzkiby+H5lWw= Date: Sat, 23 Sep 2023 15:54:59 +0100 Message-ID: <20230923155459.GB24287@haasn.xyz> From: Niklas Haas To: ffmpeg-devel@ffmpeg.org In-Reply-To: <20230401174737.85192-1-chemag@gmail.com> References: <20230401174737.85192-1-chemag@gmail.com> MIME-Version: 1.0 Content-Disposition: inline Subject: Re: [FFmpeg-devel] [PATCH 1/3] lavfi/vf_psnr: add warning when color ranges differ X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Chema Gonzalez Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Sat, 01 Apr 2023 10:47:35 -0700 Chema Gonzalez 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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". LGTM. The warning will probably be removed once filter graph color range negotiation exists. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".