[FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 doc/developer.texi | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/doc/developer.texi b/doc/developer.texi
index a1bfe180c9b..6a753f99da6 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
 @end example
 
 @item
-No braces around single-line blocks:
+No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
 
 @example c, good
 // Good
-if (bits_pixel == 24)
+if (bits_pixel == 24) @{
     avctx->pix_fmt = AV_PIX_FMT_BGR24;
-else if (bits_pixel == 8)
+@} else if (bits_pixel == 8) @{
     avctx->pix_fmt = AV_PIX_FMT_GRAY8;
-else @{
-    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
+@} else
     return AVERROR_INVALIDDATA;
-@}
+
 @end example
 
 @item
-- 
2.48.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[epirat07]

On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:

> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  doc/developer.texi | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index a1bfe180c9b..6a753f99da6 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>  @end example
>
>  @item
> -No braces around single-line blocks:
> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
>

I agree with this, however people in the past pointed it out when new code did this, IIRC.

So if we merge this, people should stop flagging this in reviews, else its frustrating
for new contributors following the documentation and then getting contradicting reviews.

>  @example c, good
>  // Good
> -if (bits_pixel == 24)
> +if (bits_pixel == 24) @{
>      avctx->pix_fmt = AV_PIX_FMT_BGR24;
> -else if (bits_pixel == 8)
> +@} else if (bits_pixel == 8) @{
>      avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> -else @{
> -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> +@} else
>      return AVERROR_INVALIDDATA;
> -@}
> +
>  @end example
>
>  @item
> -- 
> 2.48.1
>
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Vittorio Giovara]

On Thu, Feb 27, 2025 at 11:47 PM <epirat07@gmail.com> wrote:

> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
>
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  doc/developer.texi | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a1bfe180c9b..6a753f99da6 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >  @end example
> >
> >  @item
> > -No braces around single-line blocks:
> > +No braces around single-line blocks, unless they are followed by an
> else (to keep future patches cleaner)
> >
>
> I agree with this, however people in the past pointed it out when new code
> did this, IIRC.
>
> So if we merge this, people should stop flagging this in reviews, else its
> frustrating
> for new contributors following the documentation and then getting
> contradicting reviews.


would loooove a format defining script with a prehook that formats your
patches before sending <3
-- 
Vittorio
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 1326 bytes --]

On 2/27/2025 7:53 PM, Vittorio Giovara wrote:
> On Thu, Feb 27, 2025 at 11:47 PM <epirat07@gmail.com> wrote:
> 
>> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
>>
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>   doc/developer.texi | 11 +++++------
>>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/doc/developer.texi b/doc/developer.texi
>>> index a1bfe180c9b..6a753f99da6 100644
>>> --- a/doc/developer.texi
>>> +++ b/doc/developer.texi
>>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>>>   @end example
>>>
>>>   @item
>>> -No braces around single-line blocks:
>>> +No braces around single-line blocks, unless they are followed by an
>> else (to keep future patches cleaner)
>>>
>>
>> I agree with this, however people in the past pointed it out when new code
>> did this, IIRC.
>>
>> So if we merge this, people should stop flagging this in reviews, else its
>> frustrating
>> for new contributors following the documentation and then getting
>> contradicting reviews.
> 
> 
> would loooove a format defining script with a prehook that formats your
> patches before sending <3

That can be done automatically as one of the many jobs CI runs once we 
move to forgejo/gitlab. Same with every other check in patcheck.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Devin Heitmueller]

On Thu, Feb 27, 2025 at 5:57 PM James Almer <jamrial@gmail.com> wrote:
> > would loooove a format defining script with a prehook that formats your
> > patches before sending <3
>
> That can be done automatically as one of the many jobs CI runs once we
> move to forgejo/gitlab. Same with every other check in patcheck.

In my opinion, developers generally want to clean up style issues
prior to submission.  The problem with doing it in a CI job is that it
will just tell you after the fact that your patch didn't meet the
coding standards and you have to revise and resubmit.  The alternative
is for the CI job to "fix" the patch to conform, which people
generally don't want because what they submitted is different than
what gets committed.

For what it's worth, with the Linux Kernel we solved this years ago
with a "checkpatch.pl" which developers can run prior to committing.
Thus I could quickly run checkpatch.pl, fix anything it complains
about, and then run git commit and be confident that it conforms to
the standard.  Personally I prefer this approach as it lets me fix the
patches prior to submission, and the CI system isn't changing things
without my knowledge.

