[FFmpeg-devel] [HACK] mov.c: Handle MPEG-in-MOV (#241 and friends)

[Petter Ericson]

Strike three, then.

On Wed, Oct 17, 2012 at 05:48:37PM +0200, Michael Niedermayer wrote:
> On Wed, Oct 17, 2012 at 05:35:11PM +0200, Petter Ericson wrote:
> > On Wed, Oct 17, 2012 at 05:15:20PM +0200, Michael Niedermayer wrote:
> > > On Wed, Oct 17, 2012 at 05:04:07PM +0200, Petter Ericson wrote:
> > > > On Wed, Oct 17, 2012 at 02:35:07AM +0200, Michael Niedermayer wrote:
> > > > > On Tue, Oct 16, 2012 at 09:33:24PM +0200, Petter Ericson wrote:
> > > > > > On Tue, Oct 16, 2012 at 06:43:50PM +0200, Hendrik Leppkes wrote:
> > > > > > > On Tue, Oct 16, 2012 at 5:18 PM, Petter Ericson
> > > > > > > <petter.ericson at codemill.se> wrote:
> > > > > > > > Greetings,
> [...]
> >  mov.c |   50 +++++++++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 43 insertions(+), 7 deletions(-)
> > e6c017cc3510726ce8841b29a00899812c1520a0  0001-mov.c.patch
> > From dd0c50b40e7264c9c3d22b010f0f10831aede88f Mon Sep 17 00:00:00 2001
> > From: Petter Ericson <petter.ericson at codemill.se>
> > Date: Wed, 17 Oct 2012 16:53:19 +0200
> > Subject: [PATCH] mov.c:     Check for stsd + m1s tag indicating MOV-wrapped
> >  MPEG-PS, and force 	continued probing if found.
> > 
> > ---
> >  libavformat/mov.c |   50 +++++++++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 43 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index b817bc9..e93356f 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -2861,43 +2861,79 @@ static int mov_probe(AVProbeData *p)
> >      unsigned int offset;
> >      uint32_t tag;
> >      int score = 0;
> 
> > +    int stsd = MKTAG('s','t','s','d');
> 
> thats not making the code clearar, its used just once
> 

Agreed - fixed.

> 
> > +    int moov_offset = -1;
> >  
> >      /* check file header */
> >      offset = 0;
> >      for (;;) {
> >          /* ignore invalid offset */
> >          if ((offset + 8) > (unsigned int)p->buf_size)
> > -            return score;
> > +            break;
> >          tag = AV_RL32(p->buf + offset + 4);
> >          switch(tag) {
> >          /* check for obvious tags */
> > -        case MKTAG('j','P',' ',' '): /* jpeg 2000 signature */
> >          case MKTAG('m','o','o','v'):
> > +            moov_offset = offset + 4;
> > +        case MKTAG('j','P',' ',' '): /* jpeg 2000 signature */
> >          case MKTAG('m','d','a','t'):
> >          case MKTAG('p','n','o','t'): /* detect movs with preview pics like ew.mov and april.mov */
> >          case MKTAG('u','d','t','a'): /* Packet Video PVAuthor adds this and a lot of more junk */
> >          case MKTAG('f','t','y','p'):
> > -            return AVPROBE_SCORE_MAX;
> > +            score = FFMAX(score, AVPROBE_SCORE_MAX);
> > +            offset = AV_RB32(p->buf+offset) + offset;
> > +            break;
> >          /* those are more common words, so rate then a bit less */
> >          case MKTAG('e','d','i','w'): /* xdcam files have reverted first tags */
> >          case MKTAG('w','i','d','e'):
> >          case MKTAG('f','r','e','e'):
> >          case MKTAG('j','u','n','k'):
> >          case MKTAG('p','i','c','t'):
> > -            return AVPROBE_SCORE_MAX - 5;
> > +            score = FFMAX(score, AVPROBE_SCORE_MAX - 5);
> > +            offset = AV_RB32(p->buf+offset) + offset;
> 
> that can end in a infinite loop if the read value is 0
> 

Replaced with FFMIN(1, ...)

> 
> > +            break;
> >          case MKTAG(0x82,0x82,0x7f,0x7d):
> >          case MKTAG('s','k','i','p'):
> >          case MKTAG('u','u','i','d'):
> >          case MKTAG('p','r','f','l'):
> >              offset = AV_RB32(p->buf+offset) + offset;
> >              /* if we only find those cause probedata is too small at least rate them */
> > -            score = AVPROBE_SCORE_MAX - 50;
> > +            score = FFMAX(score, AVPROBE_SCORE_MAX - 50);
> >              break;
> >          default:
> > -            /* unrecognized tag */
> > -            return score;
> > +            offset = AV_RB32(p->buf + offset) + offset ;
> >          }
> >      }
> > +    if(tag > AVPROBE_SCORE_MAX - 50 && moov_offset != -1){
> > +        /* We should make sure that this is not a MOV-packed MPEG-PS */
> > +        offset = moov_offset;
> > +        
> > +        while(offset < (p->buf_size - 20) && /* Sufficient space */
> > +              AV_RL32(p->buf+offset) != stsd)
> > +            offset++;
> > +
> > +
> > +        /* We didn't find an stsd atom for some reason */
> > +        if(offset == (p->buf_size - 20))
> > +            return score;
> > +
> > +        /* Make sure there's only one stream */
> > +        if(AV_RB32(p->buf + offset + 8) != 1) 
> > +            return score;
> > +
> > +        /* Check for m1s tag */
> > +        if(AV_RL32(p->buf + offset + 16) != MKTAG('m','1','s',' '))
> > +            return score;
> 
> this would fail if by sheer (bad) luck the string "stsd" would occur
> before the correct stsd, this is not nearly as unlikely as it seems
> its ascii, and there might be some kind of metadata in the headers
> that too is ascii ...
> 
> all the checks should be done together, that is
> while(foo){
>     if(we have stsd && one stream && m1s)
>         return 5;
> }

Reformatted this section a bit. Placed the checks together, and keeps
going if stsd is found without the other values being present.

Cheers

/P

-- 
Petter Ericson
Systems developer, Codemill AB
petter.ericson at codemill.se