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

[Clément Bœsch]

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?

-- 
Clément B.
-------------- next part --------------
From 725f83b4e9ce5fec527a36523077fb5272e7d32f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Thu, 8 Sep 2011 23:04:58 +0200
Subject: [PATCH] ffprobe: replace fmt callback with str callback.

Also fix the specific ## GNU cpp extension with __VA_ARGS__.
---
 ffprobe.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/ffprobe.c b/ffprobe.c
index d435ee3..7bbc4f2 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -23,6 +23,7 @@
 
 #include "libavformat/avformat.h"
 #include "libavcodec/avcodec.h"
+#include "libavutil/avstring.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
 #include "libavutil/dict.h"
@@ -137,8 +138,8 @@ struct writer {
     const char *header, *footer;
     void (*print_header)(const char *);
     void (*print_footer)(const char *);
-    void (*print_fmt_f)(const char *, const char *, ...);
     void (*print_int_f)(const char *, int);
+    void (*print_str_f)(const char *, const char *);
     void (*show_tags)(struct writer *w, AVDictionary *dict);
 };
 
@@ -150,13 +151,9 @@ static void default_print_header(const char *section)
     printf("[%s]\n", section);
 }
 
-static void default_print_fmt(const char *key, const char *fmt, ...)
+static void default_print_str(const char *key, const char *value)
 {
-    va_list ap;
-    va_start(ap, fmt);
-    printf("%s=", key);
-    vprintf(fmt, ap);
-    va_end(ap);
+    printf("%s=%s", key, value);
 }
 
 static void default_print_int(const char *key, int value)
@@ -172,11 +169,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, ...) do {            \
+    char *strv = av_asprintf(f, __VA_ARGS__); \
+    w->print_str_f(k, strv);                  \
+    av_free(strv);                            \
+} while (0)
+#define print_fmt(k, f, ...) do {     \
     if (w->item_sep)                  \
         printf("%s", w->item_sep);    \
-    w->print_fmt_f(k, f, ##a);        \
+    print_fmt0(k, f, __VA_ARGS__);    \
 } while (0)
 
 #define print_int0(k, v) w->print_int_f(k, v)
@@ -380,8 +381,8 @@ static int open_input_file(AVFormatContext **fmt_ctx_ptr, const char *filename)
 #define WRITER_FUNC(func)                  \
     .print_header = func ## _print_header, \
     .print_footer = func ## _print_footer, \
-    .print_fmt_f  = func ## _print_fmt,    \
     .print_int_f  = func ## _print_int,    \
+    .print_str_f  = func ## _print_str,    \
     .show_tags    = func ## _show_tags
 
 static struct writer writers[] = {{
-- 
1.7.6.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110908/55c18754/attachment.asc>