Repository navigation
Conversation
…back This patch introduces 3 main changes to the selector comp: 1. Use process ipc struct for selector 2. Validate the selector params in the params callback. The source and sink channels can be obtained from the source/sink comp params 3. Validate source/sink period bytes before resizing buffer in prepare(). Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
| * FIXME: where should sel_channel come from? | ||
| * from a kcontrol or ipc params | ||
| */ | ||
| ret = sel_set_channel_values(cd, sourceb->source->params.channels, |
There was a problem hiding this comment.
Source/sink params (depends on pipe direction) won't be set at this point, so your code won't work correctly. Selector component should overwrite next components' params, so it needs those in/out channels count parameters.
There was a problem hiding this comment.
@tlauda yes you are right about this. I think we should set this in prepare() instead of params. Then the source/sink params would already be set.
There was a problem hiding this comment.
@ranj063 Getting in/out channels based on the connected components is not a great idea. The purpose of the selector component is to force the change. Let's assume that we have playback pipeline and in the middle of the pipeline we have the component, which only requires 1 channel. We would put selector before that component to get that 1 channel, but how the selector would know that it should change the out channels? Params from the host would set the host format, so basically all the components would have e.g. 2 channels.
In the long term we should think about changing component API to have something like in and out format for every comp. With such API your change would be valid and expected.
There was a problem hiding this comment.
@tlauda params from the host would change the channels but if a component knows it only needs 1 channel, it will ignore the channel from the host params and set it to 1 anyway.
There was a problem hiding this comment.
@ranj063 Such component can support multiple formats and 1 channel on input can be just for specific topology. With your approach we transfer responsibility to that component instead of selector, which would require for that component to have additional custom field anyway.
There was a problem hiding this comment.
@tlauda what custom field would that be? Take the detect component for example, it knows that it only needs 1 channel. and the selector can use this information from the detect comp.
There was a problem hiding this comment.
@ranj063 I'm thinking more about generic case, where you have components, which can work for every channel configuration, but for specific topology they require specific channel number. One of the components in the topology needs to decide, that the channels number must be reduced.
There was a problem hiding this comment.
There was a problem hiding this comment.
@ranj063 I'll second @tlauda statement that "In the long term we should think about changing component API to have something like in and out format for every comp" for every component in and out. A classic example is a preprocessing component with AEC that accepts on the 1st input 1-4 channels from mics and 2nd one 1-2 channels of reference signal from playback path. I assume that component can be compiled for platform specific configuration.
| * FIXME: where should sel_channel come from? | ||
| * from a kcontrol or ipc params | ||
| */ | ||
| ret = sel_set_channel_values(cd, sourceb->source->params.channels, |
There was a problem hiding this comment.
| * FIXME: where should sel_channel come from? | ||
| * from a kcontrol or ipc params | ||
| */ | ||
| ret = sel_set_channel_values(cd, sourceb->source->params.channels, |
There was a problem hiding this comment.
@ranj063 I'll second @tlauda statement that "In the long term we should think about changing component API to have something like in and out format for every comp" for every component in and out. A classic example is a preprocessing component with AEC that accepts on the 1st input 1-4 channels from mics and 2nd one 1-2 channels of reference signal from playback path. I assume that component can be compiled for platform specific configuration.
|
@ranj063 Should we close this based on our discussions or maybe you'll just leave the comp_process changes and remove the rest? |
3 participants
…back
This patch introduces 3 main changes to the selector comp:
and sink channels can be obtained from the source/sink comp params
prepare().
Does this come from the ipc struct or from a enum kcontrol?
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com