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 731474 - playbin stalls when trying to play the audio portion of a stream with flags="audio"
playbin stalls when trying to play the audio portion of a stream with flags="...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal normal
: 1.2.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-06-10 18:16 UTC by GstBlub
Modified: 2014-06-25 15:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
baseparse: avoid returning _OK for _NOT_LINKED (1.65 KB, patch)
2014-06-17 19:27 UTC, Thiago Sousa Santos
committed Details | Review

Description GstBlub 2014-06-10 18:16:54 UTC
playbin doesn't play just the audio of a stream, but it can play just the video (flags=video), or both (omitting flags):

The following does not work and always gets stuck while buffering:
gst-launch-1.0 playbin uri="http://playertest.longtailvideo.com/adaptive/bipbop/bipbop.m3u8" flags=audio
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Prerolled, waiting for buffering to finish...
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstPulseSinkClock
Buffering, setting pipeline to PAUSED ...

However, video-only works:
gst-launch-1.0 playbin uri="http://playertest.longtailvideo.com/adaptive/bipbop/bipbop.m3u8" flags=video
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Got context from element 'glimagesink0': gst.gl.GLDisplay=context, gst.gl.GLDisplay=(GstGLDisplay)"\(GstGLDisplayX11\)\ gldisplayx11-0";
libGL error: failed to authenticate magic 7
libGL error: failed to load driver: vboxvideo
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock

And also video+audio:
gst-launch-1.0 playbin uri="http://playertest.longtailvideo.com/adaptive/bipbop/bipbop.m3u8"
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Got context from element 'glimagesink0': gst.gl.GLDisplay=context, gst.gl.GLDisplay=(GstGLDisplay)"\(GstGLDisplayX11\)\ gldisplayx11-0";
libGL error: failed to authenticate magic 8
libGL error: failed to load driver: vboxvideo
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstPulseSinkClock

Using the audio-sink and video-sink properties instead of the flags property doesn't work, either.  In fact, neither audio-only nor video-only playback works this way, setting the appropriate sinks to fakesink and auto(audio|video)sink.
Comment 1 Vincent Penquerc'h 2014-06-12 10:55:48 UTC
Race condition, it seems. I tried, it got stuck, then I tried with logs, and it played through.

Got it to be stuck with logs later though, so I have something to look at.
Comment 2 Vincent Penquerc'h 2014-06-12 14:30:02 UTC
Using video-sink='fakesink sync=1' works consistently.

As is disabling the pause/play on buffering messages.

It looks like the same issue as https://bugzilla.gnome.org/show_bug.cgi?id=655790.

