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 746518 - input-selector: eos is always forwarded, even from unselected pads
input-selector: eos is always forwarded, even from unselected pads
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 736891
 
 
Reported: 2015-03-20 10:26 UTC by Thiago Sousa Santos
Modified: 2015-03-24 12:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: input-selector: verify that eos from unselected pads block (3.17 KB, patch)
2015-03-20 10:26 UTC, Thiago Sousa Santos
none Details | Review
patch (1.55 KB, patch)
2015-03-23 12:15 UTC, Jan Alexander Steffens (heftig)
committed Details | Review
tests: selector: remove weird semicolons at the end of test functions (2.15 KB, patch)
2015-03-23 22:30 UTC, Thiago Sousa Santos
committed Details | Review
tests: input-selector: new tests for EOS handling (7.70 KB, patch)
2015-03-23 22:30 UTC, Thiago Sousa Santos
none Details | Review
tests: input-selector: new tests for EOS handling (8.02 KB, patch)
2015-03-24 12:06 UTC, Thiago Sousa Santos
committed Details | Review

Description Thiago Sousa Santos 2015-03-20 10:26:48 UTC
The attached unit tests shows that input-selector is always forwarding
EOS, regardless of the pad that received it
Comment 1 Thiago Sousa Santos 2015-03-20 10:26:57 UTC
Created attachment 299933 [details] [review]
tests: input-selector: verify that eos from unselected pads block

Adds a test that creates 2 inputs for input-selector, one of which
is an empty stream (just pushes an EOS event) while the other has 1
buffer before EOS. The selected stream is the one that has the buffer.

input-selector should block the EOS on the unselected pad and forward
the buffer on the other one before forwarding the EOS (once all pads
are EOS)
Comment 2 Jan Alexander Steffens (heftig) 2015-03-23 12:15:15 UTC
Created attachment 300126 [details] [review]
patch

Try this, please. Should apply straight to master.
Comment 3 Jan Alexander Steffens (heftig) 2015-03-23 13:19:39 UTC
With the waiting working properly, the new test times out since pushing the EOS on the empty_stream_pad now blocks.
Comment 4 Thiago Sousa Santos 2015-03-23 13:22:52 UTC
I have an improved version here that deals with the blocking, still need to add more test cases, should be ready hopefully later today or tomorrow.
Comment 5 Thiago Sousa Santos 2015-03-23 22:30:02 UTC
Created attachment 300157 [details] [review]
tests: selector: remove weird semicolons at the end of test functions

Even though it works, it is not needed and seems more natural
to not have semicolons at the end of function declarations
Comment 6 Thiago Sousa Santos 2015-03-23 22:30:11 UTC
Created attachment 300158 [details] [review]
tests: input-selector: new tests for EOS handling

3 new tests:

1) Tests that a stream that is empty (just an EOS event)
   on inactive pad doesn't get through and tamper
   with the active pad that still has data

2) Tests that a stream that is shorter than the active one
   (pushes EOS earlier) doesn't has its EOS pushed

3) Tests that switching to an inactive stream that has received
   EOS will make input-selector push EOS
Comment 7 Jan Alexander Steffens (heftig) 2015-03-24 07:30:44 UTC
The selector tests pass here even with the unfixed master @ 7b0b93d. Isn't that wrong?
Comment 8 Thiago Sousa Santos 2015-03-24 11:26:00 UTC
Running suite(s): selector
77%: Checks: 9, Failures: 2, Errors: 0
elements/selector.c:525:F:general:test_input_selector_empty_stream:0: Assertion 'gst_pad_push_event (pad, gst_event_new_eos ())' failed
elements/selector.c:486:F:general:test_input_selector_switch_to_eos_stream:0: Assertion 'gst_pad_push (pad, buf) == GST_FLOW_OK' failed
Running suite(s): selector
77%: Checks: 9, Failures: 2, Errors: 0

I have them failing here.
Comment 9 Thiago Sousa Santos 2015-03-24 12:06:42 UTC
Created attachment 300193 [details] [review]
tests: input-selector: new tests for EOS handling

3 new tests:

1) Tests that a stream that is empty (just an EOS event)
   on inactive pad doesn't get through and tamper
   with the active pad that still has data

2) Tests that a stream that is shorter than the active one
   (pushes EOS earlier) doesn't has its EOS pushed

3) Tests that switching to an inactive stream that has received
   EOS will make input-selector push EOS
Comment 10 Thiago Sousa Santos 2015-03-24 12:32:13 UTC
commit ce663311e1f00df5117b0249931e4ac960123cb8
Author: Jan Alexander Steffens (heftig) <jsteffens@make.tv>
Date:   Mon Mar 23 13:05:30 2015 +0100

    input-selector: Fix waiting on EOS
    
    This apparently got broken by bc1ec4e. Since self->blocked is always
    FALSE, gst_input_selector_wait never actually waits.
    
    Using (!self->eos || self->blocked) && ... as the loop condition would
    be incorrect as well, because then the other call to the function in
    _chain would block until EOS, so the functions cannot be merged trivially.
    
    Since blocking is obsolete, gst_input_selector_wait will get removed anyway.
    As such, just inline the loop.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=746518

commit 150e8a5c972fde2275ce93e23402b29a59129a47
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Fri Mar 20 07:23:53 2015 -0300

    tests: input-selector: new tests for EOS handling
    
    3 new tests:
    
    1) Tests that a stream that is empty (just an EOS event)
       on inactive pad doesn't get through and tamper
       with the active pad that still has data
    
    2) Tests that a stream that is shorter than the active one
       (pushes EOS earlier) doesn't has its EOS pushed
    
    3) Tests that switching to an inactive stream that has received
       EOS will make input-selector push EOS
    
    https://bugzilla.gnome.org/show_bug.cgi?id=746518

commit 92d2351b2e97668e00d5d60bddf4123cf293fc9a
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Thu Mar 19 12:11:19 2015 +0000

    tests: selector: remove weird semicolons at the end of test functions
    
    Even though it works, it is not needed and seems more natural
    to not have semicolons at the end of function declarations
    
    https://bugzilla.gnome.org/show_bug.cgi?id=746518