diff mbox series

[FFmpeg-devel] swscale/yuv2rgb: fix shift values for conversion to X2RGB10

Message ID 20210921022123.27995-1-code@mstoeckl.com
State New
Headers show
Series [FFmpeg-devel] swscale/yuv2rgb: fix shift values for conversion to X2RGB10
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Manuel Stoeckl Sept. 21, 2021, 2:21 a.m. UTC
This resolves a problem where conversions from YUV to X2RGB10LE
would produce color values a factor 4 too small.

The variable 'yval' used later in the switch case 30 has range
[0,255], but the color channel values in X2RGB10 have two more
bits than 'yval' and can go up to 1023. Increasing (r|g|b)base
by 2 effectively multiplies yval by 4 and fixes this discrepancy.

Signed-off-by: Manuel Stoeckl <code@mstoeckl.com>
---
 libswscale/yuv2rgb.c                     | 6 +++---
 tests/ref/fate/filter-pixdesc-x2rgb10le  | 2 +-
 tests/ref/fate/filter-pixfmts-copy       | 2 +-
 tests/ref/fate/filter-pixfmts-crop       | 2 +-
 tests/ref/fate/filter-pixfmts-field      | 2 +-
 tests/ref/fate/filter-pixfmts-fieldorder | 2 +-
 tests/ref/fate/filter-pixfmts-hflip      | 2 +-
 tests/ref/fate/filter-pixfmts-il         | 2 +-
 tests/ref/fate/filter-pixfmts-null       | 2 +-
 tests/ref/fate/filter-pixfmts-pad        | 2 +-
 tests/ref/fate/filter-pixfmts-scale      | 2 +-
 tests/ref/fate/filter-pixfmts-transpose  | 2 +-
 tests/ref/fate/filter-pixfmts-vflip      | 2 +-
 13 files changed, 15 insertions(+), 15 deletions(-)

Comments

Michael Niedermayer Sept. 22, 2021, 4:53 p.m. UTC | #1
On Mon, Sep 20, 2021 at 10:21:23PM -0400, Manuel Stoeckl wrote:
> This resolves a problem where conversions from YUV to X2RGB10LE
> would produce color values a factor 4 too small.
> 
> The variable 'yval' used later in the switch case 30 has range
> [0,255], but the color channel values in X2RGB10 have two more
> bits than 'yval' and can go up to 1023. Increasing (r|g|b)base
> by 2 effectively multiplies yval by 4 and fixes this discrepancy.

does white have 1023 ? or 1020 ?
a multiplication by 4 would not in general produce a correctly scaled 10bit
per channel value from 8bit

thx

[...]
Manuel Stoeckl Sept. 22, 2021, 9:50 p.m. UTC | #2
> > 
> > The variable 'yval' used later in the switch case 30 has range
> > [0,255], but the color channel values in X2RGB10 have two more
> > bits than 'yval' and can go up to 1023. Increasing (r|g|b)base
> > by 2 effectively multiplies yval by 4 and fixes this discrepancy.  
> 
> does white have 1023 ? or 1020 ?
> a multiplication by 4 would not in general produce a correctly scaled
> 10bit per channel value from 8bit

Yes, the shift approach is flawed in that it converts 255 to 1020.
Unfortunately, 255 does not divide 1023, so no matter what there will
be inputs where the conversion is slightly off.

I'd prefer not to optimize this conversion here, when the code will
inevitably need to be rewritten later to properly handle conversions
from 10-bit or higher YUV input. (Unfortunately, while I am certain
this is possible, I don't yet understand how the swscale code works
enough yet well enough to do that. There are also possible problems
with gamma and dithering to consider.)
Manuel Stoeckl Sept. 24, 2021, 3:40 a.m. UTC | #3
> On Mon, Sep 20, 2021 at 10:21:23PM -0400, Manuel Stoeckl wrote:
> > This resolves a problem where conversions from YUV to X2RGB10LE
> > would produce color values a factor 4 too small.
> > 
> > The variable 'yval' used later in the switch case 30 has range
> > [0,255], but the color channel values in X2RGB10 have two more
> > bits than 'yval' and can go up to 1023. Increasing (r|g|b)base
> > by 2 effectively multiplies yval by 4 and fixes this discrepancy.  
> 
> does white have 1023 ? or 1020 ?
> a multiplication by 4 would not in general produce a correctly scaled
> 10bit per channel value from 8bit

