[vlc-devel] [PATCH] vaapi: fix profiles detection.

[Jean-Paul Saman]

This patch crashes in my tests with an NVIDIA card. See review comments below:

On Tue, Dec 6, 2011 at 2:13 PM, Francois Cartegnie <fcvlcdev at free.fr> wrote:
> The selected profile wasn't checked against card's ones.
> (ex: resulting in sending MPEG2 to a VC1/h264 only card)
> ---
>  modules/codec/avcodec/vaapi.c |   22 +++++++++++++++++++++-
>  1 files changed, 21 insertions(+), 1 deletions(-)
>
> diff --git a/modules/codec/avcodec/vaapi.c b/modules/codec/avcodec/vaapi.c
> index 710fddd..2705025 100644
> --- a/modules/codec/avcodec/vaapi.c
> +++ b/modules/codec/avcodec/vaapi.c
> @@ -95,8 +95,10 @@ static vlc_va_vaapi_t *vlc_va_vaapi_Get( void *p_va )
>  /* */
>  static int Open( vlc_va_vaapi_t *p_va, int i_codec_id )
>  {
> -    VAProfile i_profile;
> +    VAProfile i_profile, *p_profiles_list;
>     int i_surface_count;
> +    bool b_supported_profile = false;
> +    int i_profiles_nb = 0;
>
>     /* */
>     switch( i_codec_id )
> @@ -144,6 +146,24 @@ static int Open( vlc_va_vaapi_t *p_va, int i_codec_id )
>     if( vaInitialize( p_va->p_display, &p_va->i_version_major, &p_va->i_version_minor ) )
>         goto error;
>
> +    /* Check if the selected profile is supported */
> +    i_profiles_nb = vaMaxNumEntrypoints( p_va->p_display );

==> cause of crash, it should be vaMaxNumProfiles( p_va->p_display );

> +    p_profiles_list = malloc( i_profiles_nb * sizeof( VAProfile ) );

Use calloc() here: VAProfile *p_profiles_list = calloc( i_profiles_nb,
sizeof( VAProfile ) );

> +    if ( p_profiles_list &&
> +         vaQueryConfigProfiles( p_va->p_display, p_profiles_list, &i_profiles_nb ) == VA_STATUS_SUCCESS )

I would split this up, like this:

if (!p_profiles_list)
   goto error;

VAStatus status = vaQueryConfigProfiles( p_va->p_display,
p_profiles_list, &i_profiles_nb );
if ( status == VA_STATUS_SUCCESS )
{
    ...
}

> +    {
> +        while( --i_profiles_nb >= 0 )

The coding style in this file uses for-loops in this kind of
constructs. Therefor my suggestion is that you do the same.

> +        {
> +            if ( p_profiles_list[i_profiles_nb] == i_profile )
> +            {
> +                b_supported_profile = true;
> +                break;
> +            }
> +        }
> +    }
> +    free( p_profiles_list );
> +    if ( !b_supported_profile ) goto error;
> +
>     /* Create a VA configuration */
>     VAConfigAttrib attrib;
>     memset( &attrib, 0, sizeof(attrib) );
> --
> 1.7.3.4
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel

Kind regards,

Jean-Paul Saman