From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MWCUf-0006Ed-42 for qemu-devel@nongnu.org; Wed, 29 Jul 2009 12:59:09 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MWCUc-0006ER-Us for qemu-devel@nongnu.org; Wed, 29 Jul 2009 12:59:07 -0400 Received: from [199.232.76.173] (port=39621 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MWCUc-0006EO-RT for qemu-devel@nongnu.org; Wed, 29 Jul 2009 12:59:06 -0400 Received: from static-71-162-243-5.phlapa.fios.verizon.net ([71.162.243.5]:39384 helo=grelber.thyrsus.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MWCUc-0005DM-57 for qemu-devel@nongnu.org; Wed, 29 Jul 2009 12:59:06 -0400 From: Rob Landley Subject: Re: [Qemu-devel] [PATCH] alsa broken when the host is suspended (or hibernated) Date: Wed, 29 Jul 2009 11:58:50 -0500 References: <1248864666-31387-1-git-send-email-bjorn@mork.no> In-Reply-To: <1248864666-31387-1-git-send-email-bjorn@mork.no> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200907291158.51265.rob@landley.net> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: =?utf-8?q?Bj=C3=B8rn_Mork?= On Wednesday 29 July 2009 05:51:06 Bj=C3=B8rn Mork wrote: > audio output fails after resuming a host running a guest using alsa > audio output. Messages such as > > alsa: Failed to write 882 frames to 0x1804b98 > alsa: Reason: Streams pipe error > > will appear repeatedly in the monitor. This is caused by alsaaudio.c > not handling ESTRPIPE. Fix this by calling snd_pcm_resume() on > ESTRPIPE. > > This bug is similar to the vlc bug discussed on > https://trac.videolan.org/vlc/ticket/1286 and the fix is insired by > the patch attached to that bug report > > Signed-off-by: Bj=C3=B8rn Mork > --- > audio/alsaaudio.c | 22 ++++++++++++++++++++++ > 1 files changed, 22 insertions(+), 0 deletions(-) > > diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c > index d0b7cd0..1c007e5 100644 > --- a/audio/alsaaudio.c > +++ b/audio/alsaaudio.c > @@ -503,6 +503,16 @@ static int alsa_recover (snd_pcm_t *handle) > return 0; > } > > +static int alsa_resume (snd_pcm_t *handle) > +{ > + int err =3D snd_pcm_resume (handle); > + if (err < 0) { > + alsa_logerr (err, "Failed to resume handle %p\n", handle); > + return -1; > + } > + return 0; > +} > + > static snd_pcm_sframes_t alsa_get_avail (snd_pcm_t *handle) > { > snd_pcm_sframes_t avail; > @@ -580,6 +590,18 @@ static int alsa_run_out (HWVoiceOut *hw) > } > continue; > > + case -ESTRPIPE: > + /* stream is suspended and waiting for an application recovery */ > + if (alsa_resume (alsa->handle)) { > + alsa_logerr (written, "Failed to write %d > frames\n", + len); > + goto exit; > + } > + if (conf.verbose) { > + dolog ("Resuming suspended stream\n"); > + } > + continue; > + > case -EAGAIN: > goto exit; Is there a reason the whole patch couldn't just be something like: + case -ESTRPIPE: + /* manually recover after suspend/resume */ + if (snd_pcm_resume(handle) < 0) { + alsa_logerr(written, "Failed to resume handle %p", handle); + goto exit; + } else if (conf.verbose) dolog("Resuming suspended stream\n"); + continue; Why are you wrapping a single function call, and adding logging and error=20 recovery both in that wrapper and in the only caller of that wrapper? Rob =2D-=20 Latency is more important than throughput. It's that simple. - Linus Torval= ds