On 26/08/14 18:01, Ilia Mirkin wrote: > 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... > I'm inclined to go with if (!source_surface), and judging by the reporter quotation of the spec, I guess that they'll do the same. Whereas for what exact is meant in the spec & design, that would be an interesting question indeed :)
I'll bring it up with on the vdpau ML. -Emil > -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