[FFmpeg-devel] [PATCH v14 9/9] avcodec/evc: Changes in Changelog and MAINTAINERS files
Dawid Kozinski/Multimedia (PLT) /SRPOL/Staff Engineer/Samsung Electronics
d.kozinski at samsung.com
Tue Dec 13 14:22:08 EET 2022
We made some changes in our EVC wrapper implementation and would like to
submit new patches to patchwork, but it's still unclear to me how to deal
with the MAINTAINERS file.
Should I leave the following lines:
+ libxevd.c Dawid Kozinski
+ libxeve.c, Dawid Kozinski
+ evc.c, evc.h Dawid Kozinski
+ evcdec.c Dawid Kozinski
+ evc_parser.c Dawid Kozinski
or should I remove them?
We are expecting a clear and consistent standpoint on this matter.
-----Original Message-----
From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Lynne
Sent: piątek, 28 października 2022 23:08
To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v14 9/9] avcodec/evc: Changes in
Changelog and MAINTAINERS files
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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel at ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email ffmpeg-devel-request at ffmpeg.org
with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list