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

[Gwenole Beauchesne]

On Fri, 27 Feb 2009, Michael Niedermayer wrote:

>>> 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.
>
> they are linked in an AVFrame/AVPicture, is there a problem with passing
> AVFrame/AVPicture around instead of both structs?

Twice more initializations since those are void * (to be generic among all 
HW accelerators).

And I have a problem to call something like ff_vaapi_slice_data_map() with 
a Picture *pic arg whereas they are not directly linked. i.e. I'd prefer 
have some vaapi_* as the first arg to something called vaapi_*foo.

IOW, the function describes the accelerator but the arg would describe 
something that it does not directly need. Its used would need to be 
explicited (indirection level 1) because the accelerator data is "hidden" 
in that something (Picture). Yes, I happen to be a naming satanist 
sometimes. ;-)

For instance, imagine you want to implement linked lists with a
list_append(struct list *l, void *data)
but finally, you want to make it work as:
list_append(struct hey_the_list_is_hidden_in_there *p, void *data)
doesn't it look strange to you?

Well, I could also make a copy of the public struct va_context to the 
private part. e.g. make ff_vaapi_render_state_init() become
ff_vaapi_render_state_init(struct vaapi_render_state_private *rdsp, Picture *pic)
{
   struct vaapi_render_state *rds = pic->hwaccel_data;
   // copy needed members from va_context
   rdsp->display      = rds->va_context->display;
   rdsp->context_id   = rds->va_context->context_id;
   // ... rest of initialization
}

so that we don't need to lookup the va_context struct through the public 
vaapi_render_state struct. And I could avoid the public vaapi_render_state 
struct lookup for the surface id by replacing my ff_get_vaapi_surface() 
function arg with a Picture. That could suit me.