[FFmpeg-cvslog] r10953 - trunk/libavformat/mpeg.c

[Aurelien Jacobs]

Rich Felker wrote:

> On Thu, Nov 08, 2007 at 08:07:49AM +0000, M?ns Rullg?rd wrote:
> > aurel <subversion at mplayerhq.hu> writes:
> > 
> > > Author: aurel
> > > Date: Thu Nov  8 00:56:00 2007
> > > New Revision: 10953
> > >
> > > Log:
> > > correctly set audio codec when demuxing sofdec files
> > > samples: http://samples.mplayerhq.hu/game-formats/sfd/
> > >
> > > Modified:
> > >    trunk/libavformat/mpeg.c
> > >
> > > Modified: trunk/libavformat/mpeg.c
> > > ==============================================================================
> > > --- trunk/libavformat/mpeg.c	(original)
> > > +++ trunk/libavformat/mpeg.c	Thu Nov  8 00:56:00 2007
> > > @@ -107,15 +107,25 @@ static int mpegps_probe(AVProbeData *p)
> > >  typedef struct MpegDemuxContext {
> > >      int32_t header_state;
> > >      unsigned char psm_es_type[256];
> > > +    int sofdec;
> > >  } MpegDemuxContext;
> > >
> > >  static int mpegps_read_header(AVFormatContext *s,
> > >                                AVFormatParameters *ap)
> > >  {
> > >      MpegDemuxContext *m = s->priv_data;
> > > +    uint8_t buffer[8192];
> > > +    char *p;
> > > +
> > >      m->header_state = 0xff;
> > >      s->ctx_flags |= AVFMTCTX_NOHEADER;
> > >
> > > +    get_buffer(&s->pb, buffer, sizeof(buffer));
> > > +    if ((p=memchr(buffer, 'S', sizeof(buffer))))
> > > +        if (!memcmp(p, "Sofdec", 6))
> > 
> > This might read past the buffer.

Indeed, yes.
This is trivial to fix. I will apply the attached patch if we decide
to keep this code.

> > > +            m->sofdec = 1;
> > > +    url_fseek(&s->pb, -sizeof(buffer), SEEK_CUR);
> > 
> > Is there really no better way to detect these files?  Scanning 8k for
> > a magic string seems excessive.

Not that I know of. This string is often a bit after the first 4k bytes.

> > Besides, people are saying it breaks things.

But I can't reproduce this supposed breakage...

> I agree. I almost puked when I saw this code committed. Problematic
> hacks for useless game formats do not belong in one of the most
> important demuxers..

I would have been glad to here this when I posted the patch on
ffmpeg-devel 7 days ago...
Do you want me to revert this commit ?
Do you have any better solution to propose ?

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpeg_sofdec_fix.diff
Type: text/x-patch
Size: 502 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20071108/126ff502/attachment.bin>