[FFmpeg-devel] [PATCH] ffprobe: add priv_class field to Writer, and factorize writer options initialization

[Stefano Sabatini]

On date Tuesday 2012-09-11 19:01:07 +0200, Clément Bœsch encoded:
> On Tue, Sep 11, 2012 at 11:51:05AM +0200, Stefano Sabatini wrote:
> > ---
> >  ffprobe.c |   86 +++++++++++++++---------------------------------------------
> >  1 files changed, 22 insertions(+), 64 deletions(-)
> > 
> > diff --git a/ffprobe.c b/ffprobe.c
> > index f616086..adc8ec9 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -172,6 +172,7 @@ typedef struct WriterContext WriterContext;
> >  #define WRITER_FLAG_PUT_PACKETS_AND_FRAMES_IN_SAME_CHAPTER 2
> >  
> >  typedef struct Writer {
> > +    const AVClass *priv_class;      ///< private class of the writer, if any
> >      int priv_size;                  ///< private size for the writer context
> >      const char *name;
> >  
> > @@ -230,6 +231,10 @@ static void writer_close(WriterContext **wctx)
> >  
> >      if ((*wctx)->writer->uninit)
> >          (*wctx)->writer->uninit(*wctx);
> > +    if ((*wctx)->writer->priv_class) {
> > +        void *priv_ctx = (*wctx)->priv;
> > +        av_opt_free(priv_ctx);
> > +    }
> 
> Why the intermediate pointer?

Fixed.
 
> >      av_freep(&((*wctx)->priv));
> >      av_freep(wctx);
> >  }
> > @@ -251,13 +256,22 @@ static int writer_open(WriterContext **wctx, const Writer *writer,
> >  
> >      (*wctx)->class = &writer_class;
> >      (*wctx)->writer = writer;
> > +
> > +    if (writer->priv_class) {
> > +        void *priv_ctx = (*wctx)->priv;
> > +        *((const AVClass **)priv_ctx) = writer->priv_class;
> > +        av_opt_set_defaults(priv_ctx);
> > +
> > +        if (args &&
> > +            (ret = (av_set_options_string(priv_ctx, args, "=", ":"))) < 0)
> 
> One level of braces can be removed

Changed.
 
> > +            goto fail;
> > +    }
> >      if ((*wctx)->writer->init)
> >          ret = (*wctx)->writer->init(*wctx, args, opaque);
> >      if (ret < 0)
> >          goto fail;
> >  
> >      return 0;
> > -
> 
> nit: don't you think that separator is kind of cute?

Reverted.
 
> [...]
> 
> Nice simplification overall, and LGTM.
> 
> Will the writers' av_log print "[foobar @ ...]" now?

This patch should not change the behavior, and yes writers already did
that.

Will push it soon.
-- 
FFmpeg = Faithful and Frightening Marvellous Pure Energized Gangster