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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200914/91087e0b/attachment.sig>