Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
On Mon, Sep 28, 2020 at 10:38 AM Michael Niedermayer wrote: > On Sat, Sep 26, 2020 at 10:01:30PM -0700, Mark Reid wrote: > > On Mon, Sep 14, 2020 at 6:31 PM Mark Reid wrote: > > > > > > > > > > > On Mon, Sep 14, 2020 at 2:44 PM Michael Niedermayer > > > > wrote: > > > > > >> On Sun, Sep 13, 2020 at 04:04:42PM -0700, Mark Reid wrote: > > >> > On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer > > >> > > >> > wrote: > > >> > > > >> > > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com > wrote: > > >> > > > From: Mark Reid > > >> > > > > > >> > > > --- > > >> > > > libswscale/input.c | 12 +--- > > >> > > > tests/ref/fate/filter-pixfmts-scale | 8 > > >> > > > 2 files changed, 9 insertions(+), 11 deletions(-) > > >> > > > > >> > > Can you provide some tests that show that this is better ? > > >> > > Iam asking that because some of the numbers in some of the code > > >> > > (i dont remember which) where tuned to give more accurate overall > > >> results > > >> > > > > >> > > an example for tests would be converting from A->B->A then > compare to > > >> the > > >> > > orig > > >> > > > > >> > > thx > > >> > > > > >> > > > > >> > Hopefully i can explain this clearly! > > >> > > > >> > It's easier to see the error if you run a black image through the > old > > >> > conversion. > > >> > zero values don't get mapped to zero. (attached sample image) > > >> > > > >> > ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo > 4x4_zero.rawvideo > > >> > The image should be rgb 0, 0, 0 everywhere but instead it's 353, > 0, 407 > > >> > > > >> > > > >> > I think this is a error in fixed point rounding, the issue is > basically > > >> > boils down to > > >> > > > >> > 128 << 8 != 257 << 7 > > >> > and > > >> > 16 << 8 != 33 << 7 > > >> > > > >> > https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601 > > >> > the 8 bit rgb to yuv formula is > > >> > > > >> > Y = ry*r + gy*g + by*b + 16 > > >> > U = ru*r + gu*g + bu*b + 128 > > >> > V = rv*r + gv*g + bv*b + 128 > > >> > > > >> > I think the studio swing offsets at the end are calculated wrong in > the > > >> old > > >> > code. > > >> > (257 << (RGB2YUV_SHIFT + bpc - 9))) > > >> > 257 is correct for 8 bit rounding but not for 16-bit. > > >> > > > >> > the 257 i believe is from (128 << 1) + 1 > > >> > the +1 is for rounding > > >> > > > >> > for rounding 16-bit (128 << 9) + 1 = 0x10001 > > >> > > > >> > therefore I think the correct rounding any bit depth with the old > > >> formula > > >> > would be (untested) > > >> > (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) ) > > >> > > > >> > I just simplified it to > > >> > (0x10001 << (RGB2YUV_SHIFT - 1)) > > >> > > > >> > The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm > using. > > >> > > >> You quite possibly are correct, can you test that this actually works > > >> out. The test sample only covers 1 color (black) > > >> a testsample covering a wide range of the color cube would be more > > >> convincing that this change is needed and sufficient to fix this. > > >> > > >> thx > > >> > > > > > > I wrote a small python script to compare the raw gbrpf32le images if > that > > > works? I attached it and also a more colorful test pattern. > > > > > > it runs these two commands and compares the 2 raw float images > > > ffmpeg -y -i test_pattern.exr -f rawvideo original.gbrpf32le > > > ffmpeg -y -i test_pattern.exr -vf > > > format=pix_fmts=rgb48le,format=pix_fmts=gbrpf32le -f rawvideo > > > converted.gbrpf32le > > > > > > python gbrpf32le_diff.py test_pattern.exr > > > > > > without patch: > > > avg error: 237.445495855 > > > min error: 0.0 > > > max error: 468.399102688 > > > > > > with patch: > > > avg error: 15.9312244829 > > > min error: 0.0 > > > max error: 69.467689991 > > > > > > > > > These are floating points scaled to 16-bit values. > > > Even with my patch, I'm kinda disturbed how much error there is. > > > > > > > ping > > I re-wrote the python script as a c swscale test, if that helps > > replicate my results. attached is patch for that. > > it generates an image of random float values and does the > > conversion/comparison . > > > > before patch: > > gbrpf32le -> yuva444p16le -> gbrpf32le > > avg diff: 0.003852 > > min diff: 0.00 > > max diff: 0.006638 > > > > after patch: > > gbrpf32le -> yuva444p16le -> gbrpf32le > > avg diff: 0.000125 > > min diff: 0.00 > > max diff: 0.000501 > > is it better for all middle formats ? > Iam asking as it seems this should be rather easy to test with your code > good idea, yes it looks better for other intermediate formats too, see my results below. I'll submit a new version of patch that does this and with the warnings fixed. gbrpf32le -> yuv444p16le -> gbrpf32le avg diff: 0.003852 avg diff: 0.000125 min diff: 0.00 min diff: 0.00 max diff: 0.006638 max diff: 0.000501 gbrpf32le -> yuv444p -> gbrpf32le avg diff: 0.004316 avg diff: 0.001804 min diff: 0.00 min diff: 0.
Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
On Sat, Sep 26, 2020 at 10:01:30PM -0700, Mark Reid wrote: > On Mon, Sep 14, 2020 at 6:31 PM Mark Reid wrote: > > > > > > > On Mon, Sep 14, 2020 at 2:44 PM Michael Niedermayer > > wrote: > > > >> On Sun, Sep 13, 2020 at 04:04:42PM -0700, Mark Reid wrote: > >> > On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer > >> > >> > wrote: > >> > > >> > > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote: > >> > > > From: Mark Reid > >> > > > > >> > > > --- > >> > > > libswscale/input.c | 12 +--- > >> > > > tests/ref/fate/filter-pixfmts-scale | 8 > >> > > > 2 files changed, 9 insertions(+), 11 deletions(-) > >> > > > >> > > Can you provide some tests that show that this is better ? > >> > > Iam asking that because some of the numbers in some of the code > >> > > (i dont remember which) where tuned to give more accurate overall > >> results > >> > > > >> > > an example for tests would be converting from A->B->A then compare to > >> the > >> > > orig > >> > > > >> > > thx > >> > > > >> > > > >> > Hopefully i can explain this clearly! > >> > > >> > It's easier to see the error if you run a black image through the old > >> > conversion. > >> > zero values don't get mapped to zero. (attached sample image) > >> > > >> > ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo 4x4_zero.rawvideo > >> > The image should be rgb 0, 0, 0 everywhere but instead it's 353, 0, 407 > >> > > >> > > >> > I think this is a error in fixed point rounding, the issue is basically > >> > boils down to > >> > > >> > 128 << 8 != 257 << 7 > >> > and > >> > 16 << 8 != 33 << 7 > >> > > >> > https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601 > >> > the 8 bit rgb to yuv formula is > >> > > >> > Y = ry*r + gy*g + by*b + 16 > >> > U = ru*r + gu*g + bu*b + 128 > >> > V = rv*r + gv*g + bv*b + 128 > >> > > >> > I think the studio swing offsets at the end are calculated wrong in the > >> old > >> > code. > >> > (257 << (RGB2YUV_SHIFT + bpc - 9))) > >> > 257 is correct for 8 bit rounding but not for 16-bit. > >> > > >> > the 257 i believe is from (128 << 1) + 1 > >> > the +1 is for rounding > >> > > >> > for rounding 16-bit (128 << 9) + 1 = 0x10001 > >> > > >> > therefore I think the correct rounding any bit depth with the old > >> formula > >> > would be (untested) > >> > (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) ) > >> > > >> > I just simplified it to > >> > (0x10001 << (RGB2YUV_SHIFT - 1)) > >> > > >> > The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using. > >> > >> You quite possibly are correct, can you test that this actually works > >> out. The test sample only covers 1 color (black) > >> a testsample covering a wide range of the color cube would be more > >> convincing that this change is needed and sufficient to fix this. > >> > >> thx > >> > > > > I wrote a small python script to compare the raw gbrpf32le images if that > > works? I attached it and also a more colorful test pattern. > > > > it runs these two commands and compares the 2 raw float images > > ffmpeg -y -i test_pattern.exr -f rawvideo original.gbrpf32le > > ffmpeg -y -i test_pattern.exr -vf > > format=pix_fmts=rgb48le,format=pix_fmts=gbrpf32le -f rawvideo > > converted.gbrpf32le > > > > python gbrpf32le_diff.py test_pattern.exr > > > > without patch: > > avg error: 237.445495855 > > min error: 0.0 > > max error: 468.399102688 > > > > with patch: > > avg error: 15.9312244829 > > min error: 0.0 > > max error: 69.467689991 > > > > > > These are floating points scaled to 16-bit values. > > Even with my patch, I'm kinda disturbed how much error there is. > > > > ping > I re-wrote the python script as a c swscale test, if that helps > replicate my results. attached is patch for that. > it generates an image of random float values and does the > conversion/comparison . > > before patch: > gbrpf32le -> yuva444p16le -> gbrpf32le > avg diff: 0.003852 > min diff: 0.00 > max diff: 0.006638 > > after patch: > gbrpf32le -> yuva444p16le -> gbrpf32le > avg diff: 0.000125 > min diff: 0.00 > max diff: 0.000501 is it better for all middle formats ? Iam asking as it seems this should be rather easy to test with your code But from what i see above, obviously this is an improvment and should be applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
On Sat, Sep 26, 2020 at 10:01:30PM -0700, Mark Reid wrote: > On Mon, Sep 14, 2020 at 6:31 PM Mark Reid wrote: > > > > > > > On Mon, Sep 14, 2020 at 2:44 PM Michael Niedermayer > > wrote: > > > >> On Sun, Sep 13, 2020 at 04:04:42PM -0700, Mark Reid wrote: > >> > On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer > >> > >> > wrote: > >> > > >> > > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote: > >> > > > From: Mark Reid > >> > > > > >> > > > --- > >> > > > libswscale/input.c | 12 +--- > >> > > > tests/ref/fate/filter-pixfmts-scale | 8 > >> > > > 2 files changed, 9 insertions(+), 11 deletions(-) > >> > > > >> > > Can you provide some tests that show that this is better ? > >> > > Iam asking that because some of the numbers in some of the code > >> > > (i dont remember which) where tuned to give more accurate overall > >> results > >> > > > >> > > an example for tests would be converting from A->B->A then compare to > >> the > >> > > orig > >> > > > >> > > thx > >> > > > >> > > > >> > Hopefully i can explain this clearly! > >> > > >> > It's easier to see the error if you run a black image through the old > >> > conversion. > >> > zero values don't get mapped to zero. (attached sample image) > >> > > >> > ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo 4x4_zero.rawvideo > >> > The image should be rgb 0, 0, 0 everywhere but instead it's 353, 0, 407 > >> > > >> > > >> > I think this is a error in fixed point rounding, the issue is basically > >> > boils down to > >> > > >> > 128 << 8 != 257 << 7 > >> > and > >> > 16 << 8 != 33 << 7 > >> > > >> > https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601 > >> > the 8 bit rgb to yuv formula is > >> > > >> > Y = ry*r + gy*g + by*b + 16 > >> > U = ru*r + gu*g + bu*b + 128 > >> > V = rv*r + gv*g + bv*b + 128 > >> > > >> > I think the studio swing offsets at the end are calculated wrong in the > >> old > >> > code. > >> > (257 << (RGB2YUV_SHIFT + bpc - 9))) > >> > 257 is correct for 8 bit rounding but not for 16-bit. > >> > > >> > the 257 i believe is from (128 << 1) + 1 > >> > the +1 is for rounding > >> > > >> > for rounding 16-bit (128 << 9) + 1 = 0x10001 > >> > > >> > therefore I think the correct rounding any bit depth with the old > >> formula > >> > would be (untested) > >> > (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) ) > >> > > >> > I just simplified it to > >> > (0x10001 << (RGB2YUV_SHIFT - 1)) > >> > > >> > The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using. > >> > >> You quite possibly are correct, can you test that this actually works > >> out. The test sample only covers 1 color (black) > >> a testsample covering a wide range of the color cube would be more > >> convincing that this change is needed and sufficient to fix this. > >> > >> thx > >> > > > > I wrote a small python script to compare the raw gbrpf32le images if that > > works? I attached it and also a more colorful test pattern. > > > > it runs these two commands and compares the 2 raw float images > > ffmpeg -y -i test_pattern.exr -f rawvideo original.gbrpf32le > > ffmpeg -y -i test_pattern.exr -vf > > format=pix_fmts=rgb48le,format=pix_fmts=gbrpf32le -f rawvideo > > converted.gbrpf32le > > > > python gbrpf32le_diff.py test_pattern.exr > > > > without patch: > > avg error: 237.445495855 > > min error: 0.0 > > max error: 468.399102688 > > > > with patch: > > avg error: 15.9312244829 > > min error: 0.0 > > max error: 69.467689991 > > > > > > These are floating points scaled to 16-bit values. > > Even with my patch, I'm kinda disturbed how much error there is. > > > > ping > I re-wrote the python script as a c swscale test, if that helps > replicate my results. attached is patch for that. > it generates an image of random float values and does the > conversion/comparison . > > before patch: > gbrpf32le -> yuva444p16le -> gbrpf32le > avg diff: 0.003852 > min diff: 0.00 > max diff: 0.006638 > > after patch: > gbrpf32le -> yuva444p16le -> gbrpf32le > avg diff: 0.000125 > min diff: 0.00 > max diff: 0.000501 > > > > > > > >> > >> [...] > >> > >> -- > >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > >> > >> There will always be a question for which you do not know the correct > >> answer. > >> ___ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > >> To unsubscribe, visit link above, or email > >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > > > > Makefile |1 > tests/.gitignore |1 > tests/floatimg_cmp.c | 267 > +++ > 3 files changed, 269 insertions(+) > a04783297b0a5d9be6186a150606724457e7b0c5 > 0001-libswscale-tests-add-floatimg_cmp-test.patch > From 9c4bc86201037aec454a98b60301d9250fecc7ca Mon Sep 17 00:00:00 2001 > From: Mark Reid > Date: Sun, 20 Sep 2020
Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
On Mon, Sep 14, 2020 at 6:31 PM Mark Reid wrote: > > > On Mon, Sep 14, 2020 at 2:44 PM Michael Niedermayer > wrote: > >> On Sun, Sep 13, 2020 at 04:04:42PM -0700, Mark Reid wrote: >> > On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer >> >> > wrote: >> > >> > > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote: >> > > > From: Mark Reid >> > > > >> > > > --- >> > > > libswscale/input.c | 12 +--- >> > > > tests/ref/fate/filter-pixfmts-scale | 8 >> > > > 2 files changed, 9 insertions(+), 11 deletions(-) >> > > >> > > Can you provide some tests that show that this is better ? >> > > Iam asking that because some of the numbers in some of the code >> > > (i dont remember which) where tuned to give more accurate overall >> results >> > > >> > > an example for tests would be converting from A->B->A then compare to >> the >> > > orig >> > > >> > > thx >> > > >> > > >> > Hopefully i can explain this clearly! >> > >> > It's easier to see the error if you run a black image through the old >> > conversion. >> > zero values don't get mapped to zero. (attached sample image) >> > >> > ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo 4x4_zero.rawvideo >> > The image should be rgb 0, 0, 0 everywhere but instead it's 353, 0, 407 >> > >> > >> > I think this is a error in fixed point rounding, the issue is basically >> > boils down to >> > >> > 128 << 8 != 257 << 7 >> > and >> > 16 << 8 != 33 << 7 >> > >> > https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601 >> > the 8 bit rgb to yuv formula is >> > >> > Y = ry*r + gy*g + by*b + 16 >> > U = ru*r + gu*g + bu*b + 128 >> > V = rv*r + gv*g + bv*b + 128 >> > >> > I think the studio swing offsets at the end are calculated wrong in the >> old >> > code. >> > (257 << (RGB2YUV_SHIFT + bpc - 9))) >> > 257 is correct for 8 bit rounding but not for 16-bit. >> > >> > the 257 i believe is from (128 << 1) + 1 >> > the +1 is for rounding >> > >> > for rounding 16-bit (128 << 9) + 1 = 0x10001 >> > >> > therefore I think the correct rounding any bit depth with the old >> formula >> > would be (untested) >> > (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) ) >> > >> > I just simplified it to >> > (0x10001 << (RGB2YUV_SHIFT - 1)) >> > >> > The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using. >> >> You quite possibly are correct, can you test that this actually works >> out. The test sample only covers 1 color (black) >> a testsample covering a wide range of the color cube would be more >> convincing that this change is needed and sufficient to fix this. >> >> thx >> > > I wrote a small python script to compare the raw gbrpf32le images if that > works? I attached it and also a more colorful test pattern. > > it runs these two commands and compares the 2 raw float images > ffmpeg -y -i test_pattern.exr -f rawvideo original.gbrpf32le > ffmpeg -y -i test_pattern.exr -vf > format=pix_fmts=rgb48le,format=pix_fmts=gbrpf32le -f rawvideo > converted.gbrpf32le > > python gbrpf32le_diff.py test_pattern.exr > > without patch: > avg error: 237.445495855 > min error: 0.0 > max error: 468.399102688 > > with patch: > avg error: 15.9312244829 > min error: 0.0 > max error: 69.467689991 > > > These are floating points scaled to 16-bit values. > Even with my patch, I'm kinda disturbed how much error there is. > ping I re-wrote the python script as a c swscale test, if that helps replicate my results. attached is patch for that. it generates an image of random float values and does the conversion/comparison . before patch: gbrpf32le -> yuva444p16le -> gbrpf32le avg diff: 0.003852 min diff: 0.00 max diff: 0.006638 after patch: gbrpf32le -> yuva444p16le -> gbrpf32le avg diff: 0.000125 min diff: 0.00 max diff: 0.000501 > > >> >> [...] >> >> -- >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >> >> There will always be a question for which you do not know the correct >> answer. >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > 0001-libswscale-tests-add-floatimg_cmp-test.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
On Mon, Sep 14, 2020 at 2:44 PM Michael Niedermayer
wrote:
> On Sun, Sep 13, 2020 at 04:04:42PM -0700, Mark Reid wrote:
> > On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer
>
> > wrote:
> >
> > > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote:
> > > > From: Mark Reid
> > > >
> > > > ---
> > > > libswscale/input.c | 12 +---
> > > > tests/ref/fate/filter-pixfmts-scale | 8
> > > > 2 files changed, 9 insertions(+), 11 deletions(-)
> > >
> > > Can you provide some tests that show that this is better ?
> > > Iam asking that because some of the numbers in some of the code
> > > (i dont remember which) where tuned to give more accurate overall
> results
> > >
> > > an example for tests would be converting from A->B->A then compare to
> the
> > > orig
> > >
> > > thx
> > >
> > >
> > Hopefully i can explain this clearly!
> >
> > It's easier to see the error if you run a black image through the old
> > conversion.
> > zero values don't get mapped to zero. (attached sample image)
> >
> > ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo 4x4_zero.rawvideo
> > The image should be rgb 0, 0, 0 everywhere but instead it's 353, 0, 407
> >
> >
> > I think this is a error in fixed point rounding, the issue is basically
> > boils down to
> >
> > 128 << 8 != 257 << 7
> > and
> > 16 << 8 != 33 << 7
> >
> > https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601
> > the 8 bit rgb to yuv formula is
> >
> > Y = ry*r + gy*g + by*b + 16
> > U = ru*r + gu*g + bu*b + 128
> > V = rv*r + gv*g + bv*b + 128
> >
> > I think the studio swing offsets at the end are calculated wrong in the
> old
> > code.
> > (257 << (RGB2YUV_SHIFT + bpc - 9)))
> > 257 is correct for 8 bit rounding but not for 16-bit.
> >
> > the 257 i believe is from (128 << 1) + 1
> > the +1 is for rounding
> >
> > for rounding 16-bit (128 << 9) + 1 = 0x10001
> >
> > therefore I think the correct rounding any bit depth with the old formula
> > would be (untested)
> > (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) )
> >
> > I just simplified it to
> > (0x10001 << (RGB2YUV_SHIFT - 1))
> >
> > The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using.
>
> You quite possibly are correct, can you test that this actually works
> out. The test sample only covers 1 color (black)
> a testsample covering a wide range of the color cube would be more
> convincing that this change is needed and sufficient to fix this.
>
> thx
>
I wrote a small python script to compare the raw gbrpf32le images if that
works? I attached it and also a more colorful test pattern.
it runs these two commands and compares the 2 raw float images
ffmpeg -y -i test_pattern.exr -f rawvideo original.gbrpf32le
ffmpeg -y -i test_pattern.exr -vf
format=pix_fmts=rgb48le,format=pix_fmts=gbrpf32le -f rawvideo
converted.gbrpf32le
python gbrpf32le_diff.py test_pattern.exr
without patch:
avg error: 237.445495855
min error: 0.0
max error: 468.399102688
with patch:
avg error: 15.9312244829
min error: 0.0
max error: 69.467689991
These are floating points scaled to 16-bit values.
Even with my patch, I'm kinda disturbed how much error there is.
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> There will always be a question for which you do not know the correct
> answer.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
from __future__ import print_function
import struct
import sys
import subprocess
FFMPEG = './ffmpeg'
def read_f32(f):
return struct.unpack("
test_pattern.exr
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
On Sun, Sep 13, 2020 at 04:04:42PM -0700, Mark Reid wrote: > On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer > wrote: > > > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote: > > > From: Mark Reid > > > > > > --- > > > libswscale/input.c | 12 +--- > > > tests/ref/fate/filter-pixfmts-scale | 8 > > > 2 files changed, 9 insertions(+), 11 deletions(-) > > > > Can you provide some tests that show that this is better ? > > Iam asking that because some of the numbers in some of the code > > (i dont remember which) where tuned to give more accurate overall results > > > > an example for tests would be converting from A->B->A then compare to the > > orig > > > > thx > > > > > Hopefully i can explain this clearly! > > It's easier to see the error if you run a black image through the old > conversion. > zero values don't get mapped to zero. (attached sample image) > > ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo 4x4_zero.rawvideo > The image should be rgb 0, 0, 0 everywhere but instead it's 353, 0, 407 > > > I think this is a error in fixed point rounding, the issue is basically > boils down to > > 128 << 8 != 257 << 7 > and > 16 << 8 != 33 << 7 > > https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601 > the 8 bit rgb to yuv formula is > > Y = ry*r + gy*g + by*b + 16 > U = ru*r + gu*g + bu*b + 128 > V = rv*r + gv*g + bv*b + 128 > > I think the studio swing offsets at the end are calculated wrong in the old > code. > (257 << (RGB2YUV_SHIFT + bpc - 9))) > 257 is correct for 8 bit rounding but not for 16-bit. > > the 257 i believe is from (128 << 1) + 1 > the +1 is for rounding > > for rounding 16-bit (128 << 9) + 1 = 0x10001 > > therefore I think the correct rounding any bit depth with the old formula > would be (untested) > (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) ) > > I just simplified it to > (0x10001 << (RGB2YUV_SHIFT - 1)) > > The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using. You quite possibly are correct, can you test that this actually works out. The test sample only covers 1 color (black) a testsample covering a wide range of the color cube would be more convincing that this change is needed and sufficient to fix this. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB There will always be a question for which you do not know the correct answer. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
> The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using. rgb64ToUV and rgb64ToY use it too, might be an opportunity to reduce code duplication. I ran into this while working on related precision issues (color range conversion being precise only for 8bits) and just used the same formula as the others templates to solve the issue like you. Le lun. 14 sept. 2020 à 06:30, Mark Reid a écrit : > On Sun., Sep. 13, 2020, 4:04 p.m. Mark Reid, wrote: > > > > > > > On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer > > > wrote: > > > >> On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote: > >> > From: Mark Reid > >> > > >> > --- > >> > libswscale/input.c | 12 +--- > >> > tests/ref/fate/filter-pixfmts-scale | 8 > >> > 2 files changed, 9 insertions(+), 11 deletions(-) > >> > >> Can you provide some tests that show that this is better ? > >> Iam asking that because some of the numbers in some of the code > >> (i dont remember which) where tuned to give more accurate overall > results > >> > >> an example for tests would be converting from A->B->A then compare to > the > >> orig > >> > >> thx > >> > >> > > Hopefully i can explain this clearly! > > > > It's easier to see the error if you run a black image through the old > > conversion. > > zero values don't get mapped to zero. (attached sample image) > > > > ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo 4x4_zero.rawvideo > > The image should be rgb 0, 0, 0 everywhere but instead it's 353, 0, 407 > > > > > > I think this is a error in fixed point rounding, the issue is basically > > boils down to > > > > 128 << 8 != 257 << 7 > > and > > 16 << 8 != 33 << 7 > > > > https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601 > > the 8 bit rgb to yuv formula is > > > > Y = ry*r + gy*g + by*b + 16 > > U = ru*r + gu*g + bu*b + 128 > > V = rv*r + gv*g + bv*b + 128 > > > > I think the studio swing offsets at the end are calculated wrong in the > > old code. > > (257 << (RGB2YUV_SHIFT + bpc - 9))) > > 257 is correct for 8 bit rounding but not for 16-bit. > > > > the 257 i believe is from (128 << 1) + 1 > > the +1 is for rounding > > > > for rounding 16-bit (128 << 9) + 1 = 0x10001 > > > > therefore I think the correct rounding any bit depth with the old formula > > would be (untested) > > (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) ) > > > > I just simplified it to > > (0x10001 << (RGB2YUV_SHIFT - 1)) > > > > The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using. > > > > I'm still not sure if I'm correct about all this. The rounding stuff > doesn't make 100% sense to me. But this is all I've gathered from reading > the code. > > > > > > >> > >> [...] > >> -- > >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > >> > >> Everything should be made as simple as possible, but not simpler. > >> -- Albert Einstein > >> ___ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > >> To unsubscribe, visit link above, or email > >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > > > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
On Sun., Sep. 13, 2020, 4:04 p.m. Mark Reid, wrote: > > > On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer > wrote: > >> On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote: >> > From: Mark Reid >> > >> > --- >> > libswscale/input.c | 12 +--- >> > tests/ref/fate/filter-pixfmts-scale | 8 >> > 2 files changed, 9 insertions(+), 11 deletions(-) >> >> Can you provide some tests that show that this is better ? >> Iam asking that because some of the numbers in some of the code >> (i dont remember which) where tuned to give more accurate overall results >> >> an example for tests would be converting from A->B->A then compare to the >> orig >> >> thx >> >> > Hopefully i can explain this clearly! > > It's easier to see the error if you run a black image through the old > conversion. > zero values don't get mapped to zero. (attached sample image) > > ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo 4x4_zero.rawvideo > The image should be rgb 0, 0, 0 everywhere but instead it's 353, 0, 407 > > > I think this is a error in fixed point rounding, the issue is basically > boils down to > > 128 << 8 != 257 << 7 > and > 16 << 8 != 33 << 7 > > https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601 > the 8 bit rgb to yuv formula is > > Y = ry*r + gy*g + by*b + 16 > U = ru*r + gu*g + bu*b + 128 > V = rv*r + gv*g + bv*b + 128 > > I think the studio swing offsets at the end are calculated wrong in the > old code. > (257 << (RGB2YUV_SHIFT + bpc - 9))) > 257 is correct for 8 bit rounding but not for 16-bit. > > the 257 i believe is from (128 << 1) + 1 > the +1 is for rounding > > for rounding 16-bit (128 << 9) + 1 = 0x10001 > > therefore I think the correct rounding any bit depth with the old formula > would be (untested) > (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) ) > > I just simplified it to > (0x10001 << (RGB2YUV_SHIFT - 1)) > > The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using. > I'm still not sure if I'm correct about all this. The rounding stuff doesn't make 100% sense to me. But this is all I've gathered from reading the code. > >> >> [...] >> -- >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >> >> Everything should be made as simple as possible, but not simpler. >> -- Albert Einstein >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer wrote: > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote: > > From: Mark Reid > > > > --- > > libswscale/input.c | 12 +--- > > tests/ref/fate/filter-pixfmts-scale | 8 > > 2 files changed, 9 insertions(+), 11 deletions(-) > > Can you provide some tests that show that this is better ? > Iam asking that because some of the numbers in some of the code > (i dont remember which) where tuned to give more accurate overall results > > an example for tests would be converting from A->B->A then compare to the > orig > > thx > > Hopefully i can explain this clearly! It's easier to see the error if you run a black image through the old conversion. zero values don't get mapped to zero. (attached sample image) ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo 4x4_zero.rawvideo The image should be rgb 0, 0, 0 everywhere but instead it's 353, 0, 407 I think this is a error in fixed point rounding, the issue is basically boils down to 128 << 8 != 257 << 7 and 16 << 8 != 33 << 7 https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601 the 8 bit rgb to yuv formula is Y = ry*r + gy*g + by*b + 16 U = ru*r + gu*g + bu*b + 128 V = rv*r + gv*g + bv*b + 128 I think the studio swing offsets at the end are calculated wrong in the old code. (257 << (RGB2YUV_SHIFT + bpc - 9))) 257 is correct for 8 bit rounding but not for 16-bit. the 257 i believe is from (128 << 1) + 1 the +1 is for rounding for rounding 16-bit (128 << 9) + 1 = 0x10001 therefore I think the correct rounding any bit depth with the old formula would be (untested) (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) ) I just simplified it to (0x10001 << (RGB2YUV_SHIFT - 1)) The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Everything should be made as simple as possible, but not simpler. > -- Albert Einstein > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". 4x4_zero.exr Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote: > From: Mark Reid > > --- > libswscale/input.c | 12 +--- > tests/ref/fate/filter-pixfmts-scale | 8 > 2 files changed, 9 insertions(+), 11 deletions(-) Can you provide some tests that show that this is better ? Iam asking that because some of the numbers in some of the code (i dont remember which) where tuned to give more accurate overall results an example for tests would be converting from A->B->A then compare to the orig thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
From: Mark Reid
---
libswscale/input.c | 12 +---
tests/ref/fate/filter-pixfmts-scale | 8
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/libswscale/input.c b/libswscale/input.c
index 064ed5902f..67a85b0418 100644
--- a/libswscale/input.c
+++ b/libswscale/input.c
@@ -984,15 +984,14 @@ static av_always_inline void planar_rgbf32_to_uv(uint8_t
*_dstU, uint8_t *_dstV,
uint16_t *dstV = (uint16_t *)_dstV;
int32_t ru = rgb2yuv[RU_IDX], gu = rgb2yuv[GU_IDX], bu = rgb2yuv[BU_IDX];
int32_t rv = rgb2yuv[RV_IDX], gv = rgb2yuv[GV_IDX], bv = rgb2yuv[BV_IDX];
-int bpc = 16;
-int shift = 14;
+
for (i = 0; i < width; i++) {
int g = av_clip_uint16(lrintf(65535.0f * rdpx(src[0] + i)));
int b = av_clip_uint16(lrintf(65535.0f * rdpx(src[1] + i)));
int r = av_clip_uint16(lrintf(65535.0f * rdpx(src[2] + i)));
-dstU[i] = (ru*r + gu*g + bu*b + (257 << (RGB2YUV_SHIFT + bpc - 9))) >>
(RGB2YUV_SHIFT + shift - 14);
-dstV[i] = (rv*r + gv*g + bv*b + (257 << (RGB2YUV_SHIFT + bpc - 9))) >>
(RGB2YUV_SHIFT + shift - 14);
+dstU[i] = (ru*r + gu*g + bu*b + (0x10001 << (RGB2YUV_SHIFT - 1))) >>
RGB2YUV_SHIFT;
+dstV[i] = (rv*r + gv*g + bv*b + (0x10001 << (RGB2YUV_SHIFT - 1))) >>
RGB2YUV_SHIFT;
}
}
@@ -1003,14 +1002,13 @@ static av_always_inline void planar_rgbf32_to_y(uint8_t
*_dst, const uint8_t *_s
uint16_t *dst= (uint16_t *)_dst;
int32_t ry = rgb2yuv[RY_IDX], gy = rgb2yuv[GY_IDX], by = rgb2yuv[BY_IDX];
-int bpc = 16;
-int shift = 14;
+
for (i = 0; i < width; i++) {
int g = av_clip_uint16(lrintf(65535.0f * rdpx(src[0] + i)));
int b = av_clip_uint16(lrintf(65535.0f * rdpx(src[1] + i)));
int r = av_clip_uint16(lrintf(65535.0f * rdpx(src[2] + i)));
-dst[i] = ((ry*r + gy*g + by*b + (33 << (RGB2YUV_SHIFT + bpc - 9))) >>
(RGB2YUV_SHIFT + shift - 14));
+dst[i] = (ry*r + gy*g + by*b + (0x2001 << (RGB2YUV_SHIFT - 1))) >>
RGB2YUV_SHIFT;
}
}
diff --git a/tests/ref/fate/filter-pixfmts-scale
b/tests/ref/fate/filter-pixfmts-scale
index d7020ad2c3..30e7cd5b06 100644
--- a/tests/ref/fate/filter-pixfmts-scale
+++ b/tests/ref/fate/filter-pixfmts-scale
@@ -25,8 +25,8 @@ gbrap12be 1d9b57766ba9c2192403f43967cb9af0
gbrap12le bb1ba1c157717db3dd612a76d38a018e
gbrap16be c72b935a6e57a8e1c37bff08c2db55b1
gbrap16le 13eb0e62b1ac9c1c86c81521eaefab5f
-gbrapf32be 42e53d9edccbd9e09c4cd78780ba92f3
-gbrapf32le eebf3973ef94c841f0a1ceb1ed61621d
+gbrapf32be 366b804d5697276e8c481c4bdf05a00b
+gbrapf32le 558a268e6d6b907449d1056afab78f29
gbrpdc3387f925f972c61aae7eb23cdc19f0
gbrp10be0277d4c3a8498d75e2783fb81379e481
gbrp10lef3d70f8ab845c3c9b8f7452e4a6e285a
@@ -38,8 +38,8 @@ gbrp16be5fc826cfabebfc1442cb793c4b6303e2
gbrp16le1b3e0b63d47a3e1b6b20931316883bf2
gbrp9be d9c88968001e1452ff31fbc8d16b18a0
gbrp9le 2ccfed0816bf6bd4bb3a5b7591d9603a
-gbrpf32be 4614d32e4417f80e0adcc1bdcf6cde42
-gbrpf32le 1366ee77e5559672260bbe51040e28b2
+gbrpf32be f3d0cefdf11c861001880772d817aac8
+gbrpf32le 290468205c1c18a0667edfca45061aee
gray221201cc7cfc4964eacd8b3e426fd276
gray10be9452756d0b37f4f5c7cae7635e22d747
gray10le37fd2e1ec6b66410212d39a342e864df
--
2.27.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
