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 732851 - funnel: storing sticky events after event callback
funnel: storing sticky events after event callback
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.3.3
Other Linux
: Normal normal
: 1.4.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-07-07 14:04 UTC by Srimanta Panda (trollkarlen)
Modified: 2014-07-22 12:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix racy funnel eos handling (5.39 KB, patch)
2014-07-14 08:02 UTC, Srimanta Panda (trollkarlen)
needs-work Details | Review
After review comment fix (7.31 KB, patch)
2014-07-15 08:35 UTC, Srimanta Panda (trollkarlen)
needs-work Details | Review
Fix racy funnel eos handling (7.31 KB, patch)
2014-07-16 15:52 UTC, Srimanta Panda (trollkarlen)
committed Details | Review

Description Srimanta Panda (trollkarlen) 2014-07-07 14:04:04 UTC
When EOS events comes simultaneously from two sinkpads, it doesn't forward the eos to source pad, although both sinkpads have got the eos.


Reason: Sticky events are stored after the event callbacks are returned. Suppose one sinkpad has returned from event callback and not stored the sticky event yet, but by that time event callback for other sink pad is called and checked for all sinkpads eos. And it assumes first sinkpad has not received EOS. So the second sinkpad doesnot forward the EOS to sourcepad.
Comment 1 Srimanta Panda (trollkarlen) 2014-07-07 14:04:50 UTC
I am working on a patch for this issue currently.
Comment 2 Srimanta Panda (trollkarlen) 2014-07-14 08:02:05 UTC
Created attachment 280629 [details] [review]
Fix racy funnel eos handling
Comment 3 Sebastian Dröge (slomo) 2014-07-14 08:15:03 UTC
Review of attachment 280629 [details] [review]:

Looks good, just one minor change needed. Thanks for the patch!

::: plugins/elements/gstfunnel.c
@@ +313,3 @@
+    GST_PAD_STREAM_LOCK (funnel->srcpad);
+    GST_OBJECT_UNLOCK (funnel);
+    fpad->got_eos = FALSE;

You also need to reset got_eos when the pad is deactivated, i.e. when the element goes from PAUSED to READY state.
Comment 4 Srimanta Panda (trollkarlen) 2014-07-15 08:35:30 UTC
Created attachment 280690 [details] [review]
After review comment fix
Comment 5 Sebastian Dröge (slomo) 2014-07-16 15:01:49 UTC
Review of attachment 280690 [details] [review]:

::: plugins/elements/gstfunnel.c
@@ +302,2 @@
     if (GST_EVENT_TYPE (event) == GST_EVENT_EOS) {
+      GST_OBJECT_UNLOCK (funnel);

This should probably be GST_OBJECT_LOCK

@@ +315,3 @@
+    unlock = TRUE;
+    GST_PAD_STREAM_LOCK (funnel->srcpad);
+    GST_OBJECT_UNLOCK (funnel);

And this too... and why do you take the stream lock here? That seems wrong, you already have it here (and never release it anyway)
Comment 6 Srimanta Panda (trollkarlen) 2014-07-16 15:40:00 UTC
The second one is correct because while pushing the FLUSH event we are locking the srcpad ( same as the EOS event) and based on variable 'unlock' we are unlocking the stream. The first comment i will correct it and push it again.
Comment 7 Sebastian Dröge (slomo) 2014-07-16 15:45:21 UTC
Ok, but the object lock there is still wrong.
Comment 8 Srimanta Panda (trollkarlen) 2014-07-16 15:51:17 UTC
yes, that I am correcting.
Comment 9 Srimanta Panda (trollkarlen) 2014-07-16 15:52:47 UTC
Created attachment 280869 [details] [review]
Fix racy funnel eos handling
Comment 10 Sebastian Dröge (slomo) 2014-07-21 07:34:35 UTC
commit 6d05df01b0e2865eab3a53c9feef49d3f110dde1
Author: Srimanta Panda <srimanta@axis.com>
Date:   Wed Jul 9 15:48:10 2014 +0200

    funnel: Fix for racy EOS event handling
    
    When eos events are forwarded simultaneouly from two sinkpads on
    funnel, it doesnot forward the eos to sourcepad. The reason is
    sticky events are stored after the event callbacks are returned.
    Therefore while one is about to store the sticky events on the its
    sinkpad, other sinkpad starts checking for the eos events on all other
    sinkpads and assumes eos is not present yet.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=732851