Skip to content

Commit

Permalink
pcm: pcm_ioplug - fix the avail_update mmap capture copy issue
Browse files Browse the repository at this point in the history 
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: 1714332 ("pcm: ioplug: Transfer all available data")
BugLink: alsa-project#103
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
  • Loading branch information
@perexg
perexg committed Jan 21, 2021
1 parent ae7362c commit bb4c7f3
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 32 deletions.
20 changes: 14 additions & 6 deletions src/pcm/pcm.c
View file
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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

/**
Expand Down
60 changes: 34 additions & 26 deletions src/pcm/pcm_ioplug.c
View file
Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down
2 changes: 2 additions & 0 deletions src/pcm/pcm_local.h
View file
Expand Up @@ -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,
Expand Down

0 comments on commit bb4c7f3

Please sign in to comment.