[FFmpeg-devel] [PATCH 2/2] ffprobe: add basic JSON print format.

Clément Bœsch ubitux at gmail.com
Fri Sep 2 11:37:15 CEST 2011


On Fri, Sep 02, 2011 at 01:41:44AM +0200, Alexander Strasser wrote:
> Hi Clément,
> 

Hi Alexander,

[...]
> > +#define JSON_ESCAPE "\"\\\b\f\n\r\t"
> > +#define JSON_SUBST  "\"\\bfnr"
> 
>   This looks wrong. If I understood the code below correctly
> than you do a pointer subtraction of 2 pointers with the 1st
> pointing inside the first string literal and the 2nd pointing
> at the start of it. Afterwards you use that offset to index the
> second literal. 
> 

Yes, escape and substitution buffer have to be symmetric.

>   So if we assume a hypothetical memory with
> 
> the addresses  3456789
>                AB..X..
> and the        1st,2nd strings (modified and shortened for demo)
> 
>   then
> 
> strchr returns  pointer to the 1st position:  3 - 3 = 0
> strchf retruns  pointer to the 2nd position:  4 - 3 = 1
> 
>   if we use the result of the last calculation as index
> to the second string we would get the zero terminator
> (ASCII control NUL) which is probably not what we wanted.
> 
>   Making the long comment short I think the second Literal
> misses a `t' character at the end.
> 

I'm not sure I get all what you mean here, but if the issue was just about
the missing 't' character at the end of the substitution buffer, I added
it in the new attached patch.

>   Besides the above, I don't think the C standard guarantees
> that the literals after being substituted by the preprocessor
> actually must be at the same storage location. IIRC at least
> the first C standard would actually demand the opposite behaviour
> as string literals were meant to be writable.
> 

Yeah right, I noticed it yesterday, but didn't changed it assuming it
didn't applied in that case. But of course it does, so patch updated.

> > +
> > +static char *json_escape_str(const char *s)
> > +{
> > +    char *ret, *p;
> > +    int i, len = 0;
> > +
> > +    for (i = 0; s[i]; i++) {
> > +        if (strchr(JSON_ESCAPE, s[i]))     len += 2;
> > +        else if ((unsigned char)s[i] < 32) len += 6;
> > +        else                               len += 1;
> > +    }
> > +
> > +    p = ret = av_malloc(len + 1);
> > +    for (i = 0; s[i]; i++) {
> > +        char *q = strchr(JSON_ESCAPE, s[i]);
> > +        if (q) {
> > +            *p++ = '\\';
> > +            *p++ = JSON_SUBST[q - JSON_ESCAPE];
> > +        } else if ((unsigned char)s[i] < 32) {
> > +            snprintf(p, 7, "\\u00%02x", s[i] & 0xff);
> > +            p += 6;
> > +        } else {
> > +            *p++ = s[i];
> > +        }
> > +    }
> > +    *p = 0;
> > +    return ret;
> > +}
> 
>   I don't really like it, but it is at least short. Maybe someone
> comes up with a simpler idea. Unfortunately I don't have any ideas
> right now. This is really not meant to discourage you, it is just my
> personal feeling when I read the code. I think it is a bit fragile
> and may be expressed more robust/safely.
> 

Feel free to propose something else :)

> > +
> > +static void json_print_fmt(const char *key, const char *fmt, ...)
> > +{
> > +    int len;
> > +    char *raw_s = NULL, *escaped_s = NULL;
> > +    va_list ap;
> > +
> > +    va_start(ap, fmt);
> > +    len = vsnprintf(NULL, 0, fmt, ap);
> > +    va_end(ap);
> > +    if (len < 0)
> > +        goto end;
> > +
> > +    raw_s = av_malloc(len + 1);
> > +    if (!raw_s)
> > +        goto end;
> > +
> > +    va_start(ap, fmt);
> > +    len = vsnprintf(raw_s, len + 1, fmt, ap);
> > +    va_end(ap);
> > +    if (len < 0)
> > +        goto end;
> > +
> > +    escaped_s = json_escape_str(raw_s);
> > +
> > +end:
> > +    printf("    \"%s\": \"%s\"", key, escaped_s ? escaped_s : "");
> > +    av_free(raw_s);
> > +    av_free(escaped_s);
> > +}
> > +
> > +static void json_print_int(const char *key, int value)
> > +{
> > +    printf("    \"%s\": %d", key, value);
> > +}
> 
>   The key params of those to routines are used as the name of the
> object members. Those are defined as strings by the JSON RFC and
> should IMHO get escaped as well.
> 

Those keys are controlled, but escape added anyway.

> > +
> > +static void json_print_footer(const char *section)
> > +{
> > +    printf("\n  }");
> > +}
> > +
> > +static void json_print_section_start(const char *section, int multiple_entries)
> > +{
> > +    printf("\n  \"%s\":%s", section, multiple_entries ? " [" : " ");
> > +}
> 
>   If I didn't get confused this is also the name of an object member and
> therefore escaping should apply here too.
> 

Done. I also added the escape in the fmt printing for the key.

>   I understand the contents for some cases for which I demand escaping may
> be more tightly under control by the developers, but applying escaping
> consequently makes it less easy for developers to break things while making
> changes in the future. A future change may be motivated by something entirely
> different and it would be easy for the developer to forget to check all output
> writer implementations.
> 

Sure.

>   Also I am a lazy developer, that didn't yet check if one of the object
> member names could be fed with content not under our control. So if it
> gets decided against escaping the object member names, at least this
> should be double checked.
> 

I agree.

New patch attached.

-- 
Clément B.
-------------- next part --------------
From c4c1c8da165e88cd61cb4d607d6ae24d04700115 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Sat, 27 Aug 2011 20:19:44 +0200
Subject: [PATCH] ffprobe: add basic JSON print format.

---
 ffprobe.c |  144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 140 insertions(+), 4 deletions(-)

diff --git a/ffprobe.c b/ffprobe.c
index 2748224..6396842 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -130,6 +130,8 @@ struct writer {
     const char *items_sep;          ///< separator between sets of key/value couples
     const char *section_sep;        ///< separator between sections (streams, packets, ...)
     const char *header, *footer;
+    void (*print_section_start)(const char *, int);
+    void (*print_section_end)  (const char *, int);
     void (*print_header)(const char *);
     void (*print_footer)(const char *);
     void (*print_fmt_f)(const char *, const char *, ...);
@@ -138,6 +140,107 @@ struct writer {
 };
 
 
+/* JSON output */
+
+static void json_print_header(const char *section)
+{
+    printf("{\n");
+}
+
+static const char *json_escape = "\"\\\b\f\n\r\t";
+static const char *json_subst  = "\"\\bfnrt";
+
+static char *json_escape_str(const char *s)
+{
+    char *ret, *p;
+    int i, len = 0;
+
+    for (i = 0; s[i]; i++) {
+        if (strchr(json_escape, s[i]))     len += 2;
+        else if ((unsigned char)s[i] < 32) len += 6;
+        else                               len += 1;
+    }
+
+    p = ret = av_malloc(len + 1);
+    if (!p)
+        return NULL;
+    for (i = 0; s[i]; i++) {
+        char *q = strchr(json_escape, s[i]);
+        if (q) {
+            *p++ = '\\';
+            *p++ = json_subst[q - json_escape];
+        } else if ((unsigned char)s[i] < 32) {
+            snprintf(p, 7, "\\u00%02x", s[i] & 0xff);
+            p += 6;
+        } else {
+            *p++ = s[i];
+        }
+    }
+    *p = 0;
+    return ret;
+}
+
+static void json_print_fmt(const char *key, const char *fmt, ...)
+{
+    int len;
+    char *raw_str = NULL, *value_esc = NULL, *key_esc;
+    va_list ap;
+
+    va_start(ap, fmt);
+    len = vsnprintf(NULL, 0, fmt, ap);
+    va_end(ap);
+    if (len < 0)
+        goto end;
+
+    raw_str = av_malloc(len + 1);
+    if (!raw_str)
+        goto end;
+
+    va_start(ap, fmt);
+    len = vsnprintf(raw_str, len + 1, fmt, ap);
+    va_end(ap);
+    if (len < 0)
+        goto end;
+
+    value_esc = json_escape_str(raw_str);
+
+end:
+    key_esc = json_escape_str(key);
+    printf("    \"%s\": \"%s\"",
+           key_esc   ? key_esc   : "",
+           value_esc ? value_esc : "");
+    av_free(raw_str);
+    av_free(key_esc);
+    av_free(value_esc);
+}
+
+static void json_print_int(const char *key, int value)
+{
+    char *key_esc = json_escape_str(key);
+    printf("    \"%s\": %d", key_esc ? key_esc : "", value);
+    av_free(key_esc);
+}
+
+static void json_print_footer(const char *section)
+{
+    printf("\n  }");
+}
+
+static void json_print_section_start(const char *section, int multiple_entries)
+{
+    char *section_esc = json_escape_str(section);
+    printf("\n  \"%s\":%s", section_esc ? section_esc : "",
+           multiple_entries ? " [" : " ");
+    av_free(section_esc);
+}
+
+static void json_print_section_end(const char *section, int multiple_entries)
+{
+    if (multiple_entries)
+        printf("]");
+}
+
+
 /* Default output */
 
 static void default_print_header(const char *section)
@@ -219,6 +322,24 @@ static void show_packets(struct writer *w, AVFormatContext *fmt_ctx)
         show_packet(w, fmt_ctx, &pkt, i++);
 }
 
+static void json_show_tags(struct writer *w, AVDictionary *dict)
+{
+    AVDictionaryEntry *tag = NULL;
+    int first = 1;
+    if (!dict)
+        return;
+    printf(",\n    \"tags\": {\n");
+    while ((tag = av_dict_get(dict, "", tag, AV_DICT_IGNORE_SUFFIX))) {
+        if (first) {
+            print_str0(tag->key, tag->value);
+            first = 0;
+        } else {
+            print_str(tag->key, tag->value);
+        }
+    }
+    printf("\n    }");
+}
+
 static void default_show_tags(struct writer *w, AVDictionary *dict)
 {
     AVDictionaryEntry *tag = NULL;
@@ -386,6 +507,16 @@ static struct writer writers[] = {{
         .section_sep  = "\n",
         .footer       = "\n",
         WRITER_FUNC(default),
+    },{
+        .name         = "json",
+        .header       = "{",
+        .item_sep     = ",\n",
+        .items_sep    = ",",
+        .section_sep  = ",",
+        .footer       = "\n}\n",
+        .print_section_start = json_print_section_start,
+        .print_section_end   = json_print_section_end,
+        WRITER_FUNC(json),
     }
 };
 
@@ -400,9 +531,13 @@ static int get_writer(const char *name)
     return -1;
 }
 
-#define SECTION_PRINT(name, left) do {                        \
+#define SECTION_PRINT(name, multiple_entries, left) do {      \
     if (do_show_ ## name) {                                   \
+        if (w->print_section_start)                           \
+            w->print_section_start(#name, multiple_entries);  \
         show_ ## name (w, fmt_ctx);                           \
+        if (w->print_section_end)                             \
+            w->print_section_end(#name, multiple_entries);    \
         if (left)                                             \
             printf("%s", w->section_sep);                     \
     }                                                         \
@@ -427,9 +562,9 @@ static int probe_file(const char *filename)
     if (w->header)
         printf("%s", w->header);
 
-    SECTION_PRINT(packets, do_show_streams || do_show_format);
-    SECTION_PRINT(streams, do_show_format);
-    SECTION_PRINT(format,  0);
+    SECTION_PRINT(packets, 1, do_show_streams || do_show_format);
+    SECTION_PRINT(streams, 1, do_show_format);
+    SECTION_PRINT(format,  0, 0);
 
     if (w->footer)
         printf("%s", w->footer);
@@ -499,6 +634,7 @@ static const OptionDef options[] = {
       "use sexagesimal format HOURS:MM:SS.MICROSECONDS for time units" },
     { "pretty", 0, {(void*)&opt_pretty},
       "prettify the format of displayed values, make it more human readable" },
+    { "print_format", OPT_STRING | HAS_ARG, {(void*)&print_format}, "Set the output format used. Available formats: default, json" },
     { "show_format",  OPT_BOOL, {(void*)&do_show_format} , "show format/container info" },
     { "show_packets", OPT_BOOL, {(void*)&do_show_packets}, "show packets info" },
     { "show_streams", OPT_BOOL, {(void*)&do_show_streams}, "show streams info" },
-- 
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/20110902/8851f965/attachment.asc>


More information about the ffmpeg-devel mailing list