[FFmpeg-devel] [PATCH v2 2/5] swscale/swscale_unscaled: add missing gbrap10 on ff_get_unscaled_swscale

[Michael Niedermayer]

On Tue, Jan 29, 2019 at 01:08:38PM +0700, Muhammad Faiz wrote:
> On Tue, Jan 29, 2019 at 5:14 AM Michael Niedermayer
> <michael at niedermayer.cc> wrote:
> >
> > On Mon, Jan 28, 2019 at 05:07:38PM +0700, Muhammad Faiz wrote:
> > > Fix inconsistent checksums between gbrap10be and gbrap10le
> > > on fate-filter-pixfmts.
> > >
> > > Signed-off-by: Muhammad Faiz <mfcc64 at gmail.com>
> > > ---
> > >  libswscale/swscale_unscaled.c            | 3 +++
> > >  tests/ref/fate/filter-pixfmts-copy       | 2 +-
> > >  tests/ref/fate/filter-pixfmts-crop       | 2 +-
> > >  tests/ref/fate/filter-pixfmts-field      | 2 +-
> > >  tests/ref/fate/filter-pixfmts-fieldorder | 2 +-
> > >  tests/ref/fate/filter-pixfmts-hflip      | 2 +-
> > >  tests/ref/fate/filter-pixfmts-il         | 2 +-
> > >  tests/ref/fate/filter-pixfmts-null       | 2 +-
> > >  tests/ref/fate/filter-pixfmts-scale      | 2 +-
> > >  tests/ref/fate/filter-pixfmts-transpose  | 2 +-
> > >  tests/ref/fate/filter-pixfmts-vflip      | 2 +-
> > >  11 files changed, 13 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
> > > index 058f2b94db..734a527e68 100644
> > > --- a/libswscale/swscale_unscaled.c
> > > +++ b/libswscale/swscale_unscaled.c
> > > @@ -1942,6 +1942,7 @@ void ff_get_unscaled_swscale(SwsContext *c)
> > >           dstFormat == AV_PIX_FMT_GBRP12LE || dstFormat == AV_PIX_FMT_GBRP12BE ||
> > >           dstFormat == AV_PIX_FMT_GBRP14LE || dstFormat == AV_PIX_FMT_GBRP14BE ||
> > >           dstFormat == AV_PIX_FMT_GBRP16LE || dstFormat == AV_PIX_FMT_GBRP16BE ||
> > > +         dstFormat == AV_PIX_FMT_GBRAP10LE || dstFormat == AV_PIX_FMT_GBRAP10BE ||
> > >           dstFormat == AV_PIX_FMT_GBRAP12LE || dstFormat == AV_PIX_FMT_GBRAP12BE ||
> > >           dstFormat == AV_PIX_FMT_GBRAP16LE || dstFormat == AV_PIX_FMT_GBRAP16BE ))
> > >          c->swscale = Rgb16ToPlanarRgb16Wrapper;
> > > @@ -1951,6 +1952,7 @@ void ff_get_unscaled_swscale(SwsContext *c)
> > >           srcFormat == AV_PIX_FMT_GBRP10LE || srcFormat == AV_PIX_FMT_GBRP10BE ||
> > >           srcFormat == AV_PIX_FMT_GBRP12LE || srcFormat == AV_PIX_FMT_GBRP12BE ||
> > >           srcFormat == AV_PIX_FMT_GBRP14LE || srcFormat == AV_PIX_FMT_GBRP14BE ||
> > > +         srcFormat == AV_PIX_FMT_GBRAP10LE || srcFormat == AV_PIX_FMT_GBRAP10BE ||
> > >           srcFormat == AV_PIX_FMT_GBRAP12LE || srcFormat == AV_PIX_FMT_GBRAP12BE ||
> > >           srcFormat == AV_PIX_FMT_GBRAP16LE || srcFormat == AV_PIX_FMT_GBRAP16BE) &&
> > >          (dstFormat == AV_PIX_FMT_RGB48LE  || dstFormat == AV_PIX_FMT_RGB48BE  ||
> > > @@ -1997,6 +1999,7 @@ void ff_get_unscaled_swscale(SwsContext *c)
> > >          IS_DIFFERENT_ENDIANESS(srcFormat, dstFormat, AV_PIX_FMT_GBRP12) ||
> > >          IS_DIFFERENT_ENDIANESS(srcFormat, dstFormat, AV_PIX_FMT_GBRP14) ||
> > >          IS_DIFFERENT_ENDIANESS(srcFormat, dstFormat, AV_PIX_FMT_GBRP16) ||
> > > +        IS_DIFFERENT_ENDIANESS(srcFormat, dstFormat, AV_PIX_FMT_GBRAP10) ||
> > >          IS_DIFFERENT_ENDIANESS(srcFormat, dstFormat, AV_PIX_FMT_GBRAP12) ||
> > >          IS_DIFFERENT_ENDIANESS(srcFormat, dstFormat, AV_PIX_FMT_GBRAP16) ||
> > >          IS_DIFFERENT_ENDIANESS(srcFormat, dstFormat, AV_PIX_FMT_RGB444) ||
> >
> > have you tested these ?
> > iam asking as the previous iteration had a bug which was missed in testing
> 
> fate-filter-pixfmts only covers gbrap10le <-> gbrap10be conversion.
> Actually the first two changes fix gbrap10 <-> rgb(a)(64/48) conversions.
> 
> This is a simple test:
> FFMPEG=./ffmpeg
> 
> $FFMPEG -loglevel quiet -filter_complex "allyuv=d=0.01" -pix_fmt
> gbrap10le -f rawvideo -y in.rgb
> 
> for fmt in rgba64be bgra64be rgba64le bgra64be gbrap10be; do
>     $FFMPEG -loglevel quiet -s 4096x4096 -pix_fmt gbrap10le -f
> rawvideo -i in.rgb -vf format=$fmt -pix_fmt gbrap10le -f rawvideo -y
> out-$fmt.rgb
>     if cmp in.rgb out-$fmt.rgb; then
>         echo "in.rgb out-$fmt.rgb are equal"
>     fi
> done

passes on x86 & mips qemu with this (and prev) patch

thanks!



> 
> > also adding a test or tests to fate which would cover that codepath that was
> > missed could be a good idea
> 
> As I suggested in 1/5, adding a test that covers all lossless
> conversions (not only gbrap10 case) is a good idea.
> But that is beyond of this patch.
> 
> (Also note that p010 and p016 problems haven't yet been fixed.)
> 
> >
> > the change itself looks fine i think but i have not tested it beyond seeing
> > that it changes the output for some files
> >
> > Thanks
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190130/483d08a5/attachment.sig>