[alsa-devel] [PATCH] dmix: fix inconsistent status
Hi all,
Following on from my previous email with subject "1.1.3 bug report: dmix reports inconsistent status", this is a preliminary patch which fixes the issue.
Comments are welcome. In particular note that I'm not very familiar with this code, and in particular I still don't quite understand what the original commit which introduced this bug (38a2d2e) is meant to be doing.
Note also that if this patch is committable, based on the commit comment for 38a2d2e I think there might be an analogous change to be made to faf53c1.
Cheers, Cheng
--- diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -459,12 +459,9 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm) * plugin implementation */
-static snd_pcm_state_t snd_pcm_dmix_state(snd_pcm_t *pcm) +static snd_pcm_state_t snd_pcm_dmix_state_from_slave_state(snd_pcm_direct_t *dmix, snd_pcm_t *pcm, snd_pcm_state_t state) { - snd_pcm_direct_t *dmix = pcm->private_data; int err; - snd_pcm_state_t state; - state = snd_pcm_state(dmix->spcm); switch (state) { case SND_PCM_STATE_SUSPENDED: case SND_PCM_STATE_DISCONNECTED: @@ -483,6 +480,13 @@ static snd_pcm_state_t snd_pcm_dmix_state(snd_pcm_t *pcm) return dmix->state; }
+static snd_pcm_state_t snd_pcm_dmix_state(snd_pcm_t *pcm) +{ + snd_pcm_direct_t *dmix = pcm->private_data; + snd_pcm_state_t slave_state = snd_pcm_state(dmix->spcm); + return snd_pcm_dmix_state_from_slave_state(dmix, pcm, slave_state); +} + static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status) { snd_pcm_direct_t *dmix = pcm->private_data; @@ -490,6 +494,8 @@ static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status) memset(status, 0, sizeof(*status)); snd_pcm_status(dmix->spcm, status);
+ status->state = snd_pcm_dmix_state_from_slave_state(dmix, pcm, status->state); + switch (dmix->state) { case SNDRV_PCM_STATE_DRAINING: case SNDRV_PCM_STATE_RUNNING:
On Fri, 26 May 2017 11:45:53 +0200, Cheng Sun wrote:
Hi all,
Following on from my previous email with subject "1.1.3 bug report: dmix reports inconsistent status", this is a preliminary patch which fixes the issue.
Comments are welcome. In particular note that I'm not very familiar with this code, and in particular I still don't quite understand what the original commit which introduced this bug (38a2d2e) is meant to be doing.
Note also that if this patch is committable, based on the commit comment for 38a2d2e I think there might be an analogous change to be made to faf53c1.
Can it be simply like the patch below?
thanks,
Takashi
--- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -501,6 +501,7 @@ static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status) break; }
+ status->state = snd_pcm_dmix_state(pcm); status->trigger_tstamp = dmix->trigger_tstamp; status->avail = snd_pcm_mmap_playback_avail(pcm); status->avail_max = status->avail > dmix->avail_max ? status->avail : dmix->avail_max;
On Tue, 30 May 2017 17:33:08 +0200, Takashi Iwai wrote:
On Fri, 26 May 2017 11:45:53 +0200, Cheng Sun wrote:
Hi all,
Following on from my previous email with subject "1.1.3 bug report: dmix reports inconsistent status", this is a preliminary patch which fixes the issue.
Comments are welcome. In particular note that I'm not very familiar with this code, and in particular I still don't quite understand what the original commit which introduced this bug (38a2d2e) is meant to be doing.
Note also that if this patch is committable, based on the commit comment for 38a2d2e I think there might be an analogous change to be made to faf53c1.
Can it be simply like the patch below?
I came to believing this is the right way to go. Below is the proper patch. If this works for you, I'm going to apply.
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] pcm: dmix: Fix the inconsistent PCM state
The commit 38a2d2eda880 ("pcm: dmix: Do not discard slave reported delay in...") changed the handling in snd_pcm_dmix_status() for taking the actual delay from the slave PCM status. Along with it, the commit removed the line to update its own state altogether, as it had been done originally in the dshare patch (commit faf53c197cab "pcm_dshare: Do not discard slave reported delay..."), supposing that the slave PCM keeps this same state. However, for dmix/dshare, the PCM state may differ from the slave, thus these changes resulted in the inconsistent PCM state.
For dshare, the issue was already addressed by commit ad6957c61867 ("plugin:dshare: wrong state reporting"), while the fix for dmix was forgotten until now.
This patch restores the code to set the proper dmix PCM state again like in the previous versions.
Fixes: 38a2d2eda880 ("pcm: dmix: Do not discard slave reported delay in...") Reported-by: Cheng Sun chengsun9@gmail.com Signed-off-by: Takashi Iwai tiwai@suse.de --- src/pcm/pcm_dmix.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index c64ae5fc21f9..a6a8f3a84678 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -501,6 +501,7 @@ static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status) break; }
+ status->state = snd_pcm_dmix_state(pcm); status->trigger_tstamp = dmix->trigger_tstamp; status->avail = snd_pcm_mmap_playback_avail(pcm); status->avail_max = status->avail > dmix->avail_max ? status->avail : dmix->avail_max;
participants (2)
-
Cheng Sun -
Takashi Iwai