[FFmpeg-devel] [PATCH 2/3] bink: get rid of double precission float use

[Vitor Sessak]

On Tue, Feb 7, 2012 at 10:02 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Tue, Feb 07, 2012 at 09:59:51PM +0100, Vitor Sessak wrote:
>> On Tue, Feb 7, 2012 at 8:17 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Tue, Feb 07, 2012 at 08:06:19PM +0100, Vitor Sessak wrote:
>> >> On Mon, Feb 6, 2012 at 7:31 PM, Michael Niedermayer <michaelni at gmx.at>
>> >> wrote:
>> >> > Add a small table of integers and out commented code to generate it with
>> >> > the double precision code.
>> >>
>> >> Not against the idea, but note that this should not be needed for
>> >> bit-exactness: I checked that all the double values are far enough from
>> >> integers for this code to be bitexact across archs.
>> >
>> > do you think its better to leave the double prec code ?
>>
>> No, not really. I like integer code, it's way more obvious if it is
>> bit-exact or not.
>>
>> >> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> >> > ---
>> >> >  libavcodec/bink.c |   36 ++++++++++++++++++++++++------------
>> >> >  1 files changed, 24 insertions(+), 12 deletions(-)
>> >> >
>> >> > diff --git a/libavcodec/bink.c b/libavcodec/bink.c
>> >> > index ff8bc8a..c77c139 100644
>> >> > --- a/libavcodec/bink.c
>> >> > +++ b/libavcodec/bink.c
>> >> > @@ -1222,40 +1222,52 @@ static int decode_frame(AVCodecContext *avctx,
>> >> void *data, int *data_size, AVPac
>> >> >  static av_cold void binkb_calc_quant(void)
>> >> >  {
>> >> >     uint8_t inv_bink_scan[64];
>> >> > -    double s[64];
>> >> > +    static const int s[64]={
>> >> > +        1073741824,1489322693,1402911301,1262586814,1073741824,
>> >> 843633538, 581104888, 296244703,
>> >> > +
>> >>  1489322693,2065749918,1945893874,1751258219,1489322693,1170153332,
>> >> 806015634, 410903207,
>> >> > +
>> >>  1402911301,1945893874,1832991949,1649649171,1402911301,1102260336,
>> >> 759250125, 387062357,
>> >> > +        1262586814,1751258219,1649649171,1484645031,1262586814,
>> >> 992008094, 683307060, 348346918,
>> >> > +        1073741824,1489322693,1402911301,1262586814,1073741824,
>> >> 843633538, 581104888, 296244703,
>> >> > +         843633538,1170153332,1102260336, 992008094, 843633538,
>> >> 662838617, 456571181, 232757969,
>> >> > +         581104888, 806015634, 759250125, 683307060, 581104888,
>> >> 456571181, 314491699, 160326478,
>> >> > +         296244703, 410903207, 387062357, 348346918, 296244703,
>> >> 232757969, 160326478,  81733730,
>> >> > +    };
>> >> >     int i, j;
>> >> > -
>> >> > +#define C (1LL<<30)
>> >> > +#if 0
>> >> >     for (j = 0; j < 8; j++) {
>> >> >         for (i = 0; i < 8; i++) {
>> >> >             if (j && j != 4)
>> >> >                if (i && i != 4)
>> >>
>> >> > -                   s[j*8 + i] = cos(j * M_PI/16.0) * cos(i * M_PI/16.0)
>> >> * 2.0;
>> >> > +                   s[j*8 + i] = llrint(cos(j * M_PI/16.0) * cos(i *
>> >> M_PI/16.0) * 2.0 * C);
>> >>
>> >> Are you sure about this change? This will make the table different from the
>> >> reference decoder (and I bet also from the reference encoder), and thus
>> >> should change the output in a negative way.
>> >
>> > where does a table that is used outside the init function become
>> > different ?
>>
>> Does the binkb_inter_quant table is the same with and without the
>> llrint()? I suppose your int code is bitexact to the llrint()
>> version...
>
> I av_log()ed the tables and diffed before i posted the patches so
> AFAIK theres no change but i could have made a mistake

So I'm not against it.

-Vitor