Re: Bug in snd_pcm_ioplug_avail_update()?

[Rob Duncan <rduncan@tesla.com>]

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@teslamotors.com>
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@teslamotors.com>
---
 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);
+
 	snd_pcm_ioplug_hw_ptr_update(pcm);
 	if (io->data->state == SND_PCM_STATE_XRUN)
 		return -EPIPE;
@@ -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;
+			}
 		}
 	}
-	avail = snd_pcm_mmap_avail(pcm);
 	if (avail > io->avail_max)
 		io->avail_max = avail;
 	return (snd_pcm_sframes_t)avail;
-- 
2.15.1 (Apple Git-101)