[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