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 725917 - multiqueue: do not reset last push result when pushing events
multiqueue: do not reset last push result when pushing events
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal blocker
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-03-07 20:52 UTC by Thiago Sousa Santos
Modified: 2014-03-15 15:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
multiqueue: do not reset last push result when pushing events (1.20 KB, patch)
2014-03-07 20:53 UTC, Thiago Sousa Santos
committed Details | Review
tests: multiqueue: fix eos count on test for not-linked case (2.14 KB, patch)
2014-03-14 16:40 UTC, Thiago Sousa Santos
committed Details | Review

Description Thiago Sousa Santos 2014-03-07 20:52:58 UTC
Multiqueue currently always sets the src pad flow return to OK
when pushing events, ignoring any previous set value
Comment 1 Thiago Sousa Santos 2014-03-07 20:53:00 UTC
Created attachment 271266 [details] [review]
multiqueue: do not reset last push result when pushing events

Use the last result as a default when pushing a item from a single queue,
otherwise the status gets reset to _OK when pushing events.

This causes problems when mistakenly activating a not-linked stream
that is being ignored upstream as it is not being used (adaptive
scenarios), it will make the multiqueue post a buffering message
on a pad that won't receive buffers
Comment 2 Sebastian Dröge (slomo) 2014-03-08 14:21:29 UTC
Looks like the same problem exists in queue and queue2. Want to fix it there too?
Comment 3 Thiago Sousa Santos 2014-03-08 23:16:54 UTC
Sure, doing some final tests and will push right after.
Comment 4 Thiago Sousa Santos 2014-03-10 16:12:05 UTC
commit 3ed2507ebc773ceb830012ae260949a52fe442a7
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Mon Mar 10 09:49:07 2014 -0300

    queue: queue2: preserve last flow result when pushing events
    
    Avoids mistakenly returning _OK when downstream is still
    _NOT_LINKED on subsequent received pad pushes
    
    https://bugzilla.gnome.org/show_bug.cgi?id=725917

commit b6470a7b3f26c203112db57b58afe8533b877963
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Mon Mar 10 09:48:58 2014 -0300

    multiqueue: do not reset last push result when pushing events
    
    Use the last result as a default when pushing a item from a single queue,
    otherwise the status gets reset to _OK when pushing events.
    
    This causes problems when mistakenly activating a not-linked stream
    that is being ignored upstream as it is not being used (adaptive
    scenarios), it will make the multiqueue post a buffering message
    on a pad that won't receive buffers
Comment 5 Tim-Philipp Müller 2014-03-11 21:43:32 UTC
This commit appears to make the test_sparse_stream sometimes timeout for me, reproduce with make elements/multiqueue.forever
Comment 6 Thiago Sousa Santos 2014-03-11 23:34:49 UTC
Last message showing the issue:

multiqueue gstmultiqueue.c:1290:gst_multi_queue_loop:<multiqueue0> queue 1 sleeping for not-linked wakeup with newid 205, highid 204, next_time 0:00:00.000000000, high_time 0:00:09.900000000

so this queue will wait forever and never push the EOS to finish the test, it is waiting for another stream that is already EOS and shouldn't push more data.

Before waiting we should check that there is something to wait for.

Also this got me thinking about considering a TRUE return on serialized events push as a GST_FLOW_OK return.
Comment 7 Thiago Sousa Santos 2014-03-14 16:40:51 UTC
Created attachment 271923 [details] [review]
tests: multiqueue: fix eos count on test for not-linked case

So I rethinked a bit about the not-linked return handling and found out
the test is wrong, here is the commit message for this:

From the test case:

/* This test creates a multiqueue with 2 streams. One receives
 * a constant flow of buffers, the other only gets one buffer, and then
 * new-segment events, and returns not-linked. The multiqueue should not fill.
 */

If one of the queues goes EOS and the other returns NOT_LINKED the stream
can be considerered EOS as a NOT_LINKED means that one of the branches has no
sink downstream that will block the EOS message posting.
Comment 8 Sebastian Dröge (slomo) 2014-03-15 11:47:24 UTC
Comment on attachment 271923 [details] [review]
tests: multiqueue: fix eos count on test for not-linked case

Makes sense IMHO
Comment 9 Thiago Sousa Santos 2014-03-15 15:41:11 UTC
commit 22258782d8870b14c433e448cebf502dd57c30ca
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Fri Mar 14 13:32:17 2014 -0300

    tests: multiqueue: fix eos count on test for not-linked case
    
    From the test case:
    
    /* This test creates a multiqueue with 2 streams. One receives
     * a constant flow of buffers, the other only gets one buffer, and then
     * new-segment events, and returns not-linked. The multiqueue should not fil
     */
    
    If one of the queues goes EOS and the other returns NOT_LINKED the stream
    can be considerered EOS as a NOT_LINKED means that one of the branches has n
    sink downstream that will block the EOS message posting.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=725917