Created attachment 264404 [details] [review]
missing-plugins: Add funcs to generate detail and installer desc from GstStructures

With the current API of GstDiscoverer we do not have access to the GstMessage directly
but to its contained GstStructure, this lead to various application
basically copy pasting those method from pbutils in their code. Instead
add some simple API's to simplify the generation of those strings
directly from the GstStructure.

API:
  gst_missing_plugin_message_get_description_from_struct
  gst_missing_plugin_message_get_installer_detail_from_struct

Review of attachment 264404 [details] [review]:

...or maybe add API to discoverer to get the messages instead?

::: gst-libs/gst/pbutils/missing-plugins.c
@@ +413,3 @@
+/**
+ * gst_missing_plugin_message_get_installer_detail_from_struct:
+ * @structure: a missing-plugin #GstMessage of type #GST_MESSAGE_ELEMENT

A GstStrcuture, not a message

@@ +423,3 @@
+ *
+ * Returns: a newly-allocated detail string, or NULL on error. Free string
+ *          with g_free() when not needed any longer.

Since: 1.4

@@ +550,3 @@
+ *
+ * Returns: a newly-allocated description string, or NULL on error. Free
+ *          string with g_free() when not needed any longer.

Since: 1.4

First of all: which applications, which code?

@tim In rhythmbox https://git.gnome.org/browse/rhythmbox/tree/lib/rb-gst-media-types.c and in pitivi we would need the same kind of things

I don't understand the discoverer API for the missing plugin stuff, it looks very weird.

You get the GstStructure from a missing-plugin message via gst_discoverer_get_misc() [what a name...]?

Thing is, there might be multiple missing plugin messages, not just one. I think we should make new API for that and either just return a list of messages, or return an array of installer detail strings like we do in

http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/gstreamer-GstParse.html#gst-parse-context-get-missing-elements

I think any API should be limited to either messages or installer strings, the GstStructure of GstMessage is all but an implementation detail here.

Created attachment 264405 [details] [review]
missing-plugins: Add funcs to generate detail and installer desc from GstStructures

With the current API of GstDiscoverer we do not have access to the GstMessage directly
but to its contained GstStructure, this lead to various application
basically copy pasting those method from pbutils in their code. Instead
add some simple API's to simplify the generation of those strings
directly from the GstStructure.

API:
  gst_missing_plugin_message_get_description_from_struct
  gst_missing_plugin_message_get_installer_detail_from_struct

Comment on attachment 264405 [details] [review]
missing-plugins: Add funcs to generate detail and installer desc from GstStructures

Completely agreed with comment #5. Let's go that way, everything else looks rather weird API-wise.

Apps should not access the GstStructure of the missing-plugins element messages, but use the APIs provided, and the rhythmbox code is not really right on multiple levels (but there are some APIs they need from us to fix this, so not their fault).

I agree with you, I guess the best thing to do is to add a:

  gchar **
  ges_project_get_missing_elements_installer_details (GESProject *project);

method in GES then.

Yeah, the installer detail strings are what you need when you want to call the "install this please" API, so if that's what you usually want to do, then that makes sense. Might want the same for GstDiscoverer too.

I think we could add a:

  gchar **
  gst_discoverer_info_get_missing_elements_installer_details (GstDiscovererInfo **info);

Would that be good enough?

Yes, maybe also additionally for the stream info? And deprecate this weird get_misc() API please :)

Created attachment 266278 [details] [review]
discoverer: Add APIs to simply get installer details for missing plugins

Currently the API is far from optimal and the user has to work around
our badly defined API to simply install missing plugins.

API:
  new:
    gst_discoverer_info_get_missing_elements_installer_details

  deprecated:
    gst_discoverer_info_get_misc
    gst_discoverer_stream_info_get_misc

Review of attachment 266278 [details] [review]:

Looks good, just one remark. Please push afterwards :)

::: gst-libs/gst/pbutils/gstdiscoverer-types.c
@@ +1088,3 @@
+ * Returns: (transfer full): (array zero-terminated=1): An array of strings
+ * containing informations about how to install the various missing elements
+ * for @info to be usable. Free with g_strfreev.

Since: 1.4

@@ +1104,3 @@
+  }
+
+  ret = g_malloc0 (sizeof (gchar *) * info->missing_elements_details->len + 1);

I think for consistency it would be nice to return a const gchar ** here... just append a NULL to the GPtrArray once discovering is done and then return the pdata pointer directly.

Attachment 266278 [details] pushed as 622007e - discoverer: Add APIs to simply get installer details for missing plugins