[FFmpeg-devel] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[FFmpeg-devel] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Nicolas George]

ffmpeg-git@ffmpeg.org (HE12025-08-12):
> Author:     Niklas Haas <git@haasn.dev>
> AuthorDate: Mon Aug 11 15:57:24 2025 +0200
> Commit:     Niklas Haas <ffmpeg@haasn.dev>
> CommitDate: Tue Aug 12 09:01:39 2025 +0000

Pushing after less than 24 hours without approval by the maintainer is
absolutely unacceptable, forge or no forge, and a spit in my face for
the time I invested into starting to look at the code, thankfully
limited.

Never do that again.

-- 
  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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Niklas Haas]

On Tue, 12 Aug 2025 11:16:19 +0200 Nicolas George <george@nsup.org> wrote:
> ffmpeg-git@ffmpeg.org (HE12025-08-12):
> > Author:     Niklas Haas <git@haasn.dev>
> > AuthorDate: Mon Aug 11 15:57:24 2025 +0200
> > Commit:     Niklas Haas <ffmpeg@haasn.dev>
> > CommitDate: Tue Aug 12 09:01:39 2025 +0000
>
> Pushing after less than 24 hours without approval by the maintainer is
> absolutely unacceptable, forge or no forge, and a spit in my face for
> the time I invested into starting to look at the code, thankfully
> limited.

In https://ffmpeg.org/developer.html#Patch-review-process it says:

> After a patch is approved it will be committed to the repository.

I don't see anything about a time-frame requirement here.

That said, maybe we should adopt one, especially now that we actually
receive reviews in a timely manner thanks to the adoption of forgejo.

Perhaps we adopt the VLC model, which is:
- If a PR has received no activity for 3 days
- AND it has at least one approval
- AND it passes all CI checks
then it will be merged automatically, by a bot.

>
> Never do that again.
>
> --
>   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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Niklas Haas]

On Tue, 12 Aug 2025 11:34:08 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> Perhaps we adopt the VLC model, which is:
> - If a PR has received no activity for 3 days
> - AND it has at least one approval
> - AND it passes all CI checks
> then it will be merged automatically, by a bot.

Oh, and perhaps obvious, but "it has no open discussion threads or change
requests" is of course another condition.
_______________________________________________
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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Gyan Doshi]

On 2025-08-12 03:04 pm, Niklas Haas wrote:
> On Tue, 12 Aug 2025 11:16:19 +0200 Nicolas George <george@nsup.org> wrote:
>> ffmpeg-git@ffmpeg.org (HE12025-08-12):
>>> Author:     Niklas Haas <git@haasn.dev>
>>> AuthorDate: Mon Aug 11 15:57:24 2025 +0200
>>> Commit:     Niklas Haas <ffmpeg@haasn.dev>
>>> CommitDate: Tue Aug 12 09:01:39 2025 +0000
>> Pushing after less than 24 hours without approval by the maintainer is
>> absolutely unacceptable, forge or no forge, and a spit in my face for
>> the time I invested into starting to look at the code, thankfully
>> limited.
> In https://ffmpeg.org/developer.html#Patch-review-process it says:
>
>> After a patch is approved it will be committed to the repository.
> I don't see anything about a time-frame requirement here.

It's the last point in
https://ffmpeg.org/developer.html#Patches_002fCommitting

Regards,
Gyan

_______________________________________________
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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Niklas Haas]

On Tue, 12 Aug 2025 15:08:10 +0530 Gyan Doshi <ffmpeg@gyani.pro> wrote:
>
>
> On 2025-08-12 03:04 pm, Niklas Haas wrote:
> > On Tue, 12 Aug 2025 11:16:19 +0200 Nicolas George <george@nsup.org> wrote:
> >> ffmpeg-git@ffmpeg.org (HE12025-08-12):
> >>> Author:     Niklas Haas <git@haasn.dev>
> >>> AuthorDate: Mon Aug 11 15:57:24 2025 +0200
> >>> Commit:     Niklas Haas <ffmpeg@haasn.dev>
> >>> CommitDate: Tue Aug 12 09:01:39 2025 +0000
> >> Pushing after less than 24 hours without approval by the maintainer is
> >> absolutely unacceptable, forge or no forge, and a spit in my face for
> >> the time I invested into starting to look at the code, thankfully
> >> limited.
> > In https://ffmpeg.org/developer.html#Patch-review-process it says:
> >
> >> After a patch is approved it will be committed to the repository.
> > I don't see anything about a time-frame requirement here.
>
> It's the last point in
> https://ffmpeg.org/developer.html#Patches_002fCommitting

