[FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions

[Mark Reid]

On Mon, Sep 14, 2020 at 2:44 PM Michael Niedermayer <michael at niedermayer.cc>
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
> <michael at niedermayer.cc>
> > wrote:
> >
> > > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindmark at gmail.com wrote:
> > > > From: Mark Reid <mindmark at gmail.com>
> > > >
> > > > ---
> > > >  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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gbrpf32le_diff.py
Type: text/x-python-script
Size: 1521 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200914/7947e824/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test_pattern.exr
Type: application/octet-stream
Size: 48781 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200914/7947e824/attachment.obj>