[FFmpeg-devel] [PATCH] AVHWAccel infrastructure v2

[Gwenole Beauchesne]

Le 28 sept. 09 ? 17:56, Ivan Kalvachev a ?crit :

> I see very few of the benefits you declare actually into the code.
> IMHO, it only makes libavcodec bigger for no real gain at all,
> adding code that is supposed to reside in the player.

That's the problem. The player shall not have to handle decode  
context... Having the player handle decode context would be as if you  
were to have the player finds itself the right codec to use in SW mode  
or even have it fill in MpegEncContext struct itself... That's not the  
player's job IMHO.

> 1. You do remove get/release_buffer calls by creating new entries
> alloc/free_surface in the AVHWAccel, then special case handling in
> alloc/free_frame_buffer and adding big ugly hack into
> avcodec_default_get_buffer() to allocate fake frames, so codecs won't
> notice the difference.

No, the hack in avcodec_default_get_buffer() is to avoid the  
allocation of CPU side memory when we don't need the pixels. This can  
work without that hunk, it's just that you will be allocating (large)  
memory that is not used.

> This is the part I don't get, are these alloc/free from inside the
> vaapi codec? I don't see them in the patch.

Since this is an HWAccel hook, it's provided by the HWAccel codec  
implementation, not the application.

> The other option is that they must be filled by the calling
> application.

No, the application only has to fill in the hwaccel_attrs array.

> 2. HWACCEL_CAP_GET_PIXELS is described as the option that would  
> return the
> pixels, but is actually used only on find_hwaccel().
> In theory if lavcodecs is supposed to fetch rendered frame then it  
> should
> also incorporate direct calling of rendering routines.

This is the case. rendering == decoding. And this is different from  
actual rendering/display to screen.

> 3. The hwaccel_attribute are added. If I understand it correctly they
> are filled by the calling application to control the codec selection/ 
> work.
> This makes them abstract api into specific api of the general api
> (attributes, hwaccel,avctx).  (v3 moves them out of hwaccel into
> avctx).

? The files moved around only. It was just files order from svn diff  
and git diff were not the same so you thought contents were swapped  
when you probably diff'ed the patch.

> IMHO too much abstraction. Also user would have only one of the api's
> functional,

No, he can have a video decoder included in the GPU, another video  
decoder in another chip (available through some mini PCI-Express  
card), and there could even have video decoding done through GPU  
shaders. e.g. some hypothetical platform: VPU (H.264, VC-1), another  
decoder as mini PCI-Express (MPEG-2, VP6, whatever), and an  
implementation using the GPU/shaders (theora, dirac, whatever).

> have api function that allows it to select that. As for the  
> acceleration levels,
> they are supposed to be present is specific order so the fastest are
> selected first,

Hence the scoring system (VLD > MoComp for example).

> BTW what is Display doing there at all? (removed in v3).

Display == Handle, Device, Context, something that is HW accelerator  
specific and may need to be initialized from within a windowing  
system. Though the actual work is display-independent.