All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Duncan <rduncan@tesla.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>
Subject: Re: Bug in snd_pcm_ioplug_avail_update()?
Date: Tue, 17 Jul 2018 08:46:35 -0700	[thread overview]
Message-ID: <dw77emr2k1eug4.fsf@tesla.com> (raw)
In-Reply-To: <s5ho9f6jvwd.wl-tiwai@suse.de>

At 22:04 on Mon, Jul 16 2018, Takashi wrote:

> The code change is similar as what I had in my mind, but...

Version 2!

Thanks,

Rob.

>From 8890fbf81c27ed5fded12349b274c7c008c53249 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 | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c
index 6d52c27b..881a1a85 100644
--- a/src/pcm/pcm_ioplug.c
+++ b/src/pcm/pcm_ioplug.c
@@ -716,6 +716,8 @@ static snd_pcm_sframes_t snd_pcm_ioplug_avail_update(snd_pcm_t *pcm)
 	snd_pcm_ioplug_hw_ptr_update(pcm);
 	if (io->data->state == SND_PCM_STATE_XRUN)
 		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) {
@@ -728,9 +730,19 @@ 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 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;
+			}
 		}
 	}
-	avail = snd_pcm_mmap_avail(pcm);
 	if (avail > io->avail_max)
 		io->avail_max = avail;
 	return (snd_pcm_sframes_t)avail;
-- 
2.15.1

  reply	other threads:[~2018-07-17 15:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-13 22:10 Bug in snd_pcm_ioplug_avail_update()? Rob Duncan
2018-07-16 18:38 ` Takashi Iwai
2018-07-16 23:53   ` Rob Duncan
2018-07-17  5:04     ` Takashi Iwai
2018-07-17 15:46       ` Rob Duncan [this message]
2018-07-17 15:54         ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dw77emr2k1eug4.fsf@tesla.com \
    --to=rduncan@tesla.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.