[Bug 768709] streamiddemux: Reconfigure output pads when stream is changed from upstream

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=768709

Tim-Philipp Müller <t.i.m at zen.co.uk> changed:

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

--- Comment #5 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Comment on attachment 331290
  --> https://bugzilla.gnome.org/attachment.cgi?id=331290
streamiddemux: Reconfigure output pads when stream is changed from upstream

Couple of drive-by comments and nitpicks from me:


>+  if (demux->active_collection && demux->active_collection != collection) {
>+    GST_DEBUG_OBJECT (demux, "Handle stream changes : %" GST_PTR_FORMAT,
>+        collection);
>+    // release deactivated streams
>+    for (i = 0; i < gst_stream_collection_get_size (demux->active_collection);
>+        i++) {

- Please use /* old-style C comments */ not // c++ style comments.

- Please move the get_size() out of the loop (below as well)


>--- a/plugins/elements/gststreamiddemux.h
>+++ b/plugins/elements/gststreamiddemux.h
>@@ -39,6 +39,33 @@ G_BEGIN_DECLS
>   (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GST_TYPE_STREAMID_DEMUX))
> #define GST_IS_STREAMID_DEMUX_CLASS(klass) \
>   (G_TYPE_CHECK_CLASS_TYPE ((klass), GST_TYPE_STREAMID_DEMUX))
>+
>+#define GST_COLLECTION_GET_LOCK(demux) (&((GstStreamidDemux*)(demux))->collection_lock)
>+
>+#define GST_COLLECTION_LOCK(demux)   G_STMT_START { \
>+  GST_LOG_OBJECT (demux, "locking from thread %p", g_thread_self ()); \
>+  g_mutex_lock (GST_COLLECTION_GET_LOCK (demux)); \
>+  GST_LOG_OBJECT (demux, "locked from thread %p", g_thread_self ()); \
>+} G_STMT_END
>+
>+#define GST_COLLECTION_UNLOCK(demux)   G_STMT_START { \
>+  GST_LOG_OBJECT (demux, "unlocking from thread %p", g_thread_self ()); \
>+    g_mutex_unlock (GST_COLLECTION_GET_LOCK (demux)); \
>+} G_STMT_END
>+
>+#define GST_OUTPUT_GET_LOCK(demux) (&((GstStreamidDemux*)(demux))->output_lock)
>+
>+#define GST_OUTPUT_LOCK(demux)   G_STMT_START { \
>+  GST_LOG_OBJECT (demux, "locking from thread %p", g_thread_self ()); \
>+  g_mutex_lock (GST_OUTPUT_GET_LOCK (demux)); \
>+  GST_LOG_OBJECT (demux, "locked from thread %p", g_thread_self ()); \
>+} G_STMT_END
>+
>+#define GST_OUTPUT_UNLOCK(demux)   G_STMT_START { \
>+  GST_LOG_OBJECT (demux, "unlocking from thread %p", g_thread_self ()); \
>+    g_mutex_unlock (GST_OUTPUT_GET_LOCK (demux)); \
>+} G_STMT_END
>+

What's with all these new locks? Are they actually needed?

It looks to me like everything is effectively protected by the pad streaming
lock already. I didn't see any code that uses any of these locks from a
non-streaming thread. Did I miss anything?


>--- a/tests/check/elements/streamiddemux.c
>+++ b/tests/check/elements/streamiddemux.c

Unit test \o/

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.