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

Timothy B. Terriberry tterribe at xiph.org
Wed May 25 03:55:20 CEST 2005


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.

> 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.

> 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.

> empty vs. whitespace-only lines cosmetics...
Fixed in attached patch.

>>+int ds_unget_packet(demux_stream_t *ds,unsigned char *start){
>>+    if(start<ds->buffer||start>&ds->buffer[ds->buffer_pos]){
>>+        return -1;
>>+    }
>>+    ds->buffer_pos=start-ds->buffer;
>>+    return 0;
>>+}
>>+
> 
> 
> 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.

I'll address Rich's comment here, as well: the purpose of this is not to
put back data packets that appear before header packets. One of the
intentions of the Theora team (not necessarily one I agree with, but I
am not the entire team) is in the future to support additional, optional
headers beyond the required three (one potential example being a header
containing an ICC profile). The API supports this now, so that if such
things are added to the format, a library upgrade is all that is
required, without requiring additional changes to mplayer's codebase.
With the demuxer reorganization, I took the opportunity to use this API
properly. Since it doesn't know in advance how many headers there are,
the code reads header packets until it reaches the first data packet,
then stuffs them into your crazy extradata format, and puts the data
packet back into the demuxer so that the first frame is properly decoded.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: mplayer.theora-exp.patch-0.9.1
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20050524/83ef568d/attachment.txt>


More information about the MPlayer-dev-eng mailing list