From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 0ED274B91E for ; Tue, 22 Jul 2025 11:03:57 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 974C568C9ED; Tue, 22 Jul 2025 14:03:53 +0300 (EEST) Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 6ED5E68BE03 for ; Tue, 22 Jul 2025 14:03:46 +0300 (EEST) Received: from haasn.dev (unknown [10.30.1.1]) by haasn.dev (Postfix) with UTF8SMTP id 8A30D403F0; Tue, 22 Jul 2025 13:03:45 +0200 (CEST) From: Niklas Haas To: ffmpeg-devel@ffmpeg.org Date: Tue, 22 Jul 2025 13:03:43 +0200 Message-ID: <20250722110343.88000-1-ffmpeg@haasn.xyz> X-Mailer: git-send-email 2.50.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avfilter/vf_premultiply: use correct premultiplication formula 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: Niklas Haas 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: From: Niklas Haas The previous formula was introduced without justification in 6e713841e8, and the only thing Paul had to say about it over IRC was that it was copied from an unspecified source on the internet. I decided to do some testing and came to the conclusion that this term not only produces "illegal" files, but also lowers PSNR score, over the naive implementation without this extra term. Here are the results of a round-trip test, using allrgb/allyuv (respectively) as the input, and fade=alpha=yes:n=256 to cycle through every possible alpha value, comparing the round-trip output against the input: Before patch: PSNR r:26.677431 g:26.677431 b:26.677431 a:inf average:27.926818 min:6.012093 max:55.400791 PSNR y:26.677431 u:21.101981 v:21.101981 a:inf average:23.548981 min:9.013835 max:53.182303 (full) PSNR y:27.348055 u:21.101981 v:21.101981 a:inf average:23.625238 min:9.554991 max:45.652221 (limited) After patch: PSNR r:27.321996 g:27.321996 b:27.321996 a:inf average:28.571384 min:6.012093 max:52.424553 PSNR y:27.321996 u:23.187879 v:23.187879 a:inf average:25.431773 min:9.013835 max:50.199232 (full) PSNR y:27.868544 u:23.187879 v:23.187879 a:inf average:25.515660 min:9.554991 max:45.078298 (limited) It's worth pointing out that the worst case PSNR is slightly higher for the previous version only because that version can produce values that are illegally high (i.e. R > A), which artificially inflates PSNR for input pairs like (R = 255, A = 2) because it discards less information than it should. As a second evaluation, here is a comparison against the reference value computed with a floating point formats: Before patch: PSNR r:30.734183 g:30.734183 b:30.734183 a:inf average:31.983570 min:12.058264 max:inf After patch: PSNR r:32.611851 g:32.611851 b:32.611851 a:inf average:33.861238 min:12.058264 max:inf --- libavfilter/vf_premultiply.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libavfilter/vf_premultiply.c b/libavfilter/vf_premultiply.c index 322fc39094..9647563f57 100644 --- a/libavfilter/vf_premultiply.c +++ b/libavfilter/vf_premultiply.c @@ -103,7 +103,7 @@ static void premultiply8(const uint8_t *msrc, const uint8_t *asrc, for (y = 0; y < h; y++) { for (x = 0; x < w; x++) { - dst[x] = ((msrc[x] * (((asrc[x] >> 1) & 1) + asrc[x])) + 128) >> 8; + dst[x] = (msrc[x] * asrc[x] + 128) >> 8; } dst += dlinesize; @@ -123,7 +123,7 @@ static void premultiply8yuv(const uint8_t *msrc, const uint8_t *asrc, for (y = 0; y < h; y++) { for (x = 0; x < w; x++) { - dst[x] = ((((msrc[x] - 128) * (((asrc[x] >> 1) & 1) + asrc[x]))) >> 8) + 128; + dst[x] = (((msrc[x] - 128) * asrc[x]) >> 8) + 128; } dst += dlinesize; @@ -143,7 +143,7 @@ static void premultiply8offset(const uint8_t *msrc, const uint8_t *asrc, for (y = 0; y < h; y++) { for (x = 0; x < w; x++) { - dst[x] = ((((msrc[x] - offset) * (((asrc[x] >> 1) & 1) + asrc[x])) + 128) >> 8) + offset; + dst[x] = ((((msrc[x] - offset) * asrc[x]) + 128) >> 8) + offset; } dst += dlinesize; @@ -166,7 +166,7 @@ static void premultiply16(const uint8_t *mmsrc, const uint8_t *aasrc, for (y = 0; y < h; y++) { for (x = 0; x < w; x++) { - dst[x] = ((msrc[x] * (((asrc[x] >> 1) & 1) + asrc[x])) + half) >> shift; + dst[x] = (msrc[x] * asrc[x] + half) >> shift; } dst += dlinesize / 2; @@ -189,7 +189,7 @@ static void premultiply16yuv(const uint8_t *mmsrc, const uint8_t *aasrc, for (y = 0; y < h; y++) { for (x = 0; x < w; x++) { - dst[x] = ((((msrc[x] - half) * (int64_t)(((asrc[x] >> 1) & 1) + asrc[x]))) >> shift) + half; + dst[x] = (((msrc[x] - half) * (int64_t)asrc[x]) >> shift) + half; } dst += dlinesize / 2; @@ -212,7 +212,7 @@ static void premultiply16offset(const uint8_t *mmsrc, const uint8_t *aasrc, for (y = 0; y < h; y++) { for (x = 0; x < w; x++) { - dst[x] = ((((msrc[x] - offset) * (int64_t)(((asrc[x] >> 1) & 1) + asrc[x])) + half) >> shift) + offset; + dst[x] = ((((msrc[x] - offset) * (int64_t)asrc[x]) + half) >> shift) + offset; } dst += dlinesize / 2; -- 2.50.1 _______________________________________________ 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".