[FFmpeg-devel] [PATCH] Add float support to wavpack decoder

Laurent Aimar fenrir
Tue May 5 21:17:45 CEST 2009


On Tue, May 05, 2009, Reimar D?ffinger wrote:
> On Tue, May 05, 2009 at 12:02:58AM +0200, Laurent Aimar wrote:
> > I submit them to have your comments about them as I still dislike one part
> > of it: I suppose IEEE 754 float storage to convert from sign, mantissa and
> > exponent to a float value. I am open to any ideas to avoid that unless it
> > is acceptable.
> 
> Of course it is possible to avoid, you can e.g. use ldexp. But it is
> slow, so e.g. aac does not use it.
> Or you can use av_int2flt which uses it but is even slower (though
> possibly could be optimized to just use the union{} stuff where
> possible, making it inline and then also using it for AAC).
 The thing is, I need to also be able to encode non "regular" float values
like zero+/-, infinity+/-, undernormalized, ... as wavpack allows lossless
encoding of all float values. This makes av_int2flt/ldexp not usable.

 So the question is more : can I assume that ffmpeg use IEEE 754 float
storage and so do the union trick ? If not, how could I achieve this
(with reasonable speed) ?

> > +    if(S){
> > +        S <<= s->float_shift;
> 
> Why S ? That is such a useless variable name, doubly here where it seems
> to be only the mantissa.
  It is short for sample value. It is not always the mantissa, it contains
the sign and exponent encoded in it. I agree that at the end it is the mantissa.


> 
> The "S &= 0x7fffff;" could be factored out.
 I will do.
> 
> > +    (*crc) = (*crc) * 27 + S * 9 + exp * 3 + sign;
> 
> Useless ()
 That too.
> 
> > +    value.u = (sign << 31) | (exp<<23) | S;
> 
> Spaces are inconsistent.
 It escaped me.

> 
> > +    if(!got_float && avctx->sample_fmt == SAMPLE_FMT_FLT){
> > +        av_log(avctx, AV_LOG_ERROR, "Float informations not found\n");
> 
> "information" is not countable, the word "informations" does not exist.
 Thanks, I will remove the 's'.

Regards,

-- 
fenrir



More information about the ffmpeg-devel mailing list