From 4638f15de282d660c16449073536d66441ced1bd Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Fri, 6 Apr 2012 16:46:58 +0200 Subject: [PATCH 1/4] queue2: don't update the current reading_pos in flush A flush from the upstream element should not make buffering go to 0, the next pull request might be inside a range that we have and then we don't need to buffer at all. If the next pull is outside anything we have, buffering will happen as usual anyway. --- plugins/elements/gstqueue2.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/plugins/elements/gstqueue2.c b/plugins/elements/gstqueue2.c index 0bda2e7804..77f68d94f6 100644 --- a/plugins/elements/gstqueue2.c +++ b/plugins/elements/gstqueue2.c @@ -2629,12 +2629,6 @@ gst_queue2_handle_src_event (GstPad * pad, GstEvent * event) /* now unblock the getrange function */ GST_QUEUE2_MUTEX_LOCK (queue); queue->srcresult = GST_FLOW_OK; - if (queue->current) { - /* forget the highest read offset, we'll calculate a new one when we - * get the next getrange request. We need to do this in order to reset - * the buffering percentage */ - queue->current->max_reading_pos = 0; - } GST_QUEUE2_MUTEX_UNLOCK (queue); /* when using a temp file, we eat the event */ From d05d29d0c99aa60b542bd5b8e98a87ae43b38880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim-Philipp=20M=C3=BCller?= Date: Sat, 7 Apr 2012 15:20:05 +0100 Subject: [PATCH 2/4] filesrc: set default block size from local define Doesn't actually change the default value, just makes use of the define there is. Superficial testing with fakesink and jpegdec did not reveal improved performance for bigger block sizes, so leave default as it is. --- plugins/elements/gstfilesrc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/elements/gstfilesrc.c b/plugins/elements/gstfilesrc.c index 87402c8ed9..1f3017a8b9 100644 --- a/plugins/elements/gstfilesrc.c +++ b/plugins/elements/gstfilesrc.c @@ -332,6 +332,8 @@ gst_file_src_init (GstFileSrc * src, GstFileSrcClass * g_class) src->sequential = DEFAULT_SEQUENTIAL; src->is_regular = FALSE; + + gst_base_src_set_blocksize (GST_BASE_SRC (src), DEFAULT_BLOCKSIZE); } static void From 930c0197218b216ba78a04750fc4706f31aa7d98 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Fri, 24 Feb 2012 12:51:24 +0100 Subject: [PATCH 3/4] atomicqueue: fix subtle race Fix a race where the reader would see the updated the tail pointer before the write could write the data into the queue. Fix this by having a separate reader tail pointer that is only incremented after the writer wrote the data. --- gst/gstatomicqueue.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/gst/gstatomicqueue.c b/gst/gstatomicqueue.c index 5b21be63ee..1ec9a4d465 100644 --- a/gst/gstatomicqueue.c +++ b/gst/gstatomicqueue.c @@ -57,6 +57,7 @@ struct _GstAQueueMem gpointer *array; volatile gint head; volatile gint tail; + volatile gint tail_read; GstAQueueMem *next; GstAQueueMem *free; }; @@ -84,6 +85,7 @@ new_queue_mem (guint size, gint pos) mem->array = g_new0 (gpointer, mem->size + 1); mem->head = pos; mem->tail = pos; + mem->tail_read = pos; mem->next = NULL; mem->free = NULL; @@ -234,7 +236,7 @@ gst_atomic_queue_peek (GstAtomicQueue * queue) head_mem = g_atomic_pointer_get (&queue->head_mem); head = g_atomic_int_get (&head_mem->head); - tail = g_atomic_int_get (&head_mem->tail); + tail = g_atomic_int_get (&head_mem->tail_read); size = head_mem->size; /* when we are not empty, we can continue */ @@ -291,7 +293,7 @@ gst_atomic_queue_pop (GstAtomicQueue * queue) head_mem = g_atomic_pointer_get (&queue->head_mem); head = g_atomic_int_get (&head_mem->head); - tail = g_atomic_int_get (&head_mem->tail); + tail = g_atomic_int_get (&head_mem->tail_read); size = head_mem->size; /* when we are not empty, we can continue */ @@ -380,6 +382,9 @@ gst_atomic_queue_push (GstAtomicQueue * queue, gpointer data) tail + 1)); tail_mem->array[tail & size] = data; + + /* and now the readers can read */ + g_atomic_int_inc (&tail_mem->tail_read); } /** @@ -408,7 +413,7 @@ gst_atomic_queue_length (GstAtomicQueue * queue) head = g_atomic_int_get (&head_mem->head); tail_mem = g_atomic_pointer_get (&queue->tail_mem); - tail = g_atomic_int_get (&tail_mem->tail); + tail = g_atomic_int_get (&tail_mem->tail_read); #ifdef LOW_MEM if (g_atomic_int_dec_and_test (&queue->num_readers)) From f251e3073b75c9f82333fc102e391b77aae2ee3a Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Fri, 24 Feb 2012 15:24:42 +0100 Subject: [PATCH 4/4] atomicqueue: fix race After a writer has written to its reserved write location, it can only make the location available for reading if all of the writers with lower locations have finished. --- gst/gstatomicqueue.c | 47 ++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/gst/gstatomicqueue.c b/gst/gstatomicqueue.c index 1ec9a4d465..e7bfccace8 100644 --- a/gst/gstatomicqueue.c +++ b/gst/gstatomicqueue.c @@ -56,7 +56,7 @@ struct _GstAQueueMem gint size; gpointer *array; volatile gint head; - volatile gint tail; + volatile gint tail_write; volatile gint tail_read; GstAQueueMem *next; GstAQueueMem *free; @@ -84,7 +84,7 @@ new_queue_mem (guint size, gint pos) mem->size = clp2 (MAX (size, 16)) - 1; mem->array = g_new0 (gpointer, mem->size + 1); mem->head = pos; - mem->tail = pos; + mem->tail_write = pos; mem->tail_read = pos; mem->next = NULL; mem->free = NULL; @@ -297,8 +297,9 @@ gst_atomic_queue_pop (GstAtomicQueue * queue) size = head_mem->size; /* when we are not empty, we can continue */ - if (G_LIKELY (head != tail)) - break; + if G_LIKELY + (head != tail) + break; /* else array empty, try to take next */ next = g_atomic_pointer_get (&head_mem->next); @@ -307,9 +308,10 @@ gst_atomic_queue_pop (GstAtomicQueue * queue) /* now we try to move the next array as the head memory. If we fail to do that, * some other reader managed to do it first and we retry */ - if (!G_ATOMIC_POINTER_COMPARE_AND_EXCHANGE (&queue->head_mem, head_mem, - next)) - continue; + if G_UNLIKELY + (!G_ATOMIC_POINTER_COMPARE_AND_EXCHANGE (&queue->head_mem, head_mem, + next)) + continue; /* when we managed to swing the head pointer the old head is now * useless and we add it to the freelist. We can't free the memory yet @@ -318,8 +320,8 @@ gst_atomic_queue_pop (GstAtomicQueue * queue) } ret = head_mem->array[head & size]; - } while (!g_atomic_int_compare_and_exchange (&head_mem->head, head, - head + 1)); + } while G_UNLIKELY + (!g_atomic_int_compare_and_exchange (&head_mem->head, head, head + 1)); #ifdef LOW_MEM /* decrement number of readers, when we reach 0 readers we can be sure that @@ -354,37 +356,44 @@ gst_atomic_queue_push (GstAtomicQueue * queue, gpointer data) tail_mem = g_atomic_pointer_get (&queue->tail_mem); head = g_atomic_int_get (&tail_mem->head); - tail = g_atomic_int_get (&tail_mem->tail); + tail = g_atomic_int_get (&tail_mem->tail_write); size = tail_mem->size; /* we're not full, continue */ - if (tail - head <= size) - break; + if G_LIKELY + (tail - head <= size) + break; /* else we need to grow the array, we store a mask so we have to add 1 */ mem = new_queue_mem ((size << 1) + 1, tail); /* try to make our new array visible to other writers */ - if (!G_ATOMIC_POINTER_COMPARE_AND_EXCHANGE (&queue->tail_mem, tail_mem, - mem)) { + if G_UNLIKELY + (!G_ATOMIC_POINTER_COMPARE_AND_EXCHANGE (&queue->tail_mem, tail_mem, + mem)) { /* we tried to swap the new writer array but something changed. This is * because some other writer beat us to it, we free our memory and try * again */ free_queue_mem (mem); continue; - } + } /* make sure that readers can find our new array as well. The one who * manages to swap the pointer is the only one who can set the next * pointer to the new array */ g_atomic_pointer_set (&tail_mem->next, mem); } - } while (!g_atomic_int_compare_and_exchange (&tail_mem->tail, tail, - tail + 1)); + } while G_UNLIKELY + (!g_atomic_int_compare_and_exchange (&tail_mem->tail_write, tail, tail + 1)); tail_mem->array[tail & size] = data; - /* and now the readers can read */ - g_atomic_int_inc (&tail_mem->tail_read); + /* now wait until all writers have completed their write before we move the + * tail_read to this new item. It is possible that other writers are still + * updating the previous array slots and we don't want to reveal their changes + * before they are done. FIXME, it would be nice if we didn't have to busy + * wait here. */ + while G_UNLIKELY + (!g_atomic_int_compare_and_exchange (&tail_mem->tail_read, tail, tail + 1)); } /**