From 3064bf4a86ad7ee927af3ebac2aeffebac1d1ce2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim-Philipp=20M=C3=BCller?= Date: Thu, 12 Apr 2012 13:56:48 +0100 Subject: [PATCH] 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 --- gst/asfdemux/asfpacket.c | 49 ++++++++++++++++++++++++++++++++-------- gst/asfdemux/asfpacket.h | 6 ++--- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/gst/asfdemux/asfpacket.c b/gst/asfdemux/asfpacket.c index 6100673f7b..6efaee0c8c 100644 --- 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 dd8bd86f01..a1722c85cb 100644 --- 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