[FFmpeg-devel] [PATCH v5 04/14] fftools/textformat: Introduce AVTextFormatOptions for avtext_context_open()

[softworkz .]

> -----Original Message-----
> From: Stefano Sabatini <stefasab at gmail.com>
> Sent: Donnerstag, 24. April 2025 00:49
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Cc: softworkz <softworkz at hotmail.com>
> Subject: Re: [FFmpeg-devel] [PATCH v5 04/14] fftools/textformat:
> Introduce AVTextFormatOptions for avtext_context_open()
> 
> On date Tuesday 2025-04-22 21:55:33 +0000, softworkz wrote:
> > From: softworkz <softworkz at hotmail.com>
> >
> > This allows future addition of options without
> > changes to the signature of avtext_context_open().
> >
> > Signed-off-by: softworkz <softworkz at hotmail.com>
> > ---
> >  fftools/ffprobe.c                 | 13 +++++++++----
> >  fftools/textformat/avtextformat.c | 21 ++++++++-------------
> >  fftools/textformat/avtextformat.h | 22 +++++++++++++++-------
> >  3 files changed, 32 insertions(+), 24 deletions(-)
> >
> > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> > index f5c83925b9..1277b1e4f9 100644
> > --- a/fftools/ffprobe.c
> > +++ b/fftools/ffprobe.c
> > @@ -3168,10 +3168,15 @@ int main(int argc, char **argv)
> >      if (ret < 0)
> >          goto end;
> >
> > -    if ((ret = avtext_context_open(&tctx, f, wctx, f_args,
> > -                           sections, FF_ARRAY_ELEMS(sections),
> show_value_unit,
> > -                            use_value_prefix,
> use_byte_value_binary_prefix, use_value_sexagesimal_format,
> > -                            show_optional_fields, show_data_hash))
> >= 0) {
> > +    AVTextFormatOptions tf_options = {
> > +        .show_optional_fields = show_optional_fields,
> > +        .show_value_unit = show_value_unit,
> > +        .use_value_prefix = use_value_prefix,
> > +        .use_byte_value_binary_prefix =
> use_byte_value_binary_prefix,
> > +        .use_value_sexagesimal_format =
> use_value_sexagesimal_format,
> > +    };
> > +
> > +    if ((ret = avtext_context_open(&tctx, f, wctx, f_args,
> sections, FF_ARRAY_ELEMS(sections), tf_options, show_data_hash)) >= 0)
> {
> >          if (f == &avtextformatter_xml)
> >              tctx->string_validation_utf8_flags |=
> AV_UTF8_FLAG_EXCLUDE_XML_INVALID_CONTROL_CODES;
> >
> > diff --git a/fftools/textformat/avtextformat.c
> b/fftools/textformat/avtextformat.c
> > index 0a221f4a9a..217d9da25e 100644
> > --- a/fftools/textformat/avtextformat.c
> > +++ b/fftools/textformat/avtextformat.c
> > @@ -125,13 +125,7 @@ void avtext_context_close(AVTextFormatContext
> **ptctx)
> >
> >
> >  int avtext_context_open(AVTextFormatContext **ptctx, const
> AVTextFormatter *formatter, AVTextWriterContext *writer_context, const
> char *args,
> > -                        const AVTextFormatSection *sections, int
> nb_sections,
> > -                        int show_value_unit,
> > -                        int use_value_prefix,
> > -                        int use_byte_value_binary_prefix,
> > -                        int use_value_sexagesimal_format,
> > -                        int show_optional_fields,
> > -                        char *show_data_hash)
> > +                        const AVTextFormatSection *sections, int
> nb_sections, AVTextFormatOptions options, char *show_data_hash)
> >  {
> >      AVTextFormatContext *tctx;
> >      int i, ret = 0;
> > @@ -155,11 +149,11 @@ int avtext_context_open(AVTextFormatContext
> **ptctx, const AVTextFormatter *form
> >          goto fail;
> >      }
> >
> > -    tctx->show_value_unit = show_value_unit;
> > -    tctx->use_value_prefix = use_value_prefix;
> > -    tctx->use_byte_value_binary_prefix =
> use_byte_value_binary_prefix;
> > -    tctx->use_value_sexagesimal_format =
> use_value_sexagesimal_format;
> > -    tctx->show_optional_fields = show_optional_fields;
> > +    tctx->show_value_unit = options.show_value_unit;
> > +    tctx->use_value_prefix = options.use_value_prefix;
> > +    tctx->use_byte_value_binary_prefix =
> options.use_byte_value_binary_prefix;
> > +    tctx->use_value_sexagesimal_format =
> options.use_value_sexagesimal_format;
> > +    tctx->show_optional_fields = options.show_optional_fields;
> >
> >      if (nb_sections > SECTION_MAX_NB_SECTIONS) {
> >          av_log(tctx, AV_LOG_ERROR, "The number of section
> definitions (%d) is larger than the maximum allowed (%d)\n",
> nb_sections, SECTION_MAX_NB_SECTIONS);
> > @@ -202,7 +196,7 @@ int avtext_context_open(AVTextFormatContext
> **ptctx, const AVTextFormatter *form
> >          av_dict_free(&opts);
> >      }
> >
> > -    if (show_data_hash)
> > +    if (show_data_hash) {
> >          if ((ret = av_hash_alloc(&tctx->hash, show_data_hash)) < 0)
> {
> >              if (ret == AVERROR(EINVAL)) {
> >                  const char *n;
> > @@ -213,6 +207,7 @@ int avtext_context_open(AVTextFormatContext
> **ptctx, const AVTextFormatter *form
> >              }
> >              goto fail;
> >          }
> > +    }
> >
> >      /* validate replace string */
> >      {
> > diff --git a/fftools/textformat/avtextformat.h
> b/fftools/textformat/avtextformat.h
> > index aea691f351..05a358132e 100644
> > --- a/fftools/textformat/avtextformat.h
> > +++ b/fftools/textformat/avtextformat.h
> > @@ -118,17 +118,25 @@ struct AVTextFormatContext {
> >      unsigned int string_validation_utf8_flags;
> >  };
> >
> > +typedef struct AVTextFormatOptions {
> > +    int show_optional_fields;
> > +    int show_value_unit;
> > +    int use_value_prefix;
> > +    int use_byte_value_binary_prefix;
> > +    int use_value_sexagesimal_format;
> > +} AVTextFormatOptions;
> > +
> >  #define AV_TEXTFORMAT_PRINT_STRING_OPTIONAL 1
> >  #define AV_TEXTFORMAT_PRINT_STRING_VALIDATE 2
> >
> > +#define AV_TEXTFORMAT_OPEN_SHOW_VALUE_UNIT               1
> > +#define AV_TEXTFORMAT_OPEN_USE_VALUE_PREFIX              2
> > +#define AV_TEXTFORMAT_OPEN_USE_BYTE_BINARY_PREFIX        4
> > +#define AV_TEXTFORMAT_OPEN_USE_VALUE_SEXAGESIMAL_FORMAT  8
> > +#define AV_TEXTFORMAT_OPEN_SHOW_OPTIONAL_FIELDS         16
> 
> Since this is a bitmap shouldn't we use a flags field directly?

Oops, forgot to remove that. I replaced this by the AVTextFormatOptions
right above....

sw