GNOME Bugzilla – Bug 754464
Add support for g_autoptr() and friends
Last modified: 2015-12-14 20:51:28 UTC
It would be nice if all GStreamer libraries (those who install header files) define the autoptr function for all their typedefs, using one of those macro: G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC G_DEFINE_AUTO_CLEANUP_FREE_FUNC G_DEFINE_AUTOPTR_CLEANUP_FUNC That would allow: - Use of G_DECLARE_* in GStreamer itself and in applications using it when subclassing a gst type. - Use of g_autoptr() in applications that don't care about other compilers than gcc/clang. Notably GStreamer itself shouldn't use it to keep compat with MSVC. Note that it doesn't mean that GStreamer itself would depend on gcc/clang since those macro does nothing when not supported by the compiler. Those macros are new in glib 2.44, and GStreamer isn't probably going to depend on it any time soon (is it?). But applications using gst often can depend on newer glib, so it would make sense to add a gst-autocleanups.h file like the one in gobject/gio/gtk but starting with a #ifdef G_DEFINE_AUTOPTR_CLEANUP_FUNC.
I'm not sure I like this idea, it will cause people to write GStreamer code that does not compile on e.g. MSVC... and then later makes porting for them more work than if they didn't use the magic autoptr stuff in the first place.
You could disable that header when building gst itself if you like. But nothing prevents people to use g_autoptr() with glib/gio types already. That's a policy you'll have to enforce when reviewing code. It is of course not really useful for gst itself but is really nice to people using gst.
Note that glib/gtk are in the same case: they can't use g_autoptr in their code, but they have that header for apps that can.
Oh, and G_DECLARE_FINAL/DERIVABLE/TYPE can be used in gst itself (when it deps on glib 2.44) but it needs the G_DEFINE_AUTOPTR_CLEANUP_FUNC() for its base class.
Created attachment 312755 [details] [review] Add gst-autocleanup.h
Created attachment 312756 [details] Script used to generate autocleanup header I wrote that quick python script to extract all records/class from a .gir file and output the corresponding G_DEFINE_AUTOPTR_CLEANUP_FUNC(). It can be used to create a header for all other gst libs that install public API.
Created attachment 312758 [details] Script to generate gst-*-autocleanups.h All files can be generated at once with: for f in `find -name "*.gir"`; do gen-autocleanup.py $f; done
Created attachment 312777 [details] [review] Add gst-*-autocleanups.h
Created attachment 312897 [details] [review] Add gst-*-autocleanups.h
Created attachment 312899 [details] [review] Add gst-*-autocleanups.h
Created attachment 312900 [details] [review] Add gst-*-autocleanups.h
Did the job for core/base/bad/rtsp-server (good doesn't expose APIs).
Should be a blocker IMHO as it's needed for G_DECLARE_*_TYPE, which is very convenient API (and portable) for people who can depend on new enough GLib.
Created attachment 315198 [details] Script used to generate patches I updated the script to add the G_DEFINE_AUTOPTR_CLEANUP_FUNC in each header instead of grouping them all in a single header. This allows using G_DECLARE_* even when including individual headers instead of having to include the global .h file. Especially important when we'll starting using it in gst itself.
Created attachment 315199 [details] [review] Add g_autoptr() support to all types
Created attachment 315200 [details] [review] Add g_autoptr() support to all types
Created attachment 315204 [details] [review] Add g_autoptr() support to all types Note that many types in mpegts does not expose their free func, they are omitted from this patch.
Created attachment 315205 [details] [review] Add g_autoptr() support to all types
Created attachment 315207 [details] [review] core: Add g_autoptr() support to all types
Created attachment 315208 [details] [review] base: Add g_autoptr() support to all types
Created attachment 315209 [details] [review] bad: Add g_autoptr() support to all types Note that many types in mpegts does not expose their free func, they are omitted from this patch.
Created attachment 315210 [details] [review] rtsp-server: Add g_autoptr() support to all types
Ok, I'll start merging this in 1.7 dev. Note, I'll do one repo at the time, and wait for the build results.
Comment on attachment 315207 [details] [review] core: Add g_autoptr() support to all types Attachment 315207 [details] pushed as 46f83f5 - core: Add g_autoptr() support to all types
Comment on attachment 315208 [details] [review] base: Add g_autoptr() support to all types Attachment 315208 [details] pushed as 429860e - base: Add g_autoptr() support to all types
Attachment 315209 [details] pushed as 598ddae - bad: Add g_autoptr() support to all types
Comment on attachment 315210 [details] [review] rtsp-server: Add g_autoptr() support to all types commit 0ea68a1b0ff01725892b666eb8a4299a75ec39b1 Author: Xavier Claessens <xavier.claessens@collabora.com> Date: Tue Nov 10 14:17:18 2015 -0500 rtsp-server: Add g_autoptr() support to all types https://bugzilla.gnome.org/show_bug.cgi?id=754464
Thanks !