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.

3 files changed, 110 insertions, 179 deletions

diff --git a/gst/gdp/dataprotocol.c b/gst/gdp/dataprotocol.c
index 398d2420e..fbb46a6a9 100644
--- a/gst/gdp/dataprotocol.c
+++ b/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
-gst_dp_header_from_buffer_any (GstBuffer * buffer, GstDPHeaderFlag flags,
-    guint * length, guint8 ** header, GstDPVersion version)
+GstBuffer *
+gst_dp_payload_buffer (GstBuffer * buffer, GstDPHeaderFlag flags)
 {
+  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);
-  g_return_val_if_fail (length, FALSE);
-  g_return_val_if_fail (header, FALSE);
-
-  *length = GST_DP_HEADER_LENGTH;
-  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);
 
   /* 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);
-  *header = h;
-  return TRUE;
+  GST_MEMDUMP ("payload header for buffer", h, GST_DP_HEADER_LENGTH);
+  gst_memory_unmap (mem, &map);
+
+  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
-gst_dp_packet_from_caps_any (const GstCaps * caps, GstDPHeaderFlag flags,
-    guint * length, guint8 ** header, guint8 ** payload, GstDPVersion version)
+GstBuffer *
+gst_dp_payload_caps (const GstCaps * caps, GstDPHeaderFlag flags)
 {
+  GstBuffer *buf;
+  GstMapInfo map;
+  GstMemory *mem;
   guint8 *h;
   guchar *string;
   guint payload_length;
 
-  g_return_val_if_fail (GST_IS_CAPS (caps), FALSE);
-  g_return_val_if_fail (length, FALSE);
-  g_return_val_if_fail (header, FALSE);
-  g_return_val_if_fail (payload, FALSE);
+  g_assert (GST_IS_CAPS (caps));
+
+  buf = gst_buffer_new ();
 
-  *length = GST_DP_HEADER_LENGTH;
-  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);
-  *header = h;
-  *payload = string;
-  return TRUE;
+  GST_MEMDUMP ("payload header for caps", h, GST_DP_HEADER_LENGTH);
+  gst_memory_unmap (mem, &map);
+
+  /* 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 ***/
 
 /**
diff --git a/gst/gdp/dataprotocol.h b/gst/gdp/dataprotocol.h
index 17eb5e1ca..ac1d0107d 100644
--- a/gst/gdp/dataprotocol.h
+++ b/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,
-                                                 GstDPHeaderFlag flags,
-                                                 guint * length,
-                                                 guint8 ** header);
+GstBuffer *     gst_dp_payload_buffer           (GstBuffer      * buffer,
+                                                 GstDPHeaderFlag  flags);
 
-gboolean        gst_dp_caps_to_header           (const GstCaps * caps,
-                                                 GstDPHeaderFlag flags,
-                                                 guint * length,
-                                                 guint8 ** header,
-                                                 guint8 ** payload);
+GstBuffer *     gst_dp_payload_caps             (const GstCaps  * caps,
+                                                 GstDPHeaderFlag  flags);
 
-gboolean        gst_dp_event_to_header          (const GstEvent * event,
-                                                 GstDPHeaderFlag flags,
-                                                 guint * length,
-                                                 guint8 ** header,
-                                                 guint8 ** payload);
+GstBuffer *     gst_dp_payload_event            (const GstEvent * event,
+                                                 GstDPHeaderFlag  flags);
 
 /* validation */
 gboolean        gst_dp_validate_header          (guint header_length,
diff --git a/gst/gdp/gstgdppay.c b/gst/gdp/gstgdppay.c
index 51c99a8fa..751f5f691 100644
--- a/gst/gdp/gstgdppay.c
+++ b/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;
-  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;
-  }
+  return gst_dp_payload_caps (caps, this->header_flag);
 }
 
 static GstBuffer *
 gst_gdp_pay_buffer_from_buffer (GstGDPPay * this, GstBuffer * buffer)
 {
-  GstBuffer *headerbuf;
-  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;
-  }
+  return gst_dp_payload_buffer (buffer, this->header_flag);
 }
 
 static GstBuffer *
 gst_gdp_buffer_from_event (GstGDPPay * this, GstEvent * event)
 {
-  GstBuffer *headerbuf;
-  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;
-  }
+  return gst_dp_payload_event (event, this->header_flag);
 }
 
 static void