[XviD-devel] [In progress] devapi4 -- mpeg matrices

[Michael Militzer]

Hello Radek,

Quoting Radek Czyz <syskin at ihug.com.au>:

> Hello Michael,
> 
> I can't help noticing that you expect XviD to reach 1.0 without its
> code being changed.

not at all. 

> As far as I understood, our goal was to take very part that doesn't
> work and make it working - one by one. You must realize that this is
> exactly what we (GomGom mostly) are doing - yet you criticize our
> work. If you expect us to do something else than that, you should tell
> us.

You said it yourself: 'take every part that doesn't work and make _it_
working' - that's what bugfixing is all about, that's what we agreed on
to do and that's what I expect you to do.

Bugfixing (at least for me) means to debug a code snippet that is known
to not work properly, then find the source of the problem and change the
lines of code that _need_ to be changed in order to fix the bug.

Bugfixing is not: "Hm, I know that there is some flaw somewhere in this
code but [I'm too busy/I'm too lazy/I don't understand the code/I don't
like the code], so I just kick everything out and copy&paste replace it
by something else I found on the internet."

I know that this example is exaggerated, so don't start a flame-war
about it. All I wanted to say is that bug-fixes should be done with as
minimal code changes as possible. Replacing a lot of code because it's
not nicely written (or whatever else) is not bugfixing but adding a new
feature (even though the behaviour for the user might not change). Such
beautifying or getting rid of ugly code can come after 1.0

To get back on the quant-matrix topic: It is a limitation, no question
and it needs to be fixed. However how severe is this limitation? Well,
not that much. Most users didn't ever realize - nonetheless it needs
to be fixed. Ok, so how should it be fixed? Well, with the least mo-
difications possible.

Now look at what we're discussing about: In order to fix this rather
small problem, it has been suggested to fully replace more than 4000
lines of code where >5 people have been working on for 2 years in order
to make it run as smoothly as it's running now. Don't you think that
this way of addressing the problem is - mildly speaking - a little bit
overkill?

What needs to be changed at all? Some global variables need to be turned
into parameters of a function - do we need to replace 4000 lines of code
in order to achieve this goal? This is just a rather simple modification
and I assume that changing the quant code is the least work (more work
will be needed to change all the remaining code from where the quant
stuff is called).

Our quant code is part of XviD for more than two years, it has been tested
and found to be good by many many people throughout this time. All I say
now is that I don't see why we should throw it all away when also just
some small punctual modifications to the code would be enough to reach our
goal.

> You wrote:
> > Later on we introduced custom matrices and in order to adopt
> > the quant code to this change, we introduced the quant matrices as
> > global variables just as a quick hack until someone adopts the code to
> > accept the quant matrices as function parameters. Now I think that the
> > modification that is needed to turn a global variable into a parameter
> > doesn't necessarily require to completely replace/rewrite the whole
> > quantization code (c, mmx, xmm, 3dn + equivalent h.263 quant code).
> 
> What's wrong in making it right this time? Hacks that fix hacks are not
> necessary safer imho.. especially when the code in question can be
> easly proven to be correct, just by comparing psnr.

As said, our code is well tested and works. We don't need to throw it away
just because some new code is nicer. We don't want to win the software
beauty contest 03, our goal for 1.0 was to have a stable milestone release.
With this goal in mind, bugfixing can only mean to fix some of the
remaining bugs _in_ the XviD code base and not _replacing_ the whole code
base with something new.

> > What just concerns me is that we've been replacing huge amounts of code
> > in dev-api-4 by newly rewritten/beautified/no tested code without a real
> > need to do so (ME split, 2pass rewrite, now the quant code replacement).
> 
> You must know that dev-api-4 IS not tested and buggy code. How do you
> expect it to change without rewriting the parts that don't work?
> I also can't help noticing that you don't care I fixed several bugs
> during the ME split, but you complain about a simple typo that was
> detected as soon as someone used the option concerned. Do you actually
> mean to say it was better to keep the buggy code?

Oh, come on. I never said: "please, don't touch anything. I like the bugs."
But as you also know yourself, every change to the code means the risk to
introduce some new bugs. Some we may find ourselves soon again (hopefully
all), but some might survive (and will annoy users which is bad). I think
we agree that parts that don't work should be fixed. But again: our h.263
quant code worked nicely - I don't see why it needs to be replaced just
because the mpeg quant code had a limitation. We just shouldn't loose
track of what we want to achieve after all. mpeg quant has a problem - 
ok, then _fix_ the mpeg quant code but nothing else. Should we ever de-
cide that we don't _like_ our h.263 quant code anymore, then we can re-
place it by Skal's code. But this is something that should come after 1.0

> Same goes to RC - if it didn't work in dev-api-4 at all, how keeping
> it was better?
> 
> > Now we're on the road to XviD 1.0 which should become our
> > first official _stable_ release and we're again and again removing stable
> > code from the XviD code base and replace it by completely different and
> > not at all as thoroughly-tested code
> 
> Really, how removing a code that is KNOWN to be buggy is wrong for
> stable release? Do you really want to add a list of known bugs to 1.0?
> With an explaination "it has been buggy for so long, you probably
> remember not to do this and this and this" ?

I think I made myself clear about the difference between 'fixing' and
'replacing' something...

> > In result this means, that all newly introduced code needs special testing
> > in a public beta testing phase:
> 
> Of course. Nothing strange about this I hope.
> 
> There were only two ways: document all the things that didn't work in
> dev-api-3 and relase api-3 as stable, or fix them. There is nothing in
> between. Since you have chosen the second option, I see no way but
> finishing the second option, simply because dev-api-4 doesn't work *at
> all* without it.

Again: Noone said: 'don't fix it' - it's just a matter of _how_ to fix a
bug. That's all.

bye,
Michael