From bb4c7f3744ed88778ee1f071f67f56b129408a47 Mon Sep 17 00:00:00 2001 From: Jaroslav Kysela Date: Thu, 21 Jan 2021 15:45:49 +0100 Subject: [PATCH] pcm: pcm_ioplug - fix the avail_update mmap capture copy issue It seems that the commit "pcm: ioplug: Transfer all available data" introduced new regressions (wrong memory access). The second issue is that the avail_update in ioplug does not move appl_ptr nor hw_ptr, so it's possible that the transfers may be repetitive. This patch moves the transfer calls to mmap_begin callback where it should be. The pointer wraps are handled by design now. Fixes: 1714332719 ("pcm: ioplug: Transfer all available data") BugLink: https://github.com/alsa-project/alsa-lib/pull/103 Signed-off-by: Jaroslav Kysela --- src/pcm/pcm.c | 20 ++++++++++----- src/pcm/pcm_ioplug.c | 60 +++++++++++++++++++++++++------------------- src/pcm/pcm_local.h | 2 ++ 3 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c index 24030b318..a57ce5d96 100644 --- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -7218,9 +7218,8 @@ int snd_pcm_mmap_begin(snd_pcm_t *pcm, } #ifndef DOC_HIDDEN -/* locked version */ -int __snd_pcm_mmap_begin(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas, - snd_pcm_uframes_t *offset, snd_pcm_uframes_t *frames) +int __snd_pcm_mmap_begin_generic(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas, + snd_pcm_uframes_t *offset, snd_pcm_uframes_t *frames) { snd_pcm_uframes_t cont; snd_pcm_uframes_t f; @@ -7229,9 +7228,6 @@ int __snd_pcm_mmap_begin(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas, assert(pcm && areas && offset && frames); - if (pcm->fast_ops->mmap_begin) - return pcm->fast_ops->mmap_begin(pcm->fast_op_arg, areas, offset, frames); - /* fallback for plugins that do not specify new callback */ xareas = snd_pcm_mmap_areas(pcm); if (xareas == NULL) @@ -7250,6 +7246,18 @@ int __snd_pcm_mmap_begin(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas, *frames = f; return 0; } + +/* locked version */ +int __snd_pcm_mmap_begin(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas, + snd_pcm_uframes_t *offset, snd_pcm_uframes_t *frames) +{ + assert(pcm && areas && offset && frames); + + if (pcm->fast_ops->mmap_begin) + return pcm->fast_ops->mmap_begin(pcm->fast_op_arg, areas, offset, frames); + + return __snd_pcm_mmap_begin_generic(pcm, areas, offset, frames); +} #endif /** diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c index e141b1f98..bf653af1a 100644 --- a/src/pcm/pcm_ioplug.c +++ b/src/pcm/pcm_ioplug.c @@ -695,6 +695,38 @@ static snd_pcm_sframes_t snd_pcm_ioplug_readn(snd_pcm_t *pcm, void **bufs, snd_p } } +static int snd_pcm_ioplug_mmap_begin_capture(snd_pcm_t *pcm, + const snd_pcm_channel_area_t **areas, + snd_pcm_uframes_t *offset, + snd_pcm_uframes_t *frames) +{ + ioplug_priv_t *io = pcm->private_data; + int err; + + err = __snd_pcm_mmap_begin_generic(pcm, areas, offset, frames); + if (err < 0) + return err; + + if (io->data->callback->transfer && + pcm->access != SND_PCM_ACCESS_RW_INTERLEAVED && + pcm->access != SND_PCM_ACCESS_RW_NONINTERLEAVED) { + snd_pcm_sframes_t result; + result = io->data->callback->transfer(io->data, *areas, *offset, *frames); + if (result < 0) + return result; + } + + return err; +} + +static int snd_pcm_ioplug_mmap_begin(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas, + snd_pcm_uframes_t *offset, snd_pcm_uframes_t *frames) +{ + if (pcm->stream == SND_PCM_STREAM_PLAYBACK) + return __snd_pcm_mmap_begin_generic(pcm, areas, offset, frames); + return snd_pcm_ioplug_mmap_begin_capture(pcm, areas, offset, frames); +} + static snd_pcm_sframes_t snd_pcm_ioplug_mmap_commit(snd_pcm_t *pcm, snd_pcm_uframes_t offset, snd_pcm_uframes_t size) @@ -705,7 +737,7 @@ static snd_pcm_sframes_t snd_pcm_ioplug_mmap_commit(snd_pcm_t *pcm, const snd_pcm_channel_area_t *areas; snd_pcm_uframes_t ofs, frames = size; - __snd_pcm_mmap_begin(pcm, &areas, &ofs, &frames); + __snd_pcm_mmap_begin_generic(pcm, &areas, &ofs, &frames); if (ofs != offset) return -EIO; return ioplug_priv_transfer_areas(pcm, areas, offset, frames); @@ -725,31 +757,6 @@ static snd_pcm_sframes_t snd_pcm_ioplug_avail_update(snd_pcm_t *pcm) return -EPIPE; avail = snd_pcm_mmap_avail(pcm); - if (pcm->stream == SND_PCM_STREAM_CAPTURE && - pcm->access != SND_PCM_ACCESS_RW_INTERLEAVED && - pcm->access != SND_PCM_ACCESS_RW_NONINTERLEAVED) { - if (io->data->callback->transfer) { - const snd_pcm_channel_area_t *areas; - snd_pcm_uframes_t offset, size = UINT_MAX; - snd_pcm_sframes_t result; - - __snd_pcm_mmap_begin(pcm, &areas, &offset, &size); - result = io->data->callback->transfer(io->data, areas, offset, size); - if (result < 0) - return result; - - /* If the available data doesn't fit in the - contiguous area at the end of the mmap we - must transfer the remaining data to the - beginning of the mmap. */ - if (size < avail) { - result = io->data->callback->transfer(io->data, areas, - 0, avail - size); - if (result < 0) - return result; - } - } - } if (avail > io->avail_max) io->avail_max = avail; return (snd_pcm_sframes_t)avail; @@ -945,6 +952,7 @@ static const snd_pcm_fast_ops_t snd_pcm_ioplug_fast_ops = { .poll_descriptors_count = snd_pcm_ioplug_poll_descriptors_count, .poll_descriptors = snd_pcm_ioplug_poll_descriptors, .poll_revents = snd_pcm_ioplug_poll_revents, + .mmap_begin = snd_pcm_ioplug_mmap_begin, }; #endif /* !DOC_HIDDEN */ diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h index bec5a4085..a63f4be0a 100644 --- a/src/pcm/pcm_local.h +++ b/src/pcm/pcm_local.h @@ -420,6 +420,8 @@ int _snd_pcm_poll_descriptor(snd_pcm_t *pcm); #define _snd_pcm_async_descriptor _snd_pcm_poll_descriptor /* FIXME */ /* locked versions */ +int __snd_pcm_mmap_begin_generic(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas, + snd_pcm_uframes_t *offset, snd_pcm_uframes_t *frames); int __snd_pcm_mmap_begin(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas, snd_pcm_uframes_t *offset, snd_pcm_uframes_t *frames); snd_pcm_sframes_t __snd_pcm_mmap_commit(snd_pcm_t *pcm,