[FFmpeg-cvslog] fate/aic: force simple idct

[Carl Eugen Hoyos]

Michael Niedermayer <michaelni <at> gmx.at> writes:

> > > fate/aic: force simple idct
> > > 
> > > This should ensure all platforms use the same idct
> > 
> > I would like to suggest to revert this:
> > As you know, I am all for hiding bugs from users 
> > (by committing hacks around bugs), but hiding 
> > user-visible bugs from fate is not a good idea 
> > imo
> 
> if you dont specify the idct then one will be choosen
> automatically,  which will not produce the same 
> output on all platforms for example armv6 without 
> neon should fail this fate test

Yes, it fails on your cubox.
I just believe that this doesn't change my argument 
that it isn't useful to fix a test that will affect 
(every) user on an affected system.

> > Uncommenting this line in dsputil_ppc.c fixes the 
> > issue for me (and unfortunately doesn't break 
> > fate):
> > c->idct_permutation_type = FF_TRANSPOSE_IDCT_PERM;
> 
> unfortunately though it will break decoding mpeg1/2/4, 
> h263 and a few others on ppc. (untested)

I thought so too (I just wasn't sure) - hoped that 
you would confirm.

> The bug you see is caused by ppc setting onle 2 of 3 
> idct functions so the idct_permutation_type is either 
> right for 2 (used by most code) or 1 (used by aic)
> 
> you can see this by comparing the code to some other 
> that sets idcts
> 
>     if ((avctx->idct_algo == FF_IDCT_AUTO) ||
>         (avctx->idct_algo == FF_IDCT_ALTIVEC)) {
>         c->idct_put              = ff_idct_put_altivec;
>         c->idct_add              = ff_idct_add_altivec;
>         c->idct_permutation_type = FF_TRANSPOSE_IDCT_PERM;
>     }
> 
> vs.
>     if (avctx->idct_algo == FF_IDCT_AUTO ||
>         avctx->idct_algo == FF_IDCT_SIMPLEARMV6) {
>         c->idct_put              = ff_simple_idct_put_armv6;
>         c->idct_add              = ff_simple_idct_add_armv6;
>         c->idct                  = ff_simple_idct_armv6;
>         c->idct_permutation_type = FF_LIBMPEG2_IDCT_PERM;
>     }
> 
> the later sets c->idct too

Thank you for the explanation!

I sent a patch that forces idct simple for aic 
on ppc.
As said, it fails on all ppc platforms (which matches 
your explanation).

Carl Eugen