asfdemux: fix performance issue, especially with high-bitrate streams
authorTim-Philipp Müller <tim.muller@collabora.co.uk>
Thu, 12 Apr 2012 12:56:48 +0000 (13:56 +0100)
committerNikhil Devshatwar <a0132237@ti.com>
Fri, 17 May 2013 09:40:56 +0000 (15:10 +0530)
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.

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

index fb95770141a9b34e2e6b271cb0b30f49e0fcf485..4eb4d06f421826e5a7d8d7fbf4353cfff7698c51 100644 (file)
@@ -381,27 +381,57 @@ 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_SIZE (prev->buf)) {
+          if (prev->buf == NULL || payload.mo_size != prev->mo_size ||
+              payload.mo_offset >= GST_BUFFER_SIZE (prev->buf) ||
+              payload.mo_offset + payload_len > GST_BUFFER_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);
+            }
+            memcpy (GST_BUFFER_DATA (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_join (prev->buf, payload.buf);
-          GST_LOG_OBJECT (demux, "Merged fragments, merged size: %u",
-              GST_BUFFER_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_and_alloc (payload.mo_size);
+        memcpy (GST_BUFFER_DATA (payload.buf), payload_data, payload_len);
+        payload.buf_filled = payload_len;
+
         gst_asf_payload_queue_for_stream (demux, &payload, stream);
       }
     }
@@ -443,6 +473,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))
index 283088101c78ca9f94498e72e1081a91ad4a77da..15ffef5b0a336c3f105db628bc81a193b0b8ba3f 100644 (file)
@@ -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,7 +59,7 @@ typedef struct {
 gboolean   gst_asf_demux_parse_packet (GstASFDemux * demux, GstBuffer * buf);
 
 #define gst_asf_payload_is_complete(payload) \
-    (GST_BUFFER_SIZE ((payload)->buf) >= (payload)->mo_size)
+    ((payload)->buf_filled >= (payload)->mo_size)
 
 G_END_DECLS