GNOME Bugzilla – Bug 584689
Should use get-{audio,video,text}-tags playbin2 actions for getting tags
Last modified: 2009-06-08 10:17:45 UTC
Hi, currently totem gets the tags for different streams by having a bus watch and by detecting from the class of the originating element of a tag message if it is for the audio or video stream. This has some disadvantages: tags for a specific stream can also be emitted by demuxers for example, which don't have audio or video in their class. Also there currently is no way to detect if tags are for an active stream or not. (See also bug #582588 comment #3 and following). The correct way to do this is by either installing an event probe on the audio/video/text sink pads or by using the get-{audio,video,text}-tags action signals of playbin2 (which internally works by an event probe on the sink pads). To use the playbin2 actions properly in totem bug #584686 needs to be fixed first as currently there's no way to detect when tags have changed or when no new tags are added anymore.
I'm working on it now (and make sure that it still works with playbin2 0.10.23).
Created attachment 135939 [details] [review] totem-playbin2-get-tags.diff Proposed patch. This checks at the first time if playbin2 has the new signals, if not it falls back to old behaviour. If the signals are there tag messages are ignored and instead the new signals are used to detect when tags have changed. As these signals are called from a random thread it's necessary to marshal them back into the main thread by g_idle_add(). Any comments or can this already be committed? ;)
I'd rather have only one codepath for that. If the next -base release is close enough, remove the old code, and bump the reqs.
The next core/base releases will be ~20th July... is this close enough? Also I noticed an issue with that patch: a) tags-changed signal handlers should only do something if the stream id is the currently selected one and b) on stream change the new tags should be get for the new stream id. I'll change the patch soon...
Isn't there any way to have a pre-release before that? That's a bit too far for that. We already bumped reqs for the last gst-plugins-base release... Feel free to commit this patch if there's no way to get a pre-release, once you're happy with it. check_playbin2_version There's no need for the "first_time" bit, we only create one bvw per program usually, so something like: if (check_playbin2_version()) { g_signal_connect(... Please also add comments about the bits we'll remove when we up the reqs, with a FIXME in there.
A pre-release will be about 2 weeks earlier, is that early enough?
This is still more than a month away, and means that I can't expect anyone to test pre-releases of Totem, or put them in distros.
commit 6cea24030a0bd7487d7e3657ce82c2e2ebdbe8a1 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Mon Jun 8 12:16:30 2009 +0200 Use the {video,audio,text}-tags-changed signals of playbin2 2009-06-08 Sebastian Dröge <sebastian.droege@collabora.co.uk> * src/backend/bacon-video-widget-gst-0.10.c: If we have playbin2 from gst-plugins-base 0.10.23.1 or newer use the new {audio,video,text}-tags-changed signals for noticing if tags have changed. This also gives us immediately the corresponding stream id and the type of the stream and makes it unnecessary to try to get the type via the tag messages' sender. This fixes bug #584689, prevents tags for unused streams to be displayed and makes sure that audio/video tags of a demuxer or other generic elements are used.