From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754479Ab2IFNII (ORCPT ); Thu, 6 Sep 2012 09:08:08 -0400 Received: from cantor2.suse.de ([195.135.220.15]:41881 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751681Ab2IFNIG (ORCPT ); Thu, 6 Sep 2012 09:08:06 -0400 Date: Thu, 06 Sep 2012 15:08:03 +0200 Message-ID: From: Takashi Iwai To: Markus Trippelsdorf Cc: Daniel Mack , Linus Torvalds , linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org, Pierre-Louis Bossart Subject: Re: snd-usb: "delay: estimated 0, actual 352" In-Reply-To: <20120906094348.GA257@x4> References: <20120906060220.GA252@x4> <504843BA.2040808@gmail.com> <20120906065340.GA257@x4> <50484BEB.4020103@gmail.com> <20120906071757.GB257@x4> <20120906094348.GA257@x4> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.1 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Thu, 6 Sep 2012 11:43:48 +0200, Markus Trippelsdorf wrote: > > On 2012.09.06 at 10:21 +0200, Takashi Iwai wrote: > > At Thu, 06 Sep 2012 09:35:26 +0200, > > Takashi Iwai wrote: > > > > In short, a patch like below may fix the issue (note: completely > > untested!) > > No it doesn't, unfortunately... OK, I start tracking down the problem a bit more deeply now. The issue happens when the first two URBs are passed to retire_playback_urb(). These are URBs filled before start_endpoints() are set, so they contain actually zero size. Even though these are a sort of dummy packets, the driver still tries to check with the queued delay account, and gives bogus errors. So, essentially the messages are harmless and nothing to worry too much, but surely it doesn't look sexy. The patch below should fix the problem. Please give it a try. thanks, Takashi === From: Takashi Iwai Subject: [PATCH] ALSA: usb-audio: Fix bogus error messages for delay accounting The recent fix for the missing fine delayed time adjustment gives strange error messages at each start of the playback stream, such as delay: estimated 0, actual 352 delay: estimated 353, actual 705 These come from the sanity check in retire_playback_urb(). Before the stream is activated via start_endpoints(), a few silent packets have been already sent. And at this point the delay account is still in the state as if the new packets are just queued, so the driver gets confused and spews the bogus error messages. For fixing the issue, we just need to check whether the received packet is valid, whether it's zero sized or not. Reported-by: Markus Trippelsdorf Cc: [v3.5+] Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index fd5e982..f782ce1 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1140,6 +1140,12 @@ static void retire_playback_urb(struct snd_usb_substream *subs, int processed = urb->transfer_buffer_length / stride; int est_delay; + /* ignore the delay accounting when procssed=0 is given, i.e. + * silent payloads are procssed before handling the actual data + */ + if (!processed) + return; + spin_lock_irqsave(&subs->lock, flags); est_delay = snd_usb_pcm_delay(subs, runtime->rate); /* update delay with exact number of samples played */ -- 1.7.11.5 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: snd-usb: "delay: estimated 0, actual 352" Date: Thu, 06 Sep 2012 15:08:03 +0200 Message-ID: References: <20120906060220.GA252@x4> <504843BA.2040808@gmail.com> <20120906065340.GA257@x4> <50484BEB.4020103@gmail.com> <20120906071757.GB257@x4> <20120906094348.GA257@x4> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 0F08E2615C9 for ; Thu, 6 Sep 2012 15:08:04 +0200 (CEST) In-Reply-To: <20120906094348.GA257@x4> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Markus Trippelsdorf Cc: Pierre-Louis Bossart , alsa-devel@alsa-project.org, Linus Torvalds , linux-kernel@vger.kernel.org, Daniel Mack List-Id: alsa-devel@alsa-project.org At Thu, 6 Sep 2012 11:43:48 +0200, Markus Trippelsdorf wrote: > > On 2012.09.06 at 10:21 +0200, Takashi Iwai wrote: > > At Thu, 06 Sep 2012 09:35:26 +0200, > > Takashi Iwai wrote: > > > > In short, a patch like below may fix the issue (note: completely > > untested!) > > No it doesn't, unfortunately... OK, I start tracking down the problem a bit more deeply now. The issue happens when the first two URBs are passed to retire_playback_urb(). These are URBs filled before start_endpoints() are set, so they contain actually zero size. Even though these are a sort of dummy packets, the driver still tries to check with the queued delay account, and gives bogus errors. So, essentially the messages are harmless and nothing to worry too much, but surely it doesn't look sexy. The patch below should fix the problem. Please give it a try. thanks, Takashi === From: Takashi Iwai Subject: [PATCH] ALSA: usb-audio: Fix bogus error messages for delay accounting The recent fix for the missing fine delayed time adjustment gives strange error messages at each start of the playback stream, such as delay: estimated 0, actual 352 delay: estimated 353, actual 705 These come from the sanity check in retire_playback_urb(). Before the stream is activated via start_endpoints(), a few silent packets have been already sent. And at this point the delay account is still in the state as if the new packets are just queued, so the driver gets confused and spews the bogus error messages. For fixing the issue, we just need to check whether the received packet is valid, whether it's zero sized or not. Reported-by: Markus Trippelsdorf Cc: [v3.5+] Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index fd5e982..f782ce1 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1140,6 +1140,12 @@ static void retire_playback_urb(struct snd_usb_substream *subs, int processed = urb->transfer_buffer_length / stride; int est_delay; + /* ignore the delay accounting when procssed=0 is given, i.e. + * silent payloads are procssed before handling the actual data + */ + if (!processed) + return; + spin_lock_irqsave(&subs->lock, flags); est_delay = snd_usb_pcm_delay(subs, runtime->rate); /* update delay with exact number of samples played */ -- 1.7.11.5