search

alsa-project / alsa-lib

The Advanced Linux Sound Architecture (ALSA) - library
GNU Lesser General Public License v2.1
366 stars 177 forks source link

pcm: plugin - fix avail_min calculation on rate plugin #218

Closed aditpape closed 2 years ago

aditpape commented 2 years ago

commit 88e4ae27, ff1f669d introduced a dynamic recalculation of the slave's avail_min value. The calculated avail_min setting did not take into account, that the avail_min value depends on the used sampling rate and must be adapted accordingly if the slave is using a different sampling rate. That leads to too large/too small calculated avail_min settings and inaccurate period wake-up events if a rate converter plugin is used.

This patch is adapting the avail_min calculation to consider a different sampling rate between actual pcm and it's slave.

Signed-off-by: Andreas Pape apape@de.adit-jv.com

perexg commented 2 years ago

Even this fix may lead to the off-by-one calculation errors (rounding / wrap etc.). It depends on the rate plugin implementation. It may be probably good to move this code to the specific plugin and re-evaluate the math there.

aditpape commented 2 years ago

Yes, my proposal improves situation but may still lead to off-by-one due to roundings.. I already though of 'duplicating' :-( plugin_may_wait_for_avail_min() into rate plugin as rate_may_wait_for_avail_min() to have access to the rate internal operations rate->ops.output_frames() to calculate the exact amount of missing frames. Can the 'plugin' code get used by other external 'rate converter' plugins beside the builtin 'rate' plugin? If yes, a generic solution in 'plugin' code must be considered.

aditpape commented 2 years ago

btw.. I guess the 'cleanest' approach would be to get rid of 'prefetching' of samples in rate/plugin_avail_update() callbacks which was the main reason for introducing the dynamic avail_min calculation at all. Similar fix was done by you in 00eafe98 for ioplug which removes the prefetching from avail_update() and do the real transfer in mmap_begin() .. But believe such change is too intrusive ...

aditpape commented 2 years ago

Updated approach to use the rate plugin internal calculations for period size proportion between pcm and slave pcm.

perexg commented 2 years ago

Thanks for your work. I applied your patch and I put cleanups to the commit d9dbb57b946b4db438bf919e30ecdc38c177aba4.