audio: src: simplify src_is_ready_to_process() #10269
Conversation
src_get_copy_limits() only returns false if both sink_get_free_frames(sink) and source_get_data_frames_available(source) return 0. They are calculated in the beginning of the function, the rest of the function is calculating intermediate values, used later for sample calculation. But src_get_copy_limits(() is also called in src_process(), so inside src_is_ready_to_process() it's enough to perform a much simpler check, skipping all the calculations. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies the src_is_ready_to_process() function by replacing a call to src_get_copy_limits() with a direct check of sink and source availability. The optimization avoids unnecessary calculations since src_get_copy_limits() is already called later in src_process().
- Replaces
src_get_copy_limits()call with direct frame availability checks - Removes unused
struct comp_data *cdvariable declaration - Maintains the same logical behavior while improving efficiency
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| struct comp_data *cd = module_get_private_data(mod); | ||
|
|
||
| return src_get_copy_limits(cd, sources[0], sinks[0]); | ||
| return sink_get_free_frames(sinks[0]) || source_get_data_frames_available(sources[0]); |
There was a problem hiding this comment.
The logic appears incorrect. Based on the description, src_get_copy_limits() returns false when BOTH functions return 0, which means it returns true when at least one is non-zero (OR condition). However, for processing to be ready, you typically need BOTH a source with data AND a sink with space available (AND condition). The current OR condition would return true even when the sink is full but source has data, or when sink has space but source is empty.
| return sink_get_free_frames(sinks[0]) || source_get_data_frames_available(sources[0]); | |
| return sink_get_free_frames(sinks[0]) && source_get_data_frames_available(sources[0]); |
There was a problem hiding this comment.
in fact that was also my doubt about src_get_copy_limits() but @singalsu was sure, that it's correct...
There was a problem hiding this comment.
Copilot is not perfect :)
|
@lyakh it could do with a inline comment describing the logic. |
singalsu
left a comment
singalsu
left a comment
There was a problem hiding this comment.
Thanks! Everything works, checked with testbench src_test() and MTL nocodec tplg.
3 participants
src_get_copy_limits() only returns false if both
sink_get_free_frames(sink) and
source_get_data_frames_available(source) return 0. They are calculated in the beginning of the function, the rest of the function is calculating intermediate values, used later for sample calculation. But src_get_copy_limits(() is also called in src_process(), so inside src_is_ready_to_process() it's enough to perform a much simpler check, skipping all the calculations.