[FFmpeg-devel] [PATCH] HWAccel infrastructure (take 6)

[Gwenole Beauchesne]

Le 20 f?vr. 09 ? 22:04, Michael Niedermayer a ?crit :

>> @@ -51,9 +52,14 @@ av_cold int ff_h263_decode_init(AVCodecContext  
>> *avctx)
>>     s->quant_precision=5;
>>     s->decode_mb= ff_h263_decode_mb;
>>     s->low_delay= 1;
>> -    avctx->pix_fmt= PIX_FMT_YUV420P;
>>     s->unrestricted_mv= 1;
>>
>> +    avctx->hwaccel = ff_query_hwaccel_codec(avctx, avctx->codec- 
>> >id);
>> +    if (avctx->hwaccel)
>> +        avctx->pix_fmt = avctx->hwaccel->pix_fmt;
>> +    else
>> +        avctx->pix_fmt = PIX_FMT_YUV420P;
>> +
>>     /* select sub codec */
>>     switch(avctx->codec->id) {
>>     case CODEC_ID_H263:
>
> avctx->pix_fmt = ff_query_pixfmt(avctx, avctx->codec->id);

purpose? ff_query_hwaccel_pixfmt(), i.e. for hwaccel formats only?

if your intent is a generic function for all codecs, that sounds  
overkill right now, IMO. What would be the default value for AVCodecs  
not implementing pix_fmts[]? We are not guaranteed that NULL ->  
YUV420P and some exceptions would be needed or fixing all codecs to  
have a sensible enough initial pix_fmts[].

> avctx->hwaccel = ff_pixfmt_to_hwaccal(avctx->codec->id, avctx- 
> >pix_fmt);

since this is a pixfmt_to_XXX function, (i) why is pix_fmt not the  
first arg, (ii) what is codec id needed for? And if a sw pixfmt is  
allowed, how would you figure out the hwaccel if there are several  
accelerators implementing the same pixfmt for the same codec?

> if(s->divx_packed || s->avctx->hwaccel)
>
>>         //we would have to scan through the whole buf to handle the  
>> weird reordering ...
>
> and update the comment if you want

I don't like commenting on two different things in the same comment,  
and if I create another comment, it turns out that keeping two checks  
would be clearer. i.e. each one with its own comment.