> -----Original Message-----
> From: Stefano Sabatini <stefa...@gmail.com>
> Sent: Donnerstag, 8. Mai 2025 01:45
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Cc: softworkz <softwo...@hotmail.com>
> Subject: Re: [FFmpeg-devel] [PATCH v10 01/15] fftools/textformat: Formatting
> and whitespace changes
>
> 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.
Hi Stefano,
In message "[PATCH] [RFC] global/clang-format: Add .clang-format configuration
for consistent formatting" I'm trying to work out a common format that matches
the existing code formatting rules.
The patches above are formatted accordingly.
Regarding the first above, FFmpeg rules say:
Don't wrap single-line blocks in braces. Use braces only if there is an
accompanying else statement.
(https://ffmpeg.org/developer.html#Code-formatting-conventions)
Now, I'm not sure whether this exactly means "single line" - clang-format only
supports "single statement/block", so if it means "single line", it might be
difficult to replicate.
Seems, we don't have any rule for ternary operator placement, afaik, the most
common rule is to place them at the beginning of lines, but I surely can revert
it.
Thanks,
sw
_______________________________________________
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".
