Thanks.
The xrun/suspend may happen at any time and we should check it right before the slave hwptr update. Otherwise the hwptr value may be screwed
I think should be "after the slave hwptr update". If hwptr is screwed, means Suspend happened then check_xrun() can return.
and get unexpected large read/write.
Reported-by: S.J. Wang shengjiu.wang@nxp.com Signed-off-by: Takashi Iwai tiwai@suse.de
src/pcm/pcm_dmix.c | 4 ++-- src/pcm/pcm_dshare.c | 4 ++-- src/pcm/pcm_dsnoop.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index d00d53bef604..111fea157228 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -426,11 +426,11 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm) snd_pcm_direct_t *dmix = pcm->private_data; int err;
if (dmix->slowptr)snd_pcm_hwsync(dmix->spcm); err = snd_pcm_direct_check_xrun(dmix, pcm); if (err < 0) return err;
if (dmix->slowptr)snd_pcm_hwsync(dmix->spcm); return snd_pcm_dmix_sync_ptr0(pcm, *dmix->spcm->hw.ptr); } diff --
Better to get slave_hw_ptr before check_xrun(), like this:
--- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -424,15 +424,17 @@ static int snd_pcm_dmix_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_ptr static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm) { snd_pcm_direct_t *dmix = pcm->private_data; + snd_pcm_uframes_t slave_hw_ptr; int err;
if (dmix->slowptr) snd_pcm_hwsync(dmix->spcm); + slave_hw_ptr = *dmix->spcm->hw.ptr; err = snd_pcm_direct_check_xrun(dmix, pcm); if (err < 0) return err;
- return snd_pcm_dmix_sync_ptr0(pcm, *dmix->spcm->hw.ptr); + return snd_pcm_dmix_sync_ptr0(pcm, slave_hw_ptr); }
git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index 0ff43a90d270..c46e8d6a53da 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -201,11 +201,11 @@ static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) snd_pcm_direct_t *dshare = pcm->private_data; int err;
if (dshare->slowptr)snd_pcm_hwsync(dshare->spcm); err = snd_pcm_direct_check_xrun(dshare, pcm); if (err < 0) return err;
if (dshare->slowptr)snd_pcm_hwsync(dshare->spcm); return snd_pcm_dshare_sync_ptr0(pcm, *dshare->spcm->hw.ptr); } diff
--- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -199,15 +199,17 @@ static int snd_pcm_dshare_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_p static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) { snd_pcm_direct_t *dshare = pcm->private_data; + snd_pcm_uframes_t slave_hw_ptr; int err;
if (dshare->slowptr) snd_pcm_hwsync(dshare->spcm); + slave_hw_ptr = *dshare->spcm->hw.ptr; err = snd_pcm_direct_check_xrun(dshare, pcm); if (err < 0) return err;
- return snd_pcm_dshare_sync_ptr0(pcm, *dshare->spcm->hw.ptr); + return snd_pcm_dshare_sync_ptr0(pcm, slave_hw_ptr);
--git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c index 729ff447b41f..9abbbef2c1b6 100644 --- a/src/pcm/pcm_dsnoop.c +++ b/src/pcm/pcm_dsnoop.c @@ -134,14 +134,14 @@ static int snd_pcm_dsnoop_sync_ptr(snd_pcm_t *pcm) snd_pcm_sframes_t diff; int err;
err = snd_pcm_direct_check_xrun(dsnoop, pcm);if (err < 0)return err; if (dsnoop->slowptr) snd_pcm_hwsync(dsnoop->spcm); old_slave_hw_ptr = dsnoop->slave_hw_ptr; snoop_timestamp(pcm); slave_hw_ptr = dsnoop->slave_hw_ptr;
err = snd_pcm_direct_check_xrun(dsnoop, pcm);if (err < 0)return err; diff = pcm_frame_diff(slave_hw_ptr, old_slave_hw_ptr, dsnoop-slave_boundary);
if (diff == 0) /* fast path */ return 0;-- 2.34.1