[MPlayer-dev-eng] RFC conformance for stream/rtp.c (on 64 bit platforms )
Bogdan Mustiata
bogdan.mustiata at gmail.com
Sun May 27 11:49:21 CEST 2007
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
More information about the MPlayer-dev-eng
mailing list