[Libva] [PATCH] MADI: Reject reference surfaces in the wrong format

[Qu, Pengfei]

Hi,Gwenole,

Thanks for your code review and comments.

>>This looks OK as a last resort solution, but it should be completed with an earlier check in vaQueryVideoProcPipelineCaps(). This is the function used to validate the filter chain somehow.
It sounds good idea. vaQueryVideoProcPipelineCaps() just offer the interface to get the supported image format capability(such as NV12, etc) by application and then the application (such as gstreamer) will be responsible for the final image format negociation/validation. Is it correct? This will change the structure VAProcPipelineParameterBuffer to support this.

>>Besides, I believe we should probably return VA_STATUS_ERROR_INVALID_IMAGE_FORMAT or any other VA_STATUS_ERROR_INVALID_SURFACE_FORMAT that yields the same error code?
Yes, you are right.  It should be changed as your comments.
Thanks,
Pengfei

-----Original Message-----
From: Gwenole Beauchesne [mailto:gb.devel at gmail.com] 
Sent: Wednesday, August 27, 2014 12:00 PM
To: Qu, Pengfei
Cc: libva at lists.freedesktop.org
Subject: Re: [Libva] [PATCH] MADI: Reject reference surfaces in the wrong format

Hi,

2014-08-14 13:04 GMT+02:00 Qu, Pengfei <pengfei.qu at intel.com>:
>
> From b62ef279566e8d2548b519f5b62abceede1042c7 Mon Sep 17 00:00:00 2001
> From: Simon Farnsworth <simon.farnsworth at onelan.co.uk>
> Date: Thu, 29 May 2014 10:21:03 +0100
> Subject: [PATCH] MADI: Reject reference surfaces in the wrong format
>
> SNB and IVB DNDI only work with NV12 surfaces. Reject anything else.
>
> Signed-off-by: Simon Farnsworth <simon.farnsworth at onelan.co.uk>
> ---
>  src/i965_post_processing.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/i965_post_processing.c b/src/i965_post_processing.c 
> index 5f5a508..44ba3e0 100755
> --- a/src/i965_post_processing.c
> +++ b/src/i965_post_processing.c
> @@ -3106,6 +3106,11 @@ pp_nv12_dndi_initialize(VADriverContextP ctx, struct i965_post_processing_contex
>          return VA_STATUS_ERROR_UNIMPLEMENTED;
>      }
>
> +    if (previous_in_obj_surface && previous_in_obj_surface->fourcc != VA_FOURCC_NV12) {
> +        WARN_ONCE("Forward temporal reference must be NV12\n");
> +        return VA_STATUS_ERROR_INVALID_PARAMETER;
> +    }
> +
>      /* source (temporal reference) YUV surface index 5 */
>      orig_w = previous_in_obj_surface->orig_width;
>      orig_h = previous_in_obj_surface->orig_height;
> @@ -3605,6 +3610,11 @@ gen7_pp_nv12_dndi_initialize(VADriverContextP ctx, struct i965_post_processing_c
>          return VA_STATUS_ERROR_UNIMPLEMENTED;
>      }
>
> +    if (previous_in_obj_surface && previous_in_obj_surface->fourcc != VA_FOURCC_NV12) {
> +        WARN_ONCE("Forward temporal reference must be NV12\n");
> +        return VA_STATUS_ERROR_INVALID_PARAMETER;
> +    }
> +
>      /* source (temporal reference) YUV surface index 4 */
>      orig_w = previous_in_obj_surface->orig_width;
>      orig_h = previous_in_obj_surface->orig_height;
> --
> 1.9.0
> _______________________________________________
> Libva mailing list
> Libva at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libva

This looks OK as a last resort solution, but it should be completed with an earlier check in vaQueryVideoProcPipelineCaps(). This is the function used to validate the filter chain somehow.

Besides, I believe we should probably return VA_STATUS_ERROR_INVALID_IMAGE_FORMAT or any other VA_STATUS_ERROR_INVALID_SURFACE_FORMAT that yields the same error code?

Thanks,
--
Gwenole Beauchesne
Intel Corporation SAS / 2 rue de Paris, 92196 Meudon Cedex, France Registration Number (RCS): Nanterre B 302 456 199