[FFmpeg-devel] [PATCH 1/2] avidec: get rid of some magic numbers in read_gab2_sub()

[Anton Khirnov]

On Sun, Feb 06, 2011 at 11:47:54AM +0100, Reimar D?ffinger wrote:
> On Sun, Feb 06, 2011 at 11:07:05AM +0100, Anton Khirnov wrote:
> > ---
> >  libavformat/avidec.c |    5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> > index 27a9d1f..0e399b5 100644
> > --- a/libavformat/avidec.c
> > +++ b/libavformat/avidec.c
> > @@ -747,9 +747,10 @@ static int avi_read_header(AVFormatContext *s, AVFormatParameters *ap)
> >  }
> >  
> >  static int read_gab2_sub(AVStream *st, AVPacket *pkt) {
> > -    if (!strcmp(pkt->data, "GAB2") && AV_RL16(pkt->data+5) == 2) {
> > +    static const uint8_t gab2_header[] = {'G', 'A', 'B', '2', 0, 2, 0};
> > +    if (!memcmp(pkt->data, gab2_header, sizeof(gab2_header))) {
> 
> This tests one additional byte.

Does it? As far as I can see they're both testing exactly 7 bytes, unless
I'm missing something.

> Also in case the AV_RL16 part is some kind of version number I would
> consider this significantly worse, magic number or not.
TBH I don't know what that number means, this "format" seems like an
undocumented hack (and we can hope no new versions of it will ever be
created).
Anyway, I don't care much about this patch, I can drop it if you think
it does more harm than good.

-- 
Anton Khirnov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110206/481035ad/attachment.pgp>