[FFmpeg-devel] [PATCH 2/3] ffprobe: replace fmt callback with str callback.

Stefano Sabatini stefano.sabatini-lala at poste.it
Fri Sep 9 00:59:58 CEST 2011


On date Thursday 2011-09-08 23:13:28 +0200, Clément Bœsch encoded:
> On Tue, Sep 06, 2011 at 03:31:36PM +0200, Stefano Sabatini wrote:
> [...]
> > >  static void default_print_int(const char *key, int value)
> > > @@ -167,11 +164,15 @@ static void default_print_footer(const char *section)
> > >  
> > >  /* Print helpers */
> > >  
> > > -#define print_fmt0(k, f, a...) w->print_fmt_f(k, f, ##a)
> > > -#define print_fmt( k, f, a...) do {   \
> > > +#define print_fmt0(k, f, a...) do {   \
> > > +    char *strv = av_asprintf(f, ##a); \
> > 
> > > +    w->print_str_f(k, strv);          \
> > > +    av_free(strv);                    \
> > > +} while (0)
> > > +#define print_fmt(k, f, a...) do {    \
> > >      if (w->item_sep)                  \
> > >          printf("%s", w->item_sep);    \
> > > -    w->print_fmt_f(k, f, ##a);        \
> > > +    print_fmt0(k, f, ##a);            \
> > >  } while (0)
> > 
> > Is print_fmt/print_fmt0 still required? (I can't see any use in the
> > code right now.)
> > 
> 
> What I changed is the callback, not the helpers; and yes
> print_fmt/print_fmt0 are still in use since a few options need to be
> printed in a "special" way (such as fraction or 64 bits integers).
> 
> > Also I noticed that ## is a GNU cpp extension, so it may fail with
> > other pre-processors.
> > 
> 
> Oups, fixed. Since the ## is already upstream, I'd like to push this new
> patch ASAP (I'll push it tomorrow if no one object).
> 
> > Also I'm afraid that all this malloc/free may slow down the overall
> > performance.
> > 
> 

> This is a concern; though, I'm not sure adding a fixed buffer length
> limitation is appropriate for the ffprobe tool. While I think there is a
> real problem with tools like ffmpeg, I'm not sure it really is an issue
> here. Do you have something better to propose for variable-length
> printing?

Could you benchmark it? allocing/freeing a buffer for every single
print looks quite expensive.

Alternatively: you pass a buffer which is reallocated if and only when
needed, this would need some custom code.

As for the ## fix please fix it separately (no need for patch, just
commit it).

Thanks.
-- 
FFmpeg = Foolish and Fabulous Miracolous Philosophical Esoteric Gymnast


More information about the ffmpeg-devel mailing list