[FFmpeg-devel] [PATCH 12/12] WMAPRO: use some type punning in decode_coeffs()

[Måns Rullgård]

Sascha Sommer <saschasommer at freenet.de> writes:

> Hi,
>
> On Sonntag, 27. September 2009, Mans Rullgard wrote:
>> ---
>>  libavcodec/wmaprodec.c |   35 ++++++++++++++++++++++-------------
>>  1 files changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavcodec/wmaprodec.c b/libavcodec/wmaprodec.c
>> index e483733..2598ff2 100644
>> --- a/libavcodec/wmaprodec.c
>> +++ b/libavcodec/wmaprodec.c
>> @@ -766,6 +766,12 @@ static int decode_channel_transform(WMAProDecodeCtx*
>> s) */
>>  static int decode_coeffs(WMAProDecodeCtx *s, int c)
>>  {
>> +    static const int fval_tab[16] = {
>> +        0x00000000, 0x3f800000, 0x40000000, 0x40400000,
>> +        0x40800000, 0x40a00000, 0x40c00000, 0x40e00000,
>> +        0x41000000, 0x41100000, 0x41200000, 0x41300000,
>> +        0x41400000, 0x41500000, 0x41600000, 0x41700000,
>> +    };
>>      int vlctable;
>>      VLC* vlc;
>>      WMAProChannelCtx* ci = &s->channel[c];
>> @@ -801,29 +807,32 @@ static int decode_coeffs(WMAProDecodeCtx *s, int c)
>>              for (i = 0; i < 4; i += 2) {
>>                  idx = get_vlc2(&s->gb, vec2_vlc.table, VLCBITS,
>> VEC2MAXDEPTH); if (idx == HUFF_VEC2_SIZE - 1) {
>> -                    vals[i] = get_vlc2(&s->gb, vec1_vlc.table, VLCBITS,
>> VEC1MAXDEPTH); -                    if (vals[i] == HUFF_VEC1_SIZE - 1)
>> -                        vals[i] += ff_wma_get_large_val(&s->gb);
>> -                    vals[i+1] = get_vlc2(&s->gb, vec1_vlc.table, VLCBITS,
>> VEC1MAXDEPTH); -                    if (vals[i+1] == HUFF_VEC1_SIZE - 1)
>> -                        vals[i+1] += ff_wma_get_large_val(&s->gb);
>> +                    int v0, v1;
>> +                    v0 = get_vlc2(&s->gb, vec1_vlc.table, VLCBITS,
>> VEC1MAXDEPTH); +                    if (v0 == HUFF_VEC1_SIZE - 1)
>> +                        v0 += ff_wma_get_large_val(&s->gb);
>> +                    v1 = get_vlc2(&s->gb, vec1_vlc.table, VLCBITS,
>> VEC1MAXDEPTH); +                    if (v1 == HUFF_VEC1_SIZE - 1)
>> +                        v1 += ff_wma_get_large_val(&s->gb);
>> +                    ((float*)vals)[i  ] = v0;
>> +                    ((float*)vals)[i+1] = v1;
>>                  } else {
>> -                    vals[i]   = symbol_to_vec2[idx] >> 4;
>> -                    vals[i+1] = symbol_to_vec2[idx] & 0xF;
>> +                    vals[i]   = fval_tab[symbol_to_vec2[idx] >> 4 ];
>> +                    vals[i+1] = fval_tab[symbol_to_vec2[idx] & 0xF];
>>                  }
>>              }
>>          } else {
>> -            vals[0] =  symbol_to_vec4[idx] >> 12;
>> -            vals[1] = (symbol_to_vec4[idx] >> 8) & 0xF;
>> -            vals[2] = (symbol_to_vec4[idx] >> 4) & 0xF;
>> -            vals[3] =  symbol_to_vec4[idx]       & 0xF;
>> +            vals[0] = fval_tab[ symbol_to_vec4[idx] >> 12      ];
>> +            vals[1] = fval_tab[(symbol_to_vec4[idx] >> 8) & 0xF];
>> +            vals[2] = fval_tab[(symbol_to_vec4[idx] >> 4) & 0xF];
>> +            vals[3] = fval_tab[ symbol_to_vec4[idx]       & 0xF];
>>          }
>>
>>          /** decode sign */
>>          for (i = 0; i < 4; i++) {
>>              if (vals[i]) {
>>                  int sign = get_bits1(&s->gb) - 1;
>> -                ci->coeffs[cur_coeff] = (vals[i] ^ sign) - sign;
>> +                *(uint32_t*)&ci->coeffs[cur_coeff] = vals[i] ^ sign<<31;
>>                  num_zeros = 0;
>>              } else {
>>                  ci->coeffs[cur_coeff] = 0;
>
> This patch looks a bit ugly but as it seems to be significantly faster I'm not 
> against it, if such code is acceptable for ffmpeg in general.
> I only wonder if the table and sign conversion code should maybe be moved to 
> another place in ffmpeg so that it can be reused in other codecs?
>
> If the code is going to stay as is, please add a comment that describes the 
> content of fval_tab and why this is needed.

I've applied the patch with an additional comment.

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