[FFmpeg-devel] [PATCH 1/2] avidec: get rid of some magic numbers in read_gab2_sub()
Reimar Döffinger
Reimar.Doeffinger
Sun Feb 6 15:17:57 CET 2011
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...
More information about the ffmpeg-devel
mailing list