[FFmpeg-devel] [PATCH][VAAPI][2/6] Add common data structures and helpers

[Gwenole Beauchesne]

On Fri, 27 Feb 2009, Michael Niedermayer wrote:

>> 1. We want *_render_state structs be stored in a specific AVFrame member,
>> not hijack some data[i] (mandatory)
>
> this is a "if its cleaner" thing, that is if its cleaner otherwise there
> is no point

IMO, the simplest approach (requiring less code) is indeed that one: let 
the user allocate *_render_state struct and pass it back to lavc through 
.data[3] (or .data[0] if YV12 pixels are not needed) + if *_render_state 
struct needs private data, let it be an extension to the public struct.

Really, this is the simplest approach. I tried others (amend AVFrame with 
hwaccel data, split public/private, etc.) and it turned out the extra code 
(both in lavc internal and in player) is no gain.

> [...]
>> 7. vaapi_render_state struct needs private data user doesn't care about
>> and doesn't have to access.
>> 7.1 this data can't be allocated in start_frame() because of 5 and 6.
>> 7.2 lazy allocation of the struct is desired to avoid many alloc/free
>> 7.3 if private data is disassociated from original public data, we need a
>> location to bind this private data to the public struct priv member.
>
> AVFrame/AVPicture.hwaccel_codec_private

Why not but in case we push the public struct to AVFrame too, wouldn't 
that become too crownded?
e.g.
.hwaccel_data for the public struct
.hwaccel_privdata for the private struct, if any

Besides, if we separate both, this becomes somewhat unconvienent from an 
HW accel codec implementation point of view. I mean, dealing with only one 
struct at a time is very convenient (one function arg, single init from a 
Picture, etc.). Splitting them would require to manage both, or link one 
to the other, somewhere.

> also
> 8. Avoid making our public interface depend on VA/VDPAU
>   that is in order of preferance
>   * use ISO C types & structs or types and structs defined by avcodec.h
>   * use void * or pointers to structs and make sure alloc/free/access is
>     limited to either codec OR player
>   * use void * or pointers to structs and clearly document the API/ABI
>     implications if they change

Understood, already changed locally. :)