On date Sunday 2025-05-04 02:57:12 +0000, softworkz wrote:
> From: softworkz <softwo...@hotmail.com>
> 
> Reviewed-by: Stefano Sabatini <stefa...@gmail.com>
> Signed-off-by: softworkz <softwo...@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Reply via email to