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

Reimar Döffinger Reimar.Doeffinger
Wed Sep 2 16:48:09 CEST 2009


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...



More information about the ffmpeg-devel mailing list