[Libva] profile used in mplayer vaapi

[gbeauchesne at splitted-desktop.com]

Hi,

> The following patch is used for mplayer vaapi. I think keeping consistence
> with FFmpeg is reasonable, so profile has 0(simple), 1(main), 3(advance).
>
> I will push the patches for va and i965 video driver.

OK, this sounds reasonnable but please increase VA-API version to 0.32
(libva_minor_version to 32). Besides, recent changes to the vtable also
caused an ABI change but the API was not increased, thus breaking current
drivers without rebuilding them.

BTW, while we are increasing the API version, I am wondering whether we
shouldn't make vtable a pointer too similarly to vtable_glx & vtable_tpi
to avoid this kind of problems again. Plus making sure any new entry in
the vtable is at the end.

Thanks,
Gwenole.

> diff --git a/ffmpeg/libavcodec/vaapi_vc1.c b/ffmpeg/libavcodec/vaapi_vc1.c
> index 3bbbd24..a676553 100644
> --- a/ffmpeg/libavcodec/vaapi_vc1.c
> +++ b/ffmpeg/libavcodec/vaapi_vc1.c
> @@ -160,6 +160,7 @@ static int vaapi_vc1_start_frame(AVCodecContext
> *avctx, av_unused const uint8_t
>      pic_param->BFM(sequence_fields,bits,syncmarker)
> = s->resync_marker;
>      NEW(pic_param->BFM(sequence_fields,bits,rangered)
> = v->rangered);
>      NEW(pic_param->BFM(sequence_fields,bits,max_b_frames)
> = s->avctx->max_b_frames);
> +    NEW(pic_param->BFM(sequence_fields,bits,profile)
> = v->profile);
>      pic_param->coded_width
> = s->avctx->coded_width;
>      pic_param->coded_height
> = s->avctx->coded_height;
>      NEW(pic_param->BFV(entrypoint_fields,value)
> = 0); /* reset all bits */
>
>
> Thanks
> Haihao
>
>> -----Original Message-----
>> From: gbeauchesne at splitted-desktop.com
>> [mailto:gbeauchesne at splitted-desktop.com]
>> Sent: Wednesday, November 24, 2010 3:35 PM
>> To: Xiang, Haihao
>> Cc: Yuan, Shengquan; Yuan, Shengquan; libva at lists.freedesktop.org
>> Subject: Re: [Libva] profile used in mplayer vaapi
>>
>> Hi,
>>
>> > On Tue, 2010-11-23 at 14:25 +0800, Yuan, Shengquan wrote:
>> >> On Tue, Nov 23, 2010 at 2:21 PM, Xiang, Haihao
>> <haihao.xiang at intel.com>
>> >> wrote:
>> >> > Hi,  Austin
>> >> >
>> >> >   VC1 has a profile flag in the sequence layer, however
>> >> > VAPictureParameterBufferVC1 hasn't a member for this flag. The i965
>> >> > video driver needs this flag, does the parameter 'profile' passed
>> in
>> >> > vaCreateConfig match the profile specified in the sequence layer?
>> >> I don't think so, some players may always choose the highest profile
>> >> for vaCreateConfig regardless of the indication in sequence layer.
>> >> We may add something into VAPictureParameterBufferVC1 to resolve i965
>> >> issue
>> >
>> > How about the following patch?
>>
>> Please place the new "profile" field at the end of the bit-fields, i.e.
>> after "max_b_frames" so that to maintain binary compatibility. Thanks.
>>
>> Besides, I suggest this field to have the following values:
>> 1: simple
>> 2: main
>> 3: advanced
>>
>> 3 is the default for VC-1 Advanced. Values for metada info were probably
>> more like 0, 4, 12, isn't it? Thus, MSB being 0, 1, 3. Besides, keeping
>> 0
>> as no meaningful value will help implementations to maintain
>> compatibility. i.e. if profile != 0 => profile information provided,
>> thus
>> being usable.
>>
>> > diff --git a/va/va.h b/va/va.h
>> > index 7f24a85..b1e4136 100644
>> > --- a/va/va.h
>> > +++ b/va/va.h
>> > @@ -857,6 +857,7 @@ typedef struct _VAPictureParameterBufferVC1
>> >      /* sequence layer for AP or meta data for SP and MP */
>> >      union {
>> >          struct {
>> > +            unsigned int profile	: 2; /* SEQUENCE_LAYER::PROFILE or
>> The
>> > MSB of METADATA::PROFILE */
>> >              unsigned int pulldown	: 1; /*
>> SEQUENCE_LAYER::PULLDOWN */
>> >              unsigned int interlace	: 1; /*
>> SEQUENCE_LAYER::INTERLACE */
>> >              unsigned int tfcntrflag	: 1; /*
>> SEQUENCE_LAYER::TFCNTRFLAG */
>>
>> Regards,
>> Gwenole.
>
>