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

[Mark Reid]

On Mon, Sep 14, 2020 at 6:31 PM Mark Reid <mindmark at gmail.com> wrote:

>
>
> 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.
>

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.000000
max diff: 0.006638

after patch:
gbrpf32le -> yuva444p16le -> gbrpf32le
avg diff: 0.000125
min diff: 0.000000
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 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: 0001-libswscale-tests-add-floatimg_cmp-test.patch
Type: application/octet-stream
Size: 9909 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200926/933ed6f9/attachment.obj>