After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 663406 - [0.11] gstpluginfeature: make most of this structure private
[0.11] gstpluginfeature: make most of this structure private
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal normal
: 0.11.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-11-04 15:20 UTC by Vincent Penquerc'h
Modified: 2012-06-26 15:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gstpluginfeature: make most of this structure private (21.48 KB, patch)
2011-11-04 15:20 UTC, Vincent Penquerc'h
needs-work Details | Review
gstpluginfeature: make most of this structure private (22.38 KB, patch)
2011-11-08 16:35 UTC, Vincent Penquerc'h
none Details | Review

Description Vincent Penquerc'h 2011-11-04 15:20:35 UTC
Also add consts.

Some code was poking at 'loaded', so I added a _set_loaded routine,
but it doesn't feel right. Better ideas welcome...
Comment 1 Vincent Penquerc'h 2011-11-04 15:20:37 UTC
Created attachment 200693 [details] [review]
gstpluginfeature: make most of this structure private
Comment 2 Tim-Philipp Müller 2011-11-07 23:25:16 UTC
Thanks for working on this.

Only had a quick glance, but my first impression is that not all functions should be public. IMHO external code doesn't really have any reason to create a plugin feature object, or set the loaded state, or set the plugin name, so these look like they should be private functions IMHO:

+void            gst_plugin_feature_set_loaded           (GstPluginFeature * feature);
+void            gst_plugin_feature_set_plugin_name      (const GstPluginFeature * feature, const gchar *);
+void            gst_plugin_feature_setup                (GstPluginFeature * feature,
+                                                         const gchar * name, GstPlugin * plugin,
+                                                         guint rank, gboolean set_loaded);
Comment 3 Vincent Penquerc'h 2011-11-07 23:52:31 UTC
Indeed. Rather dumbly, I hadn't realized there were non installed headers allowing a kind of inter-module private API. I'll move those there. Though _set_loaded still feels iffy, even if private.
Comment 4 Vincent Penquerc'h 2011-11-08 16:35:12 UTC
Created attachment 201014 [details] [review]
gstpluginfeature: make most of this structure private
Comment 5 Wim Taymans 2012-03-12 15:28:50 UTC
Not doable in 0.10 but would be nice for 0.11. Do you want to update the patch for 0.11?
Comment 6 Tim-Philipp Müller 2012-03-12 15:35:47 UTC
I'm currently messing around with the registry stuff, will have a look at this when done.
Comment 7 Vincent Penquerc'h 2012-03-12 21:41:27 UTC
Thanks. It was done on 0.11 in fact, though it probably bitrotted by now.
Comment 8 Tim-Philipp Müller 2012-06-26 15:24:12 UTC
I actually made the entire struct private a while back, which was easier and less intrusive (and I wasn't sure about some things and wanted to defer deciding on them).