[FFmpeg-devel] [PATCH 1/5] tests/fate-run: only support le format result on pixfmts

[Muhammad Faiz]

On Mon, Jan 28, 2019 at 3:07 AM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> On Sun, Jan 27, 2019 at 11:24:46PM +0700, Muhammad Faiz wrote:
> > On Sun, Jan 27, 2019 at 10:53 PM Michael Niedermayer
> > <michael at niedermayer.cc> wrote:
> > >
> > > On Sun, Jan 27, 2019 at 04:36:15PM +0700, Muhammad Faiz wrote:
> > > > regardless of the actual supported formats.
> > > > This allows filters to support only native-endian formats,
> > > > and also allows consistency checks between little-endian
> > > > and big-endian implementation.
> > > >
> > > > This also reveals bugs on gbrap10, p010, p016 format, and
> > > > super2xsai filter (mismatched checksums between little-endian
> > > > and big-endian).
> > > >
> > > > Suggested-by: Hendrik Leppkes <h.leppkes at gmail.com>
> > > > Signed-off-by: Muhammad Faiz <mfcc64 at gmail.com>
> > > > ---
> > > > Old thread is here: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-June/195930.html
> > > >
> > > >  tests/fate-run.sh                        |   6 +-
> > > >  tests/ref/fate/filter-pixfmts-copy       | 112 +++++++++++------------
> > > >  tests/ref/fate/filter-pixfmts-crop       | 112 +++++++++++------------
> > > >  tests/ref/fate/filter-pixfmts-field      | 112 +++++++++++------------
> > > >  tests/ref/fate/filter-pixfmts-fieldorder |  88 +++++++++---------
> > > >  tests/ref/fate/filter-pixfmts-hflip      | 112 +++++++++++------------
> > > >  tests/ref/fate/filter-pixfmts-il         | 112 +++++++++++------------
> > > >  tests/ref/fate/filter-pixfmts-null       | 112 +++++++++++------------
> > > >  tests/ref/fate/filter-pixfmts-scale      | 112 +++++++++++------------
> > > >  tests/ref/fate/filter-pixfmts-super2xsai |   8 +-
> > > >  tests/ref/fate/filter-pixfmts-swapuv     |  56 ++++++------
> > > >  tests/ref/fate/filter-pixfmts-transpose  |  90 +++++++++---------
> > > >  tests/ref/fate/filter-pixfmts-vflip      | 112 +++++++++++------------
> > > >  13 files changed, 573 insertions(+), 571 deletions(-)
> > > >
> > > > diff --git a/tests/fate-run.sh b/tests/fate-run.sh
> > > > index aece90a01d..faa4285e71 100755
> > > > --- a/tests/fate-run.sh
> > > > +++ b/tests/fate-run.sh
> > > > @@ -297,8 +297,10 @@ pixfmts(){
> > > >
> > > >      outertest=$test
> > > >      for pix_fmt in $pix_fmts; do
> > > > -        test=$pix_fmt
> > > > -        video_filter "${prefilter_chain}format=$pix_fmt,$filter=$filter_args" -pix_fmt $pix_fmt -frames:v $nframes
> > > > +        # force little endian format on result
> > > > +        pix_fmt_le=`echo $pix_fmt | sed 's/be$/le/'`
> > > > +        test=$pix_fmt_le
> > > > +        video_filter "${prefilter_chain}format=$pix_fmt,$filter=$filter_args" -pix_fmt $pix_fmt_le -frames:v $nframes
> > > >      done
> > >
> > > if the input to a filter is a big endian format and the output is little
> > > endian. Then there really isnt a gurantee that the filter will work with
> > > big endian data. The libavfilter core could convert before or after the filter
> > > as it preferrs. At least thats how i remember it from the last time i looked
> > > at the code.
> > > This also makes sense, as there are very good reasons to convert before,
> > > for example when doing so results in better quality or higher speed
> > > or fewer converts overall in multi input or multi output filters...
> >
> > Of course, this can be easily fixed by adding format=$pix_fmt after
> > $filter=$filter_args.
> >
>
> > >
> > > So if the output is always forced to LE then this may unintentionally
> > > remove testing a range of cases.
> > > also this removes testing the codepath for big endian formats after the
> > > convert. Or do we have remaining cases that test these ?
> >
> > Do you suggest to duplicate test to output BE and LE simultaneously?
>
> I dont really have a specific suggestion, just dont want to have some
> codepathes be lost from testing
> a solution that doesnt duplicate all the tests would be better i guess
> as it would be quicker

Do you feel that because BE disappears in the output, it means that BE
is untested?