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

[Vitor Sessak]

Benjamin Larsson wrote:
> 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.

Err, the else does not correspond to this if(). The indentation is weird 
to avoid cosmetics, after reindenting it will be

 >>              if (nbits != 5 && nbits != 7 && nbits != 8)
 >>                  return -1;
 >>          } else {
 >>              block_size = 32;
 >>              nbits = 5;
 >>          }

-Vitor