From 0dcab96d88376e31459aaeb601cc04a45a3d8966 Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Mon, 3 Apr 2017 16:41:49 +1000 Subject: [PATCH] sdp/media: caps_from_media() don't modify the input media Performing a gst_sdp_media_get_caps_from_media() would result in changing fields in the GstSDPMedia violating the const tag in the function declaration. Before there would be a line with a=rtpmap:96 VP8/90000 after, that attribute would only contain a=rtpmap:96 Fix by performing modifications on duplicated strings instead of on the internal values. Also add a simple test for checking that the representation doesn't change by a gst_sdp_media_get_caps_from_media() --- gst-libs/gst/sdp/gstsdpmessage.c | 61 +++++++++++++++++++++----------- tests/check/libs/sdp.c | 34 ++++++++++++++++++ 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/gst-libs/gst/sdp/gstsdpmessage.c b/gst-libs/gst/sdp/gstsdpmessage.c index a00ca47ba7..11d86a00f9 100644 --- a/gst-libs/gst/sdp/gstsdpmessage.c +++ b/gst-libs/gst/sdp/gstsdpmessage.c @@ -3358,6 +3358,7 @@ gst_sdp_get_attribute_for_pt (const GstSDPMedia * media, const gchar * name, return NULL; } +/* this may modify the input string (and resets) */ #define PARSE_INT(p, del, res) \ G_STMT_START { \ gchar *t = p; \ @@ -3365,12 +3366,15 @@ G_STMT_START { \ if (p == NULL) \ res = -1; \ else { \ + char prev = *p; \ *p = '\0'; \ - p++; \ res = atoi (t); \ + *p = prev; \ + p++; \ } \ } G_STMT_END +/* this may modify the string without reset */ #define PARSE_STRING(p, del, res) \ G_STMT_START { \ gchar *t = p; \ @@ -3398,44 +3402,53 @@ static gboolean gst_sdp_parse_rtpmap (const gchar * rtpmap, gint * payload, gchar ** name, gint * rate, gchar ** params) { - gchar *p, *t; + gchar *p, *t, *orig_value; - p = (gchar *) rtpmap; + p = orig_value = g_strdup (rtpmap); PARSE_INT (p, " ", *payload); if (*payload == -1) - return FALSE; + goto fail; SKIP_SPACES (p); if (*p == '\0') - return FALSE; + goto fail; PARSE_STRING (p, "/", *name); if (*name == NULL) { GST_DEBUG ("no rate, name %s", p); /* no rate, assume -1 then, this is not supposed to happen but RealMedia * streams seem to omit the rate. */ - *name = p; + *name = g_strdup (p); *rate = -1; - return TRUE; + *params = NULL; + goto out; + } else { + *name = strdup (*name); } t = p; p = strstr (p, "/"); if (p == NULL) { *rate = atoi (t); - return TRUE; + *params = NULL; + goto out; } - *p = '\0'; p++; *rate = atoi (t); - t = p; if (*p == '\0') - return TRUE; - *params = t; + *params = NULL; + else + *params = g_strdup (p); +out: + g_free (orig_value); return TRUE; + +fail: + g_free (orig_value); + return FALSE; } /** @@ -3527,8 +3540,8 @@ gst_sdp_media_get_caps_from_media (const GstSDPMedia * media, gint pt) { GstCaps *caps; const gchar *rtpmap; - const gchar *fmtp; - const gchar *framesize; + gchar *fmtp = NULL; + gchar *framesize = NULL; gchar *name = NULL; gint rate = -1; gchar *params = NULL; @@ -3600,11 +3613,11 @@ gst_sdp_media_get_caps_from_media (const GstSDPMedia * media, gint pt) } /* parse optional fmtp: field */ - if ((fmtp = gst_sdp_get_attribute_for_pt (media, "fmtp", pt))) { + if ((fmtp = g_strdup (gst_sdp_get_attribute_for_pt (media, "fmtp", pt)))) { gchar *p; gint payload = 0; - p = (gchar *) fmtp; + p = fmtp; /* p is now of the format [=];... */ PARSE_INT (p, " ", payload); @@ -3664,11 +3677,12 @@ gst_sdp_media_get_caps_from_media (const GstSDPMedia * media, gint pt) } /* parse framesize: field */ - if ((framesize = gst_sdp_media_get_attribute_val (media, "framesize"))) { + if ((framesize = + g_strdup (gst_sdp_media_get_attribute_val (media, "framesize")))) { gchar *p; /* p is now of the format - */ - p = (gchar *) framesize; + p = framesize; PARSE_INT (p, " ", payload); if (payload != -1 && payload == pt) { @@ -3679,18 +3693,25 @@ gst_sdp_media_get_caps_from_media (const GstSDPMedia * media, gint pt) /* parse rtcp-fb: field */ gst_sdp_media_add_rtcp_fb_attributes_from_media (media, pt, caps); +out: + g_free (framesize); + g_free (fmtp); + g_free (name); + g_free (params); return caps; /* ERRORS */ no_rtpmap: { GST_ERROR ("rtpmap type not given for dynamic payload %d", pt); - return NULL; + caps = NULL; + goto out; } no_rate: { GST_ERROR ("rate unknown for payload type %d", pt); - return NULL; + caps = NULL; + goto out; } } diff --git a/tests/check/libs/sdp.c b/tests/check/libs/sdp.c index 62be9bc378..7da67ead94 100644 --- a/tests/check/libs/sdp.c +++ b/tests/check/libs/sdp.c @@ -525,6 +525,39 @@ GST_START_TEST (media_from_caps_rtcp_fb_pt_101) gst_sdp_message_free (message); } +GST_END_TEST +GST_START_TEST (caps_from_media_really_const) +{ + GstSDPMessage *message; + glong length = -1; + const GstSDPMedia *media1; + gchar *serialized; + GstCaps *caps; + + /* BUG: gst_sdp_media_get_caps_from_media() used to modify the media passed + * thus violating the const tag */ + + gst_sdp_message_new (&message); + gst_sdp_message_parse_buffer ((guint8 *) sdp, length, message); + + serialized = gst_sdp_message_as_text (message); + fail_unless (g_strcmp0 (serialized, sdp) == 0); + g_free (serialized); + + media1 = gst_sdp_message_get_media (message, 0); + fail_unless (media1 != NULL); + + caps = gst_sdp_media_get_caps_from_media (media1, 96); + + serialized = gst_sdp_message_as_text (message); + fail_unless (g_strcmp0 (serialized, sdp) == 0); + g_free (serialized); + + gst_caps_unref (caps); + + gst_sdp_message_free (message); +} + GST_END_TEST /* * End of test cases @@ -540,6 +573,7 @@ sdp_suite (void) tcase_add_test (tc_chain, boxed); tcase_add_test (tc_chain, modify); tcase_add_test (tc_chain, caps_from_media); + tcase_add_test (tc_chain, caps_from_media_really_const); tcase_add_test (tc_chain, media_from_caps); tcase_add_test (tc_chain, caps_from_media_rtcp_fb); tcase_add_test (tc_chain, caps_from_media_rtcp_fb_all);