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

[Petter Ericson]

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,
> > > >
> > > > Attached is a patch that handles the files provided in #241 and duplicates.
> > > > Needless to say, it is extremely hacky and would benefit from improvements,
> > > > but as it stands, it checks for the two places I have found the m1s tag, and
> > > > forces continued probing until mpegps_probe gets enough data to find its
> > > > own header. I considered using the mpegps demuxer as a "subprocess" simliar
> > > > to how DVDemuxer is used, but this proved to be too much trouble for me,
> > > > given that it really isn't made to work in that way, and the above
> > > > approach got the desired result.
> > > >
> > > > Improvements (rudimentary MOV parsing in the probe?) very welcome.
> > > >
> > > 
> > > The fixed offset of the tag in the file seems rather arbitrary to me.
> > > Did you confirm on multiple samples that its in that exact spot every
> > > time?
> > 
> > I checked two - my own sample and the one in the original ticket. They both
> > had the first offset (0x21E). A different duplicate (#600 something) had the
> > smaller offset due to missing the initial ftyp section before the moov atom.
> > 
> > I agree that something better should be found - a fixed offset from the
> > moov atom could probably work better, but that again takes us into 
> > rudimentary parsing territory. One might refactor the switch/case to use
> > score = MAX(returnvalue, score) or something like that, which would at least
> > allow slightly more flexibility - I'll cook something up tomorrow and
> > resubmit unless someone gets there before me.
> 
> if you dont want to write a simple parser. One could simply scan the
> first X bytes for a valid looking stsd + m1v tag, that would avoid
> the fixed offset and checking 2 tags should make false positives less
> likely
> 

Attempt two: I've reworked the code to save the offset for the moov atom, and
use that as a baseline for finding the stsd, and doing some checks to see if
a) there's only one stream and b) it has format = "m1s ".

I had to do a little bit of magic in the switch/case to make it behave, so my
guess is that a sanity check there might be a good idea. Specifically, the
default case of skipping ahead the supposed atom size might benefit from 
something better (for checking subatoms or smth).

Also, no tabs.

Best

/P

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mov.c.patch
Type: text/x-diff
Size: 3915 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121017/2e784fa9/attachment.bin>