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

[Stefano Sabatini <stefasab@gmail.com>]

On date Tuesday 2025-04-22 21:55:33 +0000, softworkz wrote:
> From: softworkz <softworkz@hotmail.com>
> 
> This allows future addition of options without
> changes to the signature of avtext_context_open().
> 
> Signed-off-by: softworkz <softworkz@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?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".