After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 762330 - Probe only flush events, is not possible
Probe only flush events, is not possible
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal normal
: 1.8.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 767500 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-02-19 15:35 UTC by Linus Svensson
Modified: 2016-06-17 11:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Unit test for flush probe (2.49 KB, patch)
2016-02-19 15:35 UTC, Linus Svensson
committed Details | Review
pad: add PROBE_TYPE_EVENT_FLUSH to PROBE_TYPE_ALL_BOTH (1.27 KB, patch)
2016-03-24 20:39 UTC, Thiago Sousa Santos
none Details | Review
pad: consider PROBE_TYPE_EVENT_FLUSH when using PROBE_TYPE_ALL_BOTH (1.90 KB, patch)
2016-03-28 14:29 UTC, Thiago Sousa Santos
committed Details | Review

Description Linus Svensson 2016-02-19 15:35:32 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)
Comment 1 Sebastian Dröge (slomo) 2016-02-19 15:54:16 UTC
This is very much related to bug #761211, which also suffers from TYPE_ALL_BOTH being set if none are set.
Comment 2 Thiago Sousa Santos 2016-03-22 19:22:19 UTC
Why exactly do we convert a flush probe type to a TYPE_ALL when adding it?

Some legacy behavior?
Comment 3 Sebastian Dröge (slomo) 2016-03-23 07:58:17 UTC
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.
Comment 4 Thiago Sousa Santos 2016-03-24 20:39:24 UTC
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 5 Sebastian Dröge (slomo) 2016-03-25 08:35:17 UTC
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
Comment 6 Thiago Sousa Santos 2016-03-28 14:29:19 UTC
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.
Comment 7 Thiago Sousa Santos 2016-03-29 03:54:17 UTC
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
Comment 8 Jan Alexander Steffens (heftig) 2016-06-10 16:10:06 UTC
*** Bug 767500 has been marked as a duplicate of this bug. ***
Comment 9 Nicolas Dufresne (ndufresne) 2016-06-10 16:29:18 UTC
Shall that go in next 1.8 release, or is considered too non-trivial ?
Comment 10 Tim-Philipp Müller 2016-06-10 16:43:17 UTC
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.
Comment 11 Sebastian Dröge (slomo) 2016-06-13 06:17:27 UTC
It's probably safe, yes.
Comment 12 Tim-Philipp Müller 2016-06-17 09:46:54 UTC
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.
Comment 13 Sebastian Dröge (slomo) 2016-06-17 10:08:12 UTC
Then go ahead :)