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

[Michael Niedermayer]

On Tue, Feb 24, 2009 at 06:26:51AM +0100, Gwenole Beauchesne wrote:
> Le 24 f?vr. 09 ? 02:25, Michael Niedermayer a ?crit :
> 
> >>>> Please also point me to the patch that replaces the VDPAU  
> >>>> PIX_FMTs, I
> >>>
> >>> what should that patch replace them with?
> >>
> >> I don't know, you said "you can't register new pixel formats" and I
> >> interpreted that as "you can't add new pixel formats", i.e. no HW
> >> formats in pixfmt.h. Hence the confusion.
> >
> > i meant
> > you cannot create new pixel formats _at runtime_
> > PIX_FMT_NB prevents it ...
> 
> I agree with you but please read the patch again, there is no new  
> pixel formats created at run-time.

no, i never said there is, it just was an argument to proof that the
lists could be static const instead of generated at runtime ...


> 
> >>>> have just svn update'd the tree and I probably missed it or someone
> >>>> forgot to commit that bit.
> >>>>
> >>>> How is all this supposed to work now, specifically?
> >>>
> >>> This is very simple and really isnt "now" ...
> >>> each AVCodec has a constant list of pix_fmts, these get passed to
> >>> get_format()
> >>> the user app through its get_format() chooses one of them and
> >>> returns it
> >>> the codec looks up the matching AVHWAccel.
> >>
> >> Your precondition is not met: the list of pix_fmts is not constant  
> >> for
> >> some AVCodecs. see e.g. mpeg12.c, the pix_fmt depends on
> >> chroma_format.
> >
> > i need more sleep, i have forgotten about that, still my point stands
> > that your page of code is not needed
> > 3 static const lists for mpeg12 will do
> 
> "page of code", well, you will see that the patch is as long as  
> before. ;-) You save a few bytes though.
> 
> However, I would like to point out that the current approach has the  
> benefinit for defining the availability of the new accelerator only  
> once. e.g. a single  REGISTER_HWACCEL (MPEG2_VAAPI, mpeg2_vaapi); in  
> allcodecs.c and you are done. Now, you will have to mention that  
> elsewhere (in a pixfmt_fmt_mpeg2_420[]). Somewhat a one-definition- 
> rule man.
> 
> OK, now I take this coding rule: "no matter how the code is obfuscated  
> (for the dev), as long as it's compact and faster by a very marginal  
> number of cycles (for the user), it  should be OK". I will keep this  
> in mind next time.

I honestly dont see how your original code would be any cleaner than a
static const table with an #ifdef per HWaccel

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

It is not what we do, but why we do it that matters.
-------------- 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/20090224/0834e251/attachment.pgp>