From b2948f2453dea369a4e7d9d2eeb35507146ebae2 Mon Sep 17 00:00:00 2001 From: Haakon Sporsheim Date: Fri, 7 Sep 2007 16:46:05 +0000 Subject: [PATCH] gst-libs/gst/rtp/gstrtpbuffer.c: Fix up GstRTPHeader helper struct so that compilers will not under any circumstances... Original commit message from CVS: Based on patch by: Haakon Sporsheim * gst-libs/gst/rtp/gstrtpbuffer.c: Fix up GstRTPHeader helper struct so that compilers will not under any circumstances add padding in between our fields, as currently happens with MSVC on win32, because that would lead to us sending out RTP payloads with broken RTP headers (#471194). Fix assertion guards for gst_rtp_buffer_get_csrc() and _set_csrc(). * tests/check/Makefile.am: * tests/check/libs/.cvsignore: * tests/check/libs/rtp.c: Add some simple unit tests for GstRTPBuffer. Some are disabled because the code tested still needs fixing (set_csrc() does not work). --- ChangeLog | 17 +++++ gst-libs/gst/rtp/gstrtpbuffer.c | 27 ++++---- tests/check/Makefile.am | 7 ++ tests/check/libs/.gitignore | 6 +- tests/check/libs/rtp.c | 140 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 184 insertions(+), 13 deletions(-) create mode 100644 tests/check/libs/rtp.c diff --git a/ChangeLog b/ChangeLog index 8d11206..d33376d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,22 @@ 2007-09-07 Tim-Philipp Müller + Based on patch by: Haakon Sporsheim + + * gst-libs/gst/rtp/gstrtpbuffer.c: + Fix up GstRTPHeader helper struct so that compilers will not under + any circumstances add padding in between our fields, as currently + happens with MSVC on win32, because that would lead to us sending + out RTP payloads with broken RTP headers (#471194). + Fix assertion guards for gst_rtp_buffer_get_csrc() and _set_csrc(). + + * tests/check/Makefile.am: + * tests/check/libs/.cvsignore: + * tests/check/libs/rtp.c: + Add some simple unit tests for GstRTPBuffer. Some are disabled + because the code tested still needs fixing (set_csrc() does not work). + +2007-09-07 Tim-Philipp Müller + * win32/MANIFEST: * win32/common/gstrtsp-enumtypes.c: * win32/common/gstrtsp-enumtypes.h: diff --git a/gst-libs/gst/rtp/gstrtpbuffer.c b/gst-libs/gst/rtp/gstrtpbuffer.c index e2c7732..9ace6bd 100644 --- a/gst-libs/gst/rtp/gstrtpbuffer.c +++ b/gst-libs/gst/rtp/gstrtpbuffer.c @@ -40,6 +40,9 @@ #define GST_RTP_HEADER_LEN 12 +/* Note: we use bitfields here to make sure the compiler doesn't add padding + * between fields on certain architectures; can't assume aligned access either + */ typedef struct _GstRTPHeader { #if G_BYTE_ORDER == G_LITTLE_ENDIAN @@ -59,10 +62,10 @@ typedef struct _GstRTPHeader #else #error "G_BYTE_ORDER should be big or little endian." #endif - guint16 seq; /* sequence number */ - guint32 timestamp; /* timestamp */ - guint32 ssrc; /* synchronization source */ - guint32 csrc[1]; /* optional CSRC list */ + unsigned int seq:16; /* sequence number */ + unsigned int timestamp:32; /* timestamp */ + unsigned int ssrc:32; /* synchronization source */ + guint8 csrclist[4]; /* optional CSRC list, 32 bits each */ } GstRTPHeader; #define GST_RTP_HEADER_VERSION(buf) (((GstRTPHeader *)(GST_BUFFER_DATA (buf)))->version) @@ -74,8 +77,10 @@ typedef struct _GstRTPHeader #define GST_RTP_HEADER_SEQ(buf) (((GstRTPHeader *)(GST_BUFFER_DATA (buf)))->seq) #define GST_RTP_HEADER_TIMESTAMP(buf) (((GstRTPHeader *)(GST_BUFFER_DATA (buf)))->timestamp) #define GST_RTP_HEADER_SSRC(buf) (((GstRTPHeader *)(GST_BUFFER_DATA (buf)))->ssrc) -#define GST_RTP_HEADER_CSRC(buf,i) (((GstRTPHeader *)(GST_BUFFER_DATA (buf)))->csrc[i]) - +#define GST_RTP_HEADER_CSRC_LIST_OFFSET(buf,i) \ + GST_BUFFER_DATA (buf) + \ + G_STRUCT_OFFSET(GstRTPHeader, csrclist) + \ + ((i) * sizeof(guint32)) #define GST_RTP_HEADER_CSRC_SIZE(buf) (GST_RTP_HEADER_CSRC_COUNT(buf) * sizeof (guint32)) /** @@ -110,7 +115,7 @@ gst_rtp_buffer_allocate_data (GstBuffer * buffer, guint payload_len, GST_RTP_HEADER_VERSION (buffer) = GST_RTP_VERSION; GST_RTP_HEADER_PADDING (buffer) = FALSE; GST_RTP_HEADER_EXTENSION (buffer) = FALSE; - GST_RTP_HEADER_CSRC_COUNT (buffer) = 0; + GST_RTP_HEADER_CSRC_COUNT (buffer) = 0; /* FIXME: not csrc_count? */ GST_RTP_HEADER_MARKER (buffer) = FALSE; GST_RTP_HEADER_PAYLOAD_TYPE (buffer) = 0; GST_RTP_HEADER_SEQ (buffer) = 0; @@ -687,9 +692,9 @@ gst_rtp_buffer_get_csrc (GstBuffer * buffer, guint8 idx) { g_return_val_if_fail (GST_IS_BUFFER (buffer), 0); g_return_val_if_fail (GST_BUFFER_DATA (buffer) != NULL, 0); - g_return_val_if_fail (GST_RTP_HEADER_CSRC_COUNT (buffer) < idx, 0); + g_return_val_if_fail (idx < GST_RTP_HEADER_CSRC_COUNT (buffer), 0); - return g_ntohl (GST_RTP_HEADER_CSRC (buffer, idx)); + return GST_READ_UINT32_BE (GST_RTP_HEADER_CSRC_LIST_OFFSET (buffer, idx)); } /** @@ -705,9 +710,9 @@ gst_rtp_buffer_set_csrc (GstBuffer * buffer, guint8 idx, guint32 csrc) { g_return_if_fail (GST_IS_BUFFER (buffer)); g_return_if_fail (GST_BUFFER_DATA (buffer) != NULL); - g_return_if_fail (GST_RTP_HEADER_CSRC_COUNT (buffer) < idx); + g_return_if_fail (idx < GST_RTP_HEADER_CSRC_COUNT (buffer)); - GST_RTP_HEADER_CSRC (buffer, idx) = g_htonl (csrc); + GST_WRITE_UINT32_BE (GST_RTP_HEADER_CSRC_LIST_OFFSET (buffer, idx), csrc); } /** diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am index b4e7e8c..bfcc091 100644 --- a/tests/check/Makefile.am +++ b/tests/check/Makefile.am @@ -93,6 +93,7 @@ check_PROGRAMS = \ libs/mixer \ libs/netbuffer \ libs/pbutils \ + libs/rtp \ libs/tag \ libs/video \ pipelines/simple-launch-lines \ @@ -167,6 +168,12 @@ libs_netbuffer_LDADD = \ $(top_builddir)/gst-libs/gst/netbuffer/libgstnetbuffer-@GST_MAJORMINOR@.la \ $(LDADD) +libs_rtp_CFLAGS = \ + $(GST_PLUGINS_BASE_CFLAGS) \ + $(AM_CFLAGS) +libs_rtp_LDADD = \ + $(top_builddir)/gst-libs/gst/rtp/libgstrtp-@GST_MAJORMINOR@.la $(LDADD) + libs_tag_CFLAGS = \ $(GST_PLUGINS_BASE_CFLAGS) \ $(AM_CFLAGS) diff --git a/tests/check/libs/.gitignore b/tests/check/libs/.gitignore index 2cc35d0..d57728a 100644 --- a/tests/check/libs/.gitignore +++ b/tests/check/libs/.gitignore @@ -2,9 +2,11 @@ audio cddabasesrc fft +mixer netbuffer +pbutils +rtp tag utils video -pbutils -mixer + diff --git a/tests/check/libs/rtp.c b/tests/check/libs/rtp.c new file mode 100644 index 0000000..a3a0e6a --- /dev/null +++ b/tests/check/libs/rtp.c @@ -0,0 +1,140 @@ +/* GStreamer unit tests for the RTP support library + * + * Copyright (C) 2007 Tim-Philipp Müller + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Library General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Library General Public License for more details. + * + * You should have received a copy of the GNU Library General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include + +#include +#include + +#define RTP_HEADER_LEN 12 + +GST_START_TEST (test_rtp_buffer) +{ + GstBuffer *buf; + guint8 *data; + + /* check GstRTPHeader structure alignment and packing */ + buf = gst_rtp_buffer_new_allocate (16, 4, 3); + fail_unless (buf != NULL); + fail_unless_equals_int (GST_BUFFER_SIZE (buf), + RTP_HEADER_LEN + 16 + 4 + 4 * 3); + data = GST_BUFFER_DATA (buf); + + /* check version in bitfield */ + gst_rtp_buffer_set_version (buf, 3); + fail_unless_equals_int (gst_rtp_buffer_get_version (buf), 3); + fail_unless_equals_int ((data[0] & 0xC0) >> 6, 3); + gst_rtp_buffer_set_version (buf, 0); + fail_unless_equals_int (gst_rtp_buffer_get_version (buf), 0); + fail_unless_equals_int ((data[0] & 0xC0) >> 6, 0); + + /* check sequence offset */ + gst_rtp_buffer_set_seq (buf, 0xF2C9); + fail_unless_equals_int (gst_rtp_buffer_get_seq (buf), 0xF2C9); + fail_unless_equals_int (GST_READ_UINT16_BE (data + 2), 0xF2C9); + gst_rtp_buffer_set_seq (buf, 0); + fail_unless_equals_int (gst_rtp_buffer_get_seq (buf), 0); + fail_unless_equals_int (GST_READ_UINT16_BE (data + 2), 0); + + /* check timestamp offset */ + gst_rtp_buffer_set_timestamp (buf, 432191); + fail_unless_equals_int (GST_READ_UINT32_BE (data + 4), 432191); + fail_unless_equals_int (gst_rtp_buffer_get_timestamp (buf), 432191); + gst_rtp_buffer_set_timestamp (buf, 0); + fail_unless_equals_int (gst_rtp_buffer_get_timestamp (buf), 0); + fail_unless_equals_int (GST_READ_UINT32_BE (data + 4), 0); + + /* check ssrc offset */ + gst_rtp_buffer_set_ssrc (buf, 0xf04043C2); + fail_unless_equals_int (gst_rtp_buffer_get_ssrc (buf), 0xf04043c2); + fail_unless_equals_int (GST_READ_UINT32_BE (data + 4 + 4), 0xf04043c2); + gst_rtp_buffer_set_ssrc (buf, 0); + fail_unless_equals_int (gst_rtp_buffer_get_ssrc (buf), 0); + fail_unless_equals_int (GST_READ_UINT32_BE (data + 4 + 4), 0); + gst_buffer_unref (buf); + + /* FIXME: this is broken, the _set_csrc doesn't work because the csrc count + * is initialised to 0 and _set_csrc() then has an assertion to make sure + * index is < the value in the struct ... */ +#if 0 + /* and again, this time with CSRCs */ + { + volatile guint32 ret; + + buf = gst_rtp_buffer_new_allocate (16, 4, 3); + fail_unless (buf != NULL); + fail_unless_equals_int (GST_BUFFER_SIZE (buf), + RTP_HEADER_LEN + 16 + 4 + 4 * 3); + + data = GST_BUFFER_DATA (buf); + + /* the default value is 0, because we haven't set any yet (FIXME?) */ + fail_unless_equals_int (gst_rtp_buffer_get_csrc_count (buf), 0); + ASSERT_CRITICAL (ret = gst_rtp_buffer_get_csrc (buf, 0)); + + data += RTP_HEADER_LEN; /* skip the other header stuff */ + gst_rtp_buffer_set_csrc (buf, 0, 0xf7c0); + fail_unless_equals_int (GST_READ_UINT32_BE (data + 0 * 4), 0xf7c0); + fail_unless_equals_int (gst_rtp_buffer_get_csrc_count (buf), 1); /* FIXME? */ + gst_rtp_buffer_set_csrc (buf, 1, 0xf7c1); + fail_unless_equals_int (GST_READ_UINT32_BE (data + 1 * 4), 0xf7c1); + fail_unless_equals_int (gst_rtp_buffer_get_csrc_count (buf), 2); /* FIXME? */ + gst_rtp_buffer_set_csrc (buf, 2, 0xf7c2); + fail_unless_equals_int (GST_READ_UINT32_BE (data + 2 * 4), 0xf7c2); + fail_unless_equals_int (gst_rtp_buffer_get_csrc_count (buf), 3); /* FIXME? */ + ASSERT_CRITICAL (gst_rtp_buffer_set_csrc (buf, 3, 0xf123)); + gst_buffer_unref (buf); + } +#endif +} + +GST_END_TEST; + +static Suite * +rtp_suite (void) +{ + Suite *s = suite_create ("rtp support library"); + TCase *tc_chain = tcase_create ("general"); + + suite_add_tcase (s, tc_chain); + tcase_add_test (tc_chain, test_rtp_buffer); + return s; +} + +int +main (int argc, char **argv) +{ + int nf; + + Suite *s = rtp_suite (); + SRunner *sr = srunner_create (s); + + gst_check_init (&argc, &argv); + + srunner_run_all (sr, CK_NORMAL); + nf = srunner_ntests_failed (sr); + srunner_free (sr); + + return nf; +} -- 2.7.4