[FFmpeg-cvslog] random thoughts about refactoring (was: Re: r20938 - trunk/libavcodec/h263.c)

[Diego Biurrun]

On Tue, Jan 05, 2010 at 08:43:51PM +0100, Michael Niedermayer wrote:
> On Tue, Jan 05, 2010 at 11:29:46AM +0100, Diego Biurrun wrote:
> > On Tue, Jan 05, 2010 at 03:20:47AM +0100, Michael Niedermayer wrote:
> > > On Mon, Jan 04, 2010 at 08:23:23PM +0100, Diego Biurrun wrote:
> > > > 
> > > > How hard can it be - here is the patch...
> > > 
> > > id say too hard, but lets see, there where 4 hunks, your patch contains 3.
> > > Something has been lost ...
> > 
> > I couldn't come up with a good replacement right away, but
> > 
> >   dc must be in the [-255,255] range
> > 
> > might work.
> 
> hmm, i dont know, its not checking dc ...

Right, better suggestions welcome.

> > > i still think copy and pasting the 4 hunks into patch is going to be the
> > > quicker way to fix this, thats not disputing that you eventually will come
> > > up with a convertion to proprer comments just that the work seems out of
> > > proportion to the purely cosmetic gain
> > 
> > What is out of proportion is the ensuing discussion. What is
> > disheartening is the lack of motivation to actually fix the current
> > mess.
> 
> My fix is to revert this. You refuse to do that. I dont really mind
> an alternative fix but i cannot fix what you precive as mess because
> i do not precive these //printf("some error condition") as mess.
> To me a note on toilet paper is as good as one on gold framed under glass

Crufty code is not good, it will spread, do you know the broken window
theory?

http://en.wikipedia.org/wiki/Fixing_Broken_Windows
http://www.codinghorror.com/blog/archives/000326.html

> > and the fools that try anyway receive nasty surprises. In the end
> > people get frustrated for wasting their time and you get frustrated
> > because nobody lends a helping hand.
> 
> To be honest and blunt, i dont think the changes you will do to this
> code will improve its readability. Thus i feel my time is wasted if
> i did spend time on this. 
> But id be honestly happy to be proofen wrong, for example if you instead
> of doing (IMO silly) cosmetic changes between //printf(" and //
> would add new doxygen comments to document what the individual functions
> do and maybe there are even case where code could be factored or changed
> to be easier to read but i think pure doxy alone would already be usefull
> then iam very much willing to help.

This was more of a general comment, not so much about H.263 in general.
You misunderstood it to be about this particular case; I probably was
not clear enough.

My changes are just the preliminaries to deeper changes: Splitting the
decoding and encoding code into separate files.  I started working on
this, but the code is an intermingled mess that will be difficult to
disentangle.  And even getting the trivial preliminaries committed is
very difficult, so my motivation to continue working on this is not
exactly on the rise...

> > Does "H.264" ring a bell?
> 
> H.264 is an entirely different beast, its much more complex than h.263
> and even i as author tend to need to read spec & code often. But to a large
> extend the same commnents apply, h264 itself is very complex and quite
> messy, any implementation that performs at reasonable speed will be
> messy & complex. Improvments to our implementation are welcome of course
> but the h263 code should be easier ...

You yourself said that you would welcome help on H.264, yet you are not
receiving any.  There are people here who know H.264 well enough to lend
a helping hand in theory, but in practice the implementation in lavc is
just too impenetrable.  h264.c is around 10.000 lines if you count svq3.c
which it directly includes!

Also, our H.264 decoder is not a speed demon.  If it were that might
warrant some tradeoffs in readability.  Jason outlined ways to speed
it up some time ago, but they were never implemented...

I tried refactoring parts of it some time ago but failed horribly.
Since you have declared that even the most minor speed regression
is unacceptable, I don't think anybody will muster the courage to
try in the future...

Don't forget that hardly anybody works on refactoring at all.  I only
remember Aurelien, Mans and myself off the top of my head.  Since
Aurelien is nowhere to be found you seem to be out of appealing
alternatives.  It seems you can only

- hire somebody competent to do it,
- do it yourself or
- make the most out of the little help I can offer you...

Diego