[MPlayer-dev-eng] [PATCH] Add video acceleration infrastructure

[Uoti Urpala]

On Mon, 2009-09-28 at 17:57 +0200, Gwenole Beauchesne wrote:
> On Mon, 28 Sep 2009, Uoti Urpala wrote:
> > And it's not clear whether hardware decoding should be the default even 
> > if it is available. For codecs that require little CPU like MPEG2 
> > hardware decoding is likely to create extra problems but give little if 
> > any benefit.
> 
> Power saving? A video decoder is around 200 mW.

At least on a desktop machine the disadvantages are likely to be much
more significant. And I'm not fully convinced of the power saving
advantages on a laptop either.


> > Is there currently any case where using VAAPI decoding would be the
> > preferred choice in practice?
> 
> I would say the preferred choice is whatever is available and used by 
> FFmpeg.

I meant my question in more practical terms. Is there currently any case
where users would get some real and significant practical benefit from
using the functionality added by your patches as opposed to some
previously existing decoding method?

>  That could be VAAPI, VDPAU, something else. Actually FFmpeg does 
> the decision based on whatever the user tells it through config 
> attributes. e.g. you can have several HW decoders available on a system: 

You must lack understanding about the practice of video handling if you
think like this. The generic functionality exposed by this
infrastructure is way inferior to VDPAU. If your system has VDPAU
support you'd want to use it through vo_vdpau and certainly not through
the infrastructure added by these patches.


> >> +static uintptr_t                 *video_accel_attrs;
> >> +static unsigned int               video_accel_attrs_size;
> >> +static enum VideoAccelStatus      video_accel_status = VIDEO_ACCEL_STATUS_IDLE;
> >> +static VideoAccelerator          *video_accels;
> >> +static unsigned int               video_accels_count;
> >> +static VideoAccelerator          *video_accel;
> >
> > You should use a context struct instead of adding global/static data.
> 
> I believe that a context created in video_accel.c and stored in 
> global/static data in VOs wouldn't change things much, would it?

I have already added support for VOs without static data in git, and xv
(the one you change) does not have any static data any more. And even if
that hadn't already been implemented it would not be a good idea to add
more such code which would make future cleanups harder.