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 740949 - inputselector: sticky events haven't send out when active track reach EOS.
inputselector: sticky events haven't send out when active track reach EOS.
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2014-12-01 06:29 UTC by kevin
Modified: 2014-12-24 12:47 UTC
See Also:
GNOME target: ---
GNOME version: ---

patch to fix the issue. (1.11 KB, patch)
2014-12-01 06:35 UTC, kevin
needs-work Details | Review
patch for block method. (3.27 KB, patch)
2014-12-15 08:42 UTC, kevin
none Details | Review
Updated patch. Use block method. Please review. (4.69 KB, patch)
2014-12-16 06:19 UTC, kevin
reviewed Details | Review
update the patch (5.67 KB, patch)
2014-12-23 07:01 UTC, kevin
needs-work Details | Review
update the patch based on review. the patch is based on latest code. (5.50 KB, patch)
2014-12-24 02:27 UTC, kevin
needs-work Details | Review
update the patch based on review. the patch is based on latest code. update typo. (5.50 KB, patch)
2014-12-24 02:40 UTC, kevin
committed Details | Review

Description kevin 2014-12-01 06:29:34 UTC
I have one stream which has multi audio tracks. One audio track is shorter than other tracks. when select the shorter audio track after the shorter audio track reach EOS, audio sink can't received EOS event.
The root cause is the sticky event is send in chain function. But the short audio track already reach EOS, so chain function can't be called. The resolution is send the sticky event when track switching. Attached the patch to fix it.

I also found chain function haven't handle active track reach EOS. How to handle it when sync mode? drop buffer when received without sync? How to handle switch to long track again? there will be mute long time as dropped buffer. How to handle switch shorter track again? Is sticky EOS event still there?
Comment 1 kevin 2014-12-01 06:35:35 UTC
Created attachment 291860 [details] [review]
patch to fix the issue.
Comment 2 Sebastian Dröge (slomo) 2014-12-01 08:47:39 UTC
Comment on attachment 291860 [details] [review]
patch to fix the issue.

This is not correct as it would send a serialized event from the application thread.

Instead the best would probably be to send EOS from the EOS of the other track
Comment 3 kevin 2014-12-01 09:40:38 UTC
If send EOS until other track reach EOS. How audio sink finish preroll if need PAUSE during wait other track reach EOS?
Comment 4 Sebastian Dröge (slomo) 2014-12-04 17:17:02 UTC
Ah, I see what you mean. Hmm... I think input-selector should block when it receives an EOS event for an unselected track. And when this track is then later selected, the EOS event will be sent directly.
Comment 5 kevin 2014-12-05 03:29:11 UTC
Yes, my patch is send the sticky event when the activated track reach EOS. If you can't accept my patch. How to send the EOS event? Who will trick the send?
Comment 6 Sebastian Dröge (slomo) 2014-12-05 08:32:17 UTC
As I said, your patch is wrong because it will send the EOS event from the application thread or whoever switches the pad.

What will need to happen is what I outlined in comment 4. The sinkpads of input-selector need to block on EOS (and handle flushing and everything correctly), and when you switch to one of those sinkpads it would be unblocked and then directly forward the EOS event.
Comment 7 kevin 2014-12-10 10:06:17 UTC
Add one semaphore to block EOS event for unactive track. Post the semaphore when the track actived. Is it right?
Comment 8 Sebastian Dröge (slomo) 2014-12-10 14:16:04 UTC
Yes, a GCond would be best... there might even be one already that you could reuse for this.
Comment 9 kevin 2014-12-12 06:37:52 UTC
I still have concern. How to handle select long track -> EOS track (send out the holded EOS event) -> long track -> EOS track (how to handle this?).
Comment 10 Sebastian Dröge (slomo) 2014-12-12 09:21:54 UTC
You won't handle that at all. The input-selector is EOS once a currently active track went EOS, and it won't be possible to switch to any other track (EOS or not) afterwards.

