bufferlist: Use a GQueue instead of a GList

Adding a buffer to the end of a GstBufferList is supposed to be a fast
operation, but it was not since the iterator does not advance its
nextpointer when adding buffers and GList does not have a tail pointer.
Using a GQueue to store the buffers makes it easier to add buffers to
the end of the list and this operation will now be much more efficient.

Adding an entire GList of buffers using
gst_buffer_list_iterator_add_list() will however have to iterate over
the list being added to be able to update the tail pointer in the
GQueue.

diff --git a/gst/gstbufferlist.c b/gst/gstbufferlist.c
index 358b5d1..887320c 100644 (file)
--- a/gst/gstbufferlist.c
+++ b/gst/gstbufferlist.c
@@ -142,7 +142,7 @@ struct _GstBufferList
 {
   GstMiniObject mini_object;
 
-  GList *buffers;
+  GQueue *buffers;
 };
 
 struct _GstBufferListClass
@@ -180,7 +180,7 @@ _gst_buffer_list_initialize (void)
 static void
 gst_buffer_list_init (GstBufferList * list)
 {
-  list->buffers = NULL;
+  list->buffers = g_queue_new ();
 
   GST_LOG ("init %p", list);
 }
@@ -194,14 +194,14 @@ gst_buffer_list_finalize (GstBufferList * list)
 
   GST_LOG ("finalize %p", list);
 
-  tmp = list->buffers;
+  tmp = list->buffers->head;
   while (tmp) {
     if (tmp->data != GROUP_START && tmp->data != STOLEN) {
       gst_buffer_unref (GST_BUFFER_CAST (tmp->data));
     }
     tmp = tmp->next;
   }
-  g_list_free (list->buffers);
+  g_queue_free (list->buffers);
 
 /* Not chaining up because GstMiniObject::finalize() does nothing
   GST_MINI_OBJECT_CLASS (gst_buffer_list_parent_class)->finalize
@@ -212,17 +212,16 @@ static GstBufferList *
 _gst_buffer_list_copy (GstBufferList * list)
 {
   GstBufferList *list_copy;
+  GQueue *buffers_copy;
   GList *tmp;
 
   g_return_val_if_fail (list != NULL, NULL);
 
-  list_copy = gst_buffer_list_new ();
-
   /* shallow copy of list and pointers */
-  list_copy->buffers = g_list_copy (list->buffers);
+  buffers_copy = g_queue_copy (list->buffers);
 
   /* ref all buffers in the list */
-  tmp = list_copy->buffers;
+  tmp = list->buffers->head;
   while (tmp) {
     if (tmp->data != GROUP_START && tmp->data != STOLEN) {
       tmp->data = gst_buffer_ref (GST_BUFFER_CAST (tmp->data));
@@ -230,6 +229,10 @@ _gst_buffer_list_copy (GstBufferList * list)
     tmp = g_list_next (tmp);
   }
 
+  list_copy = gst_buffer_list_new ();
+  g_queue_free (list_copy->buffers);
+  list_copy->buffers = buffers_copy;
+
   return list_copy;
 }
 
@@ -285,7 +288,7 @@ gst_buffer_list_n_groups (GstBufferList * list)
 
   g_return_val_if_fail (list != NULL, 0);
 
