GNOME Bugzilla – Bug 721289
pad: using multiple blocking probes doesn't work as expected
Last modified: 2014-03-05 17:17:33 UTC
Here's a scenario: Add blocking Probe A. Probe A's callback gets invoked and returns GST_PAD_PROBE_OK, leaving the block in place. Add blocking Probe B. Probe B's callback doesn't get invoked, as pad is already blocked. Remove blocking probe A. Probe B's callback still doesn't get invoked. Pad remains blocked because of Probe B's existence, but the callback associated with the Probe B was never invoked. A more desirable implementation would track which probe had blocked the pad (in the example above, that would be Probe A). When that pad is removed, it unblocks and invokes the callback of any remaining blocking probes (in this example, Probe B). If the callback leaves the blocking probe in place, then Probe B becomes the de facto blocker and the pad remains blocked until it is removed.
On IRC we had discussed the workaround for there not being a gst_pad_probe_unblock() of adding the probe a second time and then removing the original version of the probe. This bug makes that unworkable as the removal doesn't unblock the probe because there's already another blocking probe installed. It seems the API needs to pick either one of two options: 1) The block is of the probe and not of the pad so: - All callbacks should be called on matching events even if a previous one has blocked. A pad may become blocked by two different probes - There should be a gst_pad_probe_unblock(pad, probe) that unblocks only that single probe and potentially leaves the pad blocked if there's another blocking probe in effect. 2) The block is of the pad so: - We need a gst_pad_unblock(pad) that unblocks the pad whatever the blocking probe was Options 1) seems more flexible but isn't backwards compatible as it requires all callbacks to be called even if the first has blocked the pad.
Could you provide a testcase for this to make it easier to reproduce and debug? :)
Created attachment 265095 [details] Testcase for the bug Here's a reduced testcase extracted from my app.
Created attachment 265097 [details] [review] Potential patch to gstpad.c I attempted to create a patch to add gst_pad_unblock(pad) to fix this. It seems to kind of work in my app although on the second file change I seem to be getting a second EOS that closes the pipeline. I'm running the unblock from the bus callback which is probably wrong.
What should happen here is: 1) Add probe A 2) blocked due to probe A 3) add probe B, not called but pad still blocked 4) remove probe A 5) probe B is called and can decide to block or not Should be possible to implement that easily with the cookies :)
Comment on attachment 265097 [details] [review] Potential patch to gstpad.c This should be independent of this bug, can you open a new one for it :) The API should be: void gst_pad_unblock(pad, probe_id, probe_return)
Please test if this fixes all your problems commit 9e125e7bab92259d2d3317c2bf47da081c03ba8a Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Jan 2 13:34:52 2014 +0100 pad: Add unit test for adding/removing blocking probes while a pad is blocked And make sure that these new probes are actually called if they should instead of silently blocking the pad forever. https://bugzilla.gnome.org/show_bug.cgi?id=721289 commit 93ce90f83320e845867d28b9cbeea5b596a1ef04 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Jan 2 13:33:20 2014 +0100 pad: Check if new probes need to be called when adding/removing some This allows blocking a pad, add a new blocking probe, removing the first probe and then having the second probe called. Which could then decide that data-flow should actually continue instead of blocking now. https://bugzilla.gnome.org/show_bug.cgi?id=721289