That said, I'm not necessarily against a CI job which validates it
meets the coding standards and kicks it back, to catch cases where the
developer failed to run checkpatch prior to submission.  This approach
only really works well though if the developer had an automated means
to run the check his/herself prior to submission.

Devin

-- 
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1626 bytes --]

On Thu, Feb 27, 2025 at 07:40:14PM -0500, Devin Heitmueller wrote:
> On Thu, Feb 27, 2025 at 5:57 PM James Almer <jamrial@gmail.com> wrote:
> > > would loooove a format defining script with a prehook that formats your
> > > patches before sending <3
> >
> > That can be done automatically as one of the many jobs CI runs once we
> > move to forgejo/gitlab. Same with every other check in patcheck.
> 
> In my opinion, developers generally want to clean up style issues
> prior to submission.  The problem with doing it in a CI job is that it
> will just tell you after the fact that your patch didn't meet the
> coding standards and you have to revise and resubmit.  The alternative
> is for the CI job to "fix" the patch to conform, which people
> generally don't want because what they submitted is different than
> what gets committed.
> 

> For what it's worth, with the Linux Kernel we solved this years ago
> with a "checkpatch.pl" which developers can run prior to committing.
> Thus I could quickly run checkpatch.pl, fix anything it complains
> about, and then run git commit and be confident that it conforms to
> the standard.  Personally I prefer this approach as it lets me fix the
> patches prior to submission, and the CI system isn't changing things
> without my knowledge.

we have tools/patcheck :)
I have the feeling we started forgeting about it

And I also think changing submissions in CI is problematic.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Nicolas George]

Michael Niedermayer (HE12025-02-28):
> we have tools/patcheck :)
> I have the feeling we started forgeting about it

You cannot click on it. Therefore, the coders who are somehow capable of
producing C code at quality level for FFmpeg but unable to use a command
line and unable to learn will not be able to use it and we should
definitely not cut ourselves from that promising contributor base.

Regards,

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Marvin S.]

On 28 Feb 2025, at 8:53, Nicolas George wrote:

> Michael Niedermayer (HE12025-02-28):
>> we have tools/patcheck :)
>> I have the feeling we started forgeting about it
>
> You cannot click on it. Therefore, the coders who are somehow capable of
> producing C code at quality level for FFmpeg but unable to use a command
> line and unable to learn will not be able to use it and we should
> definitely not cut ourselves from that promising contributor base.
>

I don't see how your snarky remarks here add anything valuable to the
discussion, please refrain from sending such pointless emails.

Thanks,
Marvin Scholz

> Regards,
>
> -- 
>   Nicolas George
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Devin Heitmueller]

On Thu, Feb 27, 2025 at 7:59 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
> we have tools/patcheck :)
> I have the feeling we started forgeting about it

I've been working on the codebase for more than eight years, and
didn't know it existed.  I would suggest that people reviewing patches
and rejecting them due to style issues that they recommend to
submitters to run the tool.  In my experience, developers don't want
to have their patches rejected due to codingstyle issues, and such a
tool will help avoid such submissions in the first place, but only if
those developers know the tool exists.

Cheers,

Devin
-- 
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Nicolas George]

Devin Heitmueller (HE12025-02-28):
> I've been working on the codebase for more than eight years, and
> didn't know it existed.

# @anchor{Submitting patches}
# …
# Use the patcheck tool of FFmpeg to check your patch.

Fourth point in doc/developer.texi.

Regards,

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[martin schitter]

On 2/28/25 15:07, Devin Heitmueller wrote:
> I've been working on the codebase for more than eight years, and
> didn't know it existed.  I would suggest that people reviewing patches
> and rejecting them due to style issues that they recommend to
> submitters to run the tool.  In my experience, developers don't want
> to have their patches rejected due to codingstyle issues, and such a
> tool will help avoid such submissions in the first place, but only if
> those developers know the tool exists.

This kind of checks should better happen in some kind of CI based 
pipeline supporting direct feedback to the contributors before accepting 
the patches for any further human review by maintainers or on public 
mailing lists.

That's why I really like GitLab and similar tools.

And concerning bracket handling in particular:
As long as languages do not simply enforce only one preferred coding 
style (like for example in rust and deno) you'll never find a solution, 
which makes everyone happy.

martin
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Nicolas George]

