On Tue, Aug 26, 2014 at 12:50 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> On 15/08/14 19:33, Emil Velikov wrote:
>> On 14/08/14 10:59, Christian König wrote:
>>> From: Christian König <christian.koe...@amd.com>
>>>
>>> Correctly handle that the source_surface is only optional.
>>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80561
>> Cc: mesa-sta...@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.koe...@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
typedef uint32_t VdpOutputSurface
It doesn't make sense to check for NULL... I think that
VDP_INVALID_HANDLE is in the same spirit as 'NULL'. Although perhaps
they really meant 0 (is 0 some sort of otherwise-disallowed handle?).
Not sure what the nvidia vdpau libs do...
-ilia
>
>>> + 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.veli...@gmail.com>
>>
>> Thanks
>> Emil
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
