[FFmpeg-devel] Process (Was: [PATCH][7/8] Add VA API accelerated H.264 decoding (take 4))

Gwenole Beauchesne gbeauchesne
Wed Feb 11 17:11:57 CET 2009


On Tue, 10 Feb 2009, Michael Niedermayer wrote:

>> You are not asking me to improve my patch, but to write another patch to
>> fix previous mistakes, or would you accept my new patch for VA API + the
>> suggested FFmpeg API combined at once? I pretty doubt so.
>
> let me say it one more time, NO factorizeable code (that we notice to be so)
> goes in svn, either someone factorizes it or it stays out.

Yes, "that we notice to be so" are the easy key words because if we follow 
your reasonning, ff_vdpau_{mpeg,h264}_picture_complete() could have been 
factored out in the first place. In particular with the extra HWAccel 
class.

>> If I don't understand their
>> approach, I ask. Along the way, I also arrange their patch to fit the new
>> fixed architecture + fixing cosmetics at the same time. I am not an
>> idealist, it's just that I think we learn better of our own mistakes if we
>> manage to fix them ourselves + I don't want to bother others with
>> futilities (cosmetics). And I won't ask them either to implement something
>> I only have in my mind. Because the time I would spend explaining to them
>> in details what I intend to do is usually equivalent to what I would
>> actually spend in writing the code...
>
> This approuch fails in various ways, first because you change code that
> you arent the author of you
> 1. piss the author off
> 2. make mistakes that the author would not have made.
> 3. require more time.

More time than wait for a round-trip to the author that could be hours to 
days delay vs. 30 seconds fixing a typo? You must be kidding.

> and because you dont bother explaining anything to the author you
> 1. make it hard for him to maintain the code in the future
> 2. prevent healthy discussion and teamwork

The trivial changes (typos, cosmetics, etc.) are notified afterwards, 
major changes beforehand.




More information about the ffmpeg-devel mailing list