[Mesa-dev] [PATCH v2 12/27] auxiliary/vl: use vl_*_screen_create stubs when building w/o platform

[Christian König]

Am 05.05.2017 um 13:23 schrieb Emil Velikov:
> On 5 May 2017 at 10:30, Christian König <christian.koenig at amd.com> wrote:
>> Am 04.05.2017 um 18:33 schrieb Emil Velikov:
>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>
>>> Provide a dummy stub when the user has opted w/o said platform, thus
>>> we can build the binaries without unnecessarily requiring X11/other
>>> headers.
>>>
>>> In order to avoid build and link-time issues, we remove the HAVE_DRI3
>>> guards in the VA and VDPAU state-trackers.
>>>
>>> With this change st/va will return VA_STATUS_ERROR_ALLOCATION_FAILED
>>> instead of VA_STATUS_ERROR_UNIMPLEMENTED. That is fine since upstream
>>> users of libva such as vlc and mpv do little error checking, let
>>> alone distinguish between the two.
>>>
>>> Cc: Leo Liu <leo.liu at amd.com>
>>> Cc: "Guttula, Suresh" <Suresh.Guttula at amd.com>
>>> Cc: mesa-stable at lists.freedesktop.org
>>> Cc: Christian König <christian.koenig at amd.com>
>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>> ---
>>> Christian, others
>>> How do you feel about the ALLOCATION_FAILED vs UNIMPLEMENTED situation?
>>> Doing the latter is doable, albeit it will make the code a bit ugly.
>>
>> I don't think that matters much, but resulting in UNIMPLEMENTED return code
>> when X backends are not compiled in indeed sounds cleaner.
>>
>> So price question is how much hassle would it be? Apart from that the
>> changes look good to me.
>>
> Hassle is zero, yet any solution that I can think of is quite ugly.

Why do you think it is ugly? I mean returning 
VA_STATUS_ERROR_UNIMPLEMENTED for things not implemented sounds like the 
right thing to do to me.

Having the handling in the auxiliary code on the other hand is a bit 
ugly if you ask me, because such stuff is actually the problem of the 
state tracker.

It just saves us quite a bunch of conditionally compiled code in the 
state tracker and so is still preferable from a maintenance perspective.

Maybe change the vl_*_screen_create() interface to return a negative 
error code from errno.h and then map that to a proper VA_STATUS return 
in the VA state tracker?

Regards,
Christian.

> See the example below and feel free to suggest anything else.
>
> -Emil
>
> diff --git a/src/gallium/state_trackers/va/context.c
> b/src/gallium/state_trackers/va/context.c
> index ae9154a332a..9b7688320cc 100644
> --- a/src/gallium/state_trackers/va/context.c
> +++ b/src/gallium/state_trackers/va/context.c
> @@ -26,6 +26,7 @@
>    *
>    **************************************************************************/
>
> +#include <stdbool.h>
>   #include "pipe/p_screen.h"
>   #include "pipe/p_video_codec.h"
>   #include "util/u_memory.h"
> @@ -103,6 +104,7 @@ PUBLIC VAStatus
>   VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
>   {
>      vlVaDriver *drv;
> +   bool implemented = true;
>
>      if (!ctx)
>         return VA_STATUS_ERROR_INVALID_CONTEXT;
> @@ -113,17 +115,26 @@ VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
>
>      switch (ctx->display_type) {
>      case VA_DISPLAY_ANDROID:
> -      FREE(drv);
> -      return VA_STATUS_ERROR_UNIMPLEMENTED;
> +      implemented = false;
> +      break;
>      case VA_DISPLAY_GLX:
>      case VA_DISPLAY_X11:
> +#if !defined(HAVE_X11_PLATFORM)
> +      implemented = false;
> +#endif
>         drv->vscreen = vl_dri3_screen_create(ctx->native_dpy, ctx->x11_screen);
>         if (!drv->vscreen)
>            drv->vscreen = vl_dri2_screen_create(ctx->native_dpy,
> ctx->x11_screen);
>         break;
>      case VA_DISPLAY_WAYLAND:
> +#if !defined(HAVE_WAYLAND_PLATFORM)
> +      implemented = false;
> +#endif
>      case VA_DISPLAY_DRM:
>      case VA_DISPLAY_DRM_RENDERNODES: {
> +#if !defined(HAVE_DRM_PLATFORM)
> +      implemented = false;
> +#endif
>         const struct drm_state *drm_info = (struct drm_state *) ctx->drm_state;
>
>         if (!drm_info || drm_info->fd < 0) {
> @@ -139,6 +150,11 @@ VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
>         return VA_STATUS_ERROR_INVALID_DISPLAY;
>      }
>
> +   if (!implemented) {
> +      FREE(drv);
> +      return VA_STATUS_ERROR_UNIMPLEMENTED;
> +   }
> +
>      if (!drv->vscreen)
>         goto error_screen;