[FFmpeg-devel] [PATCH] HWAccel infrastructure

[Ivan Kalvachev]

On 2/17/09, Gwenole Beauchesne <gbeauchesne at splitted-desktop.com> wrote:
> Hi,
>
> This patch implements a hardware acceleration infrastructure for FFmpeg.
> It works for both VA API and VDPAU. As an extra benefit, this also fixes a
> VDPAU HW decoding bug with some MPEG-2 bitstreams I have here.
>
> The user is expected to override AVCodecContext::get_format() to select
> the proper hardware accelerator. The pix_fmts[] arg is a list constructed
> from registered AVHWAccelCodec (allcodecs.c: REGISTER_HWACCEL_CODEC()).
>
> As a side note, and in order to illustrate this, an mplayer -va arg
> controls what accelerator to use actually. Then get_format() checks that
> user (human) requested accelerator and only selects the matching IMGFMT.
>
> AVHWAccelCodec::pix_fmt is no longer an array so that to implement HW
> acceleration case 3 (as "specified" by Reimar earlier). That is,
> avctx->hwaccel_codec->pix_fmt will stick to the HW accelerated pixel
> format whereas avctx->pix_fmt could be YV12 if the user wants to readback
> pixels in that format. Otherwise, both are equal by default.

As my point of interest is how to rework xvmc using unified method,
there are few things I've thought about that may be needed to improve
this patch.

*hwaccel_codec is currently placed into AVCodecContext,
It is public structure and I think it should be better to be
moved in mpegencctx.

I think the AVHWAccelCodec should contain all functions that each
acceleration methods defines.
This way all capabilities and supported format checks would be turn into
single if(s->hwaccel_codec) check. Now caps etc...

There is *next field in AVHWAccelCodec. That's very bad idea:
-adding new functions would be after the next.
-I don't like linked lists. It would be much simpler to have an array
of pointers to
AVHWAccelCodec. This way both the structure and the array could be const.
I think it would make sense to do the same with AVCodec.

However the above idea(s) have one problem.
Let's say that I add functions like block_init() and pack_pblock()
that are used only by XvMC. All other methods (at least the ones that
do mpeg2) should implement
dummy functions for them, only to do nothing.

Otherwise, instead of call them like this:

if(s->hwaccel_codec)
    s->hwaccel_codec->func1(s,blah);

--- they should be called like this ---

if(s->hwaccel_codec && s->hwaccel_codec->func1)
    s->hwaccel_codec->func1(s,blah);

Actually it isn't that bad.

Don't rush in implementing it. I'd wait for other people comments first.