[Linphone-developers] oRTP bug + patch: endian problem for RTCP's cum_num_packet_lost

[Nicola Baldo]

Hi all,

I've found a bug in oRTP: both at transmission and reception of RTCP 
packets, the value of the field cum_num_packet_lost is messed up by the 
use of htonl() and ntohl() macros. The problem is that these macros are 
for 32 bit values, while the cum_num_packet_lost is only 24 bits. The 
bug is subtle, you can only notice it by having linphone running on a 
lossy link and noting that cum_num_packet_lost remains zero even if 
there are losses reported within the fraction_lost field.

I've attached a patch to the current cvs version. It fixes the problem, 
however to be honest it is definitely bad style code (i.e. some 
hand-made conversions in rtcp.c and rtcpparse.c within a "#ifdef 
WORDS_BIGENDIAN" statement).
It would be nicer to have some hton24() and ntoh24() macro defined 
somewhere else. Anyway, I hope this helps...

Regards,

Nicola

? linphone/.in
? linphone/config.h.in
? linphone/linphone-1.1.0pre1
? linphone/coreapi/.libs
? linphone/coreapi/arts.lo
? linphone/coreapi/authentication.lo
? linphone/coreapi/chat.lo
? linphone/coreapi/enum.lo
? linphone/coreapi/exevents.lo
? linphone/coreapi/friend.lo
? linphone/coreapi/liblinphone.la
? linphone/coreapi/linphonecore.lo
? linphone/coreapi/lpconfig.lo
? linphone/coreapi/misc.lo
? linphone/coreapi/presence.lo
? linphone/coreapi/proxy.lo
? linphone/coreapi/sdphandler.lo
? linphone/exosip/.deps
? linphone/exosip/.libs
? linphone/exosip/Makefile
? linphone/exosip/Makefile.in
? linphone/exosip/eXosip.lo
? linphone/exosip/eXutils.lo
? linphone/exosip/jauth.lo
? linphone/exosip/jcall.lo
? linphone/exosip/jcallback.lo
? linphone/exosip/jdialog.lo
? linphone/exosip/jevents.lo
? linphone/exosip/jfreinds.lo
? linphone/exosip/jidentity.lo
? linphone/exosip/jnotify.lo
? linphone/exosip/jpipe.lo
? linphone/exosip/jpublish.lo
? linphone/exosip/jreg.lo
? linphone/exosip/jrequest.lo
? linphone/exosip/jresponse.lo
? linphone/exosip/jsubscribe.lo
? linphone/exosip/jsubscribers.lo
? linphone/exosip/libeXosip.la
? linphone/exosip/misc.lo
? linphone/exosip/sdp_offans.lo
? linphone/exosip/udp.lo
? linphone/ipkg/Makefile
? linphone/ipkg/Makefile.in
? linphone/m4/Makefile
? linphone/m4/Makefile.in
? linphone/mediastreamer/test_speex
? linphone/mediastreamer2/.deps
? linphone/mediastreamer2/.libs
? linphone/mediastreamer2/Makefile
? linphone/mediastreamer2/Makefile.in
? linphone/mediastreamer2/alsacard.lo
? linphone/mediastreamer2/hpuxsndcard.lo
? linphone/mediastreamer2/jackcard.lo
? linphone/mediastreamer2/libmediastreamer2.la
? linphone/mediastreamer2/ms.lo
? linphone/mediastreamer2/msAlawdec.lo
? linphone/mediastreamer2/msAlawenc.lo
? linphone/mediastreamer2/msbuffer.lo
? linphone/mediastreamer2/msfifo.lo
? linphone/mediastreamer2/msfilter.lo
? linphone/mediastreamer2/msqueue.lo
? linphone/mediastreamer2/msringplayer.lo
? linphone/mediastreamer2/mssndread.lo
? linphone/mediastreamer2/mssndwrite.lo
? linphone/mediastreamer2/mssync.lo
? linphone/mediastreamer2/osscard.lo
? linphone/mediastreamer2/sndcard.lo
? linphone/oRTP/acinclude.m4
? linphone/oRTP/config.guess
? linphone/oRTP/config.sub
? linphone/oRTP/depcomp
? linphone/oRTP/install-sh
? linphone/oRTP/ltmain.sh
? linphone/oRTP/missing
? linphone/oRTP/ortp-0.7.1pre2.shar.gz
? linphone/oRTP/ortp-0.7.1pre2.tar.gz
? linphone/oRTP/build/win32/Makefile
? linphone/oRTP/build/win32/Makefile.in
? linphone/oRTP/include/Makefile
? linphone/oRTP/include/Makefile.in
? linphone/oRTP/include/ortp/Makefile
? linphone/oRTP/include/ortp/Makefile.in
? linphone/oRTP/src/rtcp.c.mod
? linphone/oRTP/src/rtcpparse.c.mine
? linphone/oRTP/src/rtcpparse.c.mod
? linphone/oRTP/src/tests/.deps
? linphone/oRTP/src/tests/.libs
? linphone/oRTP/src/tests/Makefile
? linphone/oRTP/src/tests/Makefile.in
? linphone/oRTP/src/tests/mrtprecv
? linphone/oRTP/src/tests/mrtpsend
? linphone/oRTP/src/tests/rtpmemtest
? linphone/oRTP/src/tests/rtprecv
? linphone/oRTP/src/tests/rtpsend
? linphone/oRTP/src/tests/test_timer
? linphone/oRTP/src/tests/tevmrtprecv
? linphone/oRTP/src/tests/tevrtprecv
? linphone/oRTP/src/tests/tevrtpsend
? linphone/share/linphone.pc
? linphone/support/.deps
? linphone/support/Makefile
? linphone/support/Makefile.in
Index: linphone/m4/ilbc.m4
===================================================================
RCS file: /cvsroot/linphone/linphone/m4/ilbc.m4,v
retrieving revision 1.2
diff -u -r1.2 ilbc.m4
--- linphone/m4/ilbc.m4 19 May 2005 15:56:58 -0000      1.2
+++ linphone/m4/ilbc.m4 13 Jul 2005 20:58:32 -0000
@@ -13,9 +13,11 @@
        CPPFLAGS=$ILBC_CFLAGS
        LDFLAGS_save=$LDFLAGS
        LDFLAGS=$ILBC_LIBS
+       dnl AC_CHECK_LIB adds ilbc to LIBS, I don't want that !
+       LIBS_save=$LIBS
        
AC_CHECK_HEADERS(iLBC_decode.h,[AC_CHECK_LIB(ilbc,iLBC_decode,ilbc_found=yes,ilbc_found=no)
        ],ilbc_found=no)
-       
+       LIBS=$LIBS_save
        CPPFLAGS=$CPPFLAGS_save
        LDFLAGS=$LDFLAGS_save
        
Index: linphone/oRTP/src/rtcp.c
===================================================================
RCS file: /cvsroot/linphone/linphone/oRTP/src/rtcp.c,v
retrieving revision 1.8
diff -u -r1.8 rtcp.c
--- linphone/oRTP/src/rtcp.c    16 Jun 2005 13:34:21 -0000      1.8
+++ linphone/oRTP/src/rtcp.c    13 Jul 2005 20:58:33 -0000
@@ -231,7 +231,14 @@
        
        b->ssrc=htonl(session->recv_ssrc);
        b->fraction_lost=loss_fraction;
-       b->cum_num_packet_lost=htonl((guint32)stream->stats.cum_packet_loss);
+#ifdef WORDS_BIGENDIAN
+       b->cum_num_packet_lost=((guint32)stream->stats.cum_packet_loss);
+#else
+       {
+         guint32 tmp=stream->stats.cum_packet_loss;
+         b->cum_num_packet_lost = ((tmp & 0x00ff0000) >>16) | ((tmp & 
0x000000ff) <<16) | (tmp & 0x0000ff00);            
+       }
+#endif
        b->interarrival_jitter=htonl((guint32) stream->jittctl.inter_jitter);
        b->ext_high_seq_num_rec=htonl(stream->hwrcv_extseq.one);
        b->lsr=htonl(stream->last_rcv_SR_ts);
Index: linphone/oRTP/src/rtcpparse.c
===================================================================
RCS file: /cvsroot/linphone/linphone/oRTP/src/rtcpparse.c,v
retrieving revision 1.1
diff -u -r1.1 rtcpparse.c
--- linphone/oRTP/src/rtcpparse.c       16 Jun 2005 13:34:21 -0000      1.1
+++ linphone/oRTP/src/rtcpparse.c       13 Jul 2005 20:58:33 -0000
@@ -48,7 +48,15 @@
       rcv_time_lsw = (guint32) 
((double)rcv_time_tv.tv_usec*(double)(1LL<<32)*1.0e-6);
       rcv_time = (rcv_time_msw<<16) | (rcv_time_lsw >> 16);
 
-      rb->cum_num_packet_lost = ntohl(rb->cum_num_packet_lost);
+#ifdef WORDS_BIGENDIAN
+      /* no conversion needed for rb->cum_num_packet_lost */
+#else
+       {
+         guint32 tmp=rb->cum_num_packet_lost;
+         rb->cum_num_packet_lost = ((tmp & 0x00ff0000) >>16) | ((tmp & 
0x000000ff) <<16) | (tmp & 0x0000ff00);           
+       }
+#endif
+
       rb->ext_high_seq_num_rec = ntohl(rb->ext_high_seq_num_rec);
       rb->interarrival_jitter = ntohl(rb->interarrival_jitter);
       rb->lsr = ntohl(rb->lsr);