Repository navigation
Fix handling of the mixer-module adapter combo on IPC3 platforms - #7357
Conversation
There was a problem hiding this comment.
Perhaps I should use active_input_buffers here and below?
There was a problem hiding this comment.
You need j anyway to have the exact active buffers, so you can continue using j here and below
There was a problem hiding this comment.
Hmm, wouldn't j==active_input_buffers ? If j can be differnent, then I think it deserves a longer more descriptive name as it's used across this long function (i.e. not just a loop index variable anymore).
There was a problem hiding this comment.
They can differ if there's a race changing which buffers are active between the first check and this loop. I can write an assert stating that they don't differ I suppose.
bb50523 to
8bf8be2
Compare
The old code does not deal correctly with input streams going away without everything stopping. This commit ensures only active streams are considered as mixer input, and the other streams are ignored (considered zeroed out). Signed-off-by: Paul Olaru <paul.olaru@nxp.com>
Some components like mixer have special handling of the prepare function whenever they are already active. Ensure said handling has a chance to run. With the old code, that code doesn't run, mixer doesn't have a chance to return PPL_STATUS_PATH_STOP and the DAI then receives a second prepare request, which is invalid given that it's already active. Signed-off-by: Paul Olaru <paul.olaru@nxp.com>
8bf8be2 to
6542a7a
Compare
|
@lyakh @andrula-song @ranj063 can you please have a look? After integration with module_adapter, mixer was mostly broken for IPC3 platforms. This patch fixes it, with still some issues to address in next patches. |
kv2019i
left a comment
There was a problem hiding this comment.
Thanks! One minor readibility/style comment, but nothing to preventing merge.
There was a problem hiding this comment.
Hmm, wouldn't j==active_input_buffers ? If j can be differnent, then I think it deserves a longer more descriptive name as it's used across this long function (i.e. not just a loop index variable anymore).
lyakh
left a comment
There was a problem hiding this comment.
The second patch in this PR is touching some very central code and not only for IPC3
| buffer_release(sink_c); | ||
|
|
||
| /* Prepare module */ | ||
| ret = module_prepare(mod); |
There was a problem hiding this comment.
are we sure, that no module is currently relying on mod->period_bytes being assigned before .prepare()?
audio: mixer: Improve handling of inactive input streams
This helps the mixer dynamically add/remove input streams when needed. Without this commit I would have the issue where the mixer would read from inactive inputs and outside the used portion of the corresponding input buffers.
audio: module_adapter: Ensure prepare always runs on modules
This commit makes sure I can actually start two streams with a delay on different mixer inputs (as opposed to in quick succession, as if given via the & symbol in a shell)
CC: @dbaluta