I've replied to this email with a new version of the patch set, that
now ensures the channel values in the YUV->RGB table created in
ff_yuv2rgb_c_init_tables go up to 1023.

However, this does not mean the code will always convert 1.0 white to
1.0 white; for example, pure 0xffff RGBA64 white gets converted to
X2RGB10 grey level 1019. Presumably there is an off-by-one rounding
error somewhere in the RGB->YUV->RGB conversion path, but I don't think
tracking it down is worth it at the moment.
diff mbox series

Patch

diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c
index cac82f4c6f..e084bd8eec 100644
--- a/libswscale/yuv2rgb.c
+++ b/libswscale/yuv2rgb.c
@@ -966,9 +966,9 @@  av_cold int ff_yuv2rgb_c_init_tables(SwsContext *c, const int inv_table[4],
         fill_gv_table(c->table_gV, 1, cgv);
         break;
     case 30:
-        rbase = 20;
-        gbase = 10;
-        bbase = 0;
+        rbase = 22;
+        gbase = 12;
+        bbase = 2;
         needAlpha = CONFIG_SWSCALE_ALPHA && isALPHA(c->srcFormat);
         if (!needAlpha)
             abase = 30;
diff --git a/tests/ref/fate/filter-pixdesc-x2rgb10le b/tests/ref/fate/filter-pixdesc-x2rgb10le
index 94c8640a56..093d3bc3e7 100644
--- a/tests/ref/fate/filter-pixdesc-x2rgb10le
+++ b/tests/ref/fate/filter-pixdesc-x2rgb10le
@@ -1 +1 @@ 
-pixdesc-x2rgb10le    98d697ed4668daf535163d5e08c903bb
+pixdesc-x2rgb10le   d3a315972acd4cbc16837bd8c8410d17
diff --git a/tests/ref/fate/filter-pixfmts-copy b/tests/ref/fate/filter-pixfmts-copy
index 1d7657c2af..7a7e42a8b4 100644
--- a/tests/ref/fate/filter-pixfmts-copy
+++ b/tests/ref/fate/filter-pixfmts-copy
@@ -80,7 +80,7 @@  rgba                b6e1b441c365e03b5ffdf9b7b68d9a0c
 rgba64be            ae2ae04b5efedca3505f47c4dd6ea6ea
 rgba64le            b91e1d77f799eb92241a2d2d28437b15
 uyvy422             3bcf3c80047592f2211fae3260b1b65d
-x2rgb10le           b0a0c8056521beeaa3fea4985ca87176
+x2rgb10le           75665d76a2077d0106d8cd65a067c475
 xyz12be             a1ef56bf746d71f59669c28e48fc8450
 xyz12le             831ff03c1ba4ef19374686f16a064d8c
 ya16be              37c07787e544f900c87b853253bfc8dd
diff --git a/tests/ref/fate/filter-pixfmts-crop b/tests/ref/fate/filter-pixfmts-crop
index 8fc7614192..17e1db2721 100644
--- a/tests/ref/fate/filter-pixfmts-crop
+++ b/tests/ref/fate/filter-pixfmts-crop
@@ -77,7 +77,7 @@  rgb8                9b364a8f112ad9459fec47a51cc03b30
 rgba                9488ac85abceaf99a9309eac5a87697e
 rgba64be            89910046972ab3c68e2a348302cc8ca9
 rgba64le            fea8ebfc869b52adf353778f29eac7a7
-x2rgb10le           5c0789f76a713f343c2ed42a371d441d
+x2rgb10le           69385674767a31dad7118bd444958c54
 xyz12be             cb4571f9aaa7b59f999ef327276104b7
 xyz12le             cd6aae8d26b18bdb4b9d068586276d91
 ya16be              a3d18014454942a96f15a49947c0c55d
diff --git a/tests/ref/fate/filter-pixfmts-field b/tests/ref/fate/filter-pixfmts-field
index ce8e53571f..871234bd00 100644
--- a/tests/ref/fate/filter-pixfmts-field
+++ b/tests/ref/fate/filter-pixfmts-field
@@ -80,7 +80,7 @@  rgba                ee616262ca6d67b7ecfba4b36c602ce3
 rgba64be            23c8c0edaabe3eaec89ce69633fb0048
 rgba64le            dfdba4de4a7cac9abf08852666c341d3
 uyvy422             1c49e44ab3f060e85fc4a3a9464f045e
-x2rgb10le           a7a5dcdfe1d4b6bd71e40b01c735f144
+x2rgb10le           3a271577cbbd82b58381fd9649812707
 xyz12be             d2fa69ec91d3ed862f2dac3f8e7a3437
 xyz12le             02bccd5e0b6824779a1f848b0ea3e3b5
 ya16be              40403b5277364777e0671da4d38e01ac
diff --git a/tests/ref/fate/filter-pixfmts-fieldorder b/tests/ref/fate/filter-pixfmts-fieldorder
index 90d36add83..413ea2925a 100644
--- a/tests/ref/fate/filter-pixfmts-fieldorder
+++ b/tests/ref/fate/filter-pixfmts-fieldorder
@@ -71,7 +71,7 @@  rgba                1fdf872a087a32cd35b80cc7be399578
 rgba64be            5598f44514d122b9a57c5c92c20bbc61
 rgba64le            b34e6e30621ae579519a2d91a96a0acf
 uyvy422             75de70e31c435dde878002d3f22b238a
-x2rgb10le           636c90498c64abba1cc0624c5209a61f
+x2rgb10le           403e41a1387fbe3e31b3bca8313961f1
 xyz12be             15f5cda71de5fef9cec5e75e3833b6bc
 xyz12le             7be6c8781f38c21a6b8f602f62ca31e6
 ya16be              0f13e0f52586d172aaa07710fa3e8f31
diff --git a/tests/ref/fate/filter-pixfmts-hflip b/tests/ref/fate/filter-pixfmts-hflip
index 0d40b93e97..6d5ddca677 100644
--- a/tests/ref/fate/filter-pixfmts-hflip
+++ b/tests/ref/fate/filter-pixfmts-hflip
@@ -77,7 +77,7 @@  rgb8                68a3a575badadd9e4f90226209f11699
 rgba                51961c723ea6707e0a410cd3f21f15d3
 rgba64be            c910444019f4cfbf4d995227af55da8d
 rgba64le            0c810d8b3a6bca10321788e1cb145340
-x2rgb10le           9f99dce306383daf25cd1542b2517fef
+x2rgb10le           1f6145a8c273a27932d4fbe676fe56e9
 xyz12be             25f90259ff8a226befdaec3dfe82996e
 xyz12le             926c0791d59aaff61b2778e8ada3316d
 ya16be              d5b342355bdd9e3197e01b13b7c6301e
diff --git a/tests/ref/fate/filter-pixfmts-il b/tests/ref/fate/filter-pixfmts-il
index d1bc866957..18a55138e7 100644
--- a/tests/ref/fate/filter-pixfmts-il
+++ b/tests/ref/fate/filter-pixfmts-il
@@ -79,7 +79,7 @@  rgba                625d8f4bd39c4bdbf61eb5e4713aecc9
 rgba64be            db70d33aa6c06f3e0a1c77bd11284261
 rgba64le            a8a2daae04374a27219bc1c890204007
 uyvy422             d6ee3ca43356d08c392382b24b22cda5
-x2rgb10le           a01ea7dd339e028780e04971012d826d
+x2rgb10le           17fdd92376181fd55dd8e8f89d4a9724
 xyz12be             7c7d54c55f136cbbc50b18029f3be0b3
 xyz12le             090ba6b1170baf2b1358b43b971d33b0
 ya16be              7bc720918bc0132e9717acbde89874e0
diff --git a/tests/ref/fate/filter-pixfmts-null b/tests/ref/fate/filter-pixfmts-null
index 1d7657c2af..7a7e42a8b4 100644
--- a/tests/ref/fate/filter-pixfmts-null
+++ b/tests/ref/fate/filter-pixfmts-null
@@ -80,7 +80,7 @@  rgba                b6e1b441c365e03b5ffdf9b7b68d9a0c
 rgba64be            ae2ae04b5efedca3505f47c4dd6ea6ea
 rgba64le            b91e1d77f799eb92241a2d2d28437b15
 uyvy422             3bcf3c80047592f2211fae3260b1b65d
-x2rgb10le           b0a0c8056521beeaa3fea4985ca87176
+x2rgb10le           75665d76a2077d0106d8cd65a067c475
 xyz12be             a1ef56bf746d71f59669c28e48fc8450
 xyz12le             831ff03c1ba4ef19374686f16a064d8c
 ya16be              37c07787e544f900c87b853253bfc8dd
diff --git a/tests/ref/fate/filter-pixfmts-pad b/tests/ref/fate/filter-pixfmts-pad
index 9a5db82543..05d7e43b78 100644
--- a/tests/ref/fate/filter-pixfmts-pad
+++ b/tests/ref/fate/filter-pixfmts-pad
@@ -28,7 +28,7 @@  nv42                1738ad3c31c6c16e17679f5b09ce4677
 rgb0                78d500c8361ab6423a4826a00268c908
 rgb24               17f9e2e0c609009acaf2175c42d4a2a5
 rgba                b157c90191463d34fb3ce77b36c96386
-x2rgb10le           c240f8a8dfa647c57c0974d061c9652a
+x2rgb10le           dd12649f2a87df29ba583d3160ce271d
 xyz12le             85abf80b77a9236a76ba0b00fcbdea2d
 ya16le              940fafa240b9916de5f73cb20a552f24
 ya8                 5fc0f471207ddf7aa01b07027d56b672
diff --git a/tests/ref/fate/filter-pixfmts-scale b/tests/ref/fate/filter-pixfmts-scale
index f47c9b887f..cae8841504 100644
--- a/tests/ref/fate/filter-pixfmts-scale
+++ b/tests/ref/fate/filter-pixfmts-scale
@@ -80,7 +80,7 @@  rgba                85bb5d03cea1c6e8002ced3373904336
 rgba64be            ee73e57923af984b31cc7795d13929da
 rgba64le            783d2779adfafe3548bdb671ec0de69e
 uyvy422             aeb4ba4f9f003ae21f6d18089198244f
-x2rgb10le           591fe7942544c8fc40e5d30e0e589f49
+x2rgb10le           fa4a54140159d358680bc6c4fa99d363
 xyz12be             c7ba8345998c0141ddc079cdd29b1a40
 xyz12le             95f5d3a0de834cc495c9032a14987cde
 ya16be              20d4842899d61068f5fb6af478bf26a6
diff --git a/tests/ref/fate/filter-pixfmts-transpose b/tests/ref/fate/filter-pixfmts-transpose
index 0a8542b2a9..b1d770f70e 100644
--- a/tests/ref/fate/filter-pixfmts-transpose
+++ b/tests/ref/fate/filter-pixfmts-transpose
@@ -76,7 +76,7 @@  rgb8                c90feb30c3c9391ef5f470209d7b7a15
 rgba                4d76a9542143752a4ac30f82f88f68f1
 rgba64be            a60041217f4c0cd796d19d3940a12a41
 rgba64le            ad47197774858858ae7b0c177dffa459
-x2rgb10le           a64d4d901b09bea9d59eda58be5e88ff
+x2rgb10le           47e24736427cee81ad8e65d2d4ff2e2b
 xyz12be             68e5cba640f6e4ef72dff950e88b5342
 xyz12le             8b6b6a6db4d7561e80db88ccaecce7a9
 ya16be              3e161cb5f225922a80fefdc9cc02a4f9
diff --git a/tests/ref/fate/filter-pixfmts-vflip b/tests/ref/fate/filter-pixfmts-vflip
index 732db8d331..f6c1a6beda 100644
--- a/tests/ref/fate/filter-pixfmts-vflip
+++ b/tests/ref/fate/filter-pixfmts-vflip
@@ -80,7 +80,7 @@  rgba                c1a5908572737f2ae1e5d8218af65f4b
 rgba64be            17e6273323b5779b5f3f775f150c1011
 rgba64le            48f45b10503b7dd140329c3dd0d54c98
 uyvy422             3a237e8376264e0cfa78f8a3fdadec8a
-x2rgb10le           332a6f5f5012008a562cb031836da028
+x2rgb10le           cf10d49b28bf93b5a85955a89a7943e1
 xyz12be             810644e008deb231850d779aaa27cc7e
 xyz12le             829701db461b43533cf9241e0743bc61
 ya16be              55b1dbbe4d56ed0d22461685ce85520d