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

[Michael Niedermayer]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120207/65e3f479/attachment.asc>