[FFmpeg-devel] [PATCH] avfilter/vf_premultiply: use correct premultiplication formula

[Niklas Haas]

From: Niklas Haas <git at haasn.dev>

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