I see that, but note:
1) the PR *was* approved
2) nobody is listed as a maintainer for these filters

Regardless, I still think we should then just establish a clear pipeline
here to prevent this sort of discussion from happening in the future.

Timo, can something like the VLC merge bot be implemented in ForgeJo?

>
> Regards,
> Gyan
>
> _______________________________________________
> 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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Nicolas George]

Niklas Haas (HE12025-08-12):
> I see that, but note:
> 1) the PR *was* approved

How do you know that approval was worth anything?

> 2) nobody is listed as a maintainer for these filters

So as a default, they are maintained by whoever maintains libavfilter as
a whole, which means me since Stefano took steps back from development.

> Regardless, I still think we should then just establish a clear pipeline
> here to prevent this sort of discussion from happening in the future.
> 
> Timo, can something like the VLC merge bot be implemented in ForgeJo?

Technical solution will not solve human problems.

-- 
  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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Niklas Haas]

On Tue, 12 Aug 2025 11:53:20 +0200 Nicolas George <george@nsup.org> wrote:
> Niklas Haas (HE12025-08-12):
> > I see that, but note:
> > 1) the PR *was* approved
>
> How do you know that approval was worth anything?

As far as my understanding goes, only people with write access to the
repository status have the ability to merge or approve PRs. If you do not
feel somebody can be trusted with write access, that is a separate issue
irrelevant to this discussion.

That aside, the person who reviewed the PR also has about 100x more activity
than you in the past 2-year period and is also a member of the GA. I think
it's fair to assume they are qualified to review a trivial patch.

> > 2) nobody is listed as a maintainer for these filters
>
> So as a default, they are maintained by whoever maintains libavfilter as
> a whole, which means me since Stefano took steps back from development.

Where in "MAINTAINERS" is this written?

> Technical solution will not solve human problems.

Having a clearly defined process is a human solution to a human problem.
_______________________________________________
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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Nicolas George]

Niklas Haas (HE12025-08-12):
> As far as my understanding goes, only people with write access to the
> repository status have the ability to merge or approve PRs.

This is obviously wrong: write access to the repository is
all-or-nothing, expertise in parts of the code is not. This is why we
have maintainers.

With the recent clarification of who approved the series, we now know
that you had reliable but non-expert non-maitainer approval, which
should have let you wait a little less but not push immediately.

>								If you do not
> feel somebody can be trusted with write access, that is a separate issue
> irrelevant to this discussion.

Well, Michael has been giving these away like hotcakes recently, I can
think of two cases in recent months, but it is entirely unrelated to
this discussion.

> That aside, the person who reviewed the PR also has about 100x more activity
> than you

So what?

> Where in "MAINTAINERS" is this written?

An unfortunate oversight that will soon be rectified. But I believe it
should be common knowledge for somebody who has been in the project for
as long as you have, or for somebody who exchanged mails with me
recently on the very subject of libavfilter.

> Having a clearly defined process is a human solution to a human problem.

A bot is not a human solution.

-- 
  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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[James Almer]

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

On 8/12/2025 6:53 AM, Nicolas George wrote:
> Niklas Haas (HE12025-08-12):
>> I see that, but note:
>> 1) the PR *was* approved
> 
> How do you know that approval was worth anything?

Insulting other devs right as we move to a forge to attract new 
contributors sounds counterproductive. Not to mention unacceptable.


[-- 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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Nicolas George]

James Almer (HE12025-08-12):
> Insulting other devs right as we move to a forge to attract new contributors
> sounds counterproductive. Not to mention unacceptable.

Good thing that I did no such thing. And good thing that you are not
accusing me of doing such a thing, as a baseless accusation like that
would be equally unacceptable.

The question stands: on what information did Niklas base himself, on
what information can anybody of us base ourself, to distinguish an
approval by somebody who knows the code very well and (⇒ basis for
immediate pushing) from an approval by somebody who knows the code only
tangentially and could have missed subtle issues (⇒ basis for waiting a
little less before pushing, but not immediate) from an approval by a
newbie who overestimate their own competence (we had a few of those in
the past).

