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

---
 doc/ffprobe.texi  |  6 ++++++
 fftools/ffprobe.c | 23 +++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

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" },

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

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

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

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,

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

Pushed as 7c451b609c267462de152895634902f14c3ea60a

Regards,
Gyan