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 647769 - [decodebin2] Fix preroll for streams at low bitrate
[decodebin2] Fix preroll for streams at low bitrate
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.36
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 659924
 
 
Reported: 2011-04-14 11:58 UTC by Andoni Morales
Modified: 2011-11-25 10:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix preroll for streams at low bitrates (1.20 KB, patch)
2011-04-14 11:58 UTC, Andoni Morales
rejected Details | Review
Fix preroll for streams at low bitrates (4.21 KB, patch)
2011-05-09 20:31 UTC, Andoni Morales
committed Details | Review
decodebin2: fix prerolling for low bitrate streams from hlsdemux (2.04 KB, patch)
2011-11-08 14:03 UTC, Vincent Penquerc'h
none Details | Review
decodebin2: fix prerolling for low bitrate streams from hlsdemux (2.23 KB, patch)
2011-11-08 18:56 UTC, Vincent Penquerc'h
needs-work Details | Review
decodebin2: fix prerolling for low bitrate streams from hlsdemux (6.79 KB, patch)
2011-11-24 17:50 UTC, Vincent Penquerc'h
committed Details | Review

Description Andoni Morales 2011-04-14 11:58:47 UTC
Created attachment 185952 [details] [review]
Fix preroll for streams at low bitrates

Add a queue limit in time too for streams at low bitrates, where the limit in bytes can happen after more than 40 seconds.
Comment 1 Sebastian Dröge (slomo) 2011-04-14 12:19:50 UTC
The time limit was removed because of bug #584104.
Comment 2 Sebastian Dröge (slomo) 2011-04-15 09:03:05 UTC
The IRC discussion resulted in the idea that a time limit should be added if the source is "live", i.e. if the source is not seekable.
Comment 3 Sebastian Dröge (slomo) 2011-05-09 09:53:22 UTC
Do you want to work on this Andoni?
Comment 4 Andoni Morales 2011-05-09 20:31:19 UTC
Created attachment 187528 [details] [review]
Fix preroll for streams at low bitrates

Like we discussed on IRC, the limit on time is only set if upstream is seekable.
Comment 5 Sebastian Dröge (slomo) 2011-05-10 14:02:10 UTC
Thanks, will push this after release.
Comment 6 Sebastian Dröge (slomo) 2011-05-14 09:54:29 UTC
commit dd36e4cd0ecffdc51129c33d8e8a75a8488189a1
Author: Andoni Morales Alastruey <ylatuya@gmail.com>
Date:   Mon May 9 22:20:23 2011 +0200

    decodebin2: fix preroll for streams at low bitrates
    
    For streams at low bitrates we need to set a limit in time because the limit
    in bytes might not reached too late, sometimes more than 30 seconds.
    This limit can only be set if upstream is seekable (see #584104)
    Closes #647769
Comment 7 Vincent Penquerc'h 2011-09-29 11:08:14 UTC
Hello there.
I ended up here while looking at a failure to preroll on some radio stream.
The patch that was committed adds a time limit to non seekable streams, but this would seem to make more sense to add it to seekable streams, as non seekable streams are usually outputting data at a set time rate. I understand slomo's comment above to also say that. What is the rationale to keeping this time limit to seekable streams (reading 584104 does not immediately tell me why this should be so).

Adding the time limit for non seekable streams allows my unseekable UDP streamed MPEG/TS radio stream to preroll again, which makes me happy, so I'd like to know what it'd break, or if the only-on-seekable should be only-on-non-seekable.
Comment 8 Andoni Morales 2011-09-29 11:59:13 UTC
I think this patch ended-up doing the contrary that what was discussed in IRC :) This time limit should only be added for live streams:

