On Sat, Sep 26, 2020 at 10:01:30PM -0700, Mark Reid wrote: > On Mon, Sep 14, 2020 at 6:31 PM Mark Reid <mindm...@gmail.com> wrote: > > > > > > > On Mon, Sep 14, 2020 at 2:44 PM Michael Niedermayer <mich...@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 > >> <mich...@niedermayer.cc> > >> > wrote: > >> > > >> > > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote: > >> > > > From: Mark Reid <mindm...@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@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 <mindm...@gmail.com> > Date: Sun, 20 Sep 2020 20:45:08 -0700 > Subject: [PATCH] libswscale/tests: add floatimg_cmp test > > --- > libswscale/Makefile | 1 + > libswscale/tests/.gitignore | 1 + > libswscale/tests/floatimg_cmp.c | 267 ++++++++++++++++++++++++++++++++ > 3 files changed, 269 insertions(+) > create mode 100644 libswscale/tests/floatimg_cmp.c > > diff --git a/libswscale/Makefile b/libswscale/Makefile > index 5e03e6fa0a..4b8f9de425 100644 > --- a/libswscale/Makefile > +++ b/libswscale/Makefile > @@ -25,5 +25,6 @@ OBJS-$(CONFIG_SHARED) += log2_tab.o > SLIBOBJS-$(HAVE_GNU_WINDRES) += swscaleres.o > > TESTPROGS = colorspace \ > + floatimg_cmp \ > pixdesc_query \ > swscale \ > diff --git a/libswscale/tests/.gitignore b/libswscale/tests/.gitignore > index 1a26f038c4..c56abf0ee7 100644 > --- a/libswscale/tests/.gitignore > +++ b/libswscale/tests/.gitignore > @@ -1,3 +1,4 @@ > /colorspace > +/floatimg_cmp > /pixdesc_query > /swscale > diff --git a/libswscale/tests/floatimg_cmp.c b/libswscale/tests/floatimg_cmp.c > new file mode 100644 > index 0000000000..affb6fa492 > --- /dev/null > +++ b/libswscale/tests/floatimg_cmp.c This seems to produce some compiler warnings also turning this into a fate test would be a good idea i think CC libswscale/tests/floatimg_cmp.o libswscale/tests/floatimg_cmp.c: In function ‘main’: libswscale/tests/floatimg_cmp.c:118:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inFormat); ^~~~~ libswscale/tests/floatimg_cmp.c:162:13: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] uint8_t *ptr = rgbIn[p]; ^~~~~~~ libswscale/tests/floatimg_cmp.c:197:26: warning: passing argument 2 of ‘sws_scale’ from incompatible pointer type [-Wincompatible-pointer-types] res = sws_scale(sws, rgbIn, rgbStride, 0, h, (uint8_t * const *) dst, dstStride); ^~~~~ In file included from libswscale/tests/floatimg_cmp.c:35:0: ./libswscale/swscale.h:217:5: note: expected ‘const uint8_t * const* {aka const unsigned char * const*}’ but argument is of type ‘uint8_t ** {aka unsigned char **}’ int sws_scale(struct SwsContext *c, const uint8_t *const srcSlice[], ^~~~~~~~~ libswscale/tests/floatimg_cmp.c:212:26: warning: passing argument 2 of ‘sws_scale’ from incompatible pointer type [-Wincompatible-pointer-types] res = sws_scale(sws, dst, dstStride, 0, h, (uint8_t * const *) rgbOut, rgbStride); ^~~ In file included from libswscale/tests/floatimg_cmp.c:35:0: ./libswscale/swscale.h:217:5: note: expected ‘const uint8_t * const* {aka const unsigned char * const*}’ but argument is of type ‘uint8_t ** {aka unsigned char **}’ int sws_scale(struct SwsContext *c, const uint8_t *const srcSlice[], ^~~~~~~~~ libswscale/tests/floatimg_cmp.c:219:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] double sum = 0; ^~~~~~ libswscale/tests/floatimg_cmp.c:240:17: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] float diff = fabsf(v0.f - v1.f); ^~~~~ LD libswscale/tests/floatimg_cmp thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable
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".