[Mesa-dev] [PATCH 3/3] st/vdpau: check surface params before creating resources

[Christian König]

Am 18.01.2014 06:05, schrieb Ilia Mirkin:
> On Fri, Jan 17, 2014 at 11:58 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> This seems like a function that should exist in gallium/util, but I didn't
>> find it. I guess the mesa/st does these checks far enough away from actual
>> resource creation.
>>
>>   src/gallium/state_trackers/vdpau/bitmap.c        | 4 ++++
>>   src/gallium/state_trackers/vdpau/output.c        | 7 +++++++
>>   src/gallium/state_trackers/vdpau/vdpau_private.h | 8 ++++++++
>>   3 files changed, 19 insertions(+)
>>
>> diff --git a/src/gallium/state_trackers/vdpau/bitmap.c b/src/gallium/state_trackers/vdpau/bitmap.c
>> index 6110fc7..d316134 100644
>> --- a/src/gallium/state_trackers/vdpau/bitmap.c
>> +++ b/src/gallium/state_trackers/vdpau/bitmap.c
>> @@ -80,6 +80,10 @@ vlVdpBitmapSurfaceCreate(VdpDevice device,
>>      res_tmpl.usage = frequently_accessed ? PIPE_USAGE_DYNAMIC : PIPE_USAGE_STATIC;
>>
>>      pipe_mutex_lock(dev->mutex);
>> +
>> +   if (!CheckSurfaceParams(pipe->screen, &res_tmpl))
>> +      goto err_unlock;
> Ugh, and naturally I mess this up -- there should be a ret =
> VDP_STATUS_RESOURCES here. You can see the fixed version at
> https://github.com/imirkin/mesa/commits/vdpau

I was just about to complain. But it seems like this is the only remark 
I can find as well.

Patches are: Reviewed-by: Christian König <christian.koenig at amd.com>

>
>> +
>>      res = pipe->screen->resource_create(pipe->screen, &res_tmpl);
>>      if (!res) {
>>         ret = VDP_STATUS_RESOURCES;
>> diff --git a/src/gallium/state_trackers/vdpau/output.c b/src/gallium/state_trackers/vdpau/output.c
>> index 58edd37..50036ae 100644
>> --- a/src/gallium/state_trackers/vdpau/output.c
>> +++ b/src/gallium/state_trackers/vdpau/output.c
>> @@ -83,6 +83,10 @@ vlVdpOutputSurfaceCreate(VdpDevice device,
>>      res_tmpl.usage = PIPE_USAGE_STATIC;
>>
>>      pipe_mutex_lock(dev->mutex);
>> +
>> +   if (!CheckSurfaceParams(pipe->screen, &res_tmpl))
>> +      goto err_unlock;
>> +
>>      res = pipe->screen->resource_create(pipe->screen, &res_tmpl);
>>      if (!res)
>>         goto err_unlock;
>> @@ -319,6 +323,9 @@ vlVdpOutputSurfacePutBitsIndexed(VdpOutputSurface surface,
>>      pipe_mutex_lock(vlsurface->device->mutex);
>>      vlVdpResolveDelayedRendering(vlsurface->device, NULL, NULL);
>>
>> +   if (!CheckSurfaceParams(context->screen, &res_tmpl))
>> +      goto error_resource;
>> +
>>      res = context->screen->resource_create(context->screen, &res_tmpl);
>>      if (!res)
>>         goto error_resource;
>> diff --git a/src/gallium/state_trackers/vdpau/vdpau_private.h b/src/gallium/state_trackers/vdpau/vdpau_private.h
>> index 60196ac..6c2c019 100644
>> --- a/src/gallium/state_trackers/vdpau/vdpau_private.h
>> +++ b/src/gallium/state_trackers/vdpau/vdpau_private.h
>> @@ -332,6 +332,14 @@ RectToPipeBox(const VdpRect *rect, struct pipe_resource *res)
>>      return box;
>>   }
>>
>> +static inline bool
>> +CheckSurfaceParams(struct pipe_screen *screen,
>> +                   const struct pipe_resource *templ)
>> +{
>> +   return screen->is_format_supported(
>> +         screen, templ->format, templ->target, templ->nr_samples, templ->bind);
>> +}
>> +
>>   typedef struct
>>   {
>>      struct vl_screen *vscreen;
>> --
>> 1.8.3.2
>>