Re: [Gstreamer-openmax] [PATCH 1/8] core: call OMX_GetHandle in g_omx_core_new

[Clark, R. <ro...@ti...>]

On Mar 23, 2010, at 5:25 PM, Felipe Contreras wrote:

> On Thu, Mar 18, 2010 at 1:58 AM, Rob Clark <ro...@ti...> wrote:
>> This way omx_handle is always valid, and can be used in get/set_property
>> methods.
>> ---
> 
>> @@ -282,12 +285,22 @@ g_omx_core_new (void *object)
>> 
>>     core->omx_state = OMX_StateInvalid;
>> 
>> +    /* it is a bit awkward to have to pass the element's g_class to
>> +     * g_omx_core_new(), but since this is called from type_inistance_init
>> +     * function, we can't rely on G_OBJECT_TYPE()..
>> +     */
>> +    gstomx_get_component_info (core, G_TYPE_FROM_CLASS (g_class));
> 
> I don't like this. It would be better to pass the GType directly.
> 
> But also, it's messing up g_omx and gstomx_ functions... Can you move
> g_omx_core_new to gstomx.c and make it gstomx_core_new() or something
> like that?


I could do that for now. 

But actually I would like to change the g_omx_ stuff to gstomx_..  I think there are some parts of logic, such as handling the share_buffer vs !share_buffer cases, which could be consolidated in GOmxPort (or GstOmxPort).  Similar for handling of EOS.

And for upstream caps negotiation to work properly, we need to do a pad_alloc() before SendCommand(SetState, Idle) or SendCommand(EnablePort)..  which could be more easily done in one place in GOmxPort.

This is probably the subject for another patch(set)..  but I thought maybe now would be a good time to bring up the idea and see if anyone had any issue with it.


BR,
-R


> 
> Cheers.
> 
> -- 
> Felipe Contreras