The sink in preroll may or may not be a red herring.
Comment 3 GstBlub 2014-06-12 16:12:43 UTC
This race condition also seems to affect playback beyond prerolling.  With the video-sink='fakesink sync=1' work-around I can get playback to get stuck consistently when the stream goes to buffering and logging is enabled.  If I reduce the logging to the defaults, playback consistently resumes after buffering.
Comment 4 GstBlub 2014-06-12 16:20:22 UTC
(In reply to comment #3)
> This race condition also seems to affect playback beyond prerolling.  With the
> video-sink='fakesink sync=1' work-around I can get playback to get stuck
> consistently when the stream goes to buffering and logging is enabled.  If I
> reduce the logging to the defaults, playback consistently resumes after
> buffering.

"Consistently" means "most of the time", really.  I still got it to get stuck while buffering, it just took a whole lot longer with logging disabled.
Comment 5 Vincent Penquerc'h 2014-06-13 14:23:02 UTC
It seems to be some kind of race to do with prerolling.

The hang case shows alsasink waiting for preroll on an audio buffer, then the multiqueue fills up with video till filled. The sink never gets back from the preroll wait on audio, but is the only sink (an xvimagesink is actually created too, but is shut down at start).

That wait is done by the base audio sink, when it gets interrupted in the playback when the pipeline goes from PLAYING back to PAUSED.

However, another thread was concurrently doing the PLAYING->PAUSED state change, which just signalled the preroll cond var, so when the render thread goes to wait for preroll, nothing unblocks it.

I'm not sure whether there should then be a PAUSED->PLAYING transition caused by the queues filling up again. The video part of the queue fills up for a while after this.

The issue doesn't seem to be with not-linked handling in multiqueue. I tried bypassing it (when the return value of gst_pad_push is _NOT_LINKED, I just set it to OK to pretend it was accepted). Since there's no actual sink beyond this branch, i don't think it will mess with preroll handling.

So at the moment I think it's likely some logic bug to do with the interaction between prerolling and PLAYING->PAUSED early on.
Comment 6 Vincent Penquerc'h 2014-06-13 16:06:30 UTC
On second thought, it might be a multiqueue issue after all. The second multiqueue's video queue is empty, as it pushes without sync (but the return comes back with ok, not not-linked), so it's possible that the multiqueue does not send a "buffering high" message to gst-launch because its video queue is empty, so gst-launch never switches to PLAYING again - which would presumably unblock the audio thread waiting in preroll.

However, since the pad pushes in multiqueue seem to not return not-linked, I don't see an easy way to tell whethe to ignore a given single queue in the buffering level calculation. I guess I could hardcode it based on caps to test the theory first though.
Comment 7 Thiago Sousa Santos 2014-06-17 19:27:05 UTC
Created attachment 278621 [details] [review]
baseparse: avoid returning _OK for _NOT_LINKED

The problem was actually that baseparse was not returning not-linked
when it should.


When the parser receives non-aligned packets it can push a buffer
and get a not-linked return while still leaving some data still to
be parsed. This remaining data will not form a complete frame and
the subclass likely returns _OK and baseparse would take that
as the return, while it the element is actually not-linked.

This patch fixes this by storing the last flow-return from a push
and using that if a parsing operation doesn't result in data being
flushed or skipped.
Comment 8 Sebastian Dröge (slomo) 2014-06-17 19:58:51 UTC
Comment on attachment 278621 [details] [review]
baseparse: avoid returning _OK for _NOT_LINKED

Shouldn't we just go out tere immediately if something else than OK is returned when pushing?
Comment 9 Thiago Sousa Santos 2014-06-17 21:07:24 UTC
It would make baseparse accumulate data in this scenario. And when it receives a buffer with a ts=X it would be pushing data downstream that would be before X. 
I'd prefer to have the parser drop everything before returning upstream to prevent this scenario as upstream can handle buffering and not-linked returns better.
Comment 10 GstBlub 2014-06-18 21:59:20 UTC
(In reply to comment #7)
> Created an attachment (id=278621) [details] [review]
> baseparse: avoid returning _OK for _NOT_LINKED
> 
> The problem was actually that baseparse was not returning not-linked
> when it should.

I tried the attached patch and it works.
Comment 11 Sebastian Dröge (slomo) 2014-06-22 12:11:21 UTC
(In reply to comment #9)
> It would make baseparse accumulate data in this scenario. And when it receives
> a buffer with a ts=X it would be pushing data downstream that would be before
> X. 
> I'd prefer to have the parser drop everything before returning upstream to
> prevent this scenario as upstream can handle buffering and not-linked returns
> better.

I don't understand what you mean :) What I meant is that we should not pass any further data to baseparse and directly return not-linked and other errors (instead of continuing to pass the remaining data).
Comment 12 Thiago Sousa Santos 2014-06-25 13:12:11 UTC
(In reply to comment #11)
> (In reply to comment #9)
> > It would make baseparse accumulate data in this scenario. And when it receives
> > a buffer with a ts=X it would be pushing data downstream that would be before
> > X. 
> > I'd prefer to have the parser drop everything before returning upstream to
> > prevent this scenario as upstream can handle buffering and not-linked returns
> > better.
> 
> I don't understand what you mean :) What I meant is that we should not pass any
> further data to baseparse and directly return not-linked and other errors
> (instead of continuing to pass the remaining data).

Imagine that baseparse gets a buffer that contains 1+N frames and it tries to push 1 but it gets not-linked. It returns the not-linked upstream but it will have N frames remaining in its adapter. If this repeats over the next buffers received it will go accumulating N, 2N, 3N, 4N... frames in its adapter. As baseparse is not a buffering element it will never limit its adapter size.

This is the main reason for me to think that returning not-linked immediately is not a good idea. Continue pushing data downstream seem to be what we do for other elements that are not buffering or have only a single sink and source pads.
Comment 13 Sebastian Dröge (slomo) 2014-06-25 14:34:22 UTC
Makes sense, but only for not-linked. For other non-OK flow returns (eos, error and flushing at least) you should probably just return immediately and flush the adapter.
Comment 14 Thiago Sousa Santos 2014-06-25 15:31:35 UTC
commit b7516dbf7cfc0321f7ec918bfaf27dcbd49fd130
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Mon Jun 16 19:30:06 2014 -0300

    baseparse: avoid returning _OK for _NOT_LINKED
    
    When the parser receives non-aligned packets it can push a buffer
    and get a not-linked return while still leaving some data still to
    be parsed. This remaining data will not form a complete frame and
    the subclass likely returns _OK and baseparse would take that
    as the return, while it the element is actually not-linked.
    
    This patch fixes this by storing the last flow-return from a push
    and using that if a parsing operation doesn't result in data being
    flushed or skipped.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=731474
Comment 15 Thiago Sousa Santos 2014-06-25 15:45:57 UTC
Also on 1.2 branch:

commit 8528026c3b39d68ab319383b667fafd2bf76f834
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Mon Jun 16 19:30:06 2014 -0300

    baseparse: avoid returning _OK for _NOT_LINKED
    
    When the parser receives non-aligned packets it can push a buffer
    and get a not-linked return while still leaving some data still to
    be parsed. This remaining data will not form a complete frame and
    the subclass likely returns _OK and baseparse would take that
    as the return, while it the element is actually not-linked.
    
    This patch fixes this by storing the last flow-return from a push
    and using that if a parsing operation doesn't result in data being
    flushed or skipped.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=731474