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

[Gwenole Beauchesne]

On Thu, 26 Feb 2009, Gwenole Beauchesne wrote:

>>>>> +/** \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.
>>
>> iam ok with that.
>
> The problem is then what to fill in data[0]. It cannot be NULL, it cannot
> be "constant" (for all allocs) either. If the HW accelerator cannot handle
> surface pixels readback, this shall be valid. If it does support that, it
> would have allocated data[0-2] accordingly and those allocations naturally
> yield different pointers, so no problem in that case.
>
> I see two solutions:
>
> - Make data[0] a linearly increasing ID just to make sure any allocated
> AVFrame::data[0] is different
>
> - Replace: h->default_ref_list[0][i].data[0] ==
> h->default_ref_list[1][i].data[0] with some ff_compare_picture() which
> handles hwaccel_data. However, this could be a frequent function call. So,
> make it static inline in mpegvideo_common.h?

Having lavc allocate *_render_state structs sounds good but in practise, 
that's not really the ideal solution for MPlayer. How a ::get_image() 
could retrieve that hwaccel_data member? MPI allocation is well hidden 
mpcodecs_get_image() and it completely doesn't mind about AVFrame.