-- 
  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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Tristan Matthews via ffmpeg-devel]

Hi,

On Tuesday, August 12th, 2025 at 5:51 AM, Niklas Haas <ffmpeg@haasn.xyz> wrote:

> On Tue, 12 Aug 2025 15:08:10 +0530 Gyan Doshi ffmpeg@gyani.pro wrote:
> 
> > On 2025-08-12 03:04 pm, Niklas Haas wrote:
> > 
> > > On Tue, 12 Aug 2025 11:16:19 +0200 Nicolas George george@nsup.org wrote:
> > > 
> > > > ffmpeg-git@ffmpeg.org (HE12025-08-12):
> > > > 
> > > > > Author: Niklas Haas git@haasn.dev
> > > > > AuthorDate: Mon Aug 11 15:57:24 2025 +0200
> > > > > Commit: Niklas Haas ffmpeg@haasn.dev
> > > > > CommitDate: Tue Aug 12 09:01:39 2025 +0000
> > > > > Pushing after less than 24 hours without approval by the maintainer is
> > > > > absolutely unacceptable, forge or no forge, and a spit in my face for
> > > > > the time I invested into starting to look at the code, thankfully
> > > > > limited.
> > > > > In https://ffmpeg.org/developer.html#Patch-review-process it says:
> > > 
> > > > After a patch is approved it will be committed to the repository.
> > > > I don't see anything about a time-frame requirement here.
> > 
> > It's the last point in
> > https://ffmpeg.org/developer.html#Patches_002fCommitting
> 
> 
> I see that, but note:
> 1) the PR was approved
> 2) nobody is listed as a maintainer for these filters
> 
> Regardless, I still think we should then just establish a clear pipeline
> here to prevent this sort of discussion from happening in the future.
> 
> Timo, can something like the VLC merge bot be implemented in ForgeJo?
> 

For reference, that's https://code.videolan.org/Garf/homer-bot
Obviously that's gitlab specific but may help inform some choices here.

Best,
Tristan
_______________________________________________
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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Kacper Michajlow]

On Tue, 12 Aug 2025 at 14:35, Tristan Matthews via ffmpeg-devel
<ffmpeg-devel@ffmpeg.org> wrote:
>
>
>
>
>
> Hi,
>
> On Tuesday, August 12th, 2025 at 5:51 AM, Niklas Haas <ffmpeg@haasn.xyz> wrote:
>
> > On Tue, 12 Aug 2025 15:08:10 +0530 Gyan Doshi ffmpeg@gyani.pro wrote:
> >
> > > On 2025-08-12 03:04 pm, Niklas Haas wrote:
> > >
> > > > On Tue, 12 Aug 2025 11:16:19 +0200 Nicolas George george@nsup.org wrote:
> > > >
> > > > > ffmpeg-git@ffmpeg.org (HE12025-08-12):
> > > > >
> > > > > > Author: Niklas Haas git@haasn.dev
> > > > > > AuthorDate: Mon Aug 11 15:57:24 2025 +0200
> > > > > > Commit: Niklas Haas ffmpeg@haasn.dev
> > > > > > CommitDate: Tue Aug 12 09:01:39 2025 +0000
> > > > > > Pushing after less than 24 hours without approval by the maintainer is
> > > > > > absolutely unacceptable, forge or no forge, and a spit in my face for
> > > > > > the time I invested into starting to look at the code, thankfully
> > > > > > limited.
> > > > > > In https://ffmpeg.org/developer.html#Patch-review-process it says:
> > > >
> > > > > After a patch is approved it will be committed to the repository.
> > > > > I don't see anything about a time-frame requirement here.
> > >
> > > It's the last point in
> > > https://ffmpeg.org/developer.html#Patches_002fCommitting
> >
> >
> > I see that, but note:
> > 1) the PR was approved
> > 2) nobody is listed as a maintainer for these filters
> >
> > Regardless, I still think we should then just establish a clear pipeline
> > here to prevent this sort of discussion from happening in the future.
> >
> > Timo, can something like the VLC merge bot be implemented in ForgeJo?
> >
>
> For reference, that's https://code.videolan.org/Garf/homer-bot
> Obviously that's gitlab specific but may help inform some choices here.

