[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