[Bug 729760] appsrc: Changing caps and pushing buffers is not serialized

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=729760
  GStreamer | gst-plugins-base | git

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #281987|none                        |needs-work
             status|                            |

--- Comment #14 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-07-30 09:43:58 UTC ---
Review of attachment 281987:
 --> (https://bugzilla.gnome.org/review?bug=729760&attachment=281987)

::: gst-libs/gst/app/gstappsrc.c
@@ +526,3 @@
   priv->min_percent = DEFAULT_PROP_MIN_PERCENT;
+  priv->last_caps = gst_caps_new_any ();
+  priv->current_caps = gst_caps_new_any ();

Why distinguish current and last caps?

The caps property should always return the last caps, while the current caps
are the currently used ones. That's the reason, right? Makes sense :)

And why not keep them at NULL and handle that as before?

@@ +541,3 @@
+      gst_buffer_unref (caps_or_buffer);
+    } else {
+      gst_caps_unref (caps_or_buffer);

You can use gst_mini_object_unref() on caps and buffers

@@ +1058,3 @@
     if (!g_queue_is_empty (priv->queue)) {
       guint buf_size;
+      void *caps_or_buffer;

Use GstMiniObject* as the type of these everywhere

@@ +1061,3 @@
+
+      caps_or_buffer = g_queue_peek_head (priv->queue);
+      if (GST_IS_CAPS (caps_or_buffer)) {

Why peek and later pop? Just integrating that loop directly into the while
seems more useful

while (!empty) {
  obj = pop();
  if (caps) {
    negotiate();
  } else {
    handle buffer();
  }
}

Seems clearer

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.