[FFmpeg-devel] [RFC] replace some static with asm_visibility or?so

[Balatoni Denes]

Hi!

Thanks Michael, for addressing my points in detail.

Wednesday 30 January 2008 13:13-kor Michael Niedermayer ezt ?rta:
> There are 2 problems ...
> 1. If he knew the amount of time at the begin chances are he wouldnt even
>    start fixing anything.

This does lead to another (so far implicit) point. I don't see why a 
contribution that is an independent module (not touching any other code 
significantly), and is an improvement to ffmpeg is only acceptable when it 
is "perfect". Imho such contributions could be comitted to svn, as they don't 
lead to any regression - at least not that I can see now - but an 
improvement, even though perhaps not as big an improvement as if the 
contribution was in a perfect condition. I think just because such a patch 
that I described above is comitted as is, that doesn't mean it shouldn't be 
improved later - on the contrary it should by all means, although not 
neccessarily by the original contributor (as he might not have time for 
example). As far as I can see, what I described would lead to faster feature 
additions to ffmpeg, happier contributors (they are more often rewarded, by 
having their contribution comitted), and there are no disadvantages I see.
I don't belive that only letting "perfect" contributions comitted would really 
urge contributors fix their patch - they might jump through all the hoops 
once, but maybe not any time later (i.e. they stop sending patches).

Patches that would fit my description would be for example  
Ian Caulfield's MLP decoder, some of Christophe Gisquet's optimizations,  
Siarhei Siamashka's arm idct optimizations (this also shows a double 
standard, because Mans - who has SVN commit rights - was free to commit 
suboptimal code, that has never been accepted from anybody without SVN commit 
rights - I must note however, that IMO it's good that Mans' code was made 
available early in SVN) or even my sparc idct optimizations - which 
eventually were comitted, but none of the others I have mentioned have been 
up to now (for quite some time).

> 2. Reviews take time, and spoting missing doxygen and such is VERY easy.
>    Realizing that there are fundamental design issues or otherwise that
>    something can be redesigned so it is much simpler and still otherwise
>    equivalent takes much more time and understanding of the
>    code. So these things are not always found in the first review iteration.
>    Now one could argue that the first review should be more complete and
>    more time should be spend to not miss anything. But that would cause
>    significantly longer review cycles.

Well, I understand this, maybe there is not much to do in this regard.

> And if the author choose not to
>    fix anything due to the amount of work, the review time would be
>    wasted.

Yes, otherwise the author's time is wasted, by perfecting patches that are 
never comitted.

> And please dont forget reviewing is about checking that the code and its
> design is (near) optimal, much more than it is about checking that
> indention and doxygen matches the rules. So skiping that would definitly
> not do any good. It would be like spellchecking a document whos content
> makes no sense.

I understand, even agree. The review is needed, but imho some things can be 
comitted sooner, and fixed later (I say later, not never) in SVN - perhaps 
not neccessarily by the original author (if he doesn't have more time), but 
others giving a helping hand.

Okay, my drivel might be a bit utopistic, but I felt I had to share these 
thoughts with you :)

bye
Denes