[Mplayer-cvslog] CVS: main/libmpcodecs vd_xvid4.c,1.2,1.3

[Ivan Kalvachev]

On Tue, 05 Oct 2004 22:39:15 +0200
Guillaume POIRIER <guillaume.poirier at ifsic.univ-rennes1.fr> wrote:

> Hi,
> Le mar 05/10/2004 _ 15:00, Diego Biurrun a _crit :
> > Reimar D_ffinger writes:
> > > 
> > > >>Renames xvid_ini to xvid_gbl_init and dec_p to xvid_dec_create
> > > >>That'll make the move to 1.1.x front-end smoother.
> > > > 
> > > > WHAT THE HELL ARE YOU TALKING ABOUT????!!!
> > > > 
> > > > THIS COMMIT IS PURE COSMETIC, I DON"T SEE ANY FUNCTIONAL CHANGE.
> > > > IT IS NOT POSSIBLE TO IMPROVE ANYTHING!!!
> > > > 
> > > > Revert immediately
> 
> How should I respond to an upper-case only email... Hum, it's not in my
> temper to write back with the same aggressive tone.

Probably I should have written "Rever immediately" part with uppercase too,
so you could not miss it. The right asnwer of such mail is :

"Reverted"

As you have heared I got some real life troubles, I had to sort out.
I didn't had time to read all the mails, so low volume mail list like cvs
were my favorites.
As the person that have commited xvid4 before I do care for maintaining it.
You can see that I actually do some small improvements from time to time
and I don't server only as proxy.
The changes that GomGom have done are not trivial and need better examination.

In the decoder vd_xvid4.c, he change the init function of the video system, so he can
pass the aspect information. I still haven't reviewed that code, but for me it looks risky
and could have some possible itches with e.g. direct rendering (that's off by default).
The problem is that e.g. lavcodec use an callback functions (get_buffer, get_format..)
so we could init the video system in it. XviD have only decoder function. 
As I said It involves a lot of code and should be checked carefully, something I
didn't had time to do.

>From the quick look of vd_xivd4.c I also sow that the new options for postprocessing
could be compleatly removed and instead the PP level from control() system to be used,
just like divx4linux and binnary decoders.


About ve_xvid4.c. I think that most of these helper functions didn't exist when I
added the rational code. I like putting such functions of the top of the file to avoid
explict declaration; You know that when you change function you have to hunt down
all declarations. The moved functions are not exacly the same, they also change
the way value is returned (yes Reimar, it is possible to return struct, it value is simply copied
to the target, like if it have been intiger. The code is written my michaelni, btw)

I had suppresed and reverted the changes GomGom have done to aspect code. Not only 
because it envolves cosmetics, but also because I plan to get the rational functions in 
separate modue/file and use it for x264 aspect calculations. The new aspect code itself
looks better, but I couldn't find any improvements over the old one, so for now it is delayed.

The most importan thing in this module is the emmting of delayed frames, I had some time
implementing the functionality and hunting an segfault. As result I had implemented an
proper uninit free() code. Now the big comments and ugly #ifdef are removed.


So I would like to commit the changes I have done so far.

Would You like to revert the changes and so we could become friends or
I will revert the changes by myself and keep been mad on you?


  Ivan Kalvachev
 iive

P.S.

I won't mind if GomGom got CVS write access and maintain xvid modules. 
I'm just not sure he will like flames from time to time, especially when he broke the rules.

As long as cosmetics is rule we should follow it. 
I wouldn't mind if we remove that rule, as long as all developers agree on it.
Personally I think that having automatic indent could help more on hunting bugs.
I would be happy to run an indent on mencoder.c for a change....
I still keep 2 line long list of options that make code look much better.)


P.S.2

You can really get your cvs write access revoked if you commit code that you don't understand.
And I am not talking about security or trojan horses:)
People can send horribly broken patches, but it is developers responsibility to reject or improve them.
In short if you commit broken patch, you get the cola punishment, not the patch sender.

P.S.3

I'm gonna send this mail in dev, so we could discuss it there.
I already wrote what reply I expect from you in this maillist.