public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: John Keeping <john@metanate.com>
Cc: alsa-devel@alsa-project.org, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ALSA: usb-audio: Add quirk for Tascam Model 12
Date: Mon, 28 Nov 2022 14:51:15 +0100	[thread overview]
Message-ID: <87lenvyyp8.wl-tiwai@suse.de> (raw)
In-Reply-To: <20221128122353.763696-1-john@metanate.com>

On Mon, 28 Nov 2022 13:23:52 +0100,
John Keeping wrote:
> 
> Tascam's Model 12 is a mixer which can also operate as a USB audio
> interface.  The audio interface uses explicit feedback but it seems that
> it does not correctly handle missing isochronous frames.
> 
> When injecting an xrun (or doing anything else that pauses the playback
> stream) the feedback rate climbs (for example, at 44,100Hz nominal, I
> see a stable rate around 44,099 but xrun injection sees this peak at
> around 44,135 in most cases) and glitches are heard in the audio stream
> for several seconds - this is significantly worse than the single glitch
> expected for an underrun.
> 
> While the stream does normally recover and the feedback rate returns to
> a stable value, I have seen some occurrences where this does not happen
> and the rate continues to increase while no audio is heard from the
> output.  I have not found a solid reproduction for this.
> 
> This misbehaviour can be avoided by totally resetting the stream state
> by switching the interface to alt 0 and back before restarting the
> playback stream.
> 
> Add a new quirk flag which forces the endpoint and interface to be
> reconfigured whenever the stream is stopped, and use this for the Tascam
> Model 12.
> 
> Signed-off-by: John Keeping <john@metanate.com>

Thanks for the patch, it's an interesting case.
About the code change:

> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -1673,6 +1673,13 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, bool keep_pending)
>  		stop_urbs(ep, false, keep_pending);
>  		if (ep->clock_ref)
>  			atomic_dec(&ep->clock_ref->locked);
> +
> +		if (ep->chip->quirk_flags & QUIRK_FLAG_FORCE_IFACE_RESET &&
> +		    usb_pipeout(ep->pipe)) {
> +			ep->need_setup = true;
> +			if (ep->iface_ref)
> +				ep->iface_ref->need_setup = true;
> +		}

Is this the forced reset always safe?  Imagine that you have
individual playback and capture streams, and what if only one of them
gets stopped and restarted while another keeps running?


thanks,

Takashi

  reply	other threads:[~2022-11-28 13:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28 12:23 [PATCH] ALSA: usb-audio: Add quirk for Tascam Model 12 John Keeping
2022-11-28 13:51 ` Takashi Iwai [this message]
2022-11-28 19:20   ` John Keeping
2022-11-29  7:13     ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lenvyyp8.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=john@metanate.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox