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 744308 - decodebin: race between setting use-buffering and max-size-time
decodebin: race between setting use-buffering and max-size-time
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.4.5
Other All
: Normal normal
: 1.4.6
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-02-11 05:06 UTC by Duncan Palmer
Modified: 2015-03-24 16:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix for the problem (3.41 KB, patch)
2015-02-11 05:09 UTC, Duncan Palmer
none Details | Review
Patch against master (5.06 KB, patch)
2015-02-15 23:27 UTC, Duncan Palmer
reviewed Details | Review
Set multiqueue sizes before use-buffering (3.57 KB, patch)
2015-02-24 06:16 UTC, Duncan Palmer
committed Details | Review

Description Duncan Palmer 2015-02-11 05:06:48 UTC
I'm using uridecodebin to play some HLS content, and have the 'use-buffering' property set to TRUE, and also set 'buffer-duration' and 'buffer-size'.

On a variant switch, a new decode group is created in the decodebin. When this decode group becomes active, the use-buffering property on the multiqueue in that group is set to TRUE, and then the max-size-time property is changed from 10 seconds (set when the multiqueue is created) to buffer-duration seconds (3.5 seconds in my case). Buffering messages are sometimes emitted by the multiqueue in the window between setting use-buffering and max-size-time - there's a race. This can clearly be seen in the log snippet below, which is filtered for clarity:

     decodebin gstdecodebin2.c:3671:gst_decode_group_reset_buffering:<decodebin0> Group reset buffering 0x8fb828 multiqueue9
    multiqueue gstmultiqueue.c:977:gst_multi_queue_post_buffering:<multiqueue9> Going to post buffering: 80%
     decodebin gstdecodebin2.c:3190:decodebin_set_queue_size:<multiqueue9> use buffering 1
     decodebin gstdecodebin2.c:3234:decodebin_set_queue_size:<multiqueue9> setting limits 9175040 bytes, 0 buffers, 3500000000
    multiqueue gstmultiqueue.c:938:update_buffering:<multiqueue9> buffering 230 percent
Comment 1 Duncan Palmer 2015-02-11 05:08:01 UTC
oops, that log snippet is missing a line. Try this one instead:

     decodebin gstdecodebin2.c:3671:gst_decode_group_reset_buffering:<decodebin0> Group reset buffering 0x8fb828 multiqueue9
    multiqueue gstmultiqueue.c:977:gst_multi_queue_post_buffering:<multiqueue9> Going to post buffering: 80%
     decodebin gstdecodebin2.c:3190:decodebin_set_queue_size:<multiqueue9> use buffering 1
     decodebin gstdecodebin2.c:3234:decodebin_set_queue_size:<multiqueue9> setting limits 9175040 bytes, 0 buffers, 3500000000
    multiqueue gstmultiqueue.c:938:update_buffering:<multiqueue9> buffering 230 percent
    multiqueue gstmultiqueue.c:977:gst_multi_queue_post_buffering:<multiqueue9> Going to post buffering: 100%
Comment 2 Duncan Palmer 2015-02-11 05:09:08 UTC
Created attachment 296557 [details] [review]
Fix for the problem
Comment 3 Thiago Sousa Santos 2015-02-13 21:44:09 UTC
Would you mind rebasing your patch for master? It doesn't apply cleanly there
Comment 4 Duncan Palmer 2015-02-15 23:27:48 UTC
Created attachment 296898 [details] [review]
Patch against master

Here it is.

When I call decodebin_set_queue_size() from multi_queue_overrun_cb(), or no_more_pads_cb(), I query the multiqueue for it's current buffering configuration. I'm doing this, as these signals seem not to be disconnected after the decode group is exposed, so I guess these callbacks could conceivable be called again (tho unlikely).
Comment 5 Thiago Sousa Santos 2015-02-19 23:07:06 UTC
Review of attachment 296898 [details] [review]:

Thanks for the patch. It looks good and should fix the issue. I have only one small suggestion that could make the code clearer IMHO. How about having decodebin_set_queue_size() and decodebin_set_queue_size_full(), the later would have the extra 'use_buffering' parameter while the former keeps the same parameters we have now, gets the "use-buffering" from the multiqueue and passes it to the _full() version.

Also, please attach patches in git format-patch so that it keeps the commit message when applied to other trees.
Comment 6 Duncan Palmer 2015-02-24 06:16:02 UTC
Created attachment 297730 [details] [review]
Set multiqueue sizes before use-buffering

Sounds like a good suggestion. This patch implements the changes against master. Presume the existing patch against 1.4 is ok?
Comment 7 Sebastian Dröge (slomo) 2015-03-23 09:42:09 UTC
Thiago?
Comment 8 Thiago Sousa Santos 2015-03-24 16:45:24 UTC
Sorry for the delay here, forgot about this one.

commit bf3e35a598afda180c4f392726043b272dffc1b0
Author: Duncan Palmer <dpalmer@digisoft.tv>
Date:   Mon Feb 16 09:25:03 2015 +1000

    decodebin2: Set multiqueue sizes before use-buffering.
    
    This fixes a race where the use-buffering property on a multiqueue was
    set before the queue depth was changed from it's high preroll limits to
    lower playback limits. This resulted in buffering messages being emitted
    by the multiqueue in the short window between use-buffering being
    set and the queue depth being reset.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=744308


In 1.4:
commit d6ff4bd7f3c7cfbe63cb78e83443ad450f0376de
Author: Duncan Palmer <dpalmer@digisoft.tv>
Date:   Mon Feb 16 09:25:03 2015 +1000

    decodebin2: Set multiqueue sizes before use-buffering.
    
    This fixes a race where the use-buffering property on a multiqueue was
    set before the queue depth was changed from it's high preroll limits to
    lower playback limits. This resulted in buffering messages being emitted
    by the multiqueue in the short window between use-buffering being
    set and the queue depth being reset.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=744308
    
    Conflicts:
        gst/playback/gstdecodebin2.c