[FFmpeg-devel] [PATCH] WMA Voice decoder

[Ronald S. Bultje]

Hi,

On Feb 9, 2010, at 6:26 PM, Vitor Sessak <vitor1001 at gmail.com> wrote:
> Ronald S. Bultje wrote:
>>
>> On Sat, Feb 6, 2010 at 11:19 AM, Vitor Sessak <vitor1001 at gmail.com>  
>> wrote:
>>>> +    } else /* FCB_TYPE_EXC_PULSES */ {
>>>> +        int offset_nbits = 5 - frame_desc->log_n_blocks;
>>>> +
>>>> +        fcb.no_repeat_mask = -1;
>>>> +        for (n = 0; n < 5; n++) {
>>>> +            float pulse = get_bits1(gb) ? 1.0 : -1.0;
>>>> +            int idx1, idx2;
>>>> +
>>>> +            idx1           = get_bits(gb, offset_nbits);
>>>> +            fcb.x[fcb.n]   = n + 5 * idx1;
>>>> +            fcb.y[fcb.n++] = pulse;
>>>> +            if (n < frame_desc->dbl_pulses) {
>>>> +                idx2           = get_bits(gb, offset_nbits);
>>>> +                fcb.x[fcb.n]   = n + 5 * idx2;
>>>> +                fcb.y[fcb.n++] = (idx1 >= idx2) ? pulse : -pulse;
>>>> +            }
>>>> +        }
>>>> +    }
>>>>
>>> The else{} case is very close to ff_decode_10_pulses_35bits().
>> I noticed that also, except that I can have "single" and "double"
>> pulses, whereas ff_decode_..() only does double pulses. Do you want
>> just a comment saying they're similar,
>
> IMHO the ideal is to have a comment indicating it is similar and to  
> follow the same variable names and conventions in a way to make it  
> obvious to anyone who reads both code how they are different.

Do you agree pos1/2 are inverted and should be swapped in ff_decode..()?

Ronald