[Ffmpeg-devel] Re: [PATCH] mov parser fixes and enhancements

[Michael Niedermayer]

Hi

On Sat, Feb 11, 2006 at 10:45:43PM +0100, Baptiste COUDURIER wrote:
> Michael Niedermayer wrote:
> > Hi
> > 
> > On Sat, Feb 11, 2006 at 02:47:47PM +0100, Baptiste COUDURIER wrote:
> >> Michael Niedermayer wrote:
> >>> Hi
> >>>
> >>> On Sat, Feb 11, 2006 at 01:28:18AM +0100, Baptiste COUDURIER wrote:
> >>>> Hi,
> >>>>
> >>>> This patch fixes MOV parser to parse correctly stsd atom.
> >>>> Especially, parsing this file :
> >>>>
> >>>> http://www.coudurier.com/showdown2.mov
> >>>>
> >>>> I tested the patch against all sample I had on hands and I did not
> >>>> detect any error, even mp4 and some samples from mplayer archive.
> >>>>
> >>>> There is one indentation clean, and some big code removal, code which
> >>>> isn't needed anymore, mov_read_stsd is(was ?) a big mess.
> >>> the whole mov demuxer is a big mess, but patches must be split in
> >>> selfcontained parts (1. indention clean, 2. code removial, 3. stsd fix)
> >>>
> >>> [...]
> >>>
> >> All right. Here is the stsd fix.
> > 
> > applied
> > 
> > [...]
> > 
> 
> Great, thank you, I will now check and try to clean a bit stsd parser,
> or even the whole mov.c, do you have any hints or suggestions ?

replace  code like:
#ifdef DEBUG
  av_log(NULL, AV_LOG_DEBUG, "track[%i].edit_count = %i\n", c->fc->nb_streams-1, c->streams[c->fc->nb_streams-1]->edit_count);
#endif

by some macro which gets defined to nothing if DEBUG isnt set, or remove it
completely

replace repeated complex consructs like
c->streams[c->fc->nb_streams-1]->
by their own variable like stream, st or whatever


MOV_SPLIT_CHUNKS is true and the ifdefs should be removed


simplify code:
        int sample_duration;
        int sample_count;

        sample_count=get_be32(pb);
        sample_duration = get_be32(pb);
to
        int sample_count   = get_be32(pb);
        int sample_duration= get_be32(pb);


replace av_free(x) with av_freep(&x)


change comments to doxygen compatible ones where appropriate


currently there is MOVContext.streams[] and AVFormatContext.streams[] and
MOVContext.streams[x] doesnt need to be the same stream as 
AVFormatContext.streams[x], that obviously should be fixed


and please send seperate patches if possible, that makes reviewig and 
testing much easier ...

[...]
-- 
Michael