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
Tested-by: Andreas Pape <apape@de.adit-jv.com>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>

diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 24030b318dffc5d40a99be0d4491e94dad8d5dd6..a57ce5d9664dfad1cefa963c689e46f3d10d3e95 100644 (file)
--- 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 a1463bf6b50abe964f29f74c5ac5b9dffacaf295..981843982caf8d66263c0241250db57ff7dca561 100644 (file)
--- a/src/pcm/pcm_ioplug.c
+++ b/src/pcm/pcm_ioplug.c
@@ -697,6 +697,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)
@@ -707,7 +739,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);
@@ -727,31 +759,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;
@@ -947,6 +954,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 bec5a40855dc3e1204e52837ae9e77e0060fc85a..a63f4be0a06a9c503badf7f8048d4183cee6e75f 100644 (file)
--- 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,