[MPlayer-dev-eng] RFC conformance for stream/rtp.c (on 64 bit platforms )

[Bogdan Mustiata]

Thank you for the reply,

On Sunday 27 May 2007 12:12:52 Reimar Döffinger wrote:
> Hello,
>
> On Sun, May 27, 2007 at 06:21:47AM +0300, Bogdan Mustiata wrote:
> > Conforming to RFC1889 (RTP: A Transport Protocol for Real-Time
> > Applications), the RTP header should be 4 byte aligned. Unfortunately, in
> > stream/rtp.c, ints are used to define the RTP header and they are not
> > allways 4 byte as assumed in that code on 64bit platforms.
>
> Actually int is quite certainly always 4 bytes.

Even if that's true (and I tend to belive you are right), you are still at the 
mercy of the compiler. And that's just not the way to be (tm) :). In my 
opinion you should allways use well defined, as in number of bytes, types for 
this kind of structures.

(I know for a fact that int is 2 bytes for 16bit machines so it's not 100% 
certain to be 4 bytes on every architecture. The sole fact that you don't 
have this warranty should make you consider this patch).

>
> > This patch changes the int usage to the reliable uint32_t and int32_t
> > types.
>
> Cleaning up that mess to not use structs at all to pack the data -
> possibly by moving bitstream functions from libavcodec to libavutil and
> using those - would make a lot more sense IMO.

I agree. I was just reading through mplayer's code and I've seen the mentioned 
piece of code. 

And the code, as it is _now_, is surely looking better IMHO with the patch 
applied than without. But this is what I think, and not necessarily the right 
thing to do.

>
> Greetings,
> Reimar Döffinger
> _______________________________________________
> MPlayer-dev-eng mailing list
> MPlayer-dev-eng at mplayerhq.hu
> http://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng

Thanks once again for review,
Bogdan