* [PATCH] pcm_plugin: fix appl pointer not correct when mmap_commit() return error
@ 2016-04-06 11:02 Shengjiu Wang
2016-04-07 14:56 ` Takashi Iwai
0 siblings, 1 reply; 2+ messages in thread
From: Shengjiu Wang @ 2016-04-06 11:02 UTC (permalink / raw)
To: perex, tiwai; +Cc: alsa-devel
When snd_pcm_mmap_commit() return error, the appl pointer is also updated.
which cause the avail_update()'s result wrong.
This patch move the snd_pcm_mmap_appl_forward() to the place when
snd_pcm_mmap_commit() is successfully returned.
Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
---
src/pcm/pcm_plugin.c | 48 ++++++++++++++++++++++++++++++++----------------
1 file changed, 32 insertions(+), 16 deletions(-)
diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c
index d007e8c..940491d 100644
--- a/src/pcm/pcm_plugin.c
+++ b/src/pcm/pcm_plugin.c
@@ -279,18 +279,22 @@ static snd_pcm_sframes_t snd_pcm_plugin_write_areas(snd_pcm_t *pcm,
return -EPIPE;
}
snd_atomic_write_begin(&plugin->watom);
- snd_pcm_mmap_appl_forward(pcm, frames);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) {
snd_pcm_sframes_t res;
res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result);
- if (res < 0)
+ if (res < 0) {
+ snd_atomic_write_end(&plugin->watom);
return xfer > 0 ? (snd_pcm_sframes_t)xfer : res;
+ }
frames -= res;
}
- snd_atomic_write_end(&plugin->watom);
- if (result <= 0)
+ if (result <= 0) {
+ snd_atomic_write_end(&plugin->watom);
return xfer > 0 ? (snd_pcm_sframes_t)xfer : result;
+ }
+ snd_pcm_mmap_appl_forward(pcm, frames);
+ snd_atomic_write_end(&plugin->watom);
offset += frames;
xfer += frames;
size -= frames;
@@ -325,19 +329,23 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm,
return -EPIPE;
}
snd_atomic_write_begin(&plugin->watom);
- snd_pcm_mmap_appl_forward(pcm, frames);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) {
snd_pcm_sframes_t res;
res = plugin->undo_read(slave, areas, offset, frames, slave_frames - result);
- if (res < 0)
+ if (res < 0) {
+ snd_atomic_write_end(&plugin->watom);
return xfer > 0 ? (snd_pcm_sframes_t)xfer : res;
+ }
frames -= res;
}
- snd_atomic_write_end(&plugin->watom);
- if (result <= 0)
+ if (result <= 0) {
+ snd_atomic_write_end(&plugin->watom);
return xfer > 0 ? (snd_pcm_sframes_t)xfer : result;
+ }
+ snd_pcm_mmap_appl_forward(pcm, frames);
+ snd_atomic_write_end(&plugin->watom);
offset += frames;
xfer += frames;
size -= frames;
@@ -423,19 +431,23 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
frames = plugin->write(pcm, areas, appl_offset, frames,
slave_areas, slave_offset, &slave_frames);
snd_atomic_write_begin(&plugin->watom);
- snd_pcm_mmap_appl_forward(pcm, frames);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
- snd_atomic_write_end(&plugin->watom);
if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) {
snd_pcm_sframes_t res;
res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result);
- if (res < 0)
+ if (res < 0) {
+ snd_atomic_write_end(&plugin->watom);
return xfer > 0 ? xfer : res;
+ }
frames -= res;
}
- if (result <= 0)
+ if (result <= 0) {
+ snd_atomic_write_end(&plugin->watom);
return xfer > 0 ? xfer : result;
+ }
+ snd_pcm_mmap_appl_forward(pcm, frames);
+ snd_atomic_write_end(&plugin->watom);
if (frames == cont)
appl_offset = 0;
else
@@ -490,19 +502,23 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
frames = (plugin->read)(pcm, areas, hw_offset, frames,
slave_areas, slave_offset, &slave_frames);
snd_atomic_write_begin(&plugin->watom);
- snd_pcm_mmap_hw_forward(pcm, frames);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
- snd_atomic_write_end(&plugin->watom);
if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) {
snd_pcm_sframes_t res;
res = plugin->undo_read(slave, areas, hw_offset, frames, slave_frames - result);
- if (res < 0)
+ if (res < 0) {
+ snd_atomic_write_end(&plugin->watom);
return xfer > 0 ? (snd_pcm_sframes_t)xfer : res;
+ }
frames -= res;
}
- if (result <= 0)
+ if (result <= 0) {
+ snd_atomic_write_end(&plugin->watom);
return xfer > 0 ? (snd_pcm_sframes_t)xfer : result;
+ }
+ snd_pcm_mmap_hw_forward(pcm, frames);
+ snd_atomic_write_end(&plugin->watom);
if (frames == cont)
hw_offset = 0;
else
--
1.9.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] pcm_plugin: fix appl pointer not correct when mmap_commit() return error
2016-04-06 11:02 [PATCH] pcm_plugin: fix appl pointer not correct when mmap_commit() return error Shengjiu Wang
@ 2016-04-07 14:56 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2016-04-07 14:56 UTC (permalink / raw)
To: Shengjiu Wang; +Cc: alsa-devel
On Wed, 06 Apr 2016 13:02:12 +0200,
Shengjiu Wang wrote:
>
> When snd_pcm_mmap_commit() return error, the appl pointer is also updated.
> which cause the avail_update()'s result wrong.
> This patch move the snd_pcm_mmap_appl_forward() to the place when
> snd_pcm_mmap_commit() is successfully returned.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
Thanks, applied. I also cleaned up the error paths in the patch
below.
Takashi
-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] pcm: Clean up error paths in snd_pcm_plugin_*() helpers
Minor code refactoring to unify the error return paths.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
src/pcm/pcm_plugin.c | 67 +++++++++++++++++++++++++++++++++++-----------------
1 file changed, 45 insertions(+), 22 deletions(-)
diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c
index 940491dbc84b..8527783c3569 100644
--- a/src/pcm/pcm_plugin.c
+++ b/src/pcm/pcm_plugin.c
@@ -276,7 +276,8 @@ static snd_pcm_sframes_t snd_pcm_plugin_write_areas(snd_pcm_t *pcm,
if (CHECK_SANITY(slave_frames > snd_pcm_mmap_playback_avail(slave))) {
SNDMSG("write overflow %ld > %ld", slave_frames,
snd_pcm_mmap_playback_avail(slave));
- return -EPIPE;
+ err = -EPIPE;
+ goto error;
}
snd_atomic_write_begin(&plugin->watom);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
@@ -284,14 +285,14 @@ static snd_pcm_sframes_t snd_pcm_plugin_write_areas(snd_pcm_t *pcm,
snd_pcm_sframes_t res;
res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result);
if (res < 0) {
- snd_atomic_write_end(&plugin->watom);
- return xfer > 0 ? (snd_pcm_sframes_t)xfer : res;
+ err = res;
+ goto error_atomic;
}
frames -= res;
}
if (result <= 0) {
- snd_atomic_write_end(&plugin->watom);
- return xfer > 0 ? (snd_pcm_sframes_t)xfer : result;
+ err = result;
+ goto error_atomic;
}
snd_pcm_mmap_appl_forward(pcm, frames);
snd_atomic_write_end(&plugin->watom);
@@ -300,6 +301,11 @@ static snd_pcm_sframes_t snd_pcm_plugin_write_areas(snd_pcm_t *pcm,
size -= frames;
}
return (snd_pcm_sframes_t)xfer;
+
+ error_atomic:
+ snd_atomic_write_end(&plugin->watom);
+ error:
+ return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
}
static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm,
@@ -311,6 +317,7 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm,
snd_pcm_t *slave = plugin->gen.slave;
snd_pcm_uframes_t xfer = 0;
snd_pcm_sframes_t result;
+ int err;
while (size > 0) {
snd_pcm_uframes_t frames = size;
@@ -326,7 +333,8 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm,
if (CHECK_SANITY(slave_frames > snd_pcm_mmap_capture_avail(slave))) {
SNDMSG("read overflow %ld > %ld", slave_frames,
snd_pcm_mmap_playback_avail(slave));
- return -EPIPE;
+ err = -EPIPE;
+ goto error;
}
snd_atomic_write_begin(&plugin->watom);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
@@ -335,14 +343,14 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm,
res = plugin->undo_read(slave, areas, offset, frames, slave_frames - result);
if (res < 0) {
- snd_atomic_write_end(&plugin->watom);
- return xfer > 0 ? (snd_pcm_sframes_t)xfer : res;
+ err = res;
+ goto error_atomic;
}
frames -= res;
}
if (result <= 0) {
- snd_atomic_write_end(&plugin->watom);
- return xfer > 0 ? (snd_pcm_sframes_t)xfer : result;
+ err = result;
+ goto error_atomic;
}
snd_pcm_mmap_appl_forward(pcm, frames);
snd_atomic_write_end(&plugin->watom);
@@ -351,6 +359,11 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm,
size -= frames;
}
return (snd_pcm_sframes_t)xfer;
+
+ error_atomic:
+ snd_atomic_write_end(&plugin->watom);
+ error:
+ return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
}
@@ -401,6 +414,7 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
snd_pcm_uframes_t appl_offset;
snd_pcm_sframes_t slave_size;
snd_pcm_sframes_t xfer;
+ int err;
if (pcm->stream == SND_PCM_STREAM_CAPTURE) {
snd_atomic_write_begin(&plugin->watom);
@@ -421,11 +435,10 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
snd_pcm_uframes_t slave_offset;
snd_pcm_uframes_t slave_frames = ULONG_MAX;
snd_pcm_sframes_t result;
- int err;
err = snd_pcm_mmap_begin(slave, &slave_areas, &slave_offset, &slave_frames);
if (err < 0)
- return xfer > 0 ? xfer : err;
+ goto error;
if (frames > cont)
frames = cont;
frames = plugin->write(pcm, areas, appl_offset, frames,
@@ -437,14 +450,14 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result);
if (res < 0) {
- snd_atomic_write_end(&plugin->watom);
- return xfer > 0 ? xfer : res;
+ err = res;
+ goto error_atomic;
}
frames -= res;
}
if (result <= 0) {
- snd_atomic_write_end(&plugin->watom);
- return xfer > 0 ? xfer : result;
+ err = result;
+ goto error_atomic;
}
snd_pcm_mmap_appl_forward(pcm, frames);
snd_atomic_write_end(&plugin->watom);
@@ -461,6 +474,11 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
return -EPIPE;
}
return xfer;
+
+ error_atomic:
+ snd_atomic_write_end(&plugin->watom);
+ error:
+ return xfer > 0 ? xfer : err;
}
static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
@@ -468,6 +486,7 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
snd_pcm_plugin_t *plugin = pcm->private_data;
snd_pcm_t *slave = plugin->gen.slave;
snd_pcm_sframes_t slave_size;
+ int err;
slave_size = snd_pcm_avail_update(slave);
if (pcm->stream == SND_PCM_STREAM_CAPTURE &&
@@ -492,11 +511,10 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
snd_pcm_uframes_t slave_offset;
snd_pcm_uframes_t slave_frames = ULONG_MAX;
snd_pcm_sframes_t result;
- int err;
err = snd_pcm_mmap_begin(slave, &slave_areas, &slave_offset, &slave_frames);
if (err < 0)
- return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
+ goto error;
if (frames > cont)
frames = cont;
frames = (plugin->read)(pcm, areas, hw_offset, frames,
@@ -508,14 +526,14 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
res = plugin->undo_read(slave, areas, hw_offset, frames, slave_frames - result);
if (res < 0) {
- snd_atomic_write_end(&plugin->watom);
- return xfer > 0 ? (snd_pcm_sframes_t)xfer : res;
+ err = res;
+ goto error_atomic;
}
frames -= res;
}
if (result <= 0) {
- snd_atomic_write_end(&plugin->watom);
- return xfer > 0 ? (snd_pcm_sframes_t)xfer : result;
+ err = result;
+ goto error_atomic;
}
snd_pcm_mmap_hw_forward(pcm, frames);
snd_atomic_write_end(&plugin->watom);
@@ -528,6 +546,11 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
xfer += frames;
}
return (snd_pcm_sframes_t)xfer;
+
+ error_atomic:
+ snd_atomic_write_end(&plugin->watom);
+ error:
+ return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
}
}
--
2.8.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-04-07 14:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-06 11:02 [PATCH] pcm_plugin: fix appl pointer not correct when mmap_commit() return error Shengjiu Wang
2016-04-07 14:56 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).