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 665525 - [0.11] Pad with probe won't forward flush events even though it's not blocking yet
[0.11] Pad with probe won't forward flush events even though it's not blockin...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
0.11.x
Other All
: Normal normal
: 0.11.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-12-04 01:13 UTC by Matej Knopp
Modified: 2012-02-18 20:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to forward flush events when the pad has blocking probe but is not yet blocking. (1.31 KB, patch)
2011-12-04 01:13 UTC, Matej Knopp
none Details | Review
Add GST_PAD_PROBE_TYPE_BLOCKING (3.57 KB, patch)
2011-12-05 19:57 UTC, Matej Knopp
none Details | Review
Add GST_PAD_PROBE_TYPE_HANDLE_FLUSH (2.77 KB, patch)
2011-12-05 20:22 UTC, Matej Knopp
none Details | Review

Description Matej Knopp 2011-12-04 01:13:15 UTC
Created attachment 202734 [details] [review]
Patch to forward flush events when the pad has blocking probe but is not yet blocking.

The following code doesn't seem right

    case GST_EVENT_FLUSH_START:
      GST_PAD_SET_FLUSHING (pad);

     if (G_UNLIKELY (GST_PAD_IS_BLOCKED (pad))) {
        /* flush start will have set the FLUSHING flag and will then
         * unlock all threads doing a GCond wait on the blocking pad. This
         * will typically unblock the STREAMING thread blocked on a pad. */
        GST_LOG_OBJECT (pad, "Pad is blocked, not forwarding flush-start, "
            "doing block signal.");
        GST_PAD_BLOCK_BROADCAST (pad);
        goto flushed;
      }
      break;

GST_PAD_IS_BLOCKED (pad) is true even if pad is not blocking yet, so the condition is signaled in vain. Changing the check to GST_PAD_IS_BLOCKING (pad) fixes it

Same goes for FLUSH_STOP. Although I'm not sure how the pad could get blocked while being flushed.
Comment 1 Wim Taymans 2011-12-04 18:58:52 UTC
This is intentional, as soon as a blocking probe is added, no new flush events go through. is this causing a particular problem for you? Why d you think this is wrong?
Comment 2 Matej Knopp 2011-12-04 21:38:35 UTC
It deadlocks the pipeline.

I have pipeline like this (example)
[Element1] -> [Queue] -> [Sink]

The sink is temporarily unable to render buffers so it's stalling the pipeline. The queue is full.
I want to add downdstream block probe to element1 srcpad and flush the pipeline.

I add the probe, but it is not invoked because the pipeline is taller because of the sink. That would normally be okay as I'm flushing the pipeline (seek), but in this case it won't work.

The seek start will never get to the queue. It would get there once the probe starts blocking, but that doesn't happen because the pipeline is congested.
Comment 3 Matej Knopp 2011-12-04 21:39:48 UTC
s/taller/stalled
Comment 4 Matej Knopp 2011-12-04 21:45:09 UTC
s/seek start/flush start. sorry
Comment 5 Matej Knopp 2011-12-04 21:47:08 UTC
Also a probe might be blocking, but it can also return PASS and DROP. So even though there is a BLOCK probe attached to pad it doesn't necessarily have to block it, does it?
Comment 6 Matej Knopp 2011-12-05 19:57:26 UTC
Created attachment 202862 [details] [review]
Add GST_PAD_PROBE_TYPE_BLOCKING

The patch add new probe type GST_PAD_PROBE_TYPE_BLOCKING
The probe has the type set it will be invoked for flush events and can return GST_PROBE_DROP that will prevent the flush event to be sent further down
If the pad probe doesn't have GST_PAD_PROBE_TYPE_BLOCKING set the flush event is forwarded automatically (the probe won't be invoked for flush events)
Comment 7 Matej Knopp 2011-12-05 20:22:05 UTC
Created attachment 202865 [details] [review]
Add GST_PAD_PROBE_TYPE_HANDLE_FLUSH
Comment 8 Wim Taymans 2011-12-06 17:20:22 UTC
Thanks,

I renamed the move the event to match the other event types.


commit 8e6b5f79bd0182e8582b8e18a97b095bd3c74b55
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Mon Dec 5 21:20:52 2011 +0100

    Add GST_PAD_PROBE_TYPE_HANDLE_FLUSH