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

2014-08-26 Thread 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.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 0xU.
 
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.veli...@gmail.com
 
 Thanks
 Emil
 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2014-08-26 Thread Ilia Mirkin
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 0xU.

 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


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

2014-08-26 Thread Emil Velikov
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 0xU.

 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


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

2014-08-26 Thread Ilia Mirkin
On Tue, Aug 26, 2014 at 1:18 PM, Emil Velikov emil.l.veli...@gmail.com wrote:
 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 0xU.

 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.

Well, what if you have a legitimate surface, whose handle is 0? I
don't know of anything that prevents that, although IIRC in practice
the nvidia vdpau-supplied handles start at 1.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2014-08-26 Thread Christian König

Am 26.08.2014 um 18:50 schrieb 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.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 ?


I've missed your comment and the spec is a bit flaky about this.

As far as I know zero is a perfectly valid handle and VDP_INVALID_HANDLE 
needs to be used instead.


Getting the issue on the VDPAU list is probably a good idea,
Christian.




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 0xU.


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.veli...@gmail.com

Thanks
Emil



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2014-08-26 Thread Emil Velikov
On 26/08/14 18:32, Christian König wrote:
 Am 26.08.2014 um 18:50 schrieb 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.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 ?
 
 I've missed your comment and the spec is a bit flaky about this.
 
 As far as I know zero is a perfectly valid handle and VDP_INVALID_HANDLE needs
 to be used instead.
 
Indeed you and Ilia are absolutely correct. The spec should say
VDP_INVALID_HANDLE, as confirmed by Stephen Warren [1].

Pardon for the noise.
Emil

