[FFmpeg-devel] [PATCH v14 9/9] avcodec/evc: Changes in Changelog and MAINTAINERS files

[Lynne]

Oct 27, 2022, 18:45 by michael at niedermayer.cc:

> On Tue, Oct 25, 2022 at 01:17:15PM +0200, Lynne wrote:
>
>>
>>
>>
>> Oct 24, 2022, 18:29 by jamrial at gmail.com:
>>
>> > On 10/24/2022 12:56 PM, Lynne wrote:
>> >
>> >> Oct 24, 2022, 09:42 by d.kozinski at samsung.com:
>> >>
>> >>> - Changelog update
>> >>> - MAINTAINERS update
>> >>>
>> >>> Signed-off-by: Dawid Kozinski <d.kozinski at samsung.com>
>> >>> ---
>> >>>  Changelog   | 3 ++-
>> >>>  MAINTAINERS | 5 +++++
>> >>>  2 files changed, 7 insertions(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/Changelog b/Changelog
>> >>> index ec9de1bd85..19e9ae3b1f 100644
>> >>> --- a/Changelog
>> >>> +++ b/Changelog
>> >>> @@ -45,6 +45,8 @@ version 5.1:
>> >>>  - remap_opencl filter
>> >>>  - added chromakey_cuda filter
>> >>>  - added bilateral_cuda filter
>> >>> +- eXtra-fast Essential Video Encoder (XEVE)
>> >>> +- eXtra-fast Essential Video Decoder (XEVD)
>> >>>  version 5.0:
>> >>> @@ -92,7 +94,6 @@ version 5.0:
>> >>>  - anlmf audio filter
>> >>>  - IMF demuxer (experimental)
>> >>>  -
>> >>>  version 4.4:
>> >>>  - AudioToolbox output device
>> >>>  - MacCaption demuxer
>> >>> diff --git a/MAINTAINERS b/MAINTAINERS
>> >>> index eebfa5cfb7..df8d8eca73 100644
>> >>> --- a/MAINTAINERS
>> >>> +++ b/MAINTAINERS
>> >>> @@ -200,6 +200,8 @@ Codecs:
>> >>>  libvpx*                               James Zern
>> >>>  libxavs.c                             Stefan Gehrer
>> >>>  libxavs2.c                            Huiwen Ren
>> >>> +  libxevd.c                             Dawid Kozinski
>> >>> +  libxeve.c,                            Dawid Kozinski
>> >>>  libzvbi-teletextdec.c                 Marton Balint
>> >>>  lzo.h, lzo.c                          Reimar Doeffinger
>> >>>  mdec.c                                Michael Niedermayer
>> >>> @@ -420,6 +422,9 @@ Muxers/Demuxers:
>> >>>  dv.c                                  Roman Shaposhnik
>> >>>  electronicarts.c                      Peter Ross
>> >>>  epafdec.c                             Paul B Mahol
>> >>> +  evc.c, evc.h                          Dawid Kozinski
>> >>> +  evcdec.c                              Dawid Kozinski
>> >>> +  evc_parser.c                          Dawid Kozinski
>> >>>  ffm*                                  Baptiste Coudurier
>> >>>  flic.c                                Mike Melanson
>> >>>  flvdec.c                              Michael Niedermayer
>> >>>
>> >>
>> >> Nak, that list is only for those with push access, and no
>> >> other changes may be made in the same patch.
>> >>
>> >
>> > No, it's the other way around. Those in this list may be eligible for push access.
>> > Being listed here gives them the right to NAK a patch made for a module they maintain, as well as their approval being (ideally) a requirement before making changes to it.
>> >
>>
>> Nope. Michael will give anyone on the list push access.
>>
>
> I have the feeling you dont trust me
> if thats the issue, 2 lists will not fix that
>

I trust you more than others. But in this case, I simply don't understand.


> The idea is that each developer who takes care of a bit of the code base
> (reviewing patches, approving them, fixing issues, adding features, ...)
> has the same rights as others.
> That is git write, the list is the MAINAINERs list.
>
> Its not really true that everyone in that file has write access because
> some people where forgotten and never asked, some simply dont know git well
> enough, some explicitly said they do not want git write, some sent a lot of
> messy patches and gave me pause so i didnt offer it and they also didnt ask.
> The list should be pretty close though, these are all exceptions not the rule.
>

The maintainers list used to be what jamrial said it was - an informal list
of those with good knowledge on a piece of code to make a review, independent
of whether they had push access or not. This is also how users/casual patch
senders treated it as - they added their name if they felt like they would like to be
consulted on.
The list is always a bit outdated, and that's okay.
You started treating it as a formal list of those with commit access,
and it's been somewhat chaotic. Users still think it's an informal list,
developers still think it's an informal list, only you seem to think it should be
more formal. When a user submits a patch, I wonder if they're asking for push
access or do they simply want to be consulted on for future patches?
More often than not, it's the latter.

I think there should be 2 lists, and if someone wants push access, they should
just send a patch requesting it directly rather than using the vague maintainer term
that no one pays attention to. If someone thinks they should have push access and
ask, then they probably need it.
The maintainers list could continue to be treated the same way it's been
treated.


> I fail to see the problem, btw.
> A Problem would be if someoe does something that requires to remove his git
> write or that requires us to think about "should we close that write account"
> (and yes i ignore here cases where core developers dont get along, thats not
>  a issue for a maintainer/git write list)
> If we do not hit a situation where we consider closing an account then IMO
> we havnt really had a problem with giving write access out too liberal.
> The other side OTOH certainly has occured, people sending patches over and
> over again, pinging over and over again and finally the patch is found to be ok
> and applied. That would point more toward too little write permission, or at least
> not the right person having write access, or a lack of incentives to review and apply
> patches
>

I really don't think push access should be removed from someone inactive, but
I also don't think it should be given to someone with zero commits just because
their patches never got a response, like with this patch. For such a large and
wanted feature, it'll get merged by one of us eventually.