playbin2's API is not quite stable yet, it may still change (hopefully we'll be able to get it stabilised in time though). I was planning to port totem to playbin2 in time for the next GNOME version if playbin2 works well enough and we feel the API is ready.

I'm trying it out for now, will post a patch when it's something useful.

I had some problems with my first try. Something like this is supposed to work:

Index: backend/bacon-video-widget-gst-0.10.c
===================================================================
--- backend/bacon-video-widget-gst-0.10.c	(revision 5510)
+++ backend/bacon-video-widget-gst-0.10.c	(working copy)
@@ -5101,7 +5101,7 @@
   bvw->priv->use_type = type;
   GST_DEBUG ("use_type = %d", type);
 
-  bvw->priv->play = gst_element_factory_make ("playbin", "play");
+  bvw->priv->play = gst_element_factory_make ("playbin2", "play");
   if (!bvw->priv->play) {
     g_set_error (err, BVW_ERROR, BVW_ERROR_PLUGIN_LOAD,
                  _("Failed to create a GStreamer play object. "

But it finds the xoverlay, and doesn't display anything. I never seem to get an expose event for the overlay.

*** Bug 544682 has been marked as a duplicate of this bug. ***

*** Bug 548367 has been marked as a duplicate of this bug. ***

*** Bug 434027 has been marked as a duplicate of this bug. ***

*** Bug 552339 has been marked as a duplicate of this bug. ***

I've just done some testing, and I'm getting a similar problem to Bastien; the XOverlay is getting set up and passed data properly (as far as I can tell), but nothing's getting displayed.

If you s/window/0/ in both calls to gst_x_overlay_set_xwindow_id in bvw (to make GstXOverlay create its own window), it creates its own window fine, and plays the video in it properly.

I'm attaching a debug log grabbed with GST_DEBUG=xvimagesink:5. As far as I can tell, nothing's wrong, but I'm not particularly knowledgeable about such things.

Tim?

Created attachment 121485 [details]
xvimagesink:5 log for Totem with playbin2

I should also mention that I put some debug prints ("expose XOverlay") in at calls to gst_x_overlay_expose, and they appear to be being called fine.

*** Bug 563368 has been marked as a duplicate of this bug. ***

We never seem to run through bvw_handle_application_message() on opening new files, which is where we show the video window that we hide in open_with_subtitle.

Eg. That'll make it work:
@@ -2694,6 +2697,7 @@
   bvw->priv->ignore_messages_mask = 0;
 
   /* We hide the video window for now. Will show when video of vfx comes up */
+#if 0
   if (bvw->priv->video_window) {
     gdk_window_hide (bvw->priv->video_window);
     /* We also take the whole widget until we know video size */
@@ -2701,7 +2705,7 @@
         GTK_WIDGET (bvw)->allocation.width,
         GTK_WIDGET (bvw)->allocation.height);
   }
-
+#endif

Well, it would actually work if you know how to get the stream-info. The docs for playbin2 look like cut'n'paste from playbin1, thus don't work.

Anyone with explanations on how to use the get-audio-tags/get-video-tags action signals?

*** Bug 571362 has been marked as a duplicate of this bug. ***

*** Bug 571709 has been marked as a duplicate of this bug. ***

*** Bug 576131 has been marked as a duplicate of this bug. ***

If nobody else already has this working I'm going to do it now and in the next days... I've done all simple parts already, including working video output and getting a list of language codes.

This is currently blocked by bug #576180 , also I'm currently fighting to get visualizations working...

Created attachment 131081 [details] [review]
totem-playbin2.diff

This patch ports totem to playbin2. There are still some issues with reusing the playbin2 instance though, but all in all it works quite good :) Subtitle switching for example works during playback without seeking now.

Created attachment 131131 [details] [review]
patch

New version, fixing a ref-leak of the video sink

What's with the magic flags here?
+	flags |= 0x008;
+	flags &= ~0x008;

And here:
+  if ((flags & 0x004) == 0)

And in a bunch of other places.

Doesn't that lose us support for subpicture subtitles (such as for DVDs)?
-  if (!(list =  get_lang_list_for_type (bvw, "SUBPICTURE")))
-    list = get_lang_list_for_type (bvw, "TEXT");
+  list = get_lang_list_for_type (bvw, "TEXT");


Rest looks good enough for us to commit to trunk. Even if it breaks, at least we can start looking at using some of the extra functionality.

(In reply to comment #20)
> What's with the magic flags here?
> +       flags |= 0x008;
> +       flags &= ~0x008;
> 
> And here:
> +  if ((flags & 0x004) == 0)
> 
> And in a bunch of other places.

Take a look at the output of gst-inspect playbin2 at the flags property. Those flags are for enabling/disabling visualizations or subtitles.

> Doesn't that lose us support for subpicture subtitles (such as for DVDs)?
> -  if (!(list =  get_lang_list_for_type (bvw, "SUBPICTURE")))
> -    list = get_lang_list_for_type (bvw, "TEXT");
> +  list = get_lang_list_for_type (bvw, "TEXT");

playbin2 doesn't have support for subpicture subtitles (neither has playbin1) AFAIK and there's no way to query playbin2 for subpicture stuff so I've removed that for now. I guess playbin2 will handle it as "text" in the future... 
 
> Rest looks good enough for us to commit to trunk. Even if it breaks, at least
> we can start looking at using some of the extra functionality.

Ok, shall I commit it now as is then?

The line with the flags is not very readable:

if ((flags & 0x004) == 0)

Can you define a constant for 0x004 instead so that it says:

if ((flags & SUBTITLES_MASK) == 0)

The most readable version would probably be:

if (has_subtitles (video_file))

or something like that. I don't it's necessary to do this before commit though, maybe this "flags" thing is something that needs to be fixed in many places (not just in your patch) and then I don't think this patch needs to be held back because of that issue. Adding a constant is not much work though, and I for one would certainly appreciate it.

Yeah, I've already added an enum for those flags locally now and will include it in the commit when Bastien gives his "ok" :)

That's what I was asking for. You can commit, don't forget to up the dependencies and add a check for playbin2 in the configure.ac as well.

Thanks, I've committed this patch now and will now look at the remaining issues...

2009-03-23  Sebastian Dröge  <sebastian.droege@collabora.co.uk>

	* configure.in:
	* src/backend/bacon-video-widget-gst-0.10.c:
	(bvw_handle_application_message), (parse_stream_info),
	(playbin_stream_changed_cb), (bacon_video_widget_finalize),
	(bacon_video_widget_get_subtitle),
	(bacon_video_widget_set_subtitle), (get_lang_list_for_type),
	(bacon_video_widget_get_subtitles), (setup_vis),
	(bvw_get_current_stream_num), (bvw_get_tags_of_current_stream),
	(bvw_get_caps_of_current_stream),
	(bacon_video_widget_get_metadata_string),
	(bvw_update_interfaces_delayed),
	(bvw_update_interface_implementations), (bacon_video_widget_new):
	Initial port of the GStreamer backend to playbin2 (Partially fixes
	bug #542663).

IMHO we can close this bug now as everything is ported now, everything works at least as good as before and there are new features ;) We should depend on the next GStreamer core/base releases though

*** Bug 570035 has been marked as a duplicate of this bug. ***

*** Bug 521879 has been marked as a duplicate of this bug. ***

*** Bug 386129 has been marked as a duplicate of this bug. ***