[FFmpeg-devel] [PATCH] atrac1 decoder and aea demuxer

[Benjamin Larsson]

Reimar D?ffinger wrote:
> On Wed, Sep 02, 2009 at 03:49:08PM +0200, Benjamin Larsson wrote:
>   
>> Reimar D?ffinger wrote:
>>     
>>> [...]
>>>   
>>>       
>>>> +        /* Check so that values are != 0 */
>>>> +        checksum = bsm_s + bsm_e + inb_s + inb_e;
>>>> +        if (checksum)
>>>> +            if ((bsm_s == bsm_e) && (inb_s == inb_e) && (bsm_s != inb_s))
>>>>     
>>>>         
>>> Huh, what?
>>> Is that meant to be
>>> if (bsm_s && bsm_e && bsm_s == bsm_e && inb_s == inb_e && bsm_s != inb_s)
>>>
>>> written by someone who likes obfuscation too much?
>>> If not it seriously needs some more comments/explanations.
>>>       
>> The block switch mode and info byte are repeated in an atrac frame. But 
>> they are never the same value. Hope that explains the code.
>>     
>
> Well, the checksum thing is the really curious one (particularly since
> someone seems to temporarily have forgotten the existence if the &&
> operator).
> I messed up my example, but my point was that if you check
> bsm_s == bsm_e it is a bit pointless to check bsm_s != 0 _and_ bsm_e != 0,
> because both will give the same unless your CPU is broken...
> Now, in addition I notice that another check is bsm_s != inb_s which can
> hardly be true if all are 0, can it?
> So that would result in this check:
> if (bsm_s == bsm_e && inb_s == inb_e && bsm_s != inb_s)
> Which... well... looking at the original code means that that whole
> "checksum" thing is just a very elaborate nop...
>   

True, I'll remove it.

MvH
Benjamin Larsson