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 736891 - input-selector: Can't unblock after emitting a "block" signal
input-selector: Can't unblock after emitting a "block" signal
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.2.3
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 746518
Blocks: 739620
 
 
Reported: 2014-09-18 12:20 UTC by Tzafrir Rehan
Modified: 2015-03-24 13:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch removes signal (7.23 KB, patch)
2014-11-19 11:05 UTC, Jan Alexander Steffens (heftig)
none Details | Review
patch 1/2 (4.06 KB, patch)
2015-03-23 13:26 UTC, Jan Alexander Steffens (heftig)
committed Details | Review
patch 2/2 (4.54 KB, patch)
2015-03-23 13:27 UTC, Jan Alexander Steffens (heftig)
committed Details | Review

Description Tzafrir Rehan 2014-09-18 12:20:46 UTC
After emitting a "block" signal on a selector, it is impossible to unblock the streams
Comment 1 Jan Alexander Steffens (heftig) 2014-11-19 08:19:11 UTC
Maybe the block signal should just be removed? As I understand, any existing code trying to use this will just start throwing g_criticals instead of locking up the pipeline, so that's okay.
Comment 2 Jan Alexander Steffens (heftig) 2014-11-19 11:05:07 UTC
Created attachment 290971 [details] [review]
patch removes signal

[PATCH] input-selector: Remove obsolete 'block' signal

This signal blocks the input-selector with no means of unblocking
other than a state change back to READY. Seems it was part of an
old way of switching together with the already-removed 'switch'
signal.
Comment 3 Jan Alexander Steffens (heftig) 2015-03-10 10:15:31 UTC
Please review the patch.
Comment 4 Sebastian Dröge (slomo) 2015-03-23 10:07:17 UTC
Review of attachment 290971 [details] [review]:

Seems almost ok, but please add some explanation in the commit message *why* it is ok to drop the signal. (Nobody could've used it for anything different than causing deadlocks)

::: plugins/elements/gstinputselector.c
@@ -960,3 @@
   GST_INPUT_SELECTOR_LOCK (sel);
-  /* wait or check for flushing */
-  if (gst_input_selector_wait (sel, selpad)) {

wait() actually *waited* for self->flushing or pad->flushing (or not blocking). This changes behaviour I think
Comment 5 Jan Alexander Steffens (heftig) 2015-03-23 11:27:30 UTC
(In reply to Sebastian Dröge (slomo) from comment #4)

> wait() actually *waited* for self->flushing or pad->flushing (or not 
blocking). This changes behaviour I think

Actually, no. Without the signal, self->blocked cannot ever be TRUE. Accordingly, gst_input_selector_wait will never wait and just return self->flushing.
Comment 6 Jan Alexander Steffens (heftig) 2015-03-23 13:26:51 UTC
Created attachment 300128 [details] [review]
patch 1/2

This is now on top of the patch from bug 746518.
Comment 7 Jan Alexander Steffens (heftig) 2015-03-23 13:27:12 UTC
Created attachment 300129 [details] [review]
patch 2/2
Comment 8 Thiago Sousa Santos 2015-03-24 13:11:33 UTC
Thanks for the patches, fixed.

commit 0a3ded193278ad95b04ace3dc5e2fcf7e277abff
Author: Jan Alexander Steffens (heftig) <jsteffens@make.tv>
Date:   Mon Mar 23 13:20:34 2015 +0100

    input-selector: Remove 'blocked' flag
    
    With the disappearance of the 'block' signal, this
    flag cannot be set to TRUE.
    
    gst_input_selector_wait disappears as it never waits
    and just returns self->flushing.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=736891

commit 7a1ebbe0e3a19c4e2108ddbb50095add3d265d9d
Author: Jan Alexander Steffens (heftig) <jsteffens@make.tv>
Date:   Mon Mar 23 12:12:51 2015 +0100

    input-selector: Remove obsolete 'block' signal
    
    This signal blocks the input-selector with no means of unblocking
    other than a state change back to READY. It seems this signal was
    part of an old way of synchronously switching the selector,
    together with the already-removed 'switch' signal.
    
    Removing the signal is safe, as attempting to use it could only
    end in deadlocks. Attempting to emit an unknown signal just causes
    g_criticals.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=736891