[1] http://lists.freedesktop.org/archives/vdpau/2014-August/000180.html

 Getting the issue on the VDPAU list is probably a good idea,
 Christian.
 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2014-08-15 Thread Emil Velikov
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 :)

 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(-)
 
 diff --git a/src/gallium/state_trackers/vdpau/device.c 
 b/src/gallium/state_trackers/vdpau/device.c
 index 9c5ec60..efc1fde 100644
 --- a/src/gallium/state_trackers/vdpau/device.c
 +++ b/src/gallium/state_trackers/vdpau/device.c
 @@ -42,6 +42,8 @@ vdp_imp_device_create_x11(Display *display, int screen, 
 VdpDevice *device,
VdpGetProcAddress **get_proc_address)
  {
 struct pipe_screen *pscreen;
 +   struct pipe_resource *res, res_tmpl;
 +   struct pipe_sampler_view sv_tmpl;
 vlVdpDevice *dev = NULL;
 VdpStatus ret;
  
 @@ -79,6 +81,43 @@ vdp_imp_device_create_x11(Display *display, int screen, 
 VdpDevice *device,
goto no_context;
 }
  
 +   memset(res_tmpl, 0, sizeof(res_tmpl));
 +
 +   res_tmpl.target = PIPE_TEXTURE_2D;
 +   res_tmpl.format = PIPE_FORMAT_R8G8B8A8_UNORM;
 +   res_tmpl.width0 = 1;
 +   res_tmpl.height0 = 1;
 +   res_tmpl.depth0 = 1;
 +   res_tmpl.array_size = 1;
 +   res_tmpl.bind = PIPE_BIND_SAMPLER_VIEW;
 +   res_tmpl.usage = PIPE_USAGE_DEFAULT;
 +
 +   if (!CheckSurfaceParams(pscreen, res_tmpl)) {
 +  ret = VDP_STATUS_NO_IMPLEMENTATION;
 +  goto no_resource;
 +   }
 +
 +   res = pscreen-resource_create(pscreen, res_tmpl);
 +   if (!res) {
 +  ret = VDP_STATUS_RESOURCES;
 +  goto no_resource;
 +   }
 +
 +   memset(sv_tmpl, 0, sizeof(sv_tmpl));
 +   u_sampler_view_default_template(sv_tmpl, res, res-format);
 +
 +   sv_tmpl.swizzle_r = PIPE_SWIZZLE_ONE;
 +   sv_tmpl.swizzle_g = PIPE_SWIZZLE_ONE;
 +   sv_tmpl.swizzle_b = PIPE_SWIZZLE_ONE;
 +   sv_tmpl.swizzle_a = PIPE_SWIZZLE_ONE;
 +
 +   dev-dummy_sv = dev-context-create_sampler_view(dev-context, res, 
 sv_tmpl);
 +   pipe_resource_reference(res, NULL);
 +   if (!dev-dummy_sv) {
 +  ret = VDP_STATUS_RESOURCES;
 +  goto no_resource;
 +   }
 +
 *device = vlAddDataHTAB(dev);
 if (*device == 0) {
ret = VDP_STATUS_ERROR;
 @@ -93,8 +132,9 @@ vdp_imp_device_create_x11(Display *display, int screen, 
 VdpDevice *device,
 return VDP_STATUS_OK;
  
  no_handle:
 +   pipe_sampler_view_reference(dev-dummy_sv, NULL);
 +no_resource:
 dev-context-destroy(dev-context);
 -   /* Destroy vscreen */
  no_context:
 vl_screen_destroy(dev-vscreen);
  no_vscreen:
 @@ -185,6 +225,7 @@ vlVdpDeviceFree(vlVdpDevice *dev)
  {
 pipe_mutex_destroy(dev-mutex);
 vl_compositor_cleanup(dev-compositor);
 +   pipe_sampler_view_reference(dev-dummy_sv, NULL);
 dev-context-destroy(dev-context);
 vl_screen_destroy(dev-vscreen);
 FREE(dev);
 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
 @@ -624,9 +624,9 @@ vlVdpOutputSurfaceRenderOutputSurface(VdpOutputSurface 
 destination_surface,
uint32_t flags)
  {
 vlVdpOutputSurface *dst_vlsurface;
 -   vlVdpOutputSurface *src_vlsurface;
  
 struct pipe_context *context;
 +   struct pipe_sampler_view *src_sv;
 struct vl_compositor *compositor;
 struct vl_compositor_state *cstate;
  
 @@ -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 0xU.

 +  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);
 @@ -657,7 +664,7 @@ vlVdpOutputSurfaceRenderOutputSurface(VdpOutputSurface 
 destination_surface,
  
 vl_compositor_clear_layers(cstate);
 

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

2014-08-14 Thread Christian König
From: Christian König christian.koe...@amd.com

Correctly handle that the source_surface is only optional.

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(-)

diff --git a/src/gallium/state_trackers/vdpau/device.c 
b/src/gallium/state_trackers/vdpau/device.c
index 9c5ec60..efc1fde 100644
--- a/src/gallium/state_trackers/vdpau/device.c
+++ b/src/gallium/state_trackers/vdpau/device.c
@@ -42,6 +42,8 @@ vdp_imp_device_create_x11(Display *display, int screen, 
VdpDevice *device,
   VdpGetProcAddress **get_proc_address)
 {
struct pipe_screen *pscreen;
+   struct pipe_resource *res, res_tmpl;
+   struct pipe_sampler_view sv_tmpl;
vlVdpDevice *dev = NULL;
VdpStatus ret;
 
@@ -79,6 +81,43 @@ vdp_imp_device_create_x11(Display *display, int screen, 
VdpDevice *device,
   goto no_context;
}
 
+   memset(res_tmpl, 0, sizeof(res_tmpl));
+
+   res_tmpl.target = PIPE_TEXTURE_2D;
+   res_tmpl.format = PIPE_FORMAT_R8G8B8A8_UNORM;
+   res_tmpl.width0 = 1;
+   res_tmpl.height0 = 1;
+   res_tmpl.depth0 = 1;
+   res_tmpl.array_size = 1;
+   res_tmpl.bind = PIPE_BIND_SAMPLER_VIEW;
+   res_tmpl.usage = PIPE_USAGE_DEFAULT;
+
+   if (!CheckSurfaceParams(pscreen, res_tmpl)) {
+  ret = VDP_STATUS_NO_IMPLEMENTATION;
+  goto no_resource;
+   }
+
+   res = pscreen-resource_create(pscreen, res_tmpl);
+   if (!res) {
+  ret = VDP_STATUS_RESOURCES;
+  goto no_resource;
+   }
+
+   memset(sv_tmpl, 0, sizeof(sv_tmpl));
+   u_sampler_view_default_template(sv_tmpl, res, res-format);
+
+   sv_tmpl.swizzle_r = PIPE_SWIZZLE_ONE;
+   sv_tmpl.swizzle_g = PIPE_SWIZZLE_ONE;
+   sv_tmpl.swizzle_b = PIPE_SWIZZLE_ONE;
+   sv_tmpl.swizzle_a = PIPE_SWIZZLE_ONE;
+
+   dev-dummy_sv = dev-context-create_sampler_view(dev-context, res, 
sv_tmpl);
+   pipe_resource_reference(res, NULL);
+   if (!dev-dummy_sv) {
+  ret = VDP_STATUS_RESOURCES;
+  goto no_resource;
+   }
+
*device = vlAddDataHTAB(dev);
if (*device == 0) {
   ret = VDP_STATUS_ERROR;
@@ -93,8 +132,9 @@ vdp_imp_device_create_x11(Display *display, int screen, 
VdpDevice *device,
return VDP_STATUS_OK;
 
 no_handle:
+   pipe_sampler_view_reference(dev-dummy_sv, NULL);
+no_resource:
dev-context-destroy(dev-context);
-   /* Destroy vscreen */
 no_context:
vl_screen_destroy(dev-vscreen);
 no_vscreen:
@@ -185,6 +225,7 @@ vlVdpDeviceFree(vlVdpDevice *dev)
 {
pipe_mutex_destroy(dev-mutex);
vl_compositor_cleanup(dev-compositor);
+   pipe_sampler_view_reference(dev-dummy_sv, NULL);
dev-context-destroy(dev-context);
vl_screen_destroy(dev-vscreen);
FREE(dev);
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
@@ -624,9 +624,9 @@ vlVdpOutputSurfaceRenderOutputSurface(VdpOutputSurface 
destination_surface,
   uint32_t flags)
 {
vlVdpOutputSurface *dst_vlsurface;
-   vlVdpOutputSurface *src_vlsurface;
 
struct pipe_context *context;
+   struct pipe_sampler_view *src_sv;
struct vl_compositor *compositor;
struct vl_compositor_state *cstate;
 
@@ -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) {
+  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);
@@ -657,7 +664,7 @@ vlVdpOutputSurfaceRenderOutputSurface(VdpOutputSurface 
destination_surface,
 
vl_compositor_clear_layers(cstate);
vl_compositor_set_layer_blend(cstate, 0, blend, false);
-   vl_compositor_set_rgba_layer(cstate, compositor, 0, 
src_vlsurface-sampler_view,
+   vl_compositor_set_rgba_layer(cstate, compositor, 0, src_sv,
 RectToPipe(source_rect, src_rect), NULL,
 ColorsToPipe(colors, flags, vlcolors));
STATIC_ASSERT(VL_COMPOSITOR_ROTATE_0 == VDP_OUTPUT_SURFACE_RENDER_ROTATE_0);