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 607224 - Flush the bus before reusing playbin2
Flush the bus before reusing playbin2
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: GStreamer backend
2.29.x
Other Linux
: Normal critical
: ---
Assigned To: Maintainer alias for GStreamer component of Totem
Maintainer alias for GStreamer component of Totem
Depends on:
Blocks:
 
 
Reported: 2010-01-17 15:35 UTC by Sjoerd Simons
Modified: 2010-01-24 08:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Flush playbin2's bus before reusing it (1.47 KB, patch)
2010-01-23 20:44 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Sjoerd Simons 2010-01-17 15:35:45 UTC
Since 06de49464045490a7a6d0ddf0afa4228274a5266 qtdemux will always post an error message on the bus when eos is reached. Unfortunately this breaks totems redirect handling, the following situation happens:

* qtdemux posts a redirect
* totem catches the redirect and ask playbin2 to play the new uri
* the old (redirected) qtdemux post an error on the bus on EOS
* totem catches this error and bails :/

It seems conceptually slightly strange that there is an error on EOS even if the element was successful (there were indeed no streams, just a redirect and it communicated this). tpm suggested on irc that not throwing an error means that the application will have no indication that the streaming stopped though.

Setting severity to blocker on tpms request as this breaks existing software (totem)
Comment 1 Tim-Philipp Müller 2010-01-17 17:38:03 UTC
Ah, crap. It's a bit unfortunate that this breaks totem, since posting an error seems the least-worse thing to do. Alternatively one could post an EOS I guess, but I wonder if totem will not handle that as well then if it doesn't pop messages off the bus properly.
Comment 2 Thiago Sousa Santos 2010-01-18 09:59:45 UTC
Things to remember:

* 'redirect' messages are not 'official' (if you search the GstMessage docs for 'redirect' you get nothing).

* Depending on the solution taken here, we need to fix gst-launch to not make it stall in these cases. Maybe by printing the redirect URI and then shutting down.
Comment 3 Thiago Sousa Santos 2010-01-22 15:34:39 UTC
After some discussion on #gstreamer, we agreed that this is probably a totem's bug. It possibly isn't cleaning the bus properly between track switches or redirects.
Comment 4 Edward Hervey 2010-01-22 16:46:36 UTC
There's a good chance this is related to a change done within the past few months by which playbin2 is no longer resetted to NULL but only to READY.

This will cause the bus to *NOT* flush messages, and you might end up seeing stray messages from the previous track/file.

my 0.02EUR
Comment 5 Bastien Nocera 2010-01-22 16:53:37 UTC
Yes, that'd be Sebastian's fault, and I'm happy I didn't put that code in the gnome-2-28 branch ;)
Comment 6 Sebastian Dröge (slomo) 2010-01-22 17:45:24 UTC
So we have to flush the bus manually now? Or what do you suggest?
Comment 7 Edward Hervey 2010-01-23 18:51:44 UTC
yes, something like the following:

+ gst_bus_set_flushing (bus, TRUE);
  gst_element_set_state (pipeline, READY);
+ gst_bus_set_flushing (bus, FALSE);

That did the trick for me in gstdiscoverer (which reuses uridecodebin without setting it to NULL but only to READY).
Comment 8 Sebastian Dröge (slomo) 2010-01-23 20:44:04 UTC
Created attachment 152109 [details] [review]
Flush playbin2's bus before reusing it

This makes sure that we don't get any messages from a previous URI.

Fixes bug #607224.
Comment 9 Tim-Philipp Müller 2010-01-23 21:52:34 UTC
Comment on attachment 152109 [details] [review]
Flush playbin2's bus before reusing it

Haven't tested it, but it looks good to me. Please push it to master if it fixes the issue.
Comment 10 Sebastian Dröge (slomo) 2010-01-23 22:20:11 UTC
(In reply to comment #9)
> (From update of attachment 152109 [details] [review])
> Haven't tested it, but it looks good to me. Please push it to master if it
> fixes the issue.

I have no file to test if the patch fixes it, sorry. But at least it doesn't break normal playback ;)

I'll push it after I found a file or someone else tested it.
Comment 11 Tim-Philipp Müller 2010-01-23 23:14:39 UTC
It seems to work for me. Tested with up-to-date gst-plugins-bad/mmssrc like this:

 totem 'mms://vipmms9.yacast.net/bfm_bfmtv'

(see bug #606636). Works fine with patch (oddly enough mmssrc seems to be started up twice though?), and shows an error without the patch.
Comment 12 Sebastian Dröge (slomo) 2010-01-24 08:10:06 UTC
commit b25fd88cd2be2b3643f8aca4a5f83733d40c636c
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Sat Jan 23 21:43:13 2010 +0100

    Flush playbin2's bus before reusing it
    
    This makes sure that we don't get any messages from a previous URI.
    
    Fixes bug #607224.