GNOME Bugzilla – Bug 762330
Probe only flush events, is not possible
Last modified: 2016-06-17 11:10:27 UTC
Created attachment 321665 [details] [review] Unit test for flush probe It's not possible to install a pad probe, that only reacts to FLUSH events, even though there is API for it. > gst_pad_add_probe (pad, GST_PAD_PROBE_TYPE_EVENT_FLUSH, pad_probe_cb, NULL, NULL); gst_pad_add_probe basically appends GST_PAD_PROBE_TYPE_ALL_BOTH to the mask, if the user supplied mask didn't contain any probe type regarding data. > if ((mask & GST_PAD_PROBE_TYPE_ALL_BOTH) == 0) > mask |= GST_PAD_PROBE_TYPE_ALL_BOTH; Something should be fixed here. Either this behavior is wrong, or some documentation is lacking. I made a unit test (if probes for only flush events should work)
This is very much related to bug #761211, which also suffers from TYPE_ALL_BOTH being set if none are set.
Why exactly do we convert a flush probe type to a TYPE_ALL when adding it? Some legacy behavior?
No, "convenience". If you don't provide a scheduling mode in the flags, all apply. if you don't provide a "data type" (event, buffer, etc), all of them apply. I think the problem here is that GST_PAD_PROBE_TYPE_ALL_BOTH does not include GST_PAD_PROBE_TYPE_EVENT_FLUSH.
Created attachment 324727 [details] [review] pad: add PROBE_TYPE_EVENT_FLUSH to PROBE_TYPE_ALL_BOTH Otherwise a FLUSH probe will be converted to ALL_BOTH and the callback function will get more than it expects
Comment on attachment 324727 [details] [review] pad: add PROBE_TYPE_EVENT_FLUSH to PROBE_TYPE_ALL_BOTH The intention was that you must explicitly opt-in for FLUSH events, and not have them part of the catch all value
Created attachment 324874 [details] [review] pad: consider PROBE_TYPE_EVENT_FLUSH when using PROBE_TYPE_ALL_BOTH When GST_PAD_PROBE_EVENT_FLUSH is used, the probes already have a data type and it is not needed to automatically add the default types.
commit 45b0e7aa16a19e9028d7ddd5ea4a52acaa4e5349 Author: Thiago Santos <thiagoss@osg.samsung.com> Date: Thu Mar 24 12:13:39 2016 -0300 pad: consider PROBE_TYPE_EVENT_FLUSH when using PROBE_TYPE_ALL_BOTH When GST_PAD_PROBE_EVENT_FLUSH is used, the probes already have a data type and it is not needed to automatically add the default types. https://bugzilla.gnome.org/show_bug.cgi?id=762330 commit 8db72e7df72e633b69458def08c3b87109f23996 Author: Linus Svensson <linussn@axis.com> Date: Fri Feb 19 16:18:12 2016 +0100 gstpad tests: Add a test for flush event only probes https://bugzilla.gnome.org/show_bug.cgi?id=762330
*** Bug 767500 has been marked as a duplicate of this bug. ***
Shall that go in next 1.8 release, or is considered too non-trivial ?
It looks like it could be backported, with some extra eyes reviewing/testing it. Seems like it hasn't caused any major probs in the last 2 months.
It's probably safe, yes.
Reason I want this backported is that with decodebin I see lots of: gstdecodebin2.c:2031:demuxer_source_pad_probe:<qtdemux0:audio_0> Saw event unknown in the log, the pad probe is interpreting buffers as events, that can't be good and is noisy. We could just change the handler to include the EVENT types of course.
Then go ahead :)