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

[Emil Velikov]

On 5 May 2017 at 12:37, Christian König <christian.koenig at amd.com> wrote:
> 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?
>
Not a huge fan of ifndef twinged with code. If we ignore that for a
moment - the erro+map to VA is also a bit iffy, if we think of the
wayland case.
In there we have a partial implementation (IIRC so does the i965 va
driver) which does not have a matching vl_*_screen_create call.

On second though i965 va does not report VA_STATUS_ERROR_UNIMPLEMENTED
when built w/o $platform, so perhaps we can get this as-is and polish
at a later stage?

-Emil