gdppay: refactor payloading code a little

Get rid of some indirections and inefficiencies,
just payload things directly which gives us more
control over what memory is allocated where and
how and makes things much simpler. In particular,
we can now allocate the payload header plus the
GstMemory to represent it in one go.

gst/gdp/dataprotocol.c

@@ -1,6 +1,7 @@
 /* GStreamer
  * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu>
  * Copyright (C) 2004,2006 Thomas Vander Stichele <thomas at apestaart dot org>
+ * Copyright (C) 2014 Tim-Philipp Müller <tim centricular com>
  *
  * dataprotocol.c: Functions implementing the GStreamer Data Protocol
  *
@@ -134,26 +135,25 @@ static guint16 gst_dp_crc (const guint8 * buffer, guint length);
 static guint16 gst_dp_crc_from_memory_maps (const GstMapInfo * maps,
     guint n_maps);
 
-/*** HELPER FUNCTIONS ***/
+/* payloading functions */
 
-static gboolean
+GstBuffer *
-gst_dp_header_from_buffer_any (GstBuffer * buffer, GstDPHeaderFlag flags,
+gst_dp_payload_buffer (GstBuffer * buffer, GstDPHeaderFlag flags)
-    guint * length, guint8 ** header, GstDPVersion version)
 {
+  GstBuffer *ret_buf;
+  GstMapInfo map;
+  GstMemory *mem;
   guint8 *h;
   guint16 flags_mask;
   guint16 header_crc = 0, crc = 0;
   gsize buffer_size;
 
-  g_return_val_if_fail (GST_IS_BUFFER (buffer), FALSE);
+  mem = gst_allocator_alloc (NULL, GST_DP_HEADER_LENGTH, NULL);
-  g_return_val_if_fail (length, FALSE);
+  gst_memory_map (mem, &map, GST_MAP_READWRITE);
-  g_return_val_if_fail (header, FALSE);
+  h = memset (map.data, 0, map.size);
-
-  *length = GST_DP_HEADER_LENGTH;
-  h = g_malloc0 (GST_DP_HEADER_LENGTH);
 
   /* version, flags, type */
-  GST_DP_INIT_HEADER (h, version, flags, GST_DP_PAYLOAD_BUFFER);
+  GST_DP_INIT_HEADER (h, GST_DP_VERSION_1_0, flags, GST_DP_PAYLOAD_BUFFER);
 
   if ((flags & GST_DP_HEADER_FLAG_CRC_PAYLOAD)) {
     GstMapInfo *maps;
@@ -211,32 +211,41 @@ gst_dp_header_from_buffer_any (GstBuffer * buffer, GstDPHeaderFlag flags,
   /* payload CRC */
   GST_WRITE_UINT16_BE (h + 60, crc);
 
-  GST_MEMDUMP ("created header from buffer", h, GST_DP_HEADER_LENGTH);
+  GST_MEMDUMP ("payload header for buffer", h, GST_DP_HEADER_LENGTH);
-  *header = h;
+  gst_memory_unmap (mem, &map);
-  return TRUE;
+
+  ret_buf = gst_buffer_new ();
+
+  /* header */
+  gst_buffer_append_memory (ret_buf, mem);
+
+  /* buffer data */
+  return gst_buffer_append (ret_buf, gst_buffer_ref (buffer));
 }
 
-static gboolean
+GstBuffer *
-gst_dp_packet_from_caps_any (const GstCaps * caps, GstDPHeaderFlag flags,
+gst_dp_payload_caps (const GstCaps * caps, GstDPHeaderFlag flags)
-    guint * length, guint8 ** header, guint8 ** payload, GstDPVersion version)
 {
+  GstBuffer *buf;
+  GstMapInfo map;
+  GstMemory *mem;
   guint8 *h;
   guchar *string;
   guint payload_length;
 
-  g_return_val_if_fail (GST_IS_CAPS (caps), FALSE);
+  g_assert (GST_IS_CAPS (caps));
-  g_return_val_if_fail (length, FALSE);
-  g_return_val_if_fail (header, FALSE);
-  g_return_val_if_fail (payload, FALSE);
 
-  *length = GST_DP_HEADER_LENGTH;
+  buf = gst_buffer_new ();
-  h = g_malloc0 (GST_DP_HEADER_LENGTH);
+
+  mem = gst_allocator_alloc (NULL, GST_DP_HEADER_LENGTH, NULL);
+  gst_memory_map (mem, &map, GST_MAP_READWRITE);
+  h = memset (map.data, 0, map.size);
 
   string = (guchar *) gst_caps_to_string (caps);
   payload_length = strlen ((gchar *) string) + 1;       /* include trailing 0 */
 
   /* version, flags, type */
-  GST_DP_INIT_HEADER (h, version, flags, GST_DP_PAYLOAD_CAPS);
+  GST_DP_INIT_HEADER (h, GST_DP_VERSION_1_0, flags, GST_DP_PAYLOAD_CAPS);
 
   /* buffer properties */
   GST_WRITE_UINT32_BE (h + 6, payload_length);
@@ -247,12 +256,75 @@ gst_dp_packet_from_caps_any (const GstCaps * caps, GstDPHeaderFlag flags,
 
   GST_DP_SET_CRC (h, flags, string, payload_length);
 
-  GST_MEMDUMP ("created header from caps", h, GST_DP_HEADER_LENGTH);
+  GST_MEMDUMP ("payload header for caps", h, GST_DP_HEADER_LENGTH);
-  *header = h;
+  gst_memory_unmap (mem, &map);
-  *payload = string;
+
-  return TRUE;
+  /* header */
+  gst_buffer_append_memory (buf, mem);
+
+  /* caps string */
+  gst_buffer_append_memory (buf,
+      gst_memory_new_wrapped (0, string, payload_length, 0, payload_length,
+          string, g_free));
+
+  return buf;
 }
 
+GstBuffer *
+gst_dp_payload_event (const GstEvent * event, GstDPHeaderFlag flags)
+{
+  GstBuffer *buf;
+  GstMapInfo map;
+  GstMemory *mem;
+  guint8 *h;
+  guint32 pl_length;            /* length of payload */
+  guchar *string = NULL;
+  const GstStructure *structure;
+
+  g_assert (GST_IS_EVENT (event));
+
+  buf = gst_buffer_new ();
+
+  mem = gst_allocator_alloc (NULL, GST_DP_HEADER_LENGTH, NULL);
+  gst_memory_map (mem, &map, GST_MAP_READWRITE);
+  h = memset (map.data, 0, map.size);
+
+  structure = gst_event_get_structure ((GstEvent *) event);
+  if (structure) {
+    string = (guchar *) gst_structure_to_string (structure);
+    GST_LOG ("event %p has structure, string %s", event, string);
+    pl_length = strlen ((gchar *) string) + 1;  /* include trailing 0 */
+  } else {
+    GST_LOG ("event %p has no structure", event);
+    pl_length = 0;
+  }
+
+  /* version, flags, type */
+  GST_DP_INIT_HEADER (h, GST_DP_VERSION_1_0, flags,
+      GST_DP_PAYLOAD_EVENT_NONE + GST_EVENT_TYPE (event));
+
+  /* length */
+  GST_WRITE_UINT32_BE (h + 6, pl_length);
+  /* timestamp */
+  GST_WRITE_UINT64_BE (h + 10, GST_EVENT_TIMESTAMP (event));
+
+  GST_DP_SET_CRC (h, flags, string, pl_length);
+
+  GST_MEMDUMP ("payload header for event", h, GST_DP_HEADER_LENGTH);
+  gst_memory_unmap (mem, &map);
+
+  /* header */
+  gst_buffer_append_memory (buf, mem);
+
+  /* event string */
+  if (pl_length > 0) {
+    gst_buffer_append_memory (buf,
+        gst_memory_new_wrapped (0, string, pl_length, 0, pl_length,
+            string, g_free));
+  }
+
+  return buf;
+}
 
 /*** PUBLIC FUNCTIONS ***/
 
@@ -399,68 +471,6 @@ gst_dp_header_payload_type (const guint8 * header)
   return GST_DP_HEADER_PAYLOAD_TYPE (header);
 }
 
-/* payloading functions */
-
-gboolean
-gst_dp_buffer_to_header (GstBuffer * buffer, GstDPHeaderFlag flags,
-    guint * length, guint8 ** header)
-{
-  return gst_dp_header_from_buffer_any (buffer, flags, length, header,
-      GST_DP_VERSION_1_0);
-}
-
-gboolean
-gst_dp_caps_to_header (const GstCaps * caps, GstDPHeaderFlag flags,
-    guint * length, guint8 ** header, guint8 ** payload)
-{
-  return gst_dp_packet_from_caps_any (caps, flags, length, header, payload,
-      GST_DP_VERSION_1_0);
-}
-
-gboolean
-gst_dp_event_to_header (const GstEvent * event, GstDPHeaderFlag flags,
-    guint * length, guint8 ** header, guint8 ** payload)
-{
-  guint8 *h;
-  guint32 pl_length;            /* length of payload */
-  guchar *string = NULL;
-  const GstStructure *structure;
-
-  g_return_val_if_fail (GST_IS_EVENT (event), FALSE);
-  g_return_val_if_fail (length, FALSE);
-  g_return_val_if_fail (header, FALSE);
-  g_return_val_if_fail (payload, FALSE);
-
-  *length = GST_DP_HEADER_LENGTH;
-  h = g_malloc0 (GST_DP_HEADER_LENGTH);
-
-  structure = gst_event_get_structure ((GstEvent *) event);
-  if (structure) {
-    string = (guchar *) gst_structure_to_string (structure);
-    GST_LOG ("event %p has structure, string %s", event, string);
-    pl_length = strlen ((gchar *) string) + 1;  /* include trailing 0 */
-  } else {
-    GST_LOG ("event %p has no structure", event);
-    pl_length = 0;
-  }
-
-  /* version, flags, type */
-  GST_DP_INIT_HEADER (h, GST_DP_VERSION_1_0, flags,
-      GST_DP_PAYLOAD_EVENT_NONE + GST_EVENT_TYPE (event));
-
-  /* length */
-  GST_WRITE_UINT32_BE (h + 6, pl_length);
-  /* timestamp */
-  GST_WRITE_UINT64_BE (h + 10, GST_EVENT_TIMESTAMP (event));
-
-  GST_DP_SET_CRC (h, flags, string, pl_length);
-
-  GST_MEMDUMP ("created header from event", h, GST_DP_HEADER_LENGTH);
-  *header = h;
-  *payload = string;
-  return TRUE;
-}
-
 /*** DEPACKETIZING FUNCTIONS ***/
 
 /**

gst/gdp/dataprotocol.h

@@ -87,22 +87,14 @@ GstEvent *      gst_dp_event_from_packet        (guint header_length,
                                                 const guint8 * payload);
 
 /* payloading GstBuffer/GstEvent/GstCaps */
-gboolean        gst_dp_buffer_to_header         (GstBuffer * buffer,
+GstBuffer *     gst_dp_payload_buffer           (GstBuffer      * buffer,
-                                                 GstDPHeaderFlag flags,
+                                                 GstDPHeaderFlag  flags);
-                                                 guint * length,
-                                                 guint8 ** header);
 
-gboolean        gst_dp_caps_to_header           (const GstCaps * caps,
+GstBuffer *     gst_dp_payload_caps             (const GstCaps  * caps,
-                                                 GstDPHeaderFlag flags,
+                                                 GstDPHeaderFlag  flags);
-                                                 guint * length,
-                                                 guint8 ** header,
-                                                 guint8 ** payload);
 
-gboolean        gst_dp_event_to_header          (const GstEvent * event,
+GstBuffer *     gst_dp_payload_event            (const GstEvent * event,
-                                                 GstDPHeaderFlag flags,
+                                                 GstDPHeaderFlag  flags);
-                                                 guint * length,
-                                                 guint8 ** header,
-                                                 guint8 ** payload);
 
 /* validation */
 gboolean        gst_dp_validate_header          (guint header_length,

gst/gdp/gstgdppay.c

@@ -1,5 +1,6 @@
 /* GStreamer
  * Copyright (C) 2006 Thomas Vander Stichele <thomas at apestaart dot org>
+ * Copyright (C) 2014 Tim-Philipp Müller <tim centricular com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -196,91 +197,19 @@ gst_gdp_stamp_buffer (GstGDPPay * this, GstBuffer * buffer)
 static GstBuffer *
 gst_gdp_buffer_from_caps (GstGDPPay * this, GstCaps * caps)
 {
-  GstBuffer *headerbuf;
+  return gst_dp_payload_caps (caps, this->header_flag);
-  guint8 *header, *payload;
-  guint len, plen;
-
-  if (!gst_dp_caps_to_header (caps, this->header_flag, &len, &header, &payload))
-    goto packet_failed;
-
-  GST_LOG_OBJECT (this, "creating GDP header and payload buffer from caps");
-  headerbuf = gst_buffer_new_wrapped (header, len);
-
-  plen = gst_dp_header_payload_length (header);
-
-  gst_buffer_append_memory (headerbuf,
-      gst_memory_new_wrapped (0, payload, plen, 0, plen, payload, g_free));
-
-  return headerbuf;
-
-  /* ERRORS */
-packet_failed:
-  {
-    GST_WARNING_OBJECT (this, "could not create GDP header from caps");
-    return NULL;
-  }
 }
 
 static GstBuffer *
 gst_gdp_pay_buffer_from_buffer (GstGDPPay * this, GstBuffer * buffer)
 {
-  GstBuffer *headerbuf;
+  return gst_dp_payload_buffer (buffer, this->header_flag);
-  guint8 *header;
-  guint len;
-
-  if (!gst_dp_buffer_to_header (buffer, this->header_flag, &len, &header))
-    goto no_buffer;
-
-  GST_LOG_OBJECT (this, "creating GDP header and payload buffer from buffer");
-  headerbuf = gst_buffer_new_wrapped (header, len);
-
-  /* we do not want to lose the ref on the incoming buffer */
-  gst_buffer_ref (buffer);
-
-  return gst_buffer_append (headerbuf, buffer);
-
-  /* ERRORS */
-no_buffer:
-  {
-    GST_WARNING_OBJECT (this, "could not create GDP header from buffer");
-    return NULL;
-  }
 }
 
 static GstBuffer *
 gst_gdp_buffer_from_event (GstGDPPay * this, GstEvent * event)
 {
-  GstBuffer *headerbuf;
+  return gst_dp_payload_event (event, this->header_flag);
-  GstBuffer *payloadbuf;
-  guint8 *header, *payload;
-  guint len, plen;
-  gboolean ret;
-
-  ret =
-      gst_dp_event_to_header (event, this->header_flag, &len, &header,
-      &payload);
-  if (!ret)
-    goto no_event;
-
-  GST_LOG_OBJECT (this, "creating GDP header and payload buffer from event");
-  headerbuf = gst_buffer_new_wrapped (header, len);
-
-  payloadbuf = gst_buffer_new ();
-  plen = gst_dp_header_payload_length (header);
-  if (plen && payload != NULL) {
-    gst_buffer_append_memory (payloadbuf,
-        gst_memory_new_wrapped (0, payload, plen, 0, plen, payload, g_free));
-  }
-
-  return gst_buffer_append (headerbuf, payloadbuf);
-
-  /* ERRORS */
-no_event:
-  {
-    GST_WARNING_OBJECT (this, "could not create GDP header from event %s (%d)",
-        gst_event_type_get_name (event->type), event->type);
-    return NULL;
-  }
 }
 
 static void