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

[Michael Niedermayer]

On Fri, Feb 20, 2009 at 11:41:13PM +0100, Gwenole Beauchesne wrote:
> 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?

purpose being not to merge unrelated code just because neither part
is used individually currently
and not calling a function "query_hwaccel_codec" that really querries
the pix_fmt and returns the matching hwaccel.


> 
> 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[]?

it wont be called for such codecs


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

no reason at all, i dont mind the order being changed

 
> (ii) what is codec id needed for?

2 codecs might use the same hw pixfmt but different hwaccels, maybe
this is unlikely and i dont mind codec_id being droped considering this
is just a internal function


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

same way by which you figure it out (or not) from the pix_fmt
returned from get_format().


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

I dont like having duplicated code because its easier to write comments
for it.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090221/e965e078/attachment.pgp>