[Bug 741260] gstpluginfeature: Properly expose the class.

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=741260
  GStreamer | gstreamer (core) | git

--- Comment #8 from Mathieu Duponchelle <mathieu.duponchelle at epitech.eu> 2015-01-12 01:12:44 UTC ---
(In reply to comment #6)
> Review of attachment 292334 [details]:
> 
> +1 for the idea. I think having api for this also simplifies the internal code
> (e.g. the weak pointer).
> 

The code was indeed not very well encapsulated in that regard, admittedly we
could have this API and still hide PluginFeature but :)

> ::: gst/gstpluginfeature.c
> @@ +197,3 @@
> + * @feature: feature to set plugin on
> + * @plugin: Set the plugin that provides this feature
> + *
> 
> missing doc line.

Fixed.

> @@ +204,3 @@
> +  g_return_if_fail (feature != NULL);
> +  g_return_if_fail (GST_IS_PLUGIN_FEATURE (feature));
> + * @plugin: Set the plugin that provides this feature
> 
> wouldn't you need to g_object_remove_weak_pointer () if feature->priv->plugin
> != NULL
> 

Indeed, done, I'm not sure of whether there was a case where we would reset a
different plugin but can't hurt.

> @@ +210,3 @@
> +    feature->priv->plugin_name = plugin->desc.name;
> +  } else {
> +void
> 
> do we need this?
> 

Not really, don't want to be too intrusive though.

> ::: gst/gstregistry.c
> @@ +752,3 @@
> +
> +  rank1 = gst_plugin_feature_get_rank (GST_PLUGIN_FEATURE (fac1));
> +  rank2 = gst_plugin_feature_get_rank (GST_PLUGIN_FEATURE (fac2));
> 
> nit:
> guint rank1 = gst_plugin_feature_get_rank (GST_PLUGIN_FEATURE (fac1));
> guint rank2 = gst_plugin_feature_get_rank (GST_PLUGIN_FEATURE (fac2));

Style consideration, ain't got nothing against that, done ;)

Fixed patch attached next, waiting for tim's eyeballs on our use case :)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.