[FFmpeg-devel] [PATCH v10 01/15] fftools/textformat: Formatting and whitespace changes

[Stefano Sabatini]

On date Sunday 2025-05-04 02:57:12 +0000, softworkz wrote:
> From: softworkz <softworkz at hotmail.com>
> 
> Reviewed-by: Stefano Sabatini <stefasab at gmail.com>
> Signed-off-by: softworkz <softworkz at hotmail.com>
> ---
>  fftools/textformat/avtextformat.c  | 92 +++++++++++++++---------------
>  fftools/textformat/avtextformat.h  | 20 +++----
>  fftools/textformat/avtextwriters.h | 11 ++--
>  fftools/textformat/tf_compact.c    | 91 ++++++++++++++++-------------
>  fftools/textformat/tf_default.c    | 20 +++----
>  fftools/textformat/tf_flat.c       | 26 +++++----
>  fftools/textformat/tf_ini.c        | 36 ++++++------
>  fftools/textformat/tf_json.c       | 10 ++--
>  fftools/textformat/tf_xml.c        | 30 +++++-----
>  9 files changed, 177 insertions(+), 159 deletions(-)
> 
[...]  
> -    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;
> @@ -211,7 +211,6 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form
>              }
>              return ret;
>          }
> -    }

For the record, I'm a bit against these kind of changes since they do
not really add to readability and might led to logical issues in case
an instruction is added to the else block and the parentheses are
discarded - but not so strongly opposed to block the patch though.

[...]
> -void avtext_print_section_header(AVTextFormatContext *tctx,
> -                                               const void *data,
> -                                               int section_id)
> +void avtext_print_section_header(AVTextFormatContext *tctx, const void *data, int section_id)
>  {
>      tctx->level++;
>      av_assert0(tctx->level < SECTION_MAX_NB_LEVELS);
> @@ -272,8 +269,9 @@ void avtext_print_section_header(AVTextFormatContext *tctx,

>  void avtext_print_section_footer(AVTextFormatContext *tctx)
>  {
>      int section_id = tctx->section[tctx->level]->id;
> -    int parent_section_id = tctx->level ?
> -        tctx->section[tctx->level-1]->id : SECTION_ID_NONE;
> +    int parent_section_id = tctx->level
> +        ? tctx->section[tctx->level - 1]->id
> +        : SECTION_ID_NONE;

nit: I prefer the original form (it now looks less readable).

[...]
>          if (section->flags & AV_TEXTFORMAT_SECTION_FLAG_HAS_TYPE) {
>              // add /TYPE to prefix
> @@ -185,30 +189,33 @@ static void compact_print_section_header(AVTextFormatContext *wctx, const void *
>                  char c =
>                      (*p >= '0' && *p <= '9') ||
>                      (*p >= 'a' && *p <= 'z') ||
> -                    (*p >= 'A' && *p <= 'Z') ? av_tolower(*p) : '_';
> +                    (*p >= 'A' && *p <= 'Z')
> +                    ? (char)(char)av_tolower(*p)
> +                    : '_';

Ditto.

[...]

Should be good otherwise.