Making that work would require flushing the downstream of input-selector, but which would have to be done in a way that ensures no data is flushed away. And that's far from trivial.
Comment 11 kevin 2014-12-12 09:36:00 UTC
Got it, will generate patch later.
Comment 12 kevin 2014-12-15 08:35:16 UTC
I tried block EOS event, seems it will cause block whole pipeline. Our test clip is one audio track send EOS at the begin without any valid data.
Comment 13 Sebastian Dröge (slomo) 2014-12-15 08:41:27 UTC
Is this inside playbin? I think playbin (actually decodebin) is also supposed to aggregate all EOS first and only then forward them all. It currently does not do that.

What is the backtrace of all threads and can you share your patch?
Comment 14 kevin 2014-12-15 08:42:36 UTC
Created attachment 292742 [details] [review]
patch for block method.
Comment 15 Sebastian Dröge (slomo) 2014-12-15 08:52:14 UTC
Bug #738285 has an initial patch to send EOS in decodebin after all pads are drained. Related to that is also bug #738064 and bug #740045
Comment 16 Sebastian Dröge (slomo) 2014-12-15 08:56:45 UTC
I think it should be possible to reuse the existing condition variable for that. It is also used to block unselected sinkpads until they're active (or have fallen behind too much compared to the active one... but EOS means it can't fall behind anymore as it's too far ahead already).

How does it deadlock for you, what's the backtrace of all threads?
Comment 17 kevin 2014-12-15 09:35:29 UTC
Existing condition variable used for element and will be signaled all the time when playbacking. Can it be used for pad EOS blocking?
I will check why pipeline blocked. Will back after I got the reason.
Comment 18 Sebastian Dröge (slomo) 2014-12-15 09:40:39 UTC
You could check all the conditions after it stopped waiting, and if they are not met you would just wait again.
Comment 19 kevin 2014-12-15 09:56:13 UTC
Ok. The root cause of block is haven't release lock when blocked on EOS.
Will upload patch after more test. Thanks.
Comment 20 kevin 2014-12-16 06:19:23 UTC
Created attachment 292788 [details] [review]
Updated patch. Use block method. Please review.
Comment 21 Sebastian Dröge (slomo) 2014-12-18 10:08:47 UTC
Review of attachment 292788 [details] [review]:

::: plugins/elements/gstinputselector.c
@@ +562,3 @@
         forward = (active_selpad->eos && !active_selpad->eos_sent);
         active_selpad->eos_sent = TRUE;
+        gst_input_selector_eos_wait (sel, selpad);

You probably should go out here immediately if self->flushing is TRUE

@@ +563,3 @@
         active_selpad->eos_sent = TRUE;
+        gst_input_selector_eos_wait (sel, selpad);
+        forward = TRUE;

This looks wrong, you first set forward conditionally and then unconditionally to TRUE :)

I'm not sure all this logic is correct either. See the comment in the code a few lines above. I think if what was described in that comment happened, we shouldn't wait but forward the EOS event immediately instead of waiting.

@@ +567,2 @@
       GST_DEBUG_OBJECT (pad, "received EOS");

I think if we're forwarding the EOS event here, we should set eos_block to FALSE and wake up any other pads that might currently be waiting.

