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 759110 - appsrc: Clear is_eos flag when receiving the flush event
appsrc: Clear is_eos flag when receiving the flush event
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 1.6.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-07 08:17 UTC by Kazunori Kobayashi
Modified: 2015-12-21 11:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Clear is_eos flag when receiving the flush event (1.03 KB, application/mbox)
2015-12-07 08:17 UTC, Kazunori Kobayashi
  Details
appsrc: Clear is_eos flag when receiving the flush event (1.03 KB, patch)
2015-12-07 08:26 UTC, Kazunori Kobayashi
none Details | Review
appsrc: Clear is_eos flag when receiving the flush event (1.05 KB, patch)
2015-12-08 11:00 UTC, Kazunori Kobayashi
none Details | Review
appsrc: Clear is_eos flag when receiving the flush event (2.27 KB, patch)
2015-12-18 08:57 UTC, Kazunori Kobayashi
committed Details | Review

Description Kazunori Kobayashi 2015-12-07 08:17:44 UTC
Created attachment 316867 [details]
Clear is_eos flag when receiving the flush event

The EOS event can be propagated to the downstream plugins when
is_eos flag remains set even after leaving the flushing state.
This fix allows this element to normally restart the streaming
after receiving the flush event by clearing the is_eos flag.
Comment 1 Kazunori Kobayashi 2015-12-07 08:26:57 UTC
Created attachment 316868 [details] [review]
appsrc: Clear is_eos flag when receiving the flush event
Comment 2 Sebastian Dröge (slomo) 2015-12-07 11:50:22 UTC
Review of attachment 316868 [details] [review]:

unlock_stop() is not only called for FLUSH_STOP but also for the EOS event... which then completely breaks the is_eos flag. Please use an event handler and handle FLUSH_STOP explicitly there :)
Comment 3 Kazunori Kobayashi 2015-12-08 11:00:44 UTC
Created attachment 316922 [details] [review]
appsrc: Clear is_eos flag when receiving the flush event
Comment 4 Kazunori Kobayashi 2015-12-08 11:09:42 UTC
Thank you for your review.
I've reattached the fixed patch that handles the FLUSH_STOP event.
Could you please see it again?
Comment 5 Sebastian Dröge (slomo) 2015-12-08 11:53:46 UTC
Comment on attachment 316922 [details] [review]
appsrc: Clear is_eos flag when receiving the flush event

Not in send_event() but in GstBaseSrc::event(). I think you also need to hold the mutex while changing that field.
Comment 6 Kazunori Kobayashi 2015-12-11 05:50:21 UTC
I agree on the event handling in GstBaseSrc::event(), but I cannot understand
why the is_eos clearing should be held with the mutex.
Could you tell me what race condition you are concerned about?
Comment 7 Sebastian Dröge (slomo) 2015-12-11 08:09:51 UTC
None really, just that it's protected by priv->mutex in almost all other places (except in do_seek()). It probably should be protected there too and in event(), mainly because it is used from the streaming thread and other threads (do_seek() is usually called from the application thread).
Comment 8 Kazunori Kobayashi 2015-12-18 08:57:06 UTC
Created attachment 317611 [details] [review]
appsrc: Clear is_eos flag when receiving the flush event
Comment 9 Kazunori Kobayashi 2015-12-18 09:03:02 UTC
The patch that should be fixed as you requested has been attached.
Using GstBaseSrc::event() and the protection with the mutex are included.
Could you review it?
Comment 10 Sebastian Dröge (slomo) 2015-12-19 10:36:45 UTC
commit d43f1b2a5a4361f156ab1281f7e4442a899d4de7
Author: Kazunori Kobayashi <kkobayas@igel.co.jp>
Date:   Thu Dec 3 11:53:05 2015 +0900

    appsrc: Clear is_eos flag when receiving the flush-stop event
    
    The EOS event can be propagated to the downstream elements when
    is_eos flag remains set even after leaving the flushing state.
    This fix allows this element to normally restart the streaming
    after receiving the flush event by clearing the is_eos flag.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=759110