[FFmpeg-devel] [PATCH][VAAPI][1/6] Add VA API formats

[Gwenole Beauchesne]

On Thu, 26 Feb 2009, Michael Niedermayer wrote:

> On Thu, Feb 26, 2009 at 12:46:27PM +0100, Gwenole Beauchesne wrote:
>> On Thu, 26 Feb 2009, Michael Niedermayer wrote:
>>
>>>> @@ -76,6 +76,7 @@ const uint8_t ff_mpeg1_dc_scale_table[128]={
>>>>  };
>>>>
>>>>  const enum PixelFormat ff_pixfmt_list_420[] = {
>>>> +    PIX_FMT_VAAPI_VLD,
>>>>      PIX_FMT_YUV420P,
>>>>      PIX_FMT_NONE
>>>>  };
>>>
>>> considering that this list is used by some codecs that VAAPI does not support
>>> this isnt correct
>>
>> Sure some codecs are not supported, but I thought this was your intent
>> because ::get_format() has to check for it anyway. i.e. the API can
>> specify support for a codec but the user has to check the HW actually
>> supports it. So, even if we add the HW formats on per-codec, there are
>> cases were it wouldn't be supported for real. e.g. VDPAU VC-1 in the past.
>
> The problem is that a version of lavc after this patch claims to support
> PIX_FMT_VAAPI_VLD for real video msmpeg4, mpeg4, h263 ...
> but it does not support any of it (yet)
> if this version of lavc is linked against a mplayer of lets say 3 month
> later that already does have VAAPI-mpeg4 support then its get_format() will
> select PIX_FMT_VAAPI_VLD and things would fall apart ...

OK, what about an ff_hwaccel_pixfmt_list_420[] and then convert all codecs 
to that?

Though, for h263dec.c for example, we would end up in such situation:
avctx->pix_fmt= avctx->get_format(avctx, avctx->codec->pix_fmts);
switch (avctx->codec->id){
case CODEC_ID_MPEG4:
     avctx->pix_fmt= avctx->get_format(avctx, ff_hwaccel_pixfmt_list_420);
     //...
     break;
case CODEC_ID_SOMETHING_ELSE:
     //...
     break;
case CODEC_ID_H263:
     avctx->pix_fmt= avctx->get_format(avctx, ff_hwaccel_pixfmt_list_420);
     //...
     break;
}

Is this acceptable (in style, i.e. several ::get_format() calls).