asfdemux: fix performance issue, especially with high-bitrate streams

Two things were suboptimal from a performance point of view:

a) consider a large media object such as a video keyframe, which
   may be split up into multiple fragments. We would assemble
   the media object as follows:
     buf = join (join (join (frag1, frag2), frag3), frag4)
   which causes many unnecessary memcpy()s, and malloc/free,
   which could easily add up to a multiple of the actual object
   size. To avoid this, we allocate a buffer of the size needed
   from the start and copy fragments into that directly.

b) for every fragment to join, we would create a sub-buffer
   before joining it (which would discard the sub-buffer again),
   leading to unnecessary miniobject create/free churn.

Conflicts:

	gst/asfdemux/asfpacket.c
	gst/asfdemux/asfpacket.h

diff --git a/gst/asfdemux/asfpacket.c b/gst/asfdemux/asfpacket.c
index 6100673..6efaee0 100644 (file)
--- a/gst/asfdemux/asfpacket.c
+++ b/gst/asfdemux/asfpacket.c
@@ -382,28 +382,58 @@ gst_asf_demux_parse_payload (GstASFDemux * demux, AsfPacket * packet,
 
     GST_LOG_OBJECT (demux, "payload length: %u", payload_len);
 
-    if (payload_len > 0) {
+    if (payload_len == 0) {
+      GST_DEBUG_OBJECT (demux, "skipping empty payload");
+    } else if (payload.mo_offset == 0 && payload.mo_size == payload_len) {
+      /* if the media object is not fragmented, just create a sub-buffer */
+      GST_LOG_OBJECT (demux, "unfragmented media object size %u", payload_len);
       payload.buf = asf_packet_create_payload_buffer (packet, p_data, p_size,
           payload_len);
+      payload.buf_filled = payload_len;
+      gst_asf_payload_queue_for_stream (demux, &payload, stream);
+    } else {
+      const guint8 *payload_data = *p_data;
+
+      g_assert (payload_len <= *p_size);
+
+      *p_data += payload_len;
+      *p_size -= payload_len;
 
       /* n-th fragment of a fragmented media object? */
       if (payload.mo_offset != 0) {
         AsfPayload *prev;
 
         if ((prev = asf_payload_find_previous_fragment (&payload, stream))) {
-          if (payload.mo_offset != gst_buffer_get_size (prev->buf)) {
+          if (prev->buf == NULL || payload.mo_size != prev->mo_size ||
+              payload.mo_offset >= gst_buffer_get_size (prev->buf) ||
+              payload.mo_offset + payload_len >
+              gst_buffer_get_size (prev->buf)) {
             GST_WARNING_OBJECT (demux, "Offset doesn't match previous data?!");
+          } else {
+            /* we assume fragments are payloaded with increasing mo_offset */
+            if (payload.mo_offset != prev->buf_filled) {
+              GST_WARNING_OBJECT (demux, "media object payload discontinuity: "
+                  "offset=%u vs buf_filled=%u", payload.mo_offset,
+                  prev->buf_filled);
+            }
+            gst_buffer_fill (prev->buf, payload.mo_offset,
+                payload_data, payload_len);
+            prev->buf_filled =
+                MAX (prev->buf_filled, payload.mo_offset + payload_len);
+            GST_LOG_OBJECT (demux, "Merged media object fragments, size now %u",
+                prev->buf_filled);
           }
-          /* note: buffer join/merge might not preserve buffer flags */
-          prev->buf = gst_buffer_append (prev->buf, payload.buf);
-          GST_LOG_OBJECT (demux,
-              "Merged fragments, merged size: %" G_GSIZE_FORMAT,
-              gst_buffer_get_size (prev->buf));
         } else {
-          gst_buffer_unref (payload.buf);
+          GST_DEBUG_OBJECT (demux, "n-th payload fragment, but don't have "
+              "any previous fragment, ignoring payload");
         }
-        payload.buf = NULL;
       } else {
+        GST_LOG_OBJECT (demux, "allocating buffer of size %u for fragmented "
+            "media object", payload.mo_size);
+        payload.buf = gst_buffer_new_allocate (NULL, payload.mo_size, NULL);
+        gst_buffer_fill (payload.buf, 0, payload_data, payload_len);
+        payload.buf_filled = payload_len;
+
         gst_asf_payload_queue_for_stream (demux, &payload, stream);
       }
     }
@@ -445,6 +475,7 @@ gst_asf_demux_parse_payload (GstASFDemux * demux, AsfPacket * packet,
       if (G_LIKELY (sub_payload_len > 0)) {
         payload.buf = asf_packet_create_payload_buffer (packet,
             &payload_data, &payload_len, sub_payload_len);
+        payload.buf_filled = sub_payload_len;
 
         payload.ts = ts;
         if (G_LIKELY (ts_delta))

diff --git a/gst/asfdemux/asfpacket.h b/gst/asfdemux/asfpacket.h
index dd8bd86..a1722c8 100644 (file)
--- a/gst/asfdemux/asfpacket.h
+++ b/gst/asfdemux/asfpacket.h
@@ -32,6 +32,8 @@ typedef struct {
   guint         mo_number;         /* media object number (unused)           */
   guint         mo_offset;         /* offset (timestamp for compressed data) */
   guint         mo_size;           /* size of media-object-to-be, or 0       */
+  guint         buf_filled;        /* how much of the mo data we got so far  */
+  GstBuffer    *buf;               /* buffer to assemble media-object or NULL*/
   guint         rep_data_len;      /* should never be more than 256, since   */
   guint8        rep_data[256];     /* the length should be stored in a byte  */
   GstClockTime  ts;
@@ -41,7 +43,6 @@ typedef struct {
   gboolean      interlaced;        /* default: FALSE */
   gboolean      tff;
   gboolean      rff;
-  GstBuffer    *buf;
 } AsfPayload;
 
 typedef struct {
@@ -58,9 +59,8 @@ typedef struct {
 
 gboolean   gst_asf_demux_parse_packet (GstASFDemux * demux, GstBuffer * buf);
 
-/* FIXME - gst_buffer_get_size is slow */
 #define gst_asf_payload_is_complete(payload) \
-    (gst_buffer_get_size ((payload)->buf) >= (payload)->mo_size)
+    ((payload)->buf_filled >= (payload)->mo_size)
 
 G_END_DECLS