st/va: use provided sizes and coords for vlVaGetImage

Submitted by Zhang, Boyuan on Oct. 9, 2018, 8:03 p.m.

Details

Message ID 1539115412-25566-1-git-send-email-boyuan.zhang@amd.com
State New
Headers show
Series "st/va: use provided sizes and coords for vlVaGetImage" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Zhang, Boyuan Oct. 9, 2018, 8:03 p.m.
From: Boyuan Zhang <boyuan.zhang@amd.com>

vlVaGetImage should respect the width, height, and coordinates x and y that
passed in. Therefore, pipe_box should be created with the passed in values
instead of surface width/height.

v2: add input size check, return error when size out of bounds

Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
Cc: "18.2" <mesa-stable@lists.freedesktop.org>
Reviewed-by: Leo Liu <leo.liu@amd.com>
---
 src/gallium/state_trackers/va/image.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/state_trackers/va/image.c b/src/gallium/state_trackers/va/image.c
index 3f892c9..449ae86 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -353,6 +353,23 @@  vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, int x, int y,
       return VA_STATUS_ERROR_INVALID_IMAGE;
    }
 
+   if (x < 0 || y < 0) {
+      mtx_unlock(&drv->mutex);
+      return VA_STATUS_ERROR_INVALID_PARAMETER;
+   }
+
+   if (x + width > surf->templat.width ||
+       y + height > surf->templat.height) {
+      mtx_unlock(&drv->mutex);
+      return VA_STATUS_ERROR_INVALID_PARAMETER;
+   }
+
+   if (x + width > vaimage->width ||
+       y + height > vaimage->height) {
+      mtx_unlock(&drv->mutex);
+      return VA_STATUS_ERROR_INVALID_PARAMETER;
+   }
+
    img_buf = handle_table_get(drv->htab, vaimage->buf);
    if (!img_buf) {
       mtx_unlock(&drv->mutex);
@@ -400,11 +417,14 @@  vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, int x, int y,
    }
 
    for (i = 0; i < vaimage->num_planes; i++) {
-      unsigned width, height;
+      unsigned w = align(width, 2);
+      unsigned h = align(height, 2);
       if (!views[i]) continue;
-      vlVaVideoSurfaceSize(surf, i, &width, &height);
+      vl_video_buffer_adjust_size(&w, &h, i,
+                                  surf->templat.chroma_format,
+                                  surf->templat.interlaced);
       for (j = 0; j < views[i]->texture->array_size; ++j) {
-         struct pipe_box box = {0, 0, j, width, height, 1};
+         struct pipe_box box = {x, y, j, w, h, 1};
          struct pipe_transfer *transfer;
          uint8_t *map;
          map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,

Comments

On Tue, Oct 9, 2018 at 4:03 PM <boyuan.zhang@amd.com> wrote:
>
> From: Boyuan Zhang <boyuan.zhang@amd.com>
>
> vlVaGetImage should respect the width, height, and coordinates x and y that
> passed in. Therefore, pipe_box should be created with the passed in values
> instead of surface width/height.
>
> v2: add input size check, return error when size out of bounds
>
> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
> Cc: "18.2" <mesa-stable@lists.freedesktop.org>
> Reviewed-by: Leo Liu <leo.liu@amd.com>
> ---
>  src/gallium/state_trackers/va/image.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/image.c b/src/gallium/state_trackers/va/image.c
> index 3f892c9..449ae86 100644
> --- a/src/gallium/state_trackers/va/image.c
> +++ b/src/gallium/state_trackers/va/image.c
> @@ -353,6 +353,23 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, int x, int y,
>        return VA_STATUS_ERROR_INVALID_IMAGE;
>     }
>
> +   if (x < 0 || y < 0) {
> +      mtx_unlock(&drv->mutex);
> +      return VA_STATUS_ERROR_INVALID_PARAMETER;
> +   }
> +
> +   if (x + width > surf->templat.width ||
> +       y + height > surf->templat.height) {
> +      mtx_unlock(&drv->mutex);
> +      return VA_STATUS_ERROR_INVALID_PARAMETER;
> +   }
> +
> +   if (x + width > vaimage->width ||
> +       y + height > vaimage->height) {

I believe the x/y offset is only meant for the surface, not for the
image. e.g. x=100,y=100,width=100,height=100 would be able to work
with a 100x100 image destination.

> +      mtx_unlock(&drv->mutex);
> +      return VA_STATUS_ERROR_INVALID_PARAMETER;
> +   }
> +
>     img_buf = handle_table_get(drv->htab, vaimage->buf);
>     if (!img_buf) {
>        mtx_unlock(&drv->mutex);
> @@ -400,11 +417,14 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, int x, int y,
>     }
>
>     for (i = 0; i < vaimage->num_planes; i++) {
> -      unsigned width, height;
> +      unsigned w = align(width, 2);
> +      unsigned h = align(height, 2);
>        if (!views[i]) continue;
> -      vlVaVideoSurfaceSize(surf, i, &width, &height);
> +      vl_video_buffer_adjust_size(&w, &h, i,
> +                                  surf->templat.chroma_format,
> +                                  surf->templat.interlaced);
>        for (j = 0; j < views[i]->texture->array_size; ++j) {
> -         struct pipe_box box = {0, 0, j, width, height, 1};
> +         struct pipe_box box = {x, y, j, w, h, 1};
>           struct pipe_transfer *transfer;
>           uint8_t *map;
>           map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,
> --
> 2.7.4
>
On 2018-10-09 04:09 PM, Ilia Mirkin wrote:
> On Tue, Oct 9, 2018 at 4:03 PM <boyuan.zhang@amd.com> wrote:
>> From: Boyuan Zhang <boyuan.zhang@amd.com>
>>
>> vlVaGetImage should respect the width, height, and coordinates x and y that
>> passed in. Therefore, pipe_box should be created with the passed in values
>> instead of surface width/height.
>>
>> v2: add input size check, return error when size out of bounds
>>
>> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
>> Cc: "18.2" <mesa-stable@lists.freedesktop.org>
>> Reviewed-by: Leo Liu <leo.liu@amd.com>
>> ---
>>   src/gallium/state_trackers/va/image.c | 26 +++++++++++++++++++++++---
>>   1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/va/image.c b/src/gallium/state_trackers/va/image.c
>> index 3f892c9..449ae86 100644
>> --- a/src/gallium/state_trackers/va/image.c
>> +++ b/src/gallium/state_trackers/va/image.c
>> @@ -353,6 +353,23 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, int x, int y,
>>         return VA_STATUS_ERROR_INVALID_IMAGE;
>>      }
>>
>> +   if (x < 0 || y < 0) {
>> +      mtx_unlock(&drv->mutex);
>> +      return VA_STATUS_ERROR_INVALID_PARAMETER;
>> +   }
>> +
>> +   if (x + width > surf->templat.width ||
>> +       y + height > surf->templat.height) {
>> +      mtx_unlock(&drv->mutex);
>> +      return VA_STATUS_ERROR_INVALID_PARAMETER;
>> +   }
>> +
>> +   if (x + width > vaimage->width ||
>> +       y + height > vaimage->height) {
> I believe the x/y offset is only meant for the surface, not for the
> image. e.g. x=100,y=100,width=100,height=100 would be able to work
> with a 100x100 image destination.

I just checked the doc(va.h) again, it says x/y are the coordinates of 
the upper left source pixel. I guess "source" here means surface, so I 
believe you are right.

Boyuan

>
>> +      mtx_unlock(&drv->mutex);
>> +      return VA_STATUS_ERROR_INVALID_PARAMETER;
>> +   }
>> +
>>      img_buf = handle_table_get(drv->htab, vaimage->buf);
>>      if (!img_buf) {
>>         mtx_unlock(&drv->mutex);
>> @@ -400,11 +417,14 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface, int x, int y,
>>      }
>>
>>      for (i = 0; i < vaimage->num_planes; i++) {
>> -      unsigned width, height;
>> +      unsigned w = align(width, 2);
>> +      unsigned h = align(height, 2);
>>         if (!views[i]) continue;
>> -      vlVaVideoSurfaceSize(surf, i, &width, &height);
>> +      vl_video_buffer_adjust_size(&w, &h, i,
>> +                                  surf->templat.chroma_format,
>> +                                  surf->templat.interlaced);
>>         for (j = 0; j < views[i]->texture->array_size; ++j) {
>> -         struct pipe_box box = {0, 0, j, width, height, 1};
>> +         struct pipe_box box = {x, y, j, w, h, 1};
>>            struct pipe_transfer *transfer;
>>            uint8_t *map;
>>            map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,
>> --
>> 2.7.4
>>