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

[Benjamin Larsson]

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. num_blocks == 1
means that it is a long transform either 2^7 or 2^8. Anyway aac doesn't
merge the 8 short transform mode. Feel strange to merge this one just
because it is possible. To me this clearly shows that there are 2
different coding modes and that they need to be handled differently.

> 
>>          /* 4 for short mode(low/middle bands) and 8 for short
>> mode(high band)*/
>>          num_blocks = 1 << log2_block_count;
>>  
>> +        if (num_blocks == 1) {
>> +            block_size = 32;
>> +            nbits = 5;
>> +        } else {
>>          /* mdct block size in samples: 128 (long mode, low & mid
>> bands), */
>>          /* 256 (long mode, high band) and 32 (short mode, all bands) */
>>          block_size = band_samples >> log2_block_count;
>> @@ -130,6 +134,7 @@
>>  
>>          if (nbits != 5 && nbits != 7 && nbits != 8)
>>              return -1;
>> +        }
>>  
> 
> -Vitor

MvH
Benjamin Larsson