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

[Gwenole Beauchesne]

On Thu, 26 Feb 2009, Michael Niedermayer wrote:

> i think the struct placed in data[0] should be designed so that it does not
> need any vaapi headers, that is contain pointers to structs instead of
> vaapi structs.
> then this "data[0] struct" can just be placed in avcodec.h and vaapi.h wont
> be needed.

It's hard because VADisplay, VAConfigID, VAContextID, VASurfaceID are not 
structs but either void * (VADisplay) or unsigned int (VA*ID). I think 
users of the API shall not assume anything, in case either type is 
changed. In practise, I don't see those changing anytime soon, though.

>> + * \brief This structure is used as a callback between the FFmpeg
>> + * library and the client video application.
>> + * This is created by the client application prior to calling the
>> + * FFmpeg decode functions. You must call
>> + * av_alloc_vaapi_render_state() to allocate this structure and fill
>> + * both the surface and va_context fields.
>> + */
>> +struct vaapi_render_state {
>> +    uint32_t    magic;                  ///< Magic number identifying the structure
>> +    uint32_t    state;                  ///< Holds FF_VAAPI_STATE_* values
>> +    struct vaapi_context *va_context;   ///< Pointer to display-dependent data
>> +    VASurfaceID surface;                ///< Rendering surface, never changed
>> +    // ... there are other (private) fields beyond this point
>> +};
>> +
>> +/** \brief Allocate a new VA API render state structure. */
>> +struct vaapi_render_state *av_alloc_vaapi_render_state(void);
>
> do i understand correctly that the use provided get_buffer() is called and
> then the user calls av_alloc_vaapi_render_state()?
> this is ugly
> either lavc should already call av_alloc_vaapi_render_state() before
> get_buffer()
> or it should be done in default_get_buffer() and the users get_buffer() should
> call default_get_buffer()
> or allocate it by avpicture_alloc()
> i dunno which of these would be cleanest ...

I agree that we shall not depend on a specific AVFrame::data[0] to stuff 
the HW accelerator data in. We should not delegate the call to 
default_get_buffer() to the user as he would certainly forget about it.

Why not use an hwaccel_data member for it? Though, in that case, data[0] 
shall not be NULL for correct operation of h264.

Did you mean alloc_picture() in mpegvideo.c? It doesn't seem 
avpicture_alloc() to be used anywhere but imgconvert.c.

>> +/** \brief Extract the vaapi_render_state struct from an AVPicture. */
>> +static inline struct vaapi_render_state *ff_get_vaapi_render_state(void *pic)
>
> a AVPicture isnt void *

Actuall, I meant an AVFrame. OK, I will use that and have users cast their 
Picture to it.