[Mesa-dev] [PATCH 1/2] st/vdpau: fix vlVdpOutputSurfaceRender(Output|Bitmap)Surface

[Emil Velikov]

On 15/08/14 19:33, Emil Velikov wrote:
> On 14/08/14 10:59, Christian König wrote:
>> From: Christian König <christian.koenig at amd.com>
>>
>> Correctly handle that the source_surface is only optional.
>>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80561
> Cc: mesa-stable at lists.freedesktop.org
> 
> Would be nice to get this in stable :)
> 
Hi Christian,

Did you miss my comments below, or am I miss-understanding the spec ?

>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>>  src/gallium/state_trackers/vdpau/device.c        | 43 +++++++++++++++++++++++-
>>  src/gallium/state_trackers/vdpau/output.c        | 42 +++++++++++++++--------
>>  src/gallium/state_trackers/vdpau/vdpau_private.h |  1 +
>>  3 files changed, 71 insertions(+), 15 deletions(-)
>>
[snip]
>> diff --git a/src/gallium/state_trackers/vdpau/output.c b/src/gallium/state_trackers/vdpau/output.c
>> index caae50f..3248f76 100644
>> --- a/src/gallium/state_trackers/vdpau/output.c
>> +++ b/src/gallium/state_trackers/vdpau/output.c
>> @@ -639,12 +639,19 @@ vlVdpOutputSurfaceRenderOutputSurface(VdpOutputSurface destination_surface,
>>     if (!dst_vlsurface)
>>        return VDP_STATUS_INVALID_HANDLE;
>>  
>> -   src_vlsurface = vlGetDataHTAB(source_surface);
>> -   if (!src_vlsurface)
>> -      return VDP_STATUS_INVALID_HANDLE;
>> +   if (source_surface == VDP_INVALID_HANDLE) {
> AFAICS the spec says "If source_surface is NULL..." whereas VDP_INVALID_HANDLE
> is defined as 0xffffffffU.
> 
Here

>> +      src_sv = dst_vlsurface->device->dummy_sv;
>> +
>> +   } else {
>> +      vlVdpOutputSurface *src_vlsurface = vlGetDataHTAB(source_surface);
>> +      if (!src_vlsurface)
>> +         return VDP_STATUS_INVALID_HANDLE;
>>  
>> -   if (dst_vlsurface->device != src_vlsurface->device)
>> -      return VDP_STATUS_HANDLE_DEVICE_MISMATCH;
>> +      if (dst_vlsurface->device != src_vlsurface->device)
>> +         return VDP_STATUS_HANDLE_DEVICE_MISMATCH;
>> +
>> +      src_sv = src_vlsurface->sampler_view;
>> +   }
>>  
>>     pipe_mutex_lock(dst_vlsurface->device->mutex);
>>     vlVdpResolveDelayedRendering(dst_vlsurface->device, NULL, NULL);
>> @@ -703,12 +710,19 @@ vlVdpOutputSurfaceRenderBitmapSurface(VdpOutputSurface destination_surface,
>>     if (!dst_vlsurface)
>>        return VDP_STATUS_INVALID_HANDLE;
>>  
>> -   src_vlsurface = vlGetDataHTAB(source_surface);
>> -   if (!src_vlsurface)
>> -      return VDP_STATUS_INVALID_HANDLE;
>> +   if (source_surface == VDP_INVALID_HANDLE) {
> Ditto.
> 
and here

Cheers,
Emil

> I'm not sure if we correctly handle another case from the spec (see below) but
> that can be tackled independently.
> 
> 
> "The surface is treated as having four components: red, green, blue and alpha.
> Any missing components are treated as 1.0. For example, for an A8
> VdpBitmapSurface, alpha will come from the surface but red, green and blue
> will be treated as 1.0"
> 
> 
> With the two fixed, the patch
> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
> 
> Thanks
> Emil
>