martin schitter (HE12025-02-28):
> This kind of checks should better happen in some kind of CI based pipeline
> supporting direct feedback to the contributors before accepting the patches
> for any further human review by maintainers or on public mailing lists.
> 
> That's why I really like GitLab and similar tools.

Developing in 2025: the project uses a monolithic tool, everything is in
this tool, everybody has to use it, whether they like it or not.

Developing in the early 2000s when FFmpeg grew: low-level tools are
provided, every developer is free to integrate them in their favored
working process, and indeed expected to do so.

Developers from 2025 when confronted with a project from the early
2000s: “What's a script? Help!! What's a keyboard?”
[insert here a picture of a hen who found a knife]

> As long as languages do not simply enforce only one preferred coding style
> (like for example in rust and deno) you'll never find a solution, which
> makes everyone happy.

Hard rules on cosmetic matters are a terrible idea, whether they are
enforced at the level of the language or downstream from it.

Regards,

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Vittorio Giovara]

On Fri, Feb 28, 2025 at 4:09 PM Nicolas George <george@nsup.org> wrote:

> Hard rules on cosmetic matters are a terrible idea, whether they are
> enforced at the level of the language or downstream from it.


agree to disagree :)
-- 
Vittorio
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Devin
> Heitmueller
> Sent: Freitag, 28. Februar 2025 01:40
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule
> 
> On Thu, Feb 27, 2025 at 5:57 PM James Almer <jamrial@gmail.com> wrote:
> > > would loooove a format defining script with a prehook that formats
> your
> > > patches before sending <3
> >
> > That can be done automatically as one of the many jobs CI runs once we
> > move to forgejo/gitlab. Same with every other check in patcheck.
> 
> In my opinion, developers generally want to clean up style issues
> prior to submission.  The problem with doing it in a CI job is that it
> will just tell you after the fact that your patch didn't meet the
> coding standards and you have to revise and resubmit.  The alternative
> is for the CI job to "fix" the patch to conform, which people
> generally don't want because what they submitted is different than
> what gets committed.
> 
> For what it's worth, with the Linux Kernel we solved this years ago
> with a "checkpatch.pl" which developers can run prior to committing.
> Thus I could quickly run checkpatch.pl, fix anything it complains
> about, and then run git commit and be confident that it conforms to
> the standard.  Personally I prefer this approach as it lets me fix the
> patches prior to submission, and the CI system isn't changing things
> without my knowledge.
> 
> That said, I'm not necessarily against a CI job which validates it
> meets the coding standards and kicks it back, to catch cases where the
> developer failed to run checkpatch prior to submission.  This approach
> only really works well though if the developer had an automated means
> to run the check his/herself prior to submission.

Hi Devin,

I see it the same like you do, but even a bit more extreme: For me it must happen while writing. I don't leave anything improperly formatted excepting the current spot. Deferring formatting just causes trouble all the way in combination with version control. I don’t know how others are working, but I'm often making small commits which are pieced together later or commits with something that needs to go into a previous commit. Or sometimes commits need to be split and combined in a different way. When you are doing this this with a "format later" or "format sometimes only" approach, a mix of formatted/unformatted code lines will bite you all the time when rebasing/merging things around.
So, I prefer tooling that shows warnings and assists in formatting while typing. 
I have a manually adjusted profile, but it's just an approximate solution - it could be better.

What I had pitched a few years ago is the creation of a ruleset for a widely supported linter like Clang-tidy or similar which resembles the ffmpeg coding rules in detail. Then, everybody could use it in the way they prefer - be it "live" in a full-featured IDE or just by running manually.

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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1400 bytes --]

Hi

On Thu, Feb 27, 2025 at 11:46:54PM +0100, epirat07@gmail.com wrote:
> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
> 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  doc/developer.texi | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a1bfe180c9b..6a753f99da6 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >  @end example
> >
> >  @item
> > -No braces around single-line blocks:
> > +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
> >
> 
> I agree with this, however people in the past pointed it out when new code did this, IIRC.

yes, i know, ive seen it in both directions for this one.


> 
> So if we merge this, people should stop flagging this in reviews, else its frustrating
> for new contributors following the documentation and then getting contradicting reviews.

I think as long as its in a style guide, the author of a patch can easily
point the reviewer to the guide and that should resolve it immedeatly

will apply

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[epirat07]

On 28 Feb 2025, at 0:14, Michael Niedermayer wrote:

