GNOME Bugzilla – Bug 584987
[playbin2] [gapless] Fire a track-changed message on track change.
Last modified: 2013-10-27 19:28:41 UTC
Media players care about when the user hears the start of the next track change. Currently when using playbin2 for gapless playback there is no way for the application to easily determine when the first sample of the next track is starting to play. It would be nice if playbin2 fired a track-changed message on the bus, or raised a track-changed signal, or both, when the first sample of the next track started playing on the audio hardware.
Yes, this signal/message would be a very good idea IMHO. about-to-finish is usually to early and might give a suboptimal user experience on track changes ;) Not sure if a message or signal should be used... both have their advantages and disadvantages: signals are called from the streaming thread but are instant, messages are called from the bus thread but arrive later.
Created attachment 147807 [details] [review] playbin2: Post a stream-changed message after activating a group This is useful to detect when playbin2 has really switched to the next group after about-to-finish for example. See bug #584987.
This looks like it might be useful anyway, but it doesn't actually fix the issue here - applications are interested in when the actual _playback_ of the new stream starts, not of when playbin2 internally switches. My thoughts on this (I haven't actually thought much about it though) are that playbin2 would send a downstream serialised event, and then the sink(s) would do something to turn that into a message for the application once they actually get to that point. This is particularly noticeable if you set a large buffer on the audio sink.
I would suggest exactly what Mike just said it :)
The practise in level, spectrum and other plugins is to include a timestamp. This unfortunately makes it tedious for apps to handle it. The need to clock_id=gst_clock_new_single_shot_id(clock,target_time); gst_clock_id_wait_async(clock_id,on_delayed_bus_message,(gpointer)user_data); gst_clock_id_unref(clock_id); static gboolean on_delayed_bus_message(GstClock *clock,GstClockTime time,GstClockID id,gpointer user_data) { // the callback is called from a clock thread if(GST_CLOCK_TIME_IS_VALID(time)) g_idle_add(on_delayed_idle_bus_message,user_data); else g_free(user_data); return(TRUE); } and finally handle it. I was proposing to use synced evens before, especially as GstBaseSink could turn them into a bus message in a generic way (gst_event_get_structure). Unfortunately I got no agreement. I don't remmeber the details, but it was like "the apps should do it".
Yes, I thought about doing what Mike said too after attaching this patch here. This patch provides a better way than the about-to-finish signal (it's nearer to the real track switch) but still not optimal. Something like a GST_EVENT_SINK_MESSAGE, that takes a GstMessageType and GstStructure inside the event structure, would flow with the data and be posted by sinks. Doing what Stefan suggested might be possible too but it's not obvious for me how to do it in the playbin2 case, i.e. which time should be included in the message?
Ok, I talked with Wim about this a bit and the plan would now be the following (such a sink-message event is not easy to implement, Wim tried it once and it was more work than expected): First of all, playsink keeps track of all NEWSEGMENT events and remembers the segments. On all streams. This allows us to get the running time later. Now when a switch from one to the next group happens, after the current group was deactivated playbin2 would take the stream locks and push a custom "stream-start-marker" event downstream which is synchronized to the stream. When this arrives in playsink, playsink knows on the next newsegment event that it now has the running time of the new group's streams. this then gets reported to playbin2 to trigger a clock wait that posts the track-changed message.
Created attachment 148023 [details] [review] playbin2: Post a stream-changed message after activating a group This is useful to detect when playbin2 has really switched to the next group after about-to-finish for example. See bug #584987.
This time a better solution. After bug #602275 is fixed this will provide a much better estimate about when the streams have really changed.
Created attachment 148024 [details] [review] playbin2: Post a stream-changed message after activating a group This is useful to detect when playbin2 has really switched to the next group after about-to-finish for example. See bug #584987.
Created attachment 148028 [details] [review] playbin2: Post a stream-changed message after activating a group This is useful to detect when playbin2 has really switched to the next group after about-to-finish for example. See bug #584987.
This now only depends on the new core sink-message event. If sinks implement it properly this commit will be perfect, if they do it like in basesink it will be good enough unless the sink internally queues too much data. In any case the message will be posted on the bus after all sinks have handled EOS of the previous group. commit 2289134bbedf1d391d242f6fffb6815e043d9149 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Sun Nov 15 19:36:21 2009 +0100 playbin2: Post a stream-changed message after activating a group This is useful to detect when playbin2 has really switched to the next group after about-to-finish for example. Fixes bug #584987.
commit d6dd987ffb9a2e00caffd14d7c1d34119585cf6c Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Thu Nov 19 12:10:58 2009 +0100 playbin2: Aggregate the stream-changed message by looking at the seqnum Just counting how many messages were sent and how many were received is not good enough because they might've been duplicated (e.g. by the visualization audio tee). Comparing the sequence numbers should give better results in that case.
Trying to find these 2 commits in git history I found that their hash IDs were wrong. For the record: (In reply to comment #12) > commit 2289134bbedf1d391d242f6fffb6815e043d9149 > Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> > Date: Sun Nov 15 19:36:21 2009 +0100 > > playbin2: Post a stream-changed message after activating a group http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=7e674d8605f3d12f232a4be59c5a0ee9e8389f6c (In reply to comment #13) > commit d6dd987ffb9a2e00caffd14d7c1d34119585cf6c > Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> > Date: Thu Nov 19 12:10:58 2009 +0100 > > playbin2: Aggregate the stream-changed message by looking at the seqnum http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=7e674d8605f3d12f232a4be59c5a0ee9e8389f6c I'm trying to figure out if the "playbin2-stream-changed" message got renamed to "playbin-stream-changed" (or just "stream-changed") on 1.0 (since these changes were committed in 0.10), but I don't find any of the code in master (http://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst/playback/gstplaybin2.c?id=d6dd987ffb9a2e00caffd14d7c1d34119585cf6c) that these 2 commits added.
(In reply to comment #14) > http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=7e674d8605f3d12f232a4be59c5a0ee9e8389f6c Oops, copy+paste error, the 2nd link should be: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=d6dd987ffb9a2e00caffd14d7c1d34119585cf6c
I think you should be getting a GST_MESSAGE_STREAM_START in 1.0 when the track changes.
(In reply to comment #16) > I think you should be getting a GST_MESSAGE_STREAM_START in 1.0 when the track > changes. Yes, sorry, I already figured that out [1], sorry for the noise :) Now I'm trying to figure out if I need to remove my hook to the signal "about-to-finish", since there is a FIXME comment about it[2] that points to this already-FIXED bug. Any hint? Thanks for the heads-up! [1] https://git.gnome.org/browse/banshee/commit/?id=ea128c28c2fc167039cfbe1a8ae489c55fa55c0e [2] https://git.gnome.org/browse/banshee/tree/src/Backends/Banshee.GStreamer/libbanshee/banshee-player-pipeline.c#n294
That comment doesn't make much sense to me, from what I saw Banshee does what it should do there already.
(In reply to comment #18) > That comment doesn't make much sense to me, from what I saw Banshee does what > it should do there already. Yes, I'm guessing that the comment became obsolete when Christopher committed this[1], but then does this mean that I can remove the comment, or also the "about-to-finish" hook (that is below the comment) entirely? [1] https://git.gnome.org/browse/banshee/commit/?id=b1519fded83eab0f0d2dba821dcecf802f54490b
(In reply to comment #19) > (In reply to comment #18) > > That comment doesn't make much sense to me, from what I saw Banshee does what > > it should do there already. > > Yes, I'm guessing that the comment became obsolete when Christopher committed > this[1], but then does this mean that I can remove the comment, or also the > "about-to-finish" hook (that is below the comment) entirely? Didn't get my morning coffee yet sorry: I simply tested this, and removing "about-to-finish" hook does no good obviously (makes banshee not switch to the next song). So I guess I'll just remove the comment.
Well yes, remove the comment :) Banshee changes the track in playbin from the about-to-finish signal, and updates the UI to the new track once the stream-start message is received from playbin.
(In reply to comment #21) > Well yes, remove the comment :) Banshee changes the track in playbin from the > about-to-finish signal, and updates the UI to the new track once the > stream-start message is received from playbin. Exactly, I removed this 2 days ago with a quite informative explanation about that: https://git.gnome.org/browse/banshee/commit/?id=7d91d546a6518321b0df39e8be0aea98be03be8a Thanks for all your help.