Re: [Gstreamer-openmax] [PATCH] add some utility macros to get/set core and port configs/params

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

On Mar 1, 2010, at 9:43 AM, Felipe Contreras wrote:

> On Sat, Feb 27, 2010 at 8:24 PM, Rob Clark <ro...@ti...> wrote:
>> btw, I'm thinking about this one again..
>> 
>> how about, in case of setting all params:
>> 
>>   OMX_AUDIO_PARAM_PCMMODETYPE param;
>> 
>>   G_OMX_INIT_PARAM (param);
> 
> I think it should be:
> 
> G_OMX_INIT_PARAM (OMX_AUDIO_PARAM_PCMMODETYPE, param);

is this to declare the 'param' variable, as in G_OMX_DECLARE_PARAM()?  Otherwise, why does the type need to be passed?


> 
>>   ...  set params ...
>> 
>>   G_OMX_PORT_SET_PARAM (port, OMX_IndexParamAudioPcm, &param);
> 
> I don't see a big difference to:
> 
> param.nPortIndex = 1;
> OMX_SetParameter(omx_base->gomx->omx_handle, OMX_IndexParamAudioPcm, &param);


there are two things I prefer about having a utility function/macro above the OMX_SetParameter() call:

   G_OMX_PORT_SET_PARAM (self->out_port, OMX_IndexParamAudioPcm, &param);

1) this makes it clearer *which* port it is related to, than 'param.nPortIndex = 1'..  which becomes more important with components that have many ports
2) it can deal w/ case where omx_handle isn't constructed yet.. which is important if you want to have properties on gst element that pass through to OMX config or params.


BR,
-R

> 
>> or in case where you just want to set a few params:
>> 
>>   OMX_AUDIO_PARAM_PCMMODETYPE param;
> 
> You forgot the init I guess.
> 
>>   G_OMX_PORT_GET_PARAM (port, OMX_IndexParamAudioPcm, &param);
>> 
>>   ...  set params ...
>> 
>>   G_OMX_PORT_SET_PARAM (port, OMX_IndexParamAudioPcm, &param);
> 
> Again, I don't see a big difference to:
> 
> param.nPortIndex = 1;
> OMX_GetParameter(omx_base->gomx->omx_handle, OMX_IndexParamAudioPcm, &param);
> 
> ... set params ...
> 
> OMX_SetParameter(omx_base->gomx->omx_handle, OMX_IndexParamAudioPcm, &param);
> 
> Cheers.
> 
> -- 
> Felipe Contreras