[FFmpeg-devel] [PATCH] atrac1 decoder and aea demuxer
Wed Sep 2 17:10:08 CEST 2009
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 &&
> 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.
More information about the ffmpeg-devel