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 603378 - [API][event] New event to flush all buffered data
[API][event] New event to flush all buffered data
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal normal
: 0.11.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 537382
 
 
Reported: 2009-11-30 14:38 UTC by Thiago Sousa Santos
Modified: 2012-05-18 08:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Adds the new events (5.23 KB, patch)
2009-11-30 14:41 UTC, Thiago Sousa Santos
none Details | Review
basesink patch for the new drain events (5.42 KB, patch)
2009-11-30 14:42 UTC, Thiago Sousa Santos
none Details | Review
patch for queue to handle the new drain events (1.67 KB, patch)
2009-11-30 14:43 UTC, Thiago Sousa Santos
none Details | Review
Make input selector drop buffers only when they get late (6.13 KB, patch)
2009-11-30 14:56 UTC, Thiago Sousa Santos
rejected Details | Review
Make input-selector use the drain events (3.07 KB, patch)
2009-11-30 15:00 UTC, Thiago Sousa Santos
none Details | Review
adds the clear events (7.50 KB, patch)
2009-11-30 20:15 UTC, Thiago Sousa Santos
none Details | Review
gstpad patch for clear events (2.68 KB, patch)
2009-11-30 20:15 UTC, Thiago Sousa Santos
none Details | Review
adds an utility function for clear events (2.38 KB, patch)
2009-11-30 20:15 UTC, Thiago Sousa Santos
none Details | Review
queue patch (2.33 KB, patch)
2009-11-30 20:17 UTC, Thiago Sousa Santos
none Details | Review
basesink patch (5.81 KB, patch)
2009-11-30 20:17 UTC, Thiago Sousa Santos
none Details | Review
inputselector using clear events (3.13 KB, patch)
2009-11-30 21:06 UTC, Thiago Sousa Santos
none Details | Review

Description Thiago Sousa Santos 2009-11-30 14:38:06 UTC
This event would work just like flush, but it would only drop buffers queued along the pipeline and unlock sinks that are rendering data, not segments/time informations reset.

Check https://bugzilla.gnome.org/show_bug.cgi?id=537382 for the motivation for this.

The use case here is to allow 'instantaneous' track switching. Currently, when using playbin2, a track change simply makes input-selector push data from a new pad to downstream elements. The problem here is that there are buffers queue downstream and already at the sink that will get rendered before the new track buffers are rendered. (In case of a slideshow, like the file of the referred bug, this might take a long time to happen).

This new event would drop all those buffers downstream, allowing the new activated track buffers to be rendered immediately.

There is still another problem involved in fixing #537382, multiqueue takes a long time to 'detect' that the track it is pushing is now 'not-linked' and he should retry pushing buffers on other pads (the newly activated track is on this group) and all the above would not work. I hacked my way around this for focusing on this new event solution by making input-selector property 'always-ok' true in playbin2.
Comment 1 Thiago Sousa Santos 2009-11-30 14:41:19 UTC
Created attachment 148745 [details] [review]
Adds the new events
Comment 2 Thiago Sousa Santos 2009-11-30 14:42:42 UTC
Created attachment 148746 [details] [review]
basesink patch for the new drain events
Comment 3 Thiago Sousa Santos 2009-11-30 14:43:39 UTC
Created attachment 148747 [details] [review]
patch for queue to handle the new drain events
Comment 4 Thiago Sousa Santos 2009-11-30 14:56:14 UTC
Created attachment 148750 [details] [review]
Make input selector drop buffers only when they get late

This patch was already uploaded to 537382, but I'm putting it here as well
Comment 5 Thiago Sousa Santos 2009-11-30 15:00:47 UTC
Created attachment 148752 [details] [review]
Make input-selector use the drain events

This is the attempt to make input-selector use the drain events (unsuccessful).

I suspect that the closing and sending of new segments is the problem here, because it uses times from the buffers currently passing through, that are probably much ahead of the current running time, causing buffers of the newly activated stream to be clipped/dropped. I tried commenting the code that pushed the segment closing/creation but it still didn't work.
Comment 6 Wim Taymans 2009-11-30 16:33:42 UTC
I would like to have this merged into the regular flush events. This could be done by adding new flags to the flush event (default DEEP | RESET). At least we could extend it later on then too.

