[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