rtpbin: avoid some structure copies

Don't make copied in the getter and setter for SDES in the RTPSource. This
avoids a couple of copies of the SDES structure when generating RTCP
packets.

gst/rtpmanager/rtpsession.c

@@ -875,17 +875,21 @@ rtp_session_get_sdes_string (RTPSession * sess, GstRTCPSDESType type)
  *
  * Get the SDES data as a #GstStructure
  *
- * Returns: a GstStructure with SDES items for @sess.
+ * Returns: a GstStructure with SDES items for @sess. This function returns a
+ * copy of the SDES structure, use gst_structure_free() after usage.
  */
 GstStructure *
 rtp_session_get_sdes_struct (RTPSession * sess)
 {
-  GstStructure *result;
+  const GstStructure *sdes;
+  GstStructure *result = NULL;
 
   g_return_val_if_fail (RTP_IS_SESSION (sess), NULL);
 
   RTP_SESSION_LOCK (sess);
-  result = rtp_source_get_sdes_struct (sess->source);
+  sdes = rtp_source_get_sdes_struct (sess->source);
+  if (sdes)
+    result = gst_structure_copy (sdes);
   RTP_SESSION_UNLOCK (sess);
 
   return result;
@@ -896,15 +900,16 @@ rtp_session_get_sdes_struct (RTPSession * sess)
  * @sess: an #RTSPSession
  * @sdes: a #GstStructure
  *
- * Set the SDES data as a #GstStructure.
+ * Set the SDES data as a #GstStructure. This function makes a copy of @sdes.
  */
 void
 rtp_session_set_sdes_struct (RTPSession * sess, const GstStructure * sdes)
 {
+  g_return_if_fail (sdes);
   g_return_if_fail (RTP_IS_SESSION (sess));
 
   RTP_SESSION_LOCK (sess);
-  rtp_source_set_sdes_struct (sess->source, sdes);
+  rtp_source_set_sdes_struct (sess->source, gst_structure_copy (sdes));
   RTP_SESSION_UNLOCK (sess);
 }
 
@@ -1717,10 +1722,9 @@ rtp_session_process_sdes (RTPSession * sess, GstRTCPPacket * packet,
       j++;
     }
 
+    /* takes ownership of sdes */
     changed = rtp_source_set_sdes_struct (source, sdes);
 
-    gst_structure_free (sdes);
-
     source->validated = TRUE;
 
     if (created)
@@ -2318,7 +2322,7 @@ static void
 session_sdes (RTPSession * sess, ReportData * data)
 {
   GstRTCPPacket *packet = &data->packet;
-  GstStructure *sdes;
+  const GstStructure *sdes;
   gint i, n_fields;
 
   /* add SDES packet */
@@ -2371,8 +2375,6 @@ session_sdes (RTPSession * sess, ReportData * data)
     }
   }
 
-  gst_structure_free (sdes);
-
   data->has_sdes = TRUE;
 }
 

gst/rtpmanager/rtpsource.c

@@ -286,30 +286,34 @@ rtp_source_create_stats (RTPSource * src)
  * rtp_source_get_sdes_struct:
  * @src: an #RTPSource
  *
- * Get the SDES from @src.
+ * Get the SDES from @src. See the SDES property for more details.
  *
- * Returns: %GstStructure of type "application/x-rtp-source-sdes", see
- * the SDES property for more details.
+ * Returns: %GstStructure of type "application/x-rtp-source-sdes". The result is
+ * valid until the SDES items of @src are modified.
  */
-GstStructure *
+const GstStructure *
 rtp_source_get_sdes_struct (RTPSource * src)
 {
   g_return_val_if_fail (RTP_IS_SOURCE (src), NULL);
 
-  return gst_structure_copy (src->sdes);
+  return src->sdes;
 }
 
 static gboolean
 sdes_struct_compare_func (GQuark field_id, const GValue * value,
     gpointer user_data)
 {
-  GstStructure *old = GST_STRUCTURE (user_data);
-  const gchar *field = g_quark_to_string (field_id);
+  GstStructure *old;
+  const gchar *field;
+
+  old = GST_STRUCTURE (user_data);
+  field = g_quark_to_string (field_id);
 
   if (!gst_structure_has_field (old, field))
     return FALSE;
 
   g_assert (G_VALUE_HOLDS_STRING (value));
+
   return strcmp (g_value_get_string (value), gst_structure_get_string (old,
           field)) == 0;
 }
@@ -322,15 +326,16 @@ sdes_struct_compare_func (GQuark field_id, const GValue * value,
  * Store the @sdes in @src. @sdes must be a structure of type
  * "application/x-rtp-source-sdes", see the SDES property for more details.
  *
+ * This function takes ownership of @sdes.
+ *
  * Returns: %FALSE if the SDES was unchanged.
  */
 gboolean
-rtp_source_set_sdes_struct (RTPSource * src, const GstStructure * sdes)
+rtp_source_set_sdes_struct (RTPSource * src, GstStructure * sdes)
 {
   gboolean changed;
 
   g_return_val_if_fail (RTP_IS_SOURCE (src), FALSE);
-
   g_return_val_if_fail (strcmp (gst_structure_get_name (sdes),
           "application/x-rtp-source-sdes") == 0, FALSE);
 
@@ -338,7 +343,9 @@ rtp_source_set_sdes_struct (RTPSource * src, const GstStructure * sdes)
 
   if (changed) {
     gst_structure_free (src->sdes);
-    src->sdes = gst_structure_copy (sdes);
+    src->sdes = sdes;
+  } else {
+    gst_structure_free (sdes);
   }
 
   return changed;
@@ -384,7 +391,7 @@ rtp_source_get_property (GObject * object, guint prop_id,
       g_value_set_boolean (value, rtp_source_is_sender (src));
       break;
     case PROP_SDES:
-      g_value_take_boxed (value, rtp_source_get_sdes_struct (src));
+      g_value_set_boxed (value, rtp_source_get_sdes_struct (src));
       break;
     case PROP_STATS:
       g_value_take_boxed (value, rtp_source_create_stats (src));

gst/rtpmanager/rtpsource.h

@@ -181,8 +181,9 @@ void            rtp_source_update_caps         (RTPSource *src, GstCaps *caps);
 gboolean        rtp_source_set_sdes_string     (RTPSource *src, GstRTCPSDESType type,
                                                 const gchar *data);
 gchar*          rtp_source_get_sdes_string     (RTPSource *src, GstRTCPSDESType type);
-GstStructure *  rtp_source_get_sdes_struct     (RTPSource * src);
-gboolean        rtp_source_set_sdes_struct     (RTPSource * src, const GstStructure *sdes);
+const GstStructure *
+                rtp_source_get_sdes_struct     (RTPSource * src);
+gboolean        rtp_source_set_sdes_struct     (RTPSource * src, GstStructure *sdes);
 
 /* handling network address */
 void            rtp_source_set_rtp_from        (RTPSource *src, GstNetAddress *address);