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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel