[Libva] profile used in mplayer vaapi

[Xiang, Haihao]

> -----Original Message-----
> From: gbeauchesne at splitted-desktop.com
> [mailto:gbeauchesne at splitted-desktop.com]
> Sent: Thursday, January 27, 2011 2:33 PM
> To: Xiang, Haihao
> Cc: Yuan, Shengquan; libva at lists.freedesktop.org
> Subject: RE: [Libva] profile used in mplayer vaapi
> 
> 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.


Done, 

Thanks
Haihao



> 
> 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.
> >
> >
>