Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage
On Tue, Oct 9, 2018 at 3:09 PM Boyuan Zhang wrote: > > Thanks for the explanation ilia. > > I'm curious too here that if it's legal for player to not respect the > image size when calling vlVaGetImage. If player already know the size of > image is 100x100, then why should it still call vlVaGetImage with > width/height=600? > > I mean when VA-API player calls to create image and create surface, it > should behave itself, or will be considered a player bug. If player > calls something out of range, even the driver have the size > clipped(driver trying to fix player bug), but player still won't get > expected stuff, since the requested size have been clipped by driver. > Does this make sense? Yeah, that totally makes sense. Different APIs have different approaches to such error conditions. IMHO it'd be perfectly fine to return an error status in such cases, in theory. But I'm no VA-API expert, and the docs[1] appear silent on the matter. Cheers, -ilia [1] http://intel.github.io/libva/group__api__core.html#ga3d56f2eaf0be528a512cc935aca35418 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage
Thanks for the explanation ilia. I'm curious too here that if it's legal for player to not respect the image size when calling vlVaGetImage. If player already know the size of image is 100x100, then why should it still call vlVaGetImage with width/height=600? I mean when VA-API player calls to create image and create surface, it should behave itself, or will be considered a player bug. If player calls something out of range, even the driver have the size clipped(driver trying to fix player bug), but player still won't get expected stuff, since the requested size have been clipped by driver. Does this make sense? Regards, Boyuan On 2018-10-09 02:10 PM, Ilia Mirkin wrote: On Tue, Oct 9, 2018 at 1:59 PM Boyuan Zhang wrote: Hi ilia, I saw the function u_box_clip_2d(https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/auxiliary/util/u_box.h#n74). But I still don't quite understand why we need to do this? Or say, what will happen if we don't do this box clipping here? Can you provide more information about this please? Sure. Let's say you have: 1000x1000 video surface 500x500 image Then you call vlVaGetImage(surface, x=600, y=600, width=600, height=600, image) Ideally you would retrieve the 400x400 "valid" area (from 600x600 at the surface) and stick it into the image starting at 0,0. Then let's say you have 1000x1000 video surface 100x100 image Then you call vlVaGetImage(surface, x=600, y=600, width=600, height=600, image) Ideally the image would be filled with a 100x100 square from the surface starting at 600x600 and ending at 700x700. I haven't deeply dived into the VA docs. Perhaps some or all of these are illegal. In which case the x/y/w/h need to be checked and errors returned. Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage
On Tue, Oct 9, 2018 at 1:59 PM Boyuan Zhang wrote: > > Hi ilia, > > I saw the function > u_box_clip_2d(https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/auxiliary/util/u_box.h#n74). > > But I still don't quite understand why we need to do this? Or say, what > will happen if we don't do this box clipping here? Can you provide more > information about this please? Sure. Let's say you have: 1000x1000 video surface 500x500 image Then you call vlVaGetImage(surface, x=600, y=600, width=600, height=600, image) Ideally you would retrieve the 400x400 "valid" area (from 600x600 at the surface) and stick it into the image starting at 0,0. Then let's say you have 1000x1000 video surface 100x100 image Then you call vlVaGetImage(surface, x=600, y=600, width=600, height=600, image) Ideally the image would be filled with a 100x100 square from the surface starting at 600x600 and ending at 700x700. I haven't deeply dived into the VA docs. Perhaps some or all of these are illegal. In which case the x/y/w/h need to be checked and errors returned. Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage
Hi ilia,
I saw the function
u_box_clip_2d(https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/auxiliary/util/u_box.h#n74).
But I still don't quite understand why we need to do this? Or say, what
will happen if we don't do this box clipping here? Can you provide more
information about this please?
Regards,
Boyuan
On 2018-10-05 12:11 PM, Ilia Mirkin wrote:
This is an improvement, but I think you need to clip the box to
1. Size of the surface
2. Size of the image
I think that there are clipping helpers available to do that (maybe
pipe_box_clip or so? I forget, check the auxiliary dir). Christian -
does that make sense to you?
Cheers,
-ilia
On Fri, Oct 5, 2018 at 12:01 PM wrote:
From: Boyuan Zhang
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.
Signed-off-by: Boyuan Zhang
---
src/gallium/state_trackers/va/image.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/gallium/state_trackers/va/image.c
b/src/gallium/state_trackers/va/image.c
index 3f892c9..c9f6f18 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -400,11 +400,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, , );
+ vl_video_buffer_adjust_size(, , 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage
Reviewed-by: Leo Liu
-Original Message-
From: Zhang, Boyuan
Sent: Friday, October 05, 2018 12:01 PM
To: mesa-dev@lists.freedesktop.org
Cc: Liu, Leo ; imir...@alum.mit.edu;
ckoenig.leichtzumer...@gmail.com; Zhang, Boyuan
Subject: [PATCH] st/va: use provided sizes and coords for getimage
From: Boyuan Zhang
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.
Signed-off-by: Boyuan Zhang
---
src/gallium/state_trackers/va/image.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/gallium/state_trackers/va/image.c
b/src/gallium/state_trackers/va/image.c
index 3f892c9..c9f6f18 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -400,11 +400,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, , );
+ vl_video_buffer_adjust_size(, , 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage
If that also fixes the problem, then yeah that makes perfect sense to me
as well.
Christian.
Am 05.10.2018 um 18:11 schrieb Ilia Mirkin:
This is an improvement, but I think you need to clip the box to
1. Size of the surface
2. Size of the image
I think that there are clipping helpers available to do that (maybe
pipe_box_clip or so? I forget, check the auxiliary dir). Christian -
does that make sense to you?
Cheers,
-ilia
On Fri, Oct 5, 2018 at 12:01 PM wrote:
From: Boyuan Zhang
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.
Signed-off-by: Boyuan Zhang
---
src/gallium/state_trackers/va/image.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/gallium/state_trackers/va/image.c
b/src/gallium/state_trackers/va/image.c
index 3f892c9..c9f6f18 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -400,11 +400,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, , );
+ vl_video_buffer_adjust_size(, , 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage
This is an improvement, but I think you need to clip the box to
1. Size of the surface
2. Size of the image
I think that there are clipping helpers available to do that (maybe
pipe_box_clip or so? I forget, check the auxiliary dir). Christian -
does that make sense to you?
Cheers,
-ilia
On Fri, Oct 5, 2018 at 12:01 PM wrote:
>
> From: Boyuan Zhang
>
> 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.
>
> Signed-off-by: Boyuan Zhang
> ---
> src/gallium/state_trackers/va/image.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/image.c
> b/src/gallium/state_trackers/va/image.c
> index 3f892c9..c9f6f18 100644
> --- a/src/gallium/state_trackers/va/image.c
> +++ b/src/gallium/state_trackers/va/image.c
> @@ -400,11 +400,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, , );
> + vl_video_buffer_adjust_size(, , 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
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/va: use provided sizes and coords for getimage
From: Boyuan Zhang
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.
Signed-off-by: Boyuan Zhang
---
src/gallium/state_trackers/va/image.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/gallium/state_trackers/va/image.c
b/src/gallium/state_trackers/va/image.c
index 3f892c9..c9f6f18 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -400,11 +400,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, , );
+ vl_video_buffer_adjust_size(, , 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
