GNOME Bugzilla – Bug 498222
configure improvements
Last modified: 2008-03-03 13:22:19 UTC
Please describe the problem: I have gathered a couple of patches here to improve the configuration of GStreamer plugins. I will add more specific comments for each patch. Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information:
Created attachment 99332 [details] [review] Minor improvement of the out put from configure This just improves the output from configure when either glib or libxml2 is not presesnt, since PKG_CHECK_MODULES does not add a AC_MSG_RESULT automatically for this case.
Created attachment 99333 [details] [review] Typo fix and minor improvement to AG_GST_CHECK_FEATURE This fixes a typo in the help for AG_GST_CHECK_FEATURE. It also changes the help that is included in the generated config.h file to something more appropriate.
Created attachment 99334 [details] [review] Make it easier to include and exclude plugins This is the main patch of these patches. It adds two new m4 macros: AG_GST_CHECK_PLUGIN and AG_GST_DISABLE_PLUGIN. It also modifies AG_GST_ARG_WITH_PLUGINS to work with the two new macros. The idea is to use AG_GST_CHECK_PLUGIN for each plugin which today is listed manually in GST_PLUGINS_ALL. And then use AG_GST_DISABLE_PLUGIN for any plugin which needs to be disabled for any reason (lack of some header file or whatever). It also makes it possible to check for USE_PLUGIN_<plugin name> in Makefile.am, which can be useful e.g. to not include check tests for disabled plugins.
Created attachment 99335 [details] [review] Use AG_GST_CHECK_PLUGIN, and make liboil optional This patch makes configure.ac for gst-plugins-base use the new AG_GST_CHECK_PLUGIN and AG_GST_DISABLE_PLUGIN as appropriate. It also removes the requirement to have liboil installed, as it is actually only needed for a few plugins.
Created attachment 99336 [details] [review] Use AG_GST_CHECK_PLUGIN, and make liboil optional This patch makes configure.ac for gst-plugins-good use the new AG_GST_CHECK_PLUGIN and AG_GST_DISABLE_PLUGIN as appropriate. It also removes the requirement to have liboil installed, as it is actually only needed for a few plugins.
Created attachment 99337 [details] [review] Use AG_GST_CHECK_PLUGIN, and make liboil optional This patch makes configure.ac for gst-plugins-bad use the new AG_GST_CHECK_PLUGIN and AG_GST_DISABLE_PLUGIN as appropriate. It also removes the requirement to have liboil installed, as it is actually only needed for a few plugins.
Created attachment 99338 [details] [review] Use AG_GST_CHECK_PLUGIN, and make liboil optional This patch makes configure.ac for gst-plugins-ugly use the new AG_GST_CHECK_PLUGIN and AG_GST_DISABLE_PLUGIN as appropriate. It also removes the requirement to have liboil installed, as it is actually only needed for a few plugins. Note that this patch is dry-coded as we do not use gst-plugins-ugly, but I think it should work...
That was the last patch, now go apply them. ;)
Liboil is a required dependency, since elements in gst-plugins-* are allowed to assume it exists. Having videoscale disappear because liboil is not installed is not the behavior we're looking for.
Well, since we do not use any of the plugins which require liboil, having to build and install liboil just to be allowed to build gst-plugins-* is, well, just wrong. Anyway, I have updated the gst-plugins-* patches so that liboil is required unless --without-liboil is explicitly specified. That should hopefully satisfy both your requirement for liboil to exist under normal circumstances and our wish to be able to build without it.
Created attachment 99384 [details] [review] Use AG_GST_CHECK_PLUGIN and make liboil optional for gst-plugins-base Updated the patch for gst-plugins-base.
Created attachment 99385 [details] [review] Use AG_GST_CHECK_PLUGIN and make liboil optional for gst-plugins-good Updated the patch for gst-plugins-good.
Created attachment 99386 [details] [review] Use AG_GST_CHECK_PLUGIN and make liboil optional for gst-plugins-bad Updated the patch for gst-plugins-bad.
Created attachment 99387 [details] [review] Use AG_GST_CHECK_PLUGIN and make liboil optional for gst-plugins-ugly Updated the patch for gst-plugins-ugly.
Restricting the use of liboil in gstreamer plugins is exactly the opposite direction we want to go. I suggest that you keep this patch as part of your local build system.
Of course I will keep the liboil part patch in our local repository if it is not accepted into the mainline, but I still fail to see why it should not be accepted. Making it possible to build without liboil is in no way trying to restrict the use of liboil in GStreamer plugins. It just removes an unnecessary requirement for those who know they will not need any of the affected plugins. I believe the possibility do disable the requirement for liboil could be beneficial for others as well. As I see it the --without-liboil option falls in the same category as most of the other --disable-<feature> options available for GStreamer. I.e., normal users should not use any of them, but for those of us trying to build a smaller system, e.g., for an embedded product, it is appreciated to be able to disable features you know you do not need. And with the latest version of the patch, where one will have to make an active choice by explicitly specifying --without-liboil to deactivate liboil support, one should be aware of the implications of doing so. No one should be surprised that by deactivating liboil it will deactivate plugins which requires it.
Any more comments on this? David?
Created attachment 100308 [details] [review] Use AG_GST_CHECK_PLUGIN for gst-plugins-base Updated the patch for gst-plugins-base, this time without the support to make liboil optional.
Created attachment 100309 [details] [review] Use AG_GST_CHECK_PLUGIN for gst-plugins-good Updated the patch for gst-plugins-good, this time without the support to make liboil optional.
Created attachment 100310 [details] [review] Use AG_GST_CHECK_PLUGIN for gst-plugins-bad Updated the patch for gst-plugins-bad, this time without the support to make liboil optional.
Created attachment 100312 [details] [review] Use AG_GST_CHECK_PLUGIN for gst-plugins-ugly Updated the patch for gst-plugins-ugly, this time without the support to make liboil optional.
*** Bug 501807 has been marked as a duplicate of this bug. ***
Hmm, I am not sure this really is a duplicate of Bug 501807. One can already disable the equalizer plugin by specifying a correct list of plugins to --with-plugins, but the patches here does not change that in any way. I.e., there is still no --disable-equalizer (or more likely --disable-plugin-equalizer). I think support for this could easily be added to AG_GST_CHECK_PLUGIN if one wanted to, and thereby making --with-plugins obsolete (it really is an awkward interface).
Thank you for doing the work to clean up the configure.ac files. All the patches look fine. I haven't specifically tested anything, though. Thomas has the final say on build script changes.
I started to look at patches in comment 1 and 2. in comment 1, AG_GST_PKG_CHECK_MODULES is used but this is not defined anywhere ? Is the patch missing something ? in comment 2, I think it would be good to display both the feature AND the dependent plugins. what do you think ?
AG_GST_PKG_CHECK_MODULES is already defined in common/m4/gst-check.m4. I agree with your idea to include the dependent plugins. I have uppdated the patches to accomodate for this.
Created attachment 101175 [details] [review] Typo fix and minor improvement to AG_GST_CHECK_FEATURE
Created attachment 101176 [details] [review] Make it easier to include and exclude plugins
All patches have now been applied.