On date Wednesday 2025-05-07 23:59:39 +0000, softworkz . wrote:
>
>
> > -----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.
>
So, as I wrote, I don't have strong objections against the brace
removals if you want to go with that.
> 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.
About ternary, I'd drop those changes as they conflict with the style
adopted in the whole FFmpeg codebase.
_______________________________________________
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".
