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

[Reimar Döffinger]

On Sun, Feb 06, 2011 at 03:12:25PM +0100, Anton Khirnov wrote:
> 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.

No, I missed 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.

I don't care either, it just seems a bit pointless to me...