On Tue, 17 Jul 2018 01:53:36 +0200,
Rob Duncan wrote:
>
>
> At 11:38 on Mon, Jul 16 2018, Takashi wrote:
>
> > Yeah, this looks like a bug. The code there supposed to copy the
> > whole data that is available, while snd_pcm_mmap_begin() gives the
> > range only for a single copy action, so it won't fill if the region to
> > fill is split across the buffer boundary.
> >
> > I guess we need some open-code there to achieve the whole data copy.
> > The avail value from snd_pcm_mmap_avail() can be moved before the
> > action, so that we can avoid to calculate this twice.
> >
> > If you have already some fix patch, it'd be helpful. Otherwise I'll
> > fix it up some time later.
>
> How about this?
>
> Rob.
>
> >From 467766fc0bd2fb0981d699d018114fa348ddc6ac Mon Sep 17 00:00:00 2001
> From: Rob Duncan <rduncan@xxxxxxxxxxxxxxx>
> Date: Mon, 16 Jul 2018 16:35:23 -0700
> Subject: [PATCH] pcm: ioplug: Transfer all available data
>
> The snd_pcm_mmap_begin() call returns the amount of contiguous data,
> which is less than the total available if it wraps around the buffer
> boundary.
>
> If we don't handle this split we leave stale data in the buffer that
> should have been overwritten, as well as unread data in the io_plugin
> that gets transferred on a subsequent call at the wrong offset.
>
> Signed-off-by: Rob Duncan <rduncan@xxxxxxxxxxxxxxx>
The code change is similar as what I had in my mind, but...
> ---
> src/pcm/pcm_ioplug.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c
> index 6d52c27b..90e55d0a 100644
> --- a/src/pcm/pcm_ioplug.c
> +++ b/src/pcm/pcm_ioplug.c
> @@ -713,6 +713,8 @@ static snd_pcm_sframes_t snd_pcm_ioplug_avail_update(snd_pcm_t *pcm)
> ioplug_priv_t *io = pcm->private_data;
> snd_pcm_uframes_t avail;
>
> + avail = snd_pcm_mmap_avail(pcm);
This call should be put after the hwptr update. That is...
> +
> snd_pcm_ioplug_hw_ptr_update(pcm);
> if (io->data->state == SND_PCM_STATE_XRUN)
> return -EPIPE;
... put here.
In addition:
> @@ -728,9 +730,15 @@ static snd_pcm_sframes_t snd_pcm_ioplug_avail_update(snd_pcm_t *pcm)
> result = io->data->callback->transfer(io->data, areas, offset, size);
> if (result < 0)
> return result;
> +
> + if (size < avail) {
> + result = io->data->callback->transfer(io->data, areas,
> + 0, avail - size);
> + if (result < 0)
> + return result;
> + }
Please give a short comment about the buffer boundary wrap for
readers.
Thanks!
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
