GNOME Bugzilla – Bug 756867
multiqueue: No stream-status CREATE messages for dynamically added src pads
Last modified: 2017-12-24 10:54:20 UTC
If a multiqueue is already running when a new pad gets requested, the pads are activated before getting added to the element. As a result, the unparented srcpad cannot post a stream status CREATE message for its task.
Maybe the srcpad should be added before activating it, as follows? I don't know if this would be correct. g_rec_mutex_lock (GST_STATE_GET_LOCK (mqueue)); if (GST_STATE_TARGET (mqueue) != GST_STATE_NULL) { gst_pad_set_active (sq->sinkpad, TRUE); } gst_element_add_pad (GST_ELEMENT (mqueue), sq->srcpad); gst_element_add_pad (GST_ELEMENT (mqueue), sq->sinkpad); if (GST_STATE_TARGET (mqueue) != GST_STATE_NULL) { gst_pad_set_active (sq->srcpad, TRUE); } g_rec_mutex_unlock (GST_STATE_GET_LOCK (mqueue));
If the element is already running, you need to activate the pad before adding it to the element. The pad task doesn't necessarily have to be started as part of the pad activation though, so perhaps that could be reshuffled a little.
I brought this up on irc, and heftig noted this bug report as a potential candidate. So I'm working on a rather complicated pipeline here that can reliably trigger this bug with certain hidef files, we use the thread enter/leave messages for setting thread priorities, and sometimes the enter message was not being posted to the bus. The suggested fix from Jan fixes the problem for me, however as Tim wrote this is not really a fix, as it there is a brief moment where the pad has been added to an active element, but the pad itself not yet active. The solution that Tim suggests is to start the task later on, I have started looking but so far I'm not sure when exactly the task should be started, so perhaps move the task starting into a new function that is run just after gst_element_add_pad(..., sq->sinkpad) (while the mq lock is still being held)? I'm still going through the multiqueue, so this is probably a better place to start the task.
I've started working on this yesterday. I think I have a complete patch but could not test it yet.
Created attachment 365578 [details] [review] multiqueue: Split task handling from gst_single_queue_flush
Created attachment 365579 [details] [review] multiqueue: Don't start new pads until parented Ensures the pads can post their CREATE stream-status messages.
Created attachment 365580 [details] [review] multiqueue: Check we get CREATE+ENTER stream-statuses when adding pads
I've done some testing on an embedded ARM device here with Jan's three patches on 1.6.3 (it's what we're stuck with at the moment), so I can report that the issue is now fixed, so far no other regressions or other issues have been spotted. We'll be doing some more testing with different pipelines and sources over the next day or two (up to Monday/Tuesday), will comment again when that's done.
A post-testing update: no problems or side effects have been noted with Jan's three patches, the CREATE messages are always being posted, so from our POV this is a valid fix.
Comment on attachment 365580 [details] [review] multiqueue: Check we get CREATE+ENTER stream-statuses when adding pads Can we turn this macro into a function please? You can easily pass the expected type via an argument to the function, no?
Do the fail_* macros work outside the test function?
Yes, as far as I'm aware.
There's another instance of such a macro in that file. Should I make another patch that rewrites that macro as well?
As you prefer :)
Created attachment 365913 [details] [review] multiqueue: Replace large test macro with function
Created attachment 365914 [details] [review] multiqueue: Check we get CREATE+ENTER stream-statuses when adding pads
Thanks! commit 1b02b76137bda94fe206b5f19371571d8e5ae508 Author: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com> Date: Sat Dec 23 23:43:33 2017 +0100 tests: multiqueue: Replace large test macro with function Just a bit of cleanup. https://bugzilla.gnome.org/show_bug.cgi?id=756867 commit 67a9ec6878d296277dff6447296ad13e9762e5a1 Author: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com> Date: Fri Dec 15 09:43:40 2017 +0100 tests: multiqueue: Check we get CREATE+ENTER stream-statuses when adding pads https://bugzilla.gnome.org/show_bug.cgi?id=756867 commit b17d03cb0ae25a30a2f4640181d13cbb3081e9f5 Author: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com> Date: Fri Dec 15 09:14:57 2017 +0100 multiqueue: Don't start new pads until parented Start task on new source pads added at runtime after they have been added to the element, not during activation. This ensures the pads can post their CREATE stream-status messages and the application can set thread priorities. https://bugzilla.gnome.org/show_bug.cgi?id=756867 commit 10c3a7bd5567cf56b8c10b779b23f8e01ee1ac8d Author: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com> Date: Fri Dec 15 09:14:07 2017 +0100 multiqueue: Split task handling from gst_single_queue_flush https://bugzilla.gnome.org/show_bug.cgi?id=756867