Hi Takashi, On Wed, Dec 19, 2018 at 11:30 AM Takashi Iwai <tiwai@xxxxxxx> wrote: > > On Wed, 19 Dec 2018 03:29:38 +0100, > Diego Viola wrote: > > > > Hello Takashi, > > > > I currently have an issue with speaker-test and pulseaudio, and the > > root of the problem seems to be a regression in alsa-lib. > > > > The issue occurs when I run speaker-test, it creates endless output > > with no sound, and the output does not stop until I hit ctrl-c. > > > > I've been investigating about this issue and concluded (through a git > > bisect) that this is a regression in alsa-lib, and this is the bad > > commit that introduces the bug: > > > > commit ce2095c41f2891c51f5dbd28e0317200314c5a75 > > Author: Takashi Iwai <tiwai@xxxxxxx> > > Date: Thu Mar 29 09:51:46 2018 +0200 > > > > pcm: ioplug: Implement proper drain behavior > > > > This patch fixes the draining behavior of ioplug in the following > > ways: > > > > - When no draining ioplug callback is defined, implement the draining > > loop using snd_pcm_wait*() and sync with the drain finishes. > > This is equivalent with the implementation in the kernel write(). > > Similarly as in kernel code, for non-blocking mode, it returns > > immediately after setting DRAINING state. > > > > - The hw_ptr update function checks the PCM state and stops the stream > > if the draining finishes. > > > > - When draining ioplug callback is defined, leave the whole draining > > operation to it. The callback is supposed to return -EAGAIN for > > non-blocking case, too. > > > > - When an error happens during draining, it drops the stream, for a > > safety reason. > > > > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > > > > I've sanity checked and can confirm that going back to the previous > > commit fixes the issue. > > > > My original bug report on the pulseaudio bug tracker: > > https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/593 > > > > Thanks, and please let me know if you need more information. > > Thanks for the report. Actually this looks rather like a bug in pulse > plugin itself. The commit above starts the stream before calling > pa_stream_drain(), and this seems causing the problem. IOW, > pa_stream_drain() doesn't work as advertised. > > In anyway, we should fix the regression and in this case it's safer to > cover in the alsa-lib side. Below is the fix patch. Please give it a > try. > > > Takashi > > -- 8< -- > From: Takashi Iwai <tiwai@xxxxxxx> > Subject: [PATCH] pcm: ioplug: Fix the regression of pulse plugin drain > > The recent change to support the drain via polling caused a regression > for pulse plugin; with speaker-test -c2 -twav with pulse, it leads to > either no sounds or stall. > > The only sensible behavior change in the commit wrt pulse plugin is > that now it starts the stream before calling drain callback. This > supposed to be correct, but it seems hitting a pulse plugin bug. > > The start before drain callback is only a matter of consistency, and > since this doesn't work for the single existing plugin using drain > callback, we don't need to stick with this behavior. > > For addressing the regression, we check the presence of the drain > callback and start the stream only when it doesn't exist, i.e. only in > drain-via-poll mode. > > Fixes: ce2095c41f28 ("pcm: ioplug: Implement proper drain behavior") > Reported-by: Diego Viola <diego.viola@xxxxxxxxx> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > --- > src/pcm/pcm_ioplug.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c > index 881a1a85adaf..1e25190a0f71 100644 > --- a/src/pcm/pcm_ioplug.c > +++ b/src/pcm/pcm_ioplug.c > @@ -537,9 +537,11 @@ static int snd_pcm_ioplug_drain(snd_pcm_t *pcm) > return -EBADFD; > case SND_PCM_STATE_PREPARED: > if (pcm->stream == SND_PCM_STREAM_PLAYBACK) { > - err = snd_pcm_ioplug_start(pcm); > - if (err < 0) > - goto unlock; > + if (!io->data->callback->drain) { > + err = snd_pcm_ioplug_start(pcm); > + if (err < 0) > + goto unlock; > + } > io->data->state = SND_PCM_STATE_DRAINING; > } > break; > -- > 2.19.2 > I can confirm that your patch fixes the problem. Thank you, I appreciate it a lot. Regards, Diego _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
