[MPlayer-dev-eng] PATCH to support theora-exp in mplayer

[Reimar Döffinger]

Hi,
On Tue, May 24, 2005 at 09:55:20PM -0400, Timothy B. Terriberry wrote:
> Reimar Döffinger wrote:
> > Without knowing the code...
> 
> I must say I'm getting a little tired of comments from the peanut
> gallery who "don't know the code". I've tried to be very accomodating,
> but very soon I'm going to decide it's not worth the time investment,
> and cut my losses. Find someone who "knows the code" and either commit
> it or drop it.

Sorry, but it's a big patch and in an area where there a still a few
unanswered "how to do this best" questions.
I meant I only reviewed the general part of the code, leaving the
details to others (but hopefully saving them some work nevertheless,
making it more likely they will do it).

> > Good that there aren't more fourccs than necessary ;-) *SCNR*
> Bad that they're necessary at all. But this was copied straight from the
> previous Theora support, so do not look at me. I don't make these things up.
> 
> > wtf? you want to win the obfuscated C contest? Far to go in that case,
> > but still nice try :-)
> If I was trying to write obfuscated code, you would know.

Sorry, those were really meant to be funny - obviously it didn't work.
What I meant in the last case: how about writing it like *(ed++) + 1;
(assuming I actually understand it correctly :-( )?
I think it would save everyone who has to read this code later at least
a few seconds (at least I am quite irritated by three + in a row, if you
think that's just because I'm too stupid, okay, your decision).

> > Mixing tabs and spaces...
> Just like every other piece of code in mplayer. Please stop trying to
> apply arbitrary standards that you do not follow yourself. If it were my
> code, there would be no tabs at all, but I'm not going to rewrite all of
>  mplayer for you, and you wouldn't want me to.

This chaos came because nobody looked in the patches for such things.
And I do try to follow those standards myself, I always try to match my
indetation to the existing code (though I have to admit I probably never
wrote a patch of that size).

> > Please double-check this is correct and works as expected (esp. difficult
> > to see, since it is missing a doxygen-comment, too), it has a bad smell
> > to me...
> 
> You mean like all of the other Doxygen comments, or any comments at all
> in that code? What is expected is that the next call to ds_get_packet()
> will return a pointer to "start", which was presumably returned by the
> previous call to ds_get_packet(). A trivial examination of
> ds_get_packet() shows that this is in fact what will happen, and video
> decoding up until the second I frame would be totally broken if it did not.

Yes, there aren't doxygen comments yet because that code is old,
nevertheless patches that are created now are supposed to have them for
every function...
But since Rich seems to look through your patches I will now leave this
alone, so that you don't get more annoyed and we have a chance of geting it
in...

Greetings,
Reimar Döffinger