[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 #256254|none                        |needs-work
             status|                            |

--- Comment #31 from Sebastian Dröge (slomo) <slomo at circular-chaos.org> 2013-10-03 11:39:03 UTC ---
Review of attachment 256254:
 --> (https://bugzilla.gnome.org/review?bug=707605&attachment=256254)

::: plugins/elements/gststreamiddemux.c
@@ +46,3 @@
+{
+  PROP_0,
+  PROP_NUM_SRC_PADS,

You can get this via the GstElement API, please remove the property

@@ +47,3 @@
+  PROP_0,
+  PROP_NUM_SRC_PADS,
+  PROP_ACTIVE_SRC_PAD,

This one however is useful, like in input-selector

@@ +140,3 @@
+
+  if (demux->stream_id_pairs) {
+    g_hash_table_remove_all (demux->stream_id_pairs);

You are leaking the srcpad references and stream id strings here.

And this should all also be done in GstElement::change_state() when going from
PAUSED to READY.

@@ +161,3 @@
+      break;
+    case PROP_ACTIVE_SRC_PAD:
+      g_value_set_object (value, demux->active_srcpad);

This needs locking, like all accesses to the active_srcpad not from the
streaming thread (non-serialized events for example)

@@ +190,3 @@
+
+  if (gst_streamid_demux_srcpad_has_stream_id (demux, stream_id)) {
+    GST_DEBUG_OBJECT (demux, "already created src pad");

maybe just return the pad from here?

@@ +201,3 @@
+  gst_object_ref (srcpad);
+  g_hash_table_insert (demux->stream_id_pairs, g_strdup (stream_id),
+      (gpointer) srcpad);

If you use the rwlock for the hash table, use it consistently everywhere. Needs
a write lock here

@@ +247,3 @@
+
+  g_rw_lock_reader_lock (&demux->lock);
+  srcpad = g_hash_table_lookup (demux->stream_id_pairs, stream_id);

Should probably return a new reference to the pad here. It might go away when
you handle non-serialized events and the element is currently going from
PAUSED->READY.

@@ +258,3 @@
+
+static gboolean
+gst_streamid_demux_srcpad_has_stream_id (GstStreamidDemux * demux,

This is basically the same as get_srcpad_by_stream_id(). Remove it

@@ +293,3 @@
+    if (GST_EVENT_TYPE (event) == GST_EVENT_STREAM_START) {
+      gst_event_parse_stream_start (event, &stream_id);
+      if (stream_id != NULL) {

stream_id *must* be not-null. Otherwise fail here

@@ +300,3 @@
+          demux->active_srcpad = active_srcpad;
+        }
+        GST_PAD_STREAM_LOCK (demux->active_srcpad);

Not needed, please remove

-- 
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.