-  tmp = list->buffers;
+  tmp = list->buffers->head;
   n = 0;
   while (tmp) {
     if (tmp->data == GROUP_START) {
@@ -322,7 +325,7 @@ gst_buffer_list_foreach (GstBufferList * list, GstBufferListFunc func,
   g_return_if_fail (list != NULL);
   g_return_if_fail (func != NULL);
 
-  next = list->buffers;
+  next = list->buffers->head;
   group = idx = 0;
   while (next) {
     GstBuffer *buffer;
@@ -348,7 +351,7 @@ gst_buffer_list_foreach (GstBufferList * list, GstBufferListFunc func,
       /* the function changed the buffer */
       if (buffer == NULL) {
         /* we were asked to remove the item */
-        list->buffers = g_list_delete_link (list->buffers, tmp);
+        g_queue_delete_link (list->buffers, tmp);
         idx--;
       } else {
         /* change the buffer */
@@ -393,7 +396,7 @@ gst_buffer_list_get (GstBufferList * list, guint group, guint idx)
 
   g_return_val_if_fail (list != NULL, NULL);
 
-  tmp = list->buffers;
+  tmp = list->buffers->head;
   cgroup = 0;
   while (tmp) {
     if (tmp->data == GROUP_START) {
@@ -445,7 +448,7 @@ gst_buffer_list_iterate (GstBufferList * list)
 
   it = g_slice_new (GstBufferListIterator);
   it->list = list;
-  it->next = list->buffers;
+  it->next = list->buffers->head;
   it->last_returned = NULL;
 
   return it;
@@ -524,11 +527,14 @@ gst_buffer_list_iterator_add (GstBufferListIterator * it, GstBuffer * buffer)
   g_return_if_fail (buffer != NULL);
 
   /* adding before the first group start is not allowed */
-  g_return_if_fail (it->next != it->list->buffers);
+  g_return_if_fail (it->next != it->list->buffers->head);
 
-  /* cheap insert into the GList */
-  it->list->buffers = g_list_insert_before (it->list->buffers, it->next,
-      buffer);
+  /* cheap insert into the GQueue */
+  if (it->next != NULL) {
+    g_queue_insert_before (it->list->buffers, it->next, buffer);
+  } else {
+    g_queue_push_tail (it->list->buffers, buffer);
+  }
 }
 
 /**
@@ -550,31 +556,33 @@ void
 gst_buffer_list_iterator_add_list (GstBufferListIterator * it, GList * list)
 {
   GList *last;
+  guint len;
 
   g_return_if_fail (it != NULL);
-  g_return_if_fail (it->next != it->list->buffers);
+  g_return_if_fail (it->next != it->list->buffers->head);
 
   if (list == NULL)
     return;
 
-  if (it->next) {
-    last = list;
-    while (last->next)
-      last = last->next;
+  last = list;
+  len = 1;
+  while (last->next) {
+    last = last->next;
+    len++;
+  }
 
+  if (it->next) {
     last->next = it->next;
     list->prev = it->next->prev;
     it->next->prev = last;
     if (list->prev)
       list->prev->next = list;
   } else {
-    last = it->list->buffers;
-    while (last->next)
-      last = last->next;
-
-    last->next = list;
-    list->prev = last;
+    it->list->buffers->tail->next = list;
+    list->prev = it->list->buffers->tail;
+    it->list->buffers->tail = last;
   }
+  it->list->buffers->length += len;
 }
 
 /**
@@ -599,9 +607,12 @@ gst_buffer_list_iterator_add_group (GstBufferListIterator * it)
     it->next = g_list_next (it->next);
   }
 
-  /* cheap insert of a group start into the GList */
-  it->list->buffers = g_list_insert_before (it->list->buffers, it->next,
-      GROUP_START);
+  /* cheap insert of a group start into the GQueue */
+  if (it->next != NULL) {
+    g_queue_insert_before (it->list->buffers, it->next, GROUP_START);
+  } else {
+    g_queue_push_tail (it->list->buffers, GROUP_START);
+  }
 }
 
 /**
@@ -706,7 +717,7 @@ gst_buffer_list_iterator_remove (GstBufferListIterator * it)
   if (it->last_returned->data != STOLEN) {
     gst_buffer_unref (it->last_returned->data);
   }
-  it->list->buffers = g_list_delete_link (it->list->buffers, it->last_returned);
+  g_queue_delete_link (it->list->buffers, it->last_returned);
   it->last_returned = NULL;
 }
 

diff --git a/tests/check/gst/gstbufferlist.c b/tests/check/gst/gstbufferlist.c
index 6d3d7d6..0885644 100644 (file)
--- a/tests/check/gst/gstbufferlist.c
+++ b/tests/check/gst/gstbufferlist.c
@@ -783,7 +783,14 @@ GST_START_TEST (test_list)
   gst_buffer_list_iterator_add_group (it);
   gst_buffer_list_iterator_add_list (it, l);
 
-  for (i = 0; i < 10; i++) {
+  /* add a buffer */
+  gst_buffer_list_iterator_add (it, buffer_from_string ("10"));
+
+  /* add another list */
+  l = g_list_append (NULL, buffer_from_string ("11"));
+  gst_buffer_list_iterator_add_list (it, l);
+
+  for (i = 0; i < 12; i++) {
     GstBuffer *buf;
     gchar name[10];