module_adapter: refactor the copy function - #6929
Conversation
Refactor the module_adapter_copy() function to split the simple_copy case into a separate helper function Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
|
|
||
| /* release all source buffers */ | ||
| i = 0; | ||
| list_for_item(blist, &dev->bsource_list) { |
There was a problem hiding this comment.
This list iteration is confusing as we are using a list iterator (but not using the list item, we are just using it to get the array size).
Should we not use a for/while loop here based on number of source/sinks (or is source/sink != NULL) ?
There was a problem hiding this comment.
@lgirdwood I think it is better to keep the list iterator here because this balances the buffer_acquires done using the list iterator on lne 706.
| ret = module_process(mod, mod->input_buffers, num_input_buffers, | ||
| mod->output_buffers, num_output_buffers); | ||
| ret = module_process(mod, mod->input_buffers, mod->num_input_buffers, | ||
| mod->output_buffers, mod->num_output_buffers); |
There was a problem hiding this comment.
I think there's a change in behaviour here: before this patch if the loop always took the continue path because all components were in wrong states, then num_input_buffers and num_output_buffers were 0 here, now that's different. Is that intended?
There was a problem hiding this comment.
@lyakh but that was only for the simple_copy case right?
There was a problem hiding this comment.
no, I mean old line 760 - I put a comment there
| /* check if the source dev is in the same state as the dev */ | ||
| if (!src_c->source || src_c->source->state != dev->state) { | ||
| buffer_release(src_c); | ||
| continue; |
There was a problem hiding this comment.
@lyakh if you look at the deep-buffer case in today code,
sof/src/audio/module_adapter/module_adapter.c
Line 779 in 7b100fc
There was a problem hiding this comment.
hmm, I have no idea what or how I was reading, you're right, no change here.
| ret = module_process(mod, mod->input_buffers, num_input_buffers, | ||
| mod->output_buffers, num_output_buffers); | ||
| ret = module_process(mod, mod->input_buffers, mod->num_input_buffers, | ||
| mod->output_buffers, mod->num_output_buffers); |
There was a problem hiding this comment.
no, I mean old line 760 - I put a comment there
|
@lgirdwood ok with this, you had comments earlier? this is ready to go, reviews ok and CI passes |
5 participants
Refactor the module_adapter_copy() function to split the simple_copy case into a separate helper function.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com