[FFmpeg-devel] [PATCH 2/2] libavutil: Make changes in softfloat needed for fixed point aac decoder.

[Michael Niedermayer]

On Wed, Apr 22, 2015 at 01:15:56PM +0000, Nedeljko Babic wrote:
> > >
> >> >> +static av_always_inline SoftFloat av_div_sf(SoftFloat a, SoftFloat b){
> >> >
> >> >missing documentation
> >> 
> >> I'll add it.
> >> 
> >> >is this exact ?
> >> >if not what are the gurantees to the user
> >> >
> >> 
> >> On our tests
> >> For 80.3% of input values the output is exact
> >> For 19.2% of input values the error is one LSB bit of mantissa
> >> For 0.5% of input values the error is two LSB bits of mantissa
> >
> >I think av_div_sf() should be exact, approximate
> >functions should have a clear and distinct name, be that
> >av_div_sf_approx() or whatever
> >
> 
> Ok, I'll rename the function.
> Should I leave the original div function in the code?

there should be an exact divide function left in there, yes


> 
> >
> >[...]
> >> >
> >> >> +    SoftFloat res;
> >> >> +    SoftFloat iB, tmp;
> >> >> +
> >> >> +    if (b.mant != 0)
> >> >> +    {
> >> >> +        iB = av_recip_sf(b);
> >> >> +        /* Newton iteration to double precision */
> >> >> +        tmp = av_sub_sf(FLOAT_1, av_mul_sf(b, iB));
> >> >> +        iB = av_add_sf(iB, av_mul_sf(iB, tmp));
> >> >> +        tmp = av_sub_sf(FLOAT_1, av_mul_sf(b, iB));
> >> >> +        iB = av_add_sf(iB, av_mul_sf(iB, tmp));
> >> >> +        tmp = av_sub_sf(FLOAT_1, av_mul_sf(b, iB));
> >> >> +        iB = av_add_sf(iB, av_mul_sf(iB, tmp));
> >> >> +        res = av_mul_sf(a, iB);
> >> >> +    }
> >> >> +    else
> >> >> +    {
> >> >> +        /* handle division-by-zero */
> >> >> +        res.mant = 1;
> >> >> +        res.exp = 0x7FFFFFFF;
> >> >> +    }
> >> >> +
> >> >> +    return res;
> >> >> +#endif
> >> >> +}
> >> >> +
> >> >> +//FIXME log, exp, pow
> >> >>  
> >> >
> >> >>  static inline av_const SoftFloat av_int2sf(int v, int frac_bits){
> >> >> -    return av_normalize_sf((SoftFloat){v, ONE_BITS-frac_bits});
> >> >> +    return av_normalize_sf((SoftFloat){v, frac_bits});
> >> >>  }
> >> >
> >> >missing documentation
> >> 
> >> I'll add it.
> >> 
> >> >also please make sure that the parameters make some logic sense
> >> >and do not depend on the precission choosen by the implementation
> >> >
> >> >so a "1.0" shwould be generated from the same arguments no matter
> >> >what the precision used in the implementation is
> >> 
> >> I am not sure I understand you on this.
> >> 
> >> Basic implementation of this function is the same as in original except
> >> "ONE_BITS-frac_bits" is changed with "frac_bits".
> >> This was done since algorithm is adjusted to be usable in implementation
> >> of fixed point aac decoder.
> >
> >the numbers are of the form x * 2^y
> >thus the interface to create 4.0 should be av_int2sf(1,2) not
> >int2sf(1,123)
> >The interface should be simple, logic and intuitive
> 
> As I said in the comment of this patch and before in comments to review,
> I modified the code in the softfloat to be more usable in the implementation
> of fixed point aac decoder.
> Fixed point aac decoder was developed by using our float emulation and it was
> more convenient to change ffmpeg softfloat than to make changes in aac since
> softfloat is not used anywhere in ffmpeg currently.
> 
> It is clear to me that maybe for the number that is power of two it is more
> convenient to use original way, but for other numbers it is basically the same.
> 
> On the other hand there is one subtraction less in the implementation of this
> function if precision is used as argument.
> 
> And at the end, basically, I do not have a problem with changing this function
> back to original if you want me to and comments above were just for the sake of
> discussion :)

you can add a 2nd function if you feel the subtraction matters
but there should be one with simple and a human understandable
interface

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150422/e9d3cf27/attachment.asc>