-#define AUTO_PREROLL_NOT_SEEKABLE_SIZE_TIME      0
-#define AUTO_PREROLL_SEEKABLE_SIZE_TIME          10 * GST_SECOND
+#define AUTO_PREROLL_NOT_SEEKABLE_SIZE_TIME      10 * GST_SECOND
+#define AUTO_PREROLL_SEEKABLE_SIZE_TIME          0
Comment 9 Vincent Penquerc'h 2011-09-30 21:27:01 UTC
Reopening to get the change above applied.
Andoni, could you make that a git patch, please ?
Comment 10 Sebastian Dröge (slomo) 2011-10-03 08:56:51 UTC
commit 9117681b35f122c1f5ca4b1a701435f21070b9c6
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Mon Oct 3 10:55:53 2011 +0200

    decodebin2: Use a TIME limit for pre-rolling in live streams and not in non-
    
    Fixes bug #647769 for real.
Comment 11 Julien Isorce 2011-10-07 13:16:06 UTC
but now gst-launch playbin2 uri=http://devimages.apple.com/iphone/samples/bipbop/gear1/prog_index.m3u8
does not work because of #10
Comment 12 Vincent Penquerc'h 2011-10-07 15:33:12 UTC
That stream is detected as seekable.
Maybe we should check whether we're in push mode, and act as non-seekable when in push mode. I'm unclear as to why there is a distinction between seekable and not seekable in the first place though.
Comment 13 Vincent Penquerc'h 2011-11-08 14:03:36 UTC
Created attachment 200985 [details] [review]
decodebin2: fix prerolling for low bitrate streams from hlsdemux

Such streams were detected as seekable, as the query on the typefind
element was testing the m3u8 file listing the actual streams, and
not going through the demuxer(s).

We now check for seekability when adding a new demuxer, so the query
will flow through the elements which might prevent seeking.
Comment 14 Sebastian Dröge (slomo) 2011-11-08 14:38:44 UTC
The idea is correct but are you sure this works?

Above this code gst_decode_chain_get_current_group() is already called, which would create the multiqueue. Also there could be multiple demuxers and thus multiple different results from check_upstream_seekable().
Comment 15 Vincent Penquerc'h 2011-11-08 15:04:20 UTC
It works with the test case above.
There are two demuxers in that case, yes, hlsdemux, and mpegtsdemux.
Since the test is done again after adding a new demuxer, the last call should be done on the most downstream demuxer, which seems right.

So it seems the seekability checks fails with hlsdemux, which makes that sample work, but if seekability worked in hlsdemux, but failed in mpegtsdemux instead, then the sample would not work, if I read your comment right ?

Best thing would probably be to reconfigure the multiqueue after each attempt to determine seekability, what do you think ? If you agree, I will change it so. It should be the more robust way.
Comment 16 Vincent Penquerc'h 2011-11-08 18:56:19 UTC
Created attachment 201024 [details] [review]
decodebin2: fix prerolling for low bitrate streams from hlsdemux

Such streams were detected as seekable, as the query on the typefind
element was testing the m3u8 file listing the actual streams, and
not going through the demuxer(s).

We now check for seekability when adding a new demuxer, so the query
will flow through the elements which might prevent seeking.
Comment 17 Vincent Penquerc'h 2011-11-08 18:59:01 UTC
An existing multiqueue is now always reconfigured after that new code reprobes seekability.
Still works with both my own test case and the m3u8 one.
Comment 18 Sebastian Dröge (slomo) 2011-11-09 07:41:01 UTC
That wasn't what I meant. You currently store seekability directly inside decodebin2 but if there are multiple demuxers (e.g. MXF file containing a DV container) the seekability could be different for both demuxers and you should use different multiqueue settings for the two multiqueues. Note that there always is a single multiqueue after a demuxer and that the last element of a GstDecodeChain is always a demuxer if chain->demuxer==TRUE. So it would be better to just configure the multiqueue correctly when it's instantiated and immediately before that check seekability on the current demuxer. It's not even necessary to store the result then.
Comment 19 Vincent Penquerc'h 2011-11-09 11:38:59 UTC
Hmm, I did not realize there could be more than one multiqueue.

