[FFmpeg-devel] [PATCH] Add libavsequencer.

[Stefano Sabatini]

On date Tuesday 2010-08-17 19:43:13 +0200, Michael Niedermayer encoded:
> On Sat, Aug 14, 2010 at 02:24:15AM +0200, Stefano Sabatini wrote:
> > On date Friday 2010-08-13 22:53:30 +0200, Sebastian Vater encoded:
> > > The new library is meant to contain the sequencer multimedia features for
> > > being able to playback modules and MIDI files in FFmpeg.
> > > 
> > [...]
> > > diff --git a/libavsequencer/avsequencer.c b/libavsequencer/avsequencer.c
> > > new file mode 100644
> > > index 0000000..d43284f
> > > --- /dev/null
> > > +++ b/libavsequencer/avsequencer.c
> > > @@ -0,0 +1,43 @@
> > > +/*
> > > + * Implement AVSequencer functions
> > > + * Copyright (c) 2010 Sebastian Vater <cdgs.basty at googlemail.com>
> > > + *
> > > + * This file is part of FFmpeg.
> > > + *
> > > + * FFmpeg is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU Lesser General Public
> > > + * License as published by the Free Software Foundation; either
> > > + * version 2.1 of the License, or (at your option) any later version.
> > > + *
> > > + * FFmpeg is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > + * Lesser General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU Lesser General Public
> > > + * License along with FFmpeg; if not, write to the Free Software
> > > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > > + */
> > > +
> > > +#include "config.h"
> > > +#include "avsequencer.h"
> > > +
> > > +/**
> > > + * @file
> > > + * Implement AVSequencer functions.
> > > + */
> > > +
> > > +#include <libavutil/avutil.h>
> > > +
> > 
> > > +unsigned avsequencer_version(void) {
> > > +    return LIBAVSEQUENCER_VERSION_INT;
> > > +}
> > > +
> > > +const char *avsequencer_configuration(void) {
> > > +    return FFMPEG_CONFIGURATION;
> > > +}
> > > +
> > > +const char *avsequencer_license(void) {
> > > +#define LICENSE_PREFIX "libavsequencer license: "
> > > +    return LICENSE_PREFIX FFMPEG_LICENSE + sizeof(LICENSE_PREFIX) - 1;
> > > +}
> > 
> > Nits:
> > foo(...)
> > {
> >   ...
> > }
> > 
> > Looks OK otherwise to me, assuming that Micheal is OK with
> > libavsequencer inclusion, note that libavsequencer is currently
> > disabled by default. We can consider libavsequencer API unstable, and
> > so don't worry too much about API/ABI breaks.
> > 
> > I believe it's better to keep it integrated and disabled by default
> > rather than keep it in an external repo, Sebastian will add piece by
> > piece as review will go on.
> 
> this patch here is ok if the respective maintainers are ok with it

Please send an updated version with the nits fixed, regards.
-- 
FFmpeg = Furious and Fast Muttering Powered Extended Gadget