Also instead of eos_block... isn't this more a boolean that describes if the whole selector is EOS now? So maybe just call it self->eos or self->eos_sent or self->is_eos or something like that.
Comment 22 kevin 2014-12-19 02:33:45 UTC
Thanks for your review.
I will check more and back to here later.
Comment 23 kevin 2014-12-22 03:25:04 UTC
Below code in function gst_selector_pad_event().

  prev_active_sinkpad =
      sel->active_sinkpad ? gst_object_ref (sel->active_sinkpad) : NULL;
  active_sinkpad = gst_input_selector_activate_sinkpad (sel, pad);

  if (prev_active_sinkpad != active_sinkpad) {
    if (prev_active_sinkpad)
      g_object_notify (G_OBJECT (prev_active_sinkpad), "active");
    g_object_notify (G_OBJECT (active_sinkpad), "active");
    g_object_notify (G_OBJECT (sel), "active-pad");
  if (prev_active_sinkpad)
    gst_object_unref (prev_active_sinkpad);


  /* only forward if we are dealing with the active sinkpad */
  forward = (pad == active_sinkpad);

I want get active_sinkpad again before decide forward. So active pad EOS must forward as function gst_input_selector_set_active_pad() is also called after locked.

So below code is useless as if active_selpad->eos is true, active_selpad->eos_sent must be true. And active_selpad->eos_sent = TRUE isn't reasonable.

        GstSelectorPad *active_selpad;

        /* If the active sinkpad is in EOS state but EOS
         * was not sent downstream this means that the pad
         * got EOS before it was set as active pad and that
         * the previously active pad got EOS after it was
         * active
        active_selpad = GST_SELECTOR_PAD (active_sinkpad);
        forward = (active_selpad->eos && !active_selpad->eos_sent);
        active_selpad->eos_sent = TRUE;

And will add self->eos to indicate selector into EOS state and don't wait and discard all received buffer.

Is it ok? If OK, will submit patch later.
Comment 24 Sebastian Dröge (slomo) 2014-12-22 10:18:13 UTC
Yes, you're right that this code does not make any sense at all anymore. If forward==TRUE then pad==active_sinkpad at that point and then eos=eos_sent=TRUE.

This must be a leftover from some older refactoring.
Comment 25 kevin 2014-12-23 07:01:39 UTC
Created attachment 293223 [details] [review]
update the patch
Comment 26 kevin 2014-12-23 07:02:26 UTC
The patch should work with
Comment 27 Sebastian Dröge (slomo) 2014-12-23 12:16:25 UTC
Review of attachment 293223 [details] [review]:

::: plugins/elements/gstinputselector.c
@@ +1179,3 @@
+      res = GST_FLOW_OK;
+    else
+      res = GST_FLOW_NOT_LINKED;

Should be GST_FLOW_EOS in both cases

::: plugins/elements/gstinputselector.h
@@ +79,3 @@
   gboolean blocked;
+  gboolean eos;
+  gboolean eos_blocked;

Can't you use a single variable for both? Just eos?
Comment 28 kevin 2014-12-24 02:27:35 UTC
Created attachment 293313 [details] [review]
update the patch based on review. the patch is based on latest code.
Comment 29 kevin 2014-12-24 02:40:18 UTC
Created attachment 293315 [details] [review]
update the patch based on review. the patch is based on latest code. update typo.
Comment 30 Sebastian Dröge (slomo) 2014-12-24 12:47:21 UTC
Thanks, looks mostly good now. I did some minor changes on top of it and merged it :)

commit bc1ec4ec5f4879747c0535a163e704afa7f33654
Author: Sebastian Dröge <>
Date:   Wed Dec 24 13:46:28 2014 +0100

    inputselector: Use the same waiting function for EOS and non-EOS waiting

commit a1c183417f0a93f0669f8c561a38fe39fdafd453
Author: Sebastian Dröge <>
Date:   Wed Dec 24 13:44:09 2014 +0100

    inputselector: Wake up all waitings pads directly if we forward the EOS event
    Otherwise they might wait a bit longer unnecessarily.
    Also do some minor cleanup.

commit 11e4b744f5e7b686785cfce9081193c5e5ff6a5d
Author: Song Bing <>
Date:   Wed Dec 24 10:13:51 2014 +0800

    inputselector: Block when receiving an EOS event on a deactivated pad
    ... and only unblock when either a) the pad becomes active and the event
    should be forwarded or b) the active pad went EOS itself.
    Otherwise it can happen that we switch from a longer track that is not EOS yet
    to a shorter track that already is EOS, but the shorter track won't have any
    possibility to send its EOS event downstream anymore.