On Thu, Oct 13, 2016 at 10:14:54PM +0200, Anton Khirnov wrote:
> Quoting Diego Biurrun (2016-10-13 22:09:03)
> > On Thu, Oct 13, 2016 at 09:53:47PM +0300, Martin Storsjö wrote:
> > > On Thu, 13 Oct 2016, Diego Biurrun wrote:
> > > 
> > > >On Thu, Oct 13, 2016 at 01:57:34PM +0200, Alexandra Hájková wrote:
> > > >
> > > >>>> +void checkasm_check_hevc_add_res(void)
> > > >>>> +{
> > > >>>> +    int bit_depth;
> > > >>>> +
> > > >>>> +    for (bit_depth = 8; bit_depth <= 10; bit_depth++) {
> > > >>>> +        HEVCDSPContext h;
> > > >>>> +
> > > >>>> +        ff_hevc_dsp_init(&h, bit_depth);
> > > >>>> +        check_add_res(h, bit_depth);
> > > >>>> +    }
> > > >>>
> > > >>> I didn't see you add 9-bit versions of the assembly functions, why do
> > > >>> you test 9 bits?
> > > >>>
> > > >>Because there's no 9 bit SIMD function, it's not tested but the code
> > > >>looks simpler this way.
> > > >
> > > >If there is nothing to test, don't test it. Just skip over the 9-bit
> > > >test by incrementing your counter variable by 2 instead of 1.
> > > 
> > > I would argue the other way; just because there's currently no SIMD for 
> > > it,
> > > we shouldn't skip it. Is 9 bit a valid choice here? If it is, keep it in 
> > > the
> > > test - it won't cost anything if there actually aren't any SIMD functions,
> > > and if there are, they will be tested. If it isn't valid (as in, the dsp
> > > context init functions aren't supposed to handle it), it should of course 
> > > be
> > > skipped.
> > 
> > AFAIR we do not have 9-bit code anywhere. If they ever appear, it's trivial
> > to modify the tests to also cover 9 bits.
> 
> The decoder should work with 9bit files, the relevant C functions do
> exist. It seems quite silly to me to go out of our way to not test it
> just because not SIMD currently exists.

Umm, I think that

  for (bit_depth = 8; bit_depth <= 10; bit_depth++)
  -->
  for (bit_depth = 8; bit_depth <= 10; bit_depth += 2)

is hardly "going out of our way".

Also, 12-bit, 14-bit, and 16-bit SIMD also does not exist, why not start
testing it as well right now? After all, the code might appear in the
future...

Diego
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to