something like:

 typedef enum {
   GST_FLUSH_FLAG_DEEP  = (1 << 0),
   GST_FLUSH_FLAG_RESET = (1 << 1)
 } GstFlushFlags

The RESET flag would reset the running_time. The DEEP flag would forward the event downstream instead of unlocking only the current streaming thread. Having the RESET flag bu not the DEEP flag is maybe not so useful.
Comment 7 Sebastian Dröge (slomo) 2009-11-30 16:54:33 UTC
(In reply to comment #6)
> I would like to have this merged into the regular flush events. This could be
> done by adding new flags to the flush event (default DEEP | RESET). At least we
> could extend it later on then too.
> 
> something like:
> 
>  typedef enum {
>    GST_FLUSH_FLAG_DEEP  = (1 << 0),
>    GST_FLUSH_FLAG_RESET = (1 << 1)
>  } GstFlushFlags
> 
> The RESET flag would reset the running_time. The DEEP flag would forward the
> event downstream instead of unlocking only the current streaming thread. Having
> the RESET flag bu not the DEEP flag is maybe not so useful.

For 0.11 that's a good idea but in 0.10 we need a different event for this because it is now necessary to look at the content of the event while before this wasn't necessary. All old elements would handle the flush event with flags!=DEEP|RESET as if they are DEEP|RESET and completely break playback (because the running time is reset although upstream elements think it isn't).
Comment 8 Wim Taymans 2009-11-30 17:01:18 UTC
> 
> For 0.11 that's a good idea but in 0.10 we need a different event for this
> because it is now necessary to look at the content of the event while before
> this wasn't necessary. All old elements would handle the flush event with
> flags!=DEEP|RESET as if they are DEEP|RESET and completely break playback
> (because the running time is reset although upstream elements think it isn't).

We need to change a couple of elements (like queue and basesink) at least. Other elements might need to be updated too.

But I guess you are right, old elements can forward the new event without reseting the timing info and then there is more change of backwards compat for non-updated elements.
Comment 9 Thiago Sousa Santos 2009-11-30 17:06:01 UTC
After some discussion on IRC with Wim I'll be renaming this event to CLEAR instead of DRAIN
Comment 10 Sebastian Dröge (slomo) 2009-11-30 17:16:07 UTC
Yes, sounds better :) And please add a FIXME 0.11 for merging the events by adding flags as Wim explained in comment #6.
Comment 11 Wim Taymans 2009-11-30 17:19:17 UTC
Maybe use the opportunity to make a

 gst_event_new_clear_start/stop (GstClearFlags flags)

flush-start and flush-stop then become a subset of this new event and for 0.11 we can just rename things (when needed).
Comment 12 Thiago Sousa Santos 2009-11-30 20:15:02 UTC
Created attachment 148779 [details] [review]
adds the clear events
Comment 13 Thiago Sousa Santos 2009-11-30 20:15:25 UTC
Created attachment 148780 [details] [review]
gstpad patch for clear events
Comment 14 Thiago Sousa Santos 2009-11-30 20:15:55 UTC
Created attachment 148781 [details] [review]
adds an utility function for clear events
Comment 15 Thiago Sousa Santos 2009-11-30 20:17:24 UTC
Created attachment 148782 [details] [review]
queue patch
Comment 16 Thiago Sousa Santos 2009-11-30 20:17:44 UTC
Created attachment 148783 [details] [review]
basesink patch
Comment 17 Thiago Sousa Santos 2009-11-30 21:06:26 UTC
Created attachment 148789 [details] [review]
inputselector using clear events
Comment 18 Sebastian Dröge (slomo) 2011-05-14 10:26:01 UTC
Comment on attachment 148750 [details] [review]
Make input selector drop buffers only when they get late

That's essentially solved by the following commit and Thiago agreed that this is the better approach than using the clock



commit 5229a26a7608d66b013fbcca80f8e38f875f4670
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Sat Mar 19 10:28:49 2011 +0100

    inputselector: Add sync mode that syncs inactive pads to the running time of
    
    Fixes bug #645017.
Comment 19 Sebastian Dröge (slomo) 2011-05-14 10:26:48 UTC
What's the state of this bug in general? Something like a drain/clear event would definitely be useful, at least in 0.11. Maybe as a special variant of the flush events.
Comment 20 Wim Taymans 2012-05-18 08:53:46 UTC
flush_stop now has the reset_time property. Drain is implemented with a query.