> Hi
>
> On Thu, Feb 27, 2025 at 11:46:54PM +0100, epirat07@gmail.com wrote:
>> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
>>
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>  doc/developer.texi | 11 +++++------
>>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/doc/developer.texi b/doc/developer.texi
>>> index a1bfe180c9b..6a753f99da6 100644
>>> --- a/doc/developer.texi
>>> +++ b/doc/developer.texi
>>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>>>  @end example
>>>
>>>  @item
>>> -No braces around single-line blocks:
>>> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
>>>
>>
>> I agree with this, however people in the past pointed it out when new code did this, IIRC.
>
> yes, i know, ive seen it in both directions for this one.
>
>
>>
>> So if we merge this, people should stop flagging this in reviews, else its frustrating
>> for new contributors following the documentation and then getting contradicting reviews.
>
> I think as long as its in a style guide, the author of a patch can easily
> point the reviewer to the guide and that should resolve it immedeatly
>
> will apply
>

Sounds good.

Can you maybe check the styling issue at some point?
I had updated the CSS and locally I get the right classes assigned to the
codeblocks and the CSS styling them to hint which of them are good and which
bad examples.

But on the website deployed version its not working.

Thanks,
Marvin Scholz

> thx
>
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2665 bytes --]

On Fri, Feb 28, 2025 at 12:25:43AM +0100, epirat07@gmail.com wrote:
> 
> 
> On 28 Feb 2025, at 0:14, Michael Niedermayer wrote:
> 
> > Hi
> >
> > On Thu, Feb 27, 2025 at 11:46:54PM +0100, epirat07@gmail.com wrote:
> >> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
> >>
> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >>> ---
> >>>  doc/developer.texi | 11 +++++------
> >>>  1 file changed, 5 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/doc/developer.texi b/doc/developer.texi
> >>> index a1bfe180c9b..6a753f99da6 100644
> >>> --- a/doc/developer.texi
> >>> +++ b/doc/developer.texi
> >>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >>>  @end example
> >>>
> >>>  @item
> >>> -No braces around single-line blocks:
> >>> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
> >>>
> >>
> >> I agree with this, however people in the past pointed it out when new code did this, IIRC.
> >
> > yes, i know, ive seen it in both directions for this one.
> >
> >
> >>
> >> So if we merge this, people should stop flagging this in reviews, else its frustrating
> >> for new contributors following the documentation and then getting contradicting reviews.
> >
> > I think as long as its in a style guide, the author of a patch can easily
> > point the reviewer to the guide and that should resolve it immedeatly
> >
> > will apply
> >
> 
> Sounds good.
> 
> Can you maybe check the styling issue at some point?
> I had updated the CSS and locally I get the right classes assigned to the
> codeblocks and the CSS styling them to hint which of them are good and which
> bad examples.
> 
> But on the website deployed version its not working.

there seem to be 2 style.min.css
one in ffmpeg and one generated by Makefile in ffmpeg-web
but only one on the website, this may be slightly suboptimal

ive simply copied the one from ffmpeg over on the server now, backup in
style.min.css.bak2 if it breaks something and timo wants to undo

also that change, did change the look of the frontpage, we have a bigger
download button now and some empty space
between entries in "October 30th, 2016, Results: Summer Of Code 2016."

some html/css person, should look at this i think.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Lynne]

On 27/02/2025 02:10, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   doc/developer.texi | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/developer.texi b/doc/developer.texi
> index a1bfe180c9b..6a753f99da6 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>   @end example
>   
>   @item
> -No braces around single-line blocks:
> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
>   
>   @example c, good
>   // Good
> -if (bits_pixel == 24)
> +if (bits_pixel == 24) @{
>       avctx->pix_fmt = AV_PIX_FMT_BGR24;
> -else if (bits_pixel == 8)
> +@} else if (bits_pixel == 8) @{
>       avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> -else @{
> -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> +@} else
>       return AVERROR_INVALIDDATA;
> -@}
> +
>   @end example
>   
>   @item

Strongly objecting to this patch. It's all over our codebase, and I like 
it. Literally everywhere you look.
If future patches need to add more under the branch, then it only needs 
to be done once.

Such fundamental changes need more than a single patch hastily accepted, 
and should involve the community as a whole, I believe.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2097 bytes --]