Because of course there is homer-bot... I think everyone seen
marge-bot too https://github.com/smarkets/marge-bot (also GitLab
specific)

Forgejo is API compatible with GitHub, so we could reuse solutions
that work there. Also in general Forgejo itself is more basic in
itself, doesn't support marge queues/trains, so any solution would
have to be a custom bot.

- Kacper
_______________________________________________
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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Niklas Haas]

On Tue, 12 Aug 2025 17:59:44 +0200 Kacper Michajlow <kasper93@gmail.com> wrote:
> On Tue, 12 Aug 2025 at 14:35, Tristan Matthews via ffmpeg-devel
> <ffmpeg-devel@ffmpeg.org> wrote:
> > For reference, that's https://code.videolan.org/Garf/homer-bot
> > Obviously that's gitlab specific but may help inform some choices here.
>
> Because of course there is homer-bot... I think everyone seen
> marge-bot too https://github.com/smarkets/marge-bot (also GitLab
> specific)
>
> Forgejo is API compatible with GitHub, so we could reuse solutions
> that work there. Also in general Forgejo itself is more basic in
> itself, doesn't support marge queues/trains, so any solution would
> have to be a custom bot.

For what it's worth, I was briefly discussing the idea of a merge bot
with Timo over IRC, and the conclusion we arrived at is that we could
also recreate the functionality of a merge queue that way.

Basically, the merge bot would first rebase a PR, and only merge it once
the CI passes - repeating as necessary until all "mergable" PRs are merged.

>
> - Kacper
> _______________________________________________
> 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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Yalda]

Nicolas George:
> Approval full stop is [********]. The only approval worth something comes
> from somebody who knows the code.
>
> How do you know that approval was worth anything?
>
> So as a default, they are maintained by whoever maintains libavfilter

----

I was the one who approved this PR.

I’ve had a high tolerance for remarks like yours, but I will not stand
by anymore as you question the competence of contributors without cause,
a pattern which does nothing but drive people away.

For the record:
1) I reviewed the PR in full, understood the changes, and stand by my
approval.

2) Gatekeeping over trivial or clearly beneficial patches harms the project
and drives contributors away. The only “gain” from it is polishing one’s
own ego.

3) I do not give blind approvals. I am more than capable of understanding,
implementing, and approving changes at and beyond the scope here.
Suggesting otherwise is an insult to my intelligence which I will not
accept.

And since we are being direct: your recent contributions to this project
consist largely of criticizing others without offering substantive
technical work yourself. Even minor contributors today are more in tune
with the project’s direction.

If there are genuine, technical objections to this PR, let’s address them
directly. Otherwise, undermining contributors without adding value only
hurts FFmpeg and the community at large.
_______________________________________________
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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Nicolas George]

Yalda (HE12025-08-12):
> I was the one who approved this PR.

Thanks for intervening and providing this information. It does not help
a lot, as I have no idea who you are, Yalda.

… Now that I have access to your mail address too, it seems to evoke
somebody who contributed usefully to the project in the recent years.

> I’ve had a high tolerance for remarks like yours, but I will not stand
> by anymore as you question the competence of contributors without cause,
> a pattern which does nothing but drive people away.

I acknowledge that my question could be interpreted this way, and will
therefore forgive the ad-hominem attacks at the end of this mail.

I will not apologize for the miscommunication caused by the awful tool
that is being pushed on us.

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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Yalda]

Nicolas George:
> Now that I have access to your mail address too, it seems to evoke
> somebody who contributed usefully to the project in the recent years.

For clarity, I announced my handle change in an email titled "Handle change"
on July 13th.

Your categorization is noted. It does not change the fact that the PR in
question was reviewed, understood, and approved on its merits.

This was a routine code chore/logging cleanup, not an architectural change
where I would have deferred to the maintainers and broader community.

That is the full context behind my approval.
_______________________________________________
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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Nicolas George]

Niklas Haas (HE12025-08-12):
> I don't see anything about a time-frame requirement here.

For one, it does say something about a time-frame requirement.

For two, you have not waited for approval by maintainer.

> - AND it has at least one approval

Approval full stop is bullshit. The only approval worth something comes
from somebody who knows the code.

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