[Mesa-dev] [PATCH kmscube] Detect gst_bo_map/_unmap and use it (or avoid it)

[Eric Engestrom]

On Friday, 2017-07-14 09:45:33 -0500, Daniel Díaz wrote:
> Those functions are not available on libMali, thus breaking
> builds and creating havoc.
> 
> Removing the specific parts of the code that deal with
> gbm_bo_map() and gbm_bo_unmap() renders the kmscube utility
> a little less useful, but still valuable.
> 
> Signed-off-by: Daniel Díaz <daniel.diaz at linaro.org>

Cc: Rob Clark <robdclark at gmail.com>

kmscube is his baby :)

> ---
>  Makefile.am   | 5 ++++-
>  configure.ac  | 3 +++
>  gst-decoder.c | 4 ++++
>  kmscube.c     | 2 ++
>  4 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index a36087d..ab4adbf 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -43,7 +43,6 @@ kmscube_SOURCES = \
>  	common.c \
>  	common.h \
>  	cube-smooth.c \
> -	cube-tex.c \
>  	drm-atomic.c \
>  	drm-common.c \
>  	drm-common.h \
> @@ -59,3 +58,7 @@ kmscube_LDADD += $(GST_LIBS)
>  kmscube_CFLAGS += $(GST_CFLAGS)
>  kmscube_SOURCES += cube-video.c gst-decoder.c
>  endif
> +
> +if ENABLE_GBM_MAP
> +kmscube_SOURCES += cube-tex.c
> +endif
> diff --git a/configure.ac b/configure.ac
> index 8397f7b..c7f1f4d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -65,5 +65,8 @@ if test "x$enable_gbm_modifiers" = xyes; then
>  	AC_DEFINE(HAVE_GBM_MODIFIERS, 1, [Define if you can use GBM properties.])
>  fi
>  
> +AC_CHECK_LIB([gbm], [gbm_bo_map], [HAVE_GBM_BO_MAP=yes], [])
> +AM_CONDITIONAL(ENABLE_GBM_MAP, [test "x$HAVE_GBM_BO_MAP" = "xyes"])
> +
>  AC_CONFIG_FILES([Makefile])
>  AC_OUTPUT
> diff --git a/gst-decoder.c b/gst-decoder.c
> index 2d6d581..c0dd2b2 100644
> --- a/gst-decoder.c
> +++ b/gst-decoder.c
> @@ -332,6 +332,7 @@ set_last_frame(struct decoder *dec, EGLImage frame, GstSample *samp)
>  	dec->last_samp = samp;
>  }
>  
> +#if HAVE_GBM_BO_MAP
>  // TODO this could probably be a helper re-used by cube-tex:
>  static int
>  buf_to_fd(const struct gbm *gbm, int size, void *ptr)
> @@ -357,6 +358,7 @@ buf_to_fd(const struct gbm *gbm, int size, void *ptr)
>  
>  	return fd;
>  }
> +#endif
>  
>  static EGLImage
>  buffer_to_image(struct decoder *dec, GstBuffer *buf)
> @@ -410,11 +412,13 @@ buffer_to_image(struct decoder *dec, GstBuffer *buf)
>  
>  	if (is_dmabuf_mem) {
>  		dmabuf_fd = dup(gst_dmabuf_memory_get_fd(mem));
> +#if HAVE_GBM_BO_MAP
>  	} else {

Not a fan of this; can you split the `}` on its own line and have
the #if right after?

>  		GstMapInfo map_info;
>  		gst_buffer_map(buf, &map_info, GST_MAP_READ);
>  		dmabuf_fd = buf_to_fd(dec->gbm, map_info.size, map_info.data);
>  		gst_buffer_unmap(buf, &map_info);
> +#endif
>  	}
>  
>  	if (dmabuf_fd < 0) {
> diff --git a/kmscube.c b/kmscube.c
> index 3a2c4dd..d86ed7d 100644
> --- a/kmscube.c
> +++ b/kmscube.c
> @@ -141,8 +141,10 @@ int main(int argc, char *argv[])
>  		egl = init_cube_smooth(gbm);
>  	else if (mode == VIDEO)
>  		egl = init_cube_video(gbm, video);
> +#if HAVE_GBM_BO_MAP
>  	else
>  		egl = init_cube_tex(gbm, mode);
> +#endif

Could you do something like this:

----8<----
 	else
+#if HAVE_GBM_BO_MAP
 		egl = init_cube_tex(gbm, mode);
+#else
+		printf("gbm_bo_map() support missing\n");
+#endif
---->8----

With this couple of nit-picks:
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

>  
>  	if (!egl) {
>  		printf("failed to initialize EGL\n");
> -- 
> 2.7.4
>