[Gstreamer-openmax] [PATCH 06/10] add g_omx_core_{get, set}_{config, param} helper functions

[Rob Clark]

On Mar 8, 2010, at 11:04 AM, Felipe Contreras wrote:

> On Mon, Mar 08, 2010 at 12:14:53AM +0100, Rob Clark wrote:
>> The helper functions consolidate error handling.
>> ---
> 
>> +void
>> +g_omx_core_get_param (GOmxCore *core, OMX_INDEXTYPE idx, OMX_PTR param)
> 
> Wouldn't it make sense to return the error?


I thought about that.. but at the moment there were no places that were handling errors from {Get,Set}{Parameter,Config}.

So I thought the least intrusive change was simply to do a GST_WARNING trace.

It would be a simple change later to also return the error if there was some places where we wanted to handle the error.


> 
>> +{
>> +    OMX_HANDLETYPE omx_handle = g_omx_core_get_handle (core);
>> +    OMX_ERRORTYPE err;
>> +
>> +    if (!omx_handle)
>> +        return;
> 
> I think this check should be done much sooner.


??? I guess I don't understand this point.  The check is the first thing after the g_omx_core_get_handle() call, so I don't see how it could be any sooner?

There is already an error trace in g_omx_core_get_handle(), so in this case we just need to make the function return before we call a null pointer.

The use case here is if you do a gst-inspect on an element which doesn't have a corresponding OMX component..  it is ok that the element does not work properly, but it isn't ok that gst-inspect segfaults.  This becomes relevant in the next patch where input/output-buffers properties are added.


BR,
-R


> 
>> +    err = OMX_GetParameter (omx_handle, idx, param);
>> +
>> +    if (OMX_ErrorNone != err)
> 
> The other way around: err != OMX_ErrorNone
> 
>> +        GST_WARNING_OBJECT (core->object, "OMX_GetParameter(%d) failed: %s",
>> +                idx, omx_error_to_str (err));
>> +}
> 
> -- 
> Felipe Contreras