Still, if that's the case, we want to configure each multiqueue to the most conservative option, don't we ? Because if only one demuxer can seek, the whole chain will not seek, and thus needs the "unseekable" limits, no ?
That would mean we need to walk the chains/groups and find each multiqueue when adding a new demuxer.
Comment 20 Andreas Frisch 2011-11-15 11:29:40 UTC
the patch works for me and seems to fix my bug #663858
Comment 21 Sebastian Dröge (slomo) 2011-11-15 17:04:05 UTC
(In reply to comment #19)
> Hmm, I did not realize there could be more than one multiqueue.
> 
> Still, if that's the case, we want to configure each multiqueue to the most
> conservative option, don't we ? Because if only one demuxer can seek, the whole
> chain will not seek, and thus needs the "unseekable" limits, no ?
> That would mean we need to walk the chains/groups and find each multiqueue when
> adding a new demuxer.

This is not really about seekability but more about live/non-live streams. I think we should use the limits that apply to that exact multiqueue and not the same (most conservative) limits on every multiqueue.

(Note: in 0.11 this should be replaced by the scheduling query or something. We're only using a TIME limit for non-seekable streams here because we assume that non-seekable streams are "live" and that it can take a lot of time until the BYTE/BUFFERS limits are reached for low-bitrate streams. For seekable streams we assume that we can read very fast and don't use the TIME limit to allow playback of some weird MPEG TS streams)
Comment 22 Vincent Penquerc'h 2011-11-15 17:23:37 UTC
Why not use the latency query, which has a "live" parameter, then ?
Comment 23 Sebastian Dröge (slomo) 2011-11-15 17:28:37 UTC
Because it's not really "live" in the GStreamer sense. For example for a normal HTTP stream you don't want to use the TIME limits either.

In 0.11 this would map to the scheduling query and if it returns random-access you're not going to use the TIME limit, if it's not random-access you will use the TIME limit. Unfortunately we don't have anything better in 0.10 for this other than the SEEKING query.
Comment 24 Vincent Penquerc'h 2011-11-24 17:50:40 UTC
Created attachment 202081 [details] [review]
decodebin2: fix prerolling for low bitrate streams from hlsdemux

Such streams were detected as seekable, as the query on the typefind
element was testing the m3u8 file listing the actual streams, and
not going through the demuxer(s).

We now check for seekability for each multiqueue following a demuxer,
so the query will flow through the elements which might prevent seeking.
Comment 25 Vincent Penquerc'h 2011-11-24 17:53:15 UTC
I'm not totally at ease in that code, so hopefully it's alright.
Works with my test case and the apple one above.
The new "seekable" parameter is true unconditionally when not in preroll mode, as it is ignored in that case.
Comment 26 Sebastian Dröge (slomo) 2011-11-25 10:14:33 UTC
slomo@thor:~/Projects/gstreamer/head/gst-plugins-base/gst/playback$ git log
commit a5535e76e069e1b304298ad56147b023a436bfaa
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Fri Nov 25 11:11:12 2011 +0100

    decodebin2: Set the multiqueue limits to the playing limits after overrun too
    
    We don't expect any new pads anymore and prerolling is finished now.

commit 494b2cb1a72ce3350e174ba60550a81eae5dfa56
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Fri Nov 25 11:08:58 2011 +0100

    decodebin2: Cache the upstream seekability for demuxer decode chains and use it for the non-prero
    
    After preroll the multiqueue limits are still set to the preroll
    limits if use-buffering is set to TRUE. In that case we only want
    time limits on the multiqueue if upstream is seekable.

commit 59f5d980f60a7fe10c8088bd32a08448709bdc79
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Tue Nov 8 13:55:58 2011 +0000

    decodebin2: fix prerolling for low bitrate streams from hlsdemux
    
    Such streams were detected as seekable, as the query on the typefind