[FFmpeg-devel] [PATCH] hook up the atrac1 decoder

[Benjamin Larsson]

Vitor Sessak wrote:
> Benjamin Larsson wrote:
>> Vitor Sessak wrote:
>>> Benjamin Larsson wrote:
>>>> Vitor Sessak wrote:
>>>>>>> I think a good deal of code of both branches of the if() can be
>>>>>>> factorized out.
>>>>>> What code? IMO not much of the code in the 2 cases can be 
>>>>>> factored out
>>>>>> and still be logical and clear.
>>>>> The calls to at1_imdct() and vector_fmul_window() are very similar.
>>>>> They are exactly the same if block_size == 32 and nbits == 5 when
>>>>> num_blocks == 1. Note also that the for() loop of the else{} branch
>>>>> will be run only once when num_blocks == 1, as it should. Of course,
>>>>> there will always be a if() for the memcpy().
>>>> Ok, I see how it could be factored. But the logic of passing
>>>> parameters would be highly obfuscated.
>>> Do you consider the following highly obfuscated or am I missing 
>>> something?
>>
>> Now you really have to elaborate what you are meaning.
>
> Ok, my completely untested patch was wrong, but this one (that is 
> still missing a little clean up) does not change the output for 
> samples in samples.mplayerhq.hu.
>
> -Vitor

Hmm, ok that didn't look to bad. But this code doesn't make any sence:

>          if (nbits != 5 && nbits != 7 && nbits != 8)
>              return -1;
> +        } else {
> +            block_size = 32;
> +            nbits = 5;
> +        }
If nbits is valid always set nbits to 5? That couldn't have worked. You 
should have gotten the wrong mdct_context.

MvH
Benjamin Larsson