[Bug 707605] Need "reverse-funnel" element

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=707605
  GStreamer | gstreamer (core) | git

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

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

--- Comment #55 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2013-10-31 20:50:10 UTC ---
Review of attachment 258551:
 --> (https://bugzilla.gnome.org/review?bug=707605&attachment=258551)

::: plugins/elements/gststreamiddemux.c
@@ +44,3 @@
+enum
+{
+  SIGNAL_ACTIVE_SRCPAD_CHANGED,

No need for this signal, call g_object_notify(self, "active-srcpad") instead.

@@ +103,3 @@
+
+  g_object_class_install_property (gobject_class, PROP_ACTIVE_SRC_PAD,
+      g_param_spec_object ("active-src-pad", "Active src pad",

Should probably just be called active-pad for consistency with input-selector

@@ +172,3 @@
+  switch (prop_id) {
+    case PROP_ACTIVE_SRC_PAD:
+      GST_PAD_STREAM_LOCK (demux->active_srcpad);

This is not correct, nothing is guaranteeing you here that active_srcpad might
not change at the time you try to lock it. You need to use another mutex, e.g.
the object lock as before and in the code below.

@@ +237,3 @@
+
+  GST_PAD_STREAM_LOCK (demux->active_srcpad);
+  if (demux->active_srcpad) {

Wrong lock here too

@@ +301,3 @@
+    } else if (demux->active_srcpad != active_srcpad) {
+      GST_PAD_STREAM_LOCK (demux->active_srcpad);
+      if (demux->active_srcpad != NULL) {

wrong lock

@@ +322,3 @@
+    GstPad *srcpad = NULL;
+    GST_PAD_STREAM_LOCK (demux->active_srcpad);
+    srcpad = gst_object_ref (demux->active_srcpad);

wrong lock

@@ +380,3 @@
+        gst_iterator_foreach (it,
+        (GstIteratorForeachFunction) gst_streamid_demux_release_srcpads,
demux);
+    gst_iterator_resync (it);

You must only call resync() if the iterator returned RESYNC. Otherwise you'll
loop forever here

::: tests/check/elements/streamiddemux.c
@@ +188,3 @@
+        gst_pad_get_current_caps (td.mysink[active_stream]), GST_FORMAT_BYTES,
+        g_strdup_printf ("test%d", active_stream));
+    g_object_get (td.demux, "active-src-pad", &active_srcpad, NULL);

You're leaking active_srcpad here, you probably also want to check if after the
push the active_srcpad is the one you expect

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