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 358999 - [PATCH] [GstPad] Flushing on blocked pads not handled correctly.
[PATCH] [GstPad] Flushing on blocked pads not handled correctly.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 0.10.11
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-10-02 13:41 UTC by Edward Hervey
Modified: 2006-10-02 16:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
first version of patch (8.78 KB, patch)
2006-10-02 13:44 UTC, Edward Hervey
none Details | Review
Updated version (8.86 KB, patch)
2006-10-02 14:50 UTC, Edward Hervey
none Details | Review
Cleaner version (8.81 KB, patch)
2006-10-02 15:51 UTC, Edward Hervey
committed Details | Review

Description Edward Hervey 2006-10-02 13:41:48 UTC
Currently flushing on blocked pads isn't done correctly.

FLUSH_START/FLUSH_STOP events are discarded if the pad is BLOCKED. The problem is that GST_PAD_IS_BLOCKED() isn't good enough. Nothing tells us whether the pad is actually blocking on something. If it is not, then those flushing events should be forwarded downstream.

This can be seen clearly with the following example.

[  A   src.] ---- [sink  B  src] ---- [sink  C  ]

A, B and C are elements. C is blocking the stream (sink in preroll state for example, or a filled queue).

If you block B.src (gst_pad_set_blocked_async(..TRUE...)), the pad is considered as blocked.

Let's assume you send a flushing seek, in order for the stream lock to be released (needed to safely unlink B.src for example).

Currently, B.src will discard the FLUSH_START/FLUSH_STOP events because it sees that GST_PAD_IS_BLOCKED() is TRUE, which means the downstream locking element or pad will never release the Stream Lock.
It should in fact only discard the FLUSH_START/FLUSH_STOP events if the pad is REALLY BLOCKING on an event or buffer.


The following patch adds a GstPadFlag GST_PAD_BLOCKING, which is only set between:
_the moment when an object is really blocking in the pad,
_and the moment the pad is no longer flushing

The handle_pad_block() and gst_pad_send_event() are modified accordingly, so that:
_the flag above is properly set and unset,
_and the FLUSH_START/FLUSH_STOP events are forwarded if the GST_PAD_BLOCKING flag is not set.
Comment 1 Edward Hervey 2006-10-02 13:44:22 UTC
Created attachment 73846 [details] [review]
first version of patch

Includes patch for gstpad.[ch], API documentation, additional explanations to part-blocked design document.
Comment 2 Wim Taymans 2006-10-02 14:05:57 UTC
The BLOCKING flag is only cleared when a flush arrives, it should also be cleared when _set_blocked (pad, FALSE) is called.

Maybe this:

handle_pad_block:
      BLOCKING = TRUE;
      BLOCK_WAIT()
      if (FLUSHING)
        goto flushing;
      BLOCKING = FALSE;

event:
   FLUSH_START:
     FLUSHING = TRUE;
     BLOCK_SIGNAL;

   FLUSH_STOP
     FLUSHING = FALSE;     
     BLOCKING = FALSE;

so that unlocks triggered by flushes don't unset the blocking flag but normal unblocks do.




Comment 3 Edward Hervey 2006-10-02 14:50:57 UTC
Created attachment 73851 [details] [review]
Updated version

Updated patch, taking into account wim's hint.
Comment 4 Edward Hervey 2006-10-02 15:51:01 UTC
Created attachment 73853 [details] [review]
Cleaner version

Updated patch.

Modifications:
Since GST_PAD_IS_BLOCKING is guaranteed to only exist when GST_PAD_IS_BLOCKED is TRUE, we don't need to check for both now. Makes the check smaller (and therefore faster).
Comment 5 Edward Hervey 2006-10-02 16:02:42 UTC
2006-10-02  Edward Hervey  <edward@fluendo.com>

	* docs/design/part-block.txt:
	Further explain the use of flushing on blocked pads.
	* docs/gst/gstreamer-sections.txt:
	* gst/gstpad.c: (gst_pad_is_blocking), (handle_pad_block),
	(gst_pad_push_event):
	* gst/gstpad.h:
	Added new GstPadFlag : GST_PAD_BLOCKING.
	Adds the notion of pads really blocking, which enables to properly
	handle FLUSH_START/FLUSH_STOP events on blocked pads.
	Fixes #358999
	API: gst_pad_is_blocking()
	API: GST_PAD_IS_BLOCKING() macro
	API: GST_PAD_BLOCKING GstPadFlag