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

[Mark Reid]

On Sun., Sep. 13, 2020, 4:04 p.m. Mark Reid, <mindmark at gmail.com> 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.
>

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