On Fri, Feb 28, 2025 at 03:25:06AM +0100, Lynne wrote:
> On 27/02/2025 02:10, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   doc/developer.texi | 11 +++++------
> >   1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a1bfe180c9b..6a753f99da6 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >   @end example
> >   @item
> > -No braces around single-line blocks:
> > +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
> >   @example c, good
> >   // Good
> > -if (bits_pixel == 24)
> > +if (bits_pixel == 24) @{
> >       avctx->pix_fmt = AV_PIX_FMT_BGR24;
> > -else if (bits_pixel == 8)
> > +@} else if (bits_pixel == 8) @{
> >       avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> > -else @{
> > -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> > +@} else
> >       return AVERROR_INVALIDDATA;
> > -@}
> > +
> >   @end example
> >   @item
> 
> Strongly objecting to this patch. It's all over our codebase, and I like it.
> Literally everywhere you look.
> If future patches need to add more under the branch, then it only needs to
> be done once.
> 
> Such fundamental changes need more than a single patch hastily accepted, and
> should involve the community as a whole, I believe.

Its not a change, look at tools/patcheck

or try:

+    if(this)
+        that
+    else
+        nothat
+

patcheck reports:

missing } prior to else
patcheck.stdout:11:+    else

missing whitespace between keyword and ( (feel free to ignore)
patcheck.stdout:9:+    if(this)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If one takes all money from those who grow wealth and gives it to those who
do not grow wealth, 10 years later, almost the same people who where wealthy
will be wealthy again, the same people who where poor will be poor again.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Lynne]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 2294 bytes --]



On 28/02/2025 03:33, Michael Niedermayer wrote:
> On Fri, Feb 28, 2025 at 03:25:06AM +0100, Lynne wrote:
>> On 27/02/2025 02:10, Michael Niedermayer wrote:
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>    doc/developer.texi | 11 +++++------
>>>    1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/doc/developer.texi b/doc/developer.texi
>>> index a1bfe180c9b..6a753f99da6 100644
>>> --- a/doc/developer.texi
>>> +++ b/doc/developer.texi
>>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>>>    @end example
>>>    @item
>>> -No braces around single-line blocks:
>>> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
>>>    @example c, good
>>>    // Good
>>> -if (bits_pixel == 24)
>>> +if (bits_pixel == 24) @{
>>>        avctx->pix_fmt = AV_PIX_FMT_BGR24;
>>> -else if (bits_pixel == 8)
>>> +@} else if (bits_pixel == 8) @{
>>>        avctx->pix_fmt = AV_PIX_FMT_GRAY8;
>>> -else @{
>>> -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
>>> +@} else
>>>        return AVERROR_INVALIDDATA;
>>> -@}
>>> +
>>>    @end example
>>>    @item
>>
>> Strongly objecting to this patch. It's all over our codebase, and I like it.
>> Literally everywhere you look.
>> If future patches need to add more under the branch, then it only needs to
>> be done once.
>>
>> Such fundamental changes need more than a single patch hastily accepted, and
>> should involve the community as a whole, I believe.
> 
> Its not a change, look at tools/patcheck
> 
> or try:
> 
> +    if(this)
> +        that
> +    else
> +        nothat
> +
> 
> patcheck reports:
> 
> missing } prior to else
> patcheck.stdout:11:+    else
> 
> missing whitespace between keyword and ( (feel free to ignore)
> patcheck.stdout:9:+    if(this)

Ah, okay. I didn't read properly. The double negative in the sentence 
makes it hard to understand.

Rather than
 > No braces around single-line blocks, unless they are followed by an 
else (to keep future patches cleaner)

Consider
 > Don't wrap single-line blocks in braces. Use braces only if there is 
an accompanying else statement. This keeps future code changes easier to 
keep track of.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Vittorio Giovara]

On Fri, Feb 28, 2025 at 1:25 PM Lynne <dev@lynne.ee> wrote:

>
>
> On 28/02/2025 03:33, Michael Niedermayer wrote:
> > On Fri, Feb 28, 2025 at 03:25:06AM +0100, Lynne wrote:
> >> On 27/02/2025 02:10, Michael Niedermayer wrote:
> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >>> ---
> >>>    doc/developer.texi | 11 +++++------
> >>>    1 file changed, 5 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/doc/developer.texi b/doc/developer.texi
> >>> index a1bfe180c9b..6a753f99da6 100644
> >>> --- a/doc/developer.texi
> >>> +++ b/doc/developer.texi
> >>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >>>    @end example
> >>>    @item
> >>> -No braces around single-line blocks:
> >>> +No braces around single-line blocks, unless they are followed by an
> else (to keep future patches cleaner)
> >>>    @example c, good
> >>>    // Good
> >>> -if (bits_pixel == 24)
> >>> +if (bits_pixel == 24) @{
> >>>        avctx->pix_fmt = AV_PIX_FMT_BGR24;
> >>> -else if (bits_pixel == 8)
> >>> +@} else if (bits_pixel == 8) @{
> >>>        avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> >>> -else @{
> >>> -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> >>> +@} else
> >>>        return AVERROR_INVALIDDATA;
> >>> -@}
> >>> +
> >>>    @end example
> >>>    @item
> >>
> >> Strongly objecting to this patch. It's all over our codebase, and I
> like it.
> >> Literally everywhere you look.
> >> If future patches need to add more under the branch, then it only needs
> to
> >> be done once.
> >>
> >> Such fundamental changes need more than a single patch hastily
> accepted, and
> >> should involve the community as a whole, I believe.
> >
> > Its not a change, look at tools/patcheck
> >
> > or try:
> >
> > +    if(this)
> > +        that
> > +    else
> > +        nothat
> > +
> >
> > patcheck reports:
> >
> > missing } prior to else
> > patcheck.stdout:11:+    else
> >
> > missing whitespace between keyword and ( (feel free to ignore)
> > patcheck.stdout:9:+    if(this)
>
> Ah, okay. I didn't read properly. The double negative in the sentence
> makes it hard to understand.
>
> Rather than
>  > No braces around single-line blocks, unless they are followed by an
> else (to keep future patches cleaner)
>
> Consider
>  > Don't wrap single-line blocks in braces. Use braces only if there is
> an accompanying else statement. This keeps future code changes easier to
> keep track of.


I'm not sure it's a good idea, personally I've gotten used to go enforcing
brackets all the time and it definitely feels less error-prone
I think having a formatting rule that implements this, even for a C
codebase, will be beneficial in the long run
-- 
Vittorio
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 3111 bytes --]

Hi Lynne

On Fri, Feb 28, 2025 at 01:24:46PM +0100, Lynne wrote:
> 
> 
> On 28/02/2025 03:33, Michael Niedermayer wrote:
> > On Fri, Feb 28, 2025 at 03:25:06AM +0100, Lynne wrote:
> > > On 27/02/2025 02:10, Michael Niedermayer wrote:
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >    doc/developer.texi | 11 +++++------
> > > >    1 file changed, 5 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/doc/developer.texi b/doc/developer.texi
> > > > index a1bfe180c9b..6a753f99da6 100644
> > > > --- a/doc/developer.texi
> > > > +++ b/doc/developer.texi
> > > > @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> > > >    @end example
> > > >    @item
> > > > -No braces around single-line blocks:
> > > > +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
> > > >    @example c, good
> > > >    // Good
> > > > -if (bits_pixel == 24)
> > > > +if (bits_pixel == 24) @{
> > > >        avctx->pix_fmt = AV_PIX_FMT_BGR24;
> > > > -else if (bits_pixel == 8)
> > > > +@} else if (bits_pixel == 8) @{
> > > >        avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> > > > -else @{
> > > > -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> > > > +@} else
> > > >        return AVERROR_INVALIDDATA;
> > > > -@}
> > > > +
> > > >    @end example
> > > >    @item
> > > 
> > > Strongly objecting to this patch. It's all over our codebase, and I like it.
> > > Literally everywhere you look.
> > > If future patches need to add more under the branch, then it only needs to
> > > be done once.
> > > 
> > > Such fundamental changes need more than a single patch hastily accepted, and
> > > should involve the community as a whole, I believe.
> > 
> > Its not a change, look at tools/patcheck
> > 
> > or try:
> > 
> > +    if(this)
> > +        that
> > +    else
> > +        nothat
> > +
> > 
> > patcheck reports:
> > 
> > missing } prior to else
> > patcheck.stdout:11:+    else
> > 
> > missing whitespace between keyword and ( (feel free to ignore)
> > patcheck.stdout:9:+    if(this)
> 
> Ah, okay. I didn't read properly. The double negative in the sentence makes
> it hard to understand.
> 
> Rather than
> > No braces around single-line blocks, unless they are followed by an else
> (to keep future patches cleaner)
> 
> Consider

> > Don't wrap single-line blocks in braces. Use braces only if there is an
> accompanying else statement. This keeps future code changes easier to keep
> track of.

ok, will apply with this
this will make patcheck and developer.texi consistent.

If people want to change it, it should be changed in both locations

thx


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".