diff mbox series

[FFmpeg-devel] ffprobe: add option to control optional fields display

Message ID 20210501130745.629-1-ffmpeg@gyani.pro
State Accepted
Commit 7c451b609c267462de152895634902f14c3ea60a
Headers show
Series [FFmpeg-devel] ffprobe: add option to control optional fields display
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Gyan Doshi May 1, 2021, 1:07 p.m. UTC
---
 doc/ffprobe.texi  |  6 ++++++
 fftools/ffprobe.c | 23 +++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

Comments

Gyan Doshi May 4, 2021, 9:40 a.m. UTC | #1
Ping; plan to push tomorrow.

On 2021-05-01 18:37, Gyan Doshi wrote:
> ---
>   doc/ffprobe.texi  |  6 ++++++
>   fftools/ffprobe.c | 23 +++++++++++++++++++++--
>   2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> index d7fab4ff40..59a397a225 100644
> --- a/doc/ffprobe.texi
> +++ b/doc/ffprobe.texi
> @@ -335,6 +335,12 @@ Show information about all pixel formats supported by FFmpeg.
>   Pixel format information for each format is printed within a section
>   with name "PIXEL_FORMAT".
>   
> +@item -show_optional_fields @var{value}
> +Some writers viz. JSON and XML, omit the printing of fields with invalid or non-applicable values,
> +while other writers always print them. This option enables one to control this behaviour.
> +Valid values are @code{always}/@code{1}, @code{never}/@code{0} and @code{auto}/@code{-1}.
> +Default is @var{auto}.
> +
>   @item -bitexact
>   Force bitexact output, useful to produce output which is not dependent
>   on the specific build.
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index 7b28f6b3ce..b07032bd88 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -117,6 +117,11 @@ static int use_byte_value_binary_prefix = 0;
>   static int use_value_sexagesimal_format = 0;
>   static int show_private_data            = 1;
>   
> +#define SHOW_OPTIONAL_FIELDS_AUTO       -1
> +#define SHOW_OPTIONAL_FIELDS_NEVER       0
> +#define SHOW_OPTIONAL_FIELDS_ALWAYS      1
> +static int show_optional_fields = SHOW_OPTIONAL_FIELDS_AUTO;
> +
>   static char *print_format;
>   static char *stream_specifier;
>   static char *show_data_hash;
> @@ -745,8 +750,10 @@ static inline int writer_print_string(WriterContext *wctx,
>       const struct section *section = wctx->section[wctx->level];
>       int ret = 0;
>   
> -    if ((flags & PRINT_STRING_OPT)
> -        && !(wctx->writer->flags & WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS))
> +    if (show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER ||
> +        (show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO
> +        && (flags & PRINT_STRING_OPT)
> +        && !(wctx->writer->flags & WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS)))
>           return 0;
>   
>       if (section->show_all_entries || av_dict_get(section->entries_to_show, key, NULL, 0)) {
> @@ -3244,6 +3251,17 @@ static void ffprobe_show_pixel_formats(WriterContext *w)
>       writer_print_section_footer(w);
>   }
>   
> +static int opt_show_optional_fields(void *optctx, const char *opt, const char *arg)
> +{
> +    if      (!av_strcasecmp(arg, "always")) show_optional_fields = SHOW_OPTIONAL_FIELDS_ALWAYS;
> +    else if (!av_strcasecmp(arg, "never"))  show_optional_fields = SHOW_OPTIONAL_FIELDS_NEVER;
> +    else if (!av_strcasecmp(arg, "auto"))   show_optional_fields = SHOW_OPTIONAL_FIELDS_AUTO;
> +
> +    if (show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO && av_strcasecmp(arg, "auto"))
> +        show_optional_fields = parse_number_or_die("show_optional_fields", arg, OPT_INT, SHOW_OPTIONAL_FIELDS_AUTO, SHOW_OPTIONAL_FIELDS_ALWAYS);
> +    return 0;
> +}
> +
>   static int opt_format(void *optctx, const char *opt, const char *arg)
>   {
>       iformat = av_find_input_format(arg);
> @@ -3631,6 +3649,7 @@ static const OptionDef real_options[] = {
>       { "show_library_versions", 0, { .func_arg = &opt_show_library_versions }, "show library versions" },
>       { "show_versions",         0, { .func_arg = &opt_show_versions }, "show program and library versions" },
>       { "show_pixel_formats", 0, { .func_arg = &opt_show_pixel_formats }, "show pixel format descriptions" },
> +    { "show_optional_fields", HAS_ARG, { .func_arg = &opt_show_optional_fields }, "show optional fields" },
>       { "show_private_data", OPT_BOOL, { &show_private_data }, "show private data" },
>       { "private",           OPT_BOOL, { &show_private_data }, "same as show_private_data" },
>       { "bitexact", OPT_BOOL, {&do_bitexact}, "force bitexact output" },
Derek Buitenhuis May 4, 2021, 3:42 p.m. UTC | #2
On 01/05/2021 14:07, Gyan Doshi wrote:
>  
> +@item -show_optional_fields @var{value}
> +Some writers viz. JSON and XML, omit the printing of fields with invalid or non-applicable values,
> +while other writers always print them. This option enables one to control this behaviour.
> +Valid values are @code{always}/@code{1}, @code{never}/@code{0} and @code{auto}/@code{-1}.
> +Default is @var{auto}.

Is the goal here to help badly written JSON or XML parsers?

- Derek
Gyan Doshi May 4, 2021, 4:32 p.m. UTC | #3
On 2021-05-04 21:12, Derek Buitenhuis wrote:
> On 01/05/2021 14:07, Gyan Doshi wrote:
>>   
>> +@item -show_optional_fields @var{value}
>> +Some writers viz. JSON and XML, omit the printing of fields with invalid or non-applicable values,
>> +while other writers always print them. This option enables one to control this behaviour.
>> +Valid values are @code{always}/@code{1}, @code{never}/@code{0} and @code{auto}/@code{-1}.
>> +Default is @var{auto}.
> Is the goal here to help badly written JSON or XML parsers?

No. It's to allow consumers of these writers to discover and access all 
fields if they choose to.

It's an obstacle I faced in a recent project.

Regards,
Gyan
Derek Buitenhuis May 4, 2021, 4:48 p.m. UTC | #4
On 04/05/2021 17:32, Gyan Doshi wrote:
> No. It's to allow consumers of these writers to discover and access all 
> fields if they choose to.
> 
> It's an obstacle I faced in a recent project.

For XML, the correct way to do that is to use the XSD in the doc/ directory.
That's how XML is supposed to work.

For JSON, we could provide a JSON schema maybe, but I agree JSON is kinda crappy
for that.

Not blocking the patch, just thought I'd point out there's a canonical way for XML.

- Derek
Nicolas George May 4, 2021, 4:51 p.m. UTC | #5
Derek Buitenhuis (12021-05-04):
> For XML, the correct way to do that is to use the XSD in the doc/ directory.
> That's how XML is supposed to work.

This part of XML is hugely complicated. Many people prefer to pretend it
does not exist, and I think we should avoid forcing them to use it if
there is a simpler solution.

Regards,
Derek Buitenhuis May 4, 2021, 5:14 p.m. UTC | #6
On 04/05/2021 17:51, Nicolas George wrote:
> This part of XML is hugely complicated. Many people prefer to pretend it
> does not exist, and I think we should avoid forcing them to use it if
> there is a simpler solution.

Yes, presumably this is why nobody uses the XSD :)

- Derek
Gyan Doshi May 5, 2021, 9:37 a.m. UTC | #7
Pushed as 7c451b609c267462de152895634902f14c3ea60a

Regards,
Gyan
diff mbox series

Patch

diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
index d7fab4ff40..59a397a225 100644
--- a/doc/ffprobe.texi
+++ b/doc/ffprobe.texi
@@ -335,6 +335,12 @@  Show information about all pixel formats supported by FFmpeg.
 Pixel format information for each format is printed within a section
 with name "PIXEL_FORMAT".
 
+@item -show_optional_fields @var{value}
+Some writers viz. JSON and XML, omit the printing of fields with invalid or non-applicable values,
+while other writers always print them. This option enables one to control this behaviour.
+Valid values are @code{always}/@code{1}, @code{never}/@code{0} and @code{auto}/@code{-1}.
+Default is @var{auto}.
+
 @item -bitexact
 Force bitexact output, useful to produce output which is not dependent
 on the specific build.
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 7b28f6b3ce..b07032bd88 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -117,6 +117,11 @@  static int use_byte_value_binary_prefix = 0;
 static int use_value_sexagesimal_format = 0;
 static int show_private_data            = 1;
 
+#define SHOW_OPTIONAL_FIELDS_AUTO       -1
+#define SHOW_OPTIONAL_FIELDS_NEVER       0
+#define SHOW_OPTIONAL_FIELDS_ALWAYS      1
+static int show_optional_fields = SHOW_OPTIONAL_FIELDS_AUTO;
+
 static char *print_format;
 static char *stream_specifier;
 static char *show_data_hash;
@@ -745,8 +750,10 @@  static inline int writer_print_string(WriterContext *wctx,
     const struct section *section = wctx->section[wctx->level];
     int ret = 0;
 
-    if ((flags & PRINT_STRING_OPT)
-        && !(wctx->writer->flags & WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS))
+    if (show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER ||
+        (show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO
+        && (flags & PRINT_STRING_OPT)
+        && !(wctx->writer->flags & WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS)))
         return 0;
 
     if (section->show_all_entries || av_dict_get(section->entries_to_show, key, NULL, 0)) {
@@ -3244,6 +3251,17 @@  static void ffprobe_show_pixel_formats(WriterContext *w)
     writer_print_section_footer(w);
 }
 
+static int opt_show_optional_fields(void *optctx, const char *opt, const char *arg)
+{
+    if      (!av_strcasecmp(arg, "always")) show_optional_fields = SHOW_OPTIONAL_FIELDS_ALWAYS;
+    else if (!av_strcasecmp(arg, "never"))  show_optional_fields = SHOW_OPTIONAL_FIELDS_NEVER;
+    else if (!av_strcasecmp(arg, "auto"))   show_optional_fields = SHOW_OPTIONAL_FIELDS_AUTO;
+
+    if (show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO && av_strcasecmp(arg, "auto"))
+        show_optional_fields = parse_number_or_die("show_optional_fields", arg, OPT_INT, SHOW_OPTIONAL_FIELDS_AUTO, SHOW_OPTIONAL_FIELDS_ALWAYS);
+    return 0;
+}
+
 static int opt_format(void *optctx, const char *opt, const char *arg)
 {
     iformat = av_find_input_format(arg);
@@ -3631,6 +3649,7 @@  static const OptionDef real_options[] = {
     { "show_library_versions", 0, { .func_arg = &opt_show_library_versions }, "show library versions" },
     { "show_versions",         0, { .func_arg = &opt_show_versions }, "show program and library versions" },
     { "show_pixel_formats", 0, { .func_arg = &opt_show_pixel_formats }, "show pixel format descriptions" },
+    { "show_optional_fields", HAS_ARG, { .func_arg = &opt_show_optional_fields }, "show optional fields" },
     { "show_private_data", OPT_BOOL, { &show_private_data }, "show private data" },
     { "private",           OPT_BOOL, { &show_private_data }, "same as show_private_data" },
     { "bitexact", OPT_BOOL, {&do_bitexact}, "force bitexact output" },