[FFmpeg-devel] [PATCH 07/12] WMA: use fill_float from dsputil

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Sun, Sep 27, 2009 at 11:49:23AM +0100, Mans Rullgard wrote:
>> ---
>>  libavcodec/wmadec.c |   11 +++++------
>>  1 files changed, 5 insertions(+), 6 deletions(-)
>> 
>> diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c
>> index e5f0508..52ea7a3 100644
>> --- a/libavcodec/wmadec.c
>> +++ b/libavcodec/wmadec.c
>> @@ -262,9 +262,8 @@ static int decode_exp_vlc(WMACodecContext *s, int ch)
>>          v = pow(10, last_exp * (1.0 / 16.0));
>>          max_scale = v;
>>          n = *ptr++;
>> -        do {
>> -            *q++ = v;
>> -        } while (--n);
>> +        s->dsp.fill_float(q, v, n);
>> +        q += n;
>>      }else
>>          last_exp = 36;
>>  
>> @@ -275,13 +274,13 @@ static int decode_exp_vlc(WMACodecContext *s, int ch)
>>          /* NOTE: this offset is the same as MPEG4 AAC ! */
>>          last_exp += code - 60;
>>          /* XXX: use a table */
>> +        av_log(NULL, AV_LOG_ERROR, "exp= %d\n", last_exp);
>>          v = pow(10, last_exp * (1.0 / 16.0));
>>          if (v > max_scale)
>>              max_scale = v;
>>          n = *ptr++;
>> -        do {
>> -            *q++ = v;
>> -        } while (--n);
>> +        s->dsp.fill_float(q, v, n);
>> +        q += n;
>>      }
>
> the call overhead should be quite significant here,

I wouldn't have done this if there wasn't a substantial performance
benefit.  I don't remember how much this change gave, but it was huge,
meaning at least 10% in overall decoding speed.  GCC really is that
stupid.

Since I replaced that pow() call with a table lookup in the next
patch, it would be trivial to replace that with some int* casts, thus
subverting gcc's pessimiser.  Would you be happier with this approach?

> if gcc doesnt get it and someone considers this important then the whole
> should be written in asm or inline asm should be used with C code.

I'd rather not write bitstream reading code entirely in asm.

> calling dsp really does not seems like a good idea to me
>
> (and the av_log of course shouldnt be there ...)

Oops...

-- 
M?ns Rullg?rd
mans at mansr.com