GNOME Bugzilla – Bug 778250
vaapi: build: support meson
Last modified: 2017-02-09 11:27:04 UTC
Add meson build system, as the rest of the GStreamer project. https://github.com/mesonbuild/meson/wiki
Created attachment 345065 [details] [review] [PATCH] vaapi: add meson build Here's a first crack at it.
Created attachment 345066 [details] [review] [PATCH v2] vaapi: add meson build Changes since v1: - Set USE_GST_GL_HELPERS when gstgl_dep is present - Set the GLES_VERSION_MASK used by egl code
Created attachment 345067 [details] [review] [PATCH v2] vaapi: add meson build Changes since v1: - Set USE_GST_GL_HELPERS when gstgl_dep is present - Set the GLES_VERSION_MASK used by egl code 345066 was botched :-/
Nice !
wow! What an unexpected surprise. Thanks, Scott!
Review of attachment 345067 [details] [review]: I had to make some changes in order to make it work. In the comments are those changes. And, bikeshedding a bit, there a couple of trailing white spaces that we could remove. ::: gst-libs/gst/base/meson.build @@ +15,3 @@ + version : libversion, + soversion : soversion, + install : true, install : false, ::: gst-libs/gst/vaapi/Makefile.am @@ +25,3 @@ -DIN_LIBGSTVAAPI_CORE \ -DGST_USE_UNSTABLE_API \ + -DHAVE_GSTVAAPIVERSION_H \ Is this a workaround because meson cannot generate the version header yet? ::: gst-libs/gst/vaapi/meson.build @@ +204,3 @@ +endif +if USE_EGL + gstlibvaapi_deps += [egl_dep] this needs the glib's gmodule dependency. Or better yet, delete that dead code. @@ +222,3 @@ + version : libversion, + soversion : soversion, + install : true, install : false, ::: gst/vaapi/meson.build @@ +46,3 @@ + include_directories : [configinc, libsinc], + dependencies : [gstbase_dep, gstvideo_dep, gstallocators_dep, gstpbutils_dep, + libva_dep, gstlibvaapi_dep, gstgl_dep], we need libm_dep too ::: meson.build @@ +30,3 @@ +gstvideo_dep = dependency('gstreamer-video-1.0', version : gst_req, + fallback : ['gst-plugins-base', 'video_dep']) +gstcodecparsers_dep = dependency('gstcodecparsers-1.0', version : gst_req, the dependency name is gstreamer-codecparsers-1.0 @@ +32,3 @@ +gstcodecparsers_dep = dependency('gstcodecparsers-1.0', version : gst_req, + fallback : ['gst-plugins-bad', 'gstcodecparsers_dep'], required: false) +gstgl_dep = dependency('gstgl-1.0', version : gst_req, the dependency name is gstreamer-gl-1.0 @@ +36,3 @@ +libva_dep = dependency('libva', version: '>= 0.30.4', required: false) + +if libva_dep.found() Why not, instead of branching, make libva a required dependency? @@ +37,3 @@ + +if libva_dep.found() + cc = meson.get_compiler('c') add libm_dep = cc.find_library('m', required : true)
Created attachment 345134 [details] [review] [PATCH v3] vaapi: add meson build Changes since v2: - Mark gst-lib libraries as install: false - Add gmodule dependency for egl - Add libm dependency for the plugin - Fix the names of gl and codecparsers dependencies - make libva_dep required
Created attachment 345136 [details] [review] [PATCH v4] vaapi: add meson build Changes since v3: - fix placement of meson.get_compiler Got carried away moving stuff around after testing it and wound up uploading v3 in a busted state :-(
Review of attachment 345136 [details] [review]: ::: gst-libs/gst/base/meson.build @@ +15,3 @@ + version : libversion, + soversion : soversion, + install : false, nitpick: 'install: false' is the default, so perhaps we could just remove it (less code to maintain). ::: gst-libs/gst/vaapi/gstvaapidisplay.c @@ +36,3 @@ #include "gstvaapidisplay_priv.h" #include "gstvaapiworkarounds.h" +#ifdef HAVE_GSTVAAPIVERSION_H I guess this header can be autogenerated with custom_target() ::: gst-libs/gst/vaapi/meson.build @@ +222,3 @@ + version : libversion, + soversion : soversion, + install : false, ditto ::: meson.build @@ +60,3 @@ +GLES_VERSION_MASK += glesv3_dep.found() ? 8 : 0 + +USE_ENCODERS = libva_dep.version().version_compare('>= 0.34.0') It is missing to honor the 'with_encoders' option USE_ENCODERS = libva_dep.version().version_compare('>= 0.34.0') and get_option('with_encoders') != 'no'
Created attachment 345263 [details] [review] [PATCH 1/2] make: remove gstvaapiversion.h generation hmm, instead of adding gstvaapiversion.h to the meson build, how about we just remove it. It looks funny to me and nothing like that is in the rest of the gstreamer codebase.
Created attachment 345264 [details] [review] [PATCH v5 2/2] vaapi: add meson build Changes since v4: - remove explicit install: false from static_libraries - gate USE_ENCODERS on the with_encoders option - gate USE_*_ENCODER on USE_ENCODERS
There are similar generated version headers in gstreamer and gst-plugins-base, but here it's not really needed since the lib isn't public any more.
For an uninstalled static library, we probably don't need the api-version suffix and the various versioning kwargs (but then they shouldn't hurt either).
Review of attachment 345263 [details] [review]: I kept it when we did the upstream because I assumed that a user library will reemerge in order to expose the GstVaapiDisplay. But that library-to-be needs be discussed with the community, thus, this is dead code for now. So yeah, I love the smell of removed code in the mornings <3
Comment on attachment 345264 [details] [review] [PATCH v5 2/2] vaapi: add meson build lgtm
commit 412dd13e86e834a846a26983470876ba36eace1c Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Mon Feb 6 15:46:20 2017 -0800 vaapi: add meson build https://bugzilla.gnome.org/show_bug.cgi?id=778250 commit 3cc4eb7b8138089b8cb64e3ca754784d9c194b59 Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Wed Feb 8 10:17:40 2017 -0800 make: remove gstvaapiversion.h generation https://bugzilla.gnome.org/show_bug.cgi?id=778250