Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] ALSA: usb-audio: Improve frames size computation
@ 2020-04-24  2:24 Alexander Tsoy
  2020-04-24  2:24 ` [PATCH 2/2] ALSA: usb-audio: Remove async workaround for Scarlett 2nd gen Alexander Tsoy
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Alexander Tsoy @ 2020-04-24  2:24 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai, Gregor Pintar, Roope Salmi

For computation of the the next frame size current value of fs/fps and
accumulated fractional parts of fs/fps are used, where values are stored
in Q16.16 format. This is quite natural for computing frame size for
asynchronous endpoints driven by explicit feedback, since in this case
fs/fps is a value provided by the feedback endpoint and it's already in
the Q format. If an error is accumulated over time, the device can
adjust fs/fps value to prevent buffer overruns/underruns.

But for synchronous endpoints the accuracy provided by these computations
is not enough. Due to accumulated error the driver periodically produces
frames with incorrect size (+/- 1 audio sample).

This patch fixes this issue by implementing a different algorithm for
frame size computation. It is based on accumulating of the remainders
from division fs/fps and it doesn't accumulate errors over time. This
new method is enabled for synchronous and adaptive playback endpoints.

Signed-off-by: Alexander Tsoy <alexander@tsoy.me>
---
 sound/usb/card.h     |  4 ++++
 sound/usb/endpoint.c | 43 ++++++++++++++++++++++++++++++++++++++-----
 sound/usb/endpoint.h |  1 +
 sound/usb/pcm.c      |  2 ++
 4 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index 395403a2d33f..820e564656ed 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -84,6 +84,10 @@ struct snd_usb_endpoint {
 	dma_addr_t sync_dma;		/* DMA address of syncbuf */
 
 	unsigned int pipe;		/* the data i/o pipe */
+	unsigned int framesize[2];	/* small/large frame sizes in samples */
+	unsigned int sample_rem;	/* remainder from division fs/fps */
+	unsigned int sample_accum;	/* sample accumulator */
+	unsigned int fps;		/* frames per second */
 	unsigned int freqn;		/* nominal sampling rate in fs/fps in Q16.16 format */
 	unsigned int freqm;		/* momentary sampling rate in fs/fps in Q16.16 format */
 	int	   freqshift;		/* how much to shift the feedback value to get Q16.16 */
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 4a9a2f6ef5a4..d8dc7cb56d43 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -124,12 +124,12 @@ int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep)
 
 /*
  * For streaming based on information derived from sync endpoints,
- * prepare_outbound_urb_sizes() will call next_packet_size() to
+ * prepare_outbound_urb_sizes() will call slave_next_packet_size() to
  * determine the number of samples to be sent in the next packet.
  *
- * For implicit feedback, next_packet_size() is unused.
+ * For implicit feedback, slave_next_packet_size() is unused.
  */
-int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep)
+int snd_usb_endpoint_slave_next_packet_size(struct snd_usb_endpoint *ep)
 {
 	unsigned long flags;
 	int ret;
@@ -146,6 +146,29 @@ int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep)
 	return ret;
 }
 
+/*
+ * For adaptive and synchronous endpoints, prepare_outbound_urb_sizes()
+ * will call next_packet_size() to determine the number of samples to be
+ * sent in the next packet.
+ */
+int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep)
+{
+	int ret;
+
+	if (ep->fill_max)
+		return ep->maxframesize;
+
+	ep->sample_accum += ep->sample_rem;
+	if (ep->sample_accum >= ep->fps) {
+		ep->sample_accum -= ep->fps;
+		ret = ep->framesize[1];
+	} else {
+		ret = ep->framesize[0];
+	}
+
+	return ret;
+}
+
 static void retire_outbound_urb(struct snd_usb_endpoint *ep,
 				struct snd_urb_ctx *urb_ctx)
 {
@@ -190,6 +213,8 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep,
 
 		if (ctx->packet_size[i])
 			counts = ctx->packet_size[i];
+		else if (ep->sync_master)
+			counts = snd_usb_endpoint_slave_next_packet_size(ep);
 		else
 			counts = snd_usb_endpoint_next_packet_size(ep);
 
@@ -874,10 +899,17 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
 	ep->maxpacksize = fmt->maxpacksize;
 	ep->fill_max = !!(fmt->attributes & UAC_EP_CS_ATTR_FILL_MAX);
 
-	if (snd_usb_get_speed(ep->chip->dev) == USB_SPEED_FULL)
+	if (snd_usb_get_speed(ep->chip->dev) == USB_SPEED_FULL) {
 		ep->freqn = get_usb_full_speed_rate(rate);
-	else
+		ep->fps = 1000;
+	} else {
 		ep->freqn = get_usb_high_speed_rate(rate);
+		ep->fps = 8000;
+	}
+
+	ep->sample_rem = rate % ep->fps;
+	ep->framesize[0] = rate / ep->fps;
+	ep->framesize[1] = (rate + (ep->fps - 1)) / ep->fps;
 
 	/* calculate the frequency in 16.16 format */
 	ep->freqm = ep->freqn;
@@ -936,6 +968,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
 	ep->active_mask = 0;
 	ep->unlink_mask = 0;
 	ep->phase = 0;
+	ep->sample_accum = 0;
 
 	snd_usb_endpoint_start_quirk(ep);
 
diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h
index 63a39d4fa8d8..d23fa0a8c11b 100644
--- a/sound/usb/endpoint.h
+++ b/sound/usb/endpoint.h
@@ -28,6 +28,7 @@ void snd_usb_endpoint_release(struct snd_usb_endpoint *ep);
 void snd_usb_endpoint_free(struct snd_usb_endpoint *ep);
 
 int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep);
+int snd_usb_endpoint_slave_next_packet_size(struct snd_usb_endpoint *ep);
 int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep);
 
 void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index a4e4064f9aee..b50965ab3b3a 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1579,6 +1579,8 @@ static void prepare_playback_urb(struct snd_usb_substream *subs,
 	for (i = 0; i < ctx->packets; i++) {
 		if (ctx->packet_size[i])
 			counts = ctx->packet_size[i];
+		else if (ep->sync_master)
+			counts = snd_usb_endpoint_slave_next_packet_size(ep);
 		else
 			counts = snd_usb_endpoint_next_packet_size(ep);
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ALSA: usb-audio: Remove async workaround for Scarlett 2nd gen
  2020-04-24  2:24 [PATCH 1/2] ALSA: usb-audio: Improve frames size computation Alexander Tsoy
@ 2020-04-24  2:24 ` Alexander Tsoy
  2020-04-24  6:28   ` Takashi Iwai
  2020-04-24 15:21   ` Gregor Pintar
  2020-04-24  6:28 ` [PATCH 1/2] ALSA: usb-audio: Improve frames size computation Takashi Iwai
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 12+ messages in thread
From: Alexander Tsoy @ 2020-04-24  2:24 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai, Gregor Pintar, Roope Salmi

Frame size computation has been fixed and the workaround is no longer
needed.

Signed-off-by: Alexander Tsoy <alexander@tsoy.me>
---
 sound/usb/quirks.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 351ba214a9d3..a8ece1701068 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -1806,20 +1806,6 @@ void snd_usb_audioformat_attributes_quirk(struct snd_usb_audio *chip,
 		 */
 		fp->attributes &= ~UAC_EP_CS_ATTR_FILL_MAX;
 		break;
-	case USB_ID(0x1235, 0x8200):  /* Focusrite Scarlett 2i4 2nd gen */
-	case USB_ID(0x1235, 0x8202):  /* Focusrite Scarlett 2i2 2nd gen */
-	case USB_ID(0x1235, 0x8205):  /* Focusrite Scarlett Solo 2nd gen */
-		/*
-		 * Reports that playback should use Synch: Synchronous
-		 * while still providing a feedback endpoint.
-		 * Synchronous causes snapping on some sample rates.
-		 * Force it to use Synch: Asynchronous.
-		 */
-		if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
-			fp->ep_attr &= ~USB_ENDPOINT_SYNCTYPE;
-			fp->ep_attr |= USB_ENDPOINT_SYNC_ASYNC;
-		}
-		break;
 	}
 }
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Improve frames size computation
  2020-04-24  2:24 [PATCH 1/2] ALSA: usb-audio: Improve frames size computation Alexander Tsoy
  2020-04-24  2:24 ` [PATCH 2/2] ALSA: usb-audio: Remove async workaround for Scarlett 2nd gen Alexander Tsoy
@ 2020-04-24  6:28 ` Takashi Iwai
  2020-04-24  9:19 ` Pavel Hofman
  2020-04-24 15:02 ` Gregor Pintar
  3 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2020-04-24  6:28 UTC (permalink / raw)
  To: Alexander Tsoy; +Cc: Gregor Pintar, alsa-devel, Roope Salmi

On Fri, 24 Apr 2020 04:24:48 +0200,
Alexander Tsoy wrote:
> 
> For computation of the the next frame size current value of fs/fps and
> accumulated fractional parts of fs/fps are used, where values are stored
> in Q16.16 format. This is quite natural for computing frame size for
> asynchronous endpoints driven by explicit feedback, since in this case
> fs/fps is a value provided by the feedback endpoint and it's already in
> the Q format. If an error is accumulated over time, the device can
> adjust fs/fps value to prevent buffer overruns/underruns.
> 
> But for synchronous endpoints the accuracy provided by these computations
> is not enough. Due to accumulated error the driver periodically produces
> frames with incorrect size (+/- 1 audio sample).
> 
> This patch fixes this issue by implementing a different algorithm for
> frame size computation. It is based on accumulating of the remainders
> from division fs/fps and it doesn't accumulate errors over time. This
> new method is enabled for synchronous and adaptive playback endpoints.
> 
> Signed-off-by: Alexander Tsoy <alexander@tsoy.me>

Applied to for-next branch now.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] ALSA: usb-audio: Remove async workaround for Scarlett 2nd gen
  2020-04-24  2:24 ` [PATCH 2/2] ALSA: usb-audio: Remove async workaround for Scarlett 2nd gen Alexander Tsoy
@ 2020-04-24  6:28   ` Takashi Iwai
  2020-04-24 15:21   ` Gregor Pintar
  1 sibling, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2020-04-24  6:28 UTC (permalink / raw)
  To: Alexander Tsoy; +Cc: Gregor Pintar, alsa-devel, Roope Salmi

On Fri, 24 Apr 2020 04:24:49 +0200,
Alexander Tsoy wrote:
> 
> Frame size computation has been fixed and the workaround is no longer
> needed.
> 
> Signed-off-by: Alexander Tsoy <alexander@tsoy.me>

Applied to for-next branch now.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Improve frames size computation
  2020-04-24  2:24 [PATCH 1/2] ALSA: usb-audio: Improve frames size computation Alexander Tsoy
  2020-04-24  2:24 ` [PATCH 2/2] ALSA: usb-audio: Remove async workaround for Scarlett 2nd gen Alexander Tsoy
  2020-04-24  6:28 ` [PATCH 1/2] ALSA: usb-audio: Improve frames size computation Takashi Iwai
@ 2020-04-24  9:19 ` Pavel Hofman
  2020-04-24  9:29   ` Alexander Tsoy
  2020-04-24 15:02 ` Gregor Pintar
  3 siblings, 1 reply; 12+ messages in thread
From: Pavel Hofman @ 2020-04-24  9:19 UTC (permalink / raw)
  To: Alexander Tsoy, alsa-devel; +Cc: Takashi Iwai, Gregor Pintar, Roope Salmi


Dne 24. 04. 20 v 4:24 Alexander Tsoy napsal(a):
> For computation of the the next frame size current value of fs/fps and
> accumulated fractional parts of fs/fps are used, where values are stored
> in Q16.16 format. This is quite natural for computing frame size for
> asynchronous endpoints driven by explicit feedback, since in this case
> fs/fps is a value provided by the feedback endpoint and it's already in
> the Q format. If an error is accumulated over time, the device can
> adjust fs/fps value to prevent buffer overruns/underruns.
> 
> But for synchronous endpoints the accuracy provided by these computations
> is not enough. Due to accumulated error the driver periodically produces
> frames with incorrect size (+/- 1 audio sample).
> 
> This patch fixes this issue by implementing a different algorithm for
> frame size computation. It is based on accumulating of the remainders
> from division fs/fps and it doesn't accumulate errors over time. This
> new method is enabled for synchronous and adaptive playback endpoints.
> 
> Signed-off-by: Alexander Tsoy <alexander@tsoy.me>
> ---
>  sound/usb/card.h     |  4 ++++
>  sound/usb/endpoint.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>  sound/usb/endpoint.h |  1 +
>  sound/usb/pcm.c      |  2 ++
>  4 files changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/usb/card.h b/sound/usb/card.h
> index 395403a2d33f..820e564656ed 100644
> --- a/sound/usb/card.h
> +++ b/sound/usb/card.h
> @@ -84,6 +84,10 @@ struct snd_usb_endpoint {
>  	dma_addr_t sync_dma;		/* DMA address of syncbuf */
>  
>  	unsigned int pipe;		/* the data i/o pipe */
> +	unsigned int framesize[2];	/* small/large frame sizes in samples */
> +	unsigned int sample_rem;	/* remainder from division fs/fps */
> +	unsigned int sample_accum;	/* sample accumulator */
> +	unsigned int fps;		/* frames per second */
>  	unsigned int freqn;		/* nominal sampling rate in fs/fps in Q16.16 format */
>  	unsigned int freqm;		/* momentary sampling rate in fs/fps in Q16.16 format */
>  	int	   freqshift;		/* how much to shift the feedback value to get Q16.16 */
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 4a9a2f6ef5a4..d8dc7cb56d43 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -124,12 +124,12 @@ int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep)
>  
>  /*
>   * For streaming based on information derived from sync endpoints,
> - * prepare_outbound_urb_sizes() will call next_packet_size() to
> + * prepare_outbound_urb_sizes() will call slave_next_packet_size() to
>   * determine the number of samples to be sent in the next packet.

Please should not this read

"For streaming based on information derived from async endpoints,"

or

"For streaming based on information derived from sync-master endpoints,"?

Because the next method says:

For adaptive and synchronous endpoints, prepare_outbound_urb_sizes()...

Thanks for the great patch,

Pavel.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Improve frames size computation
  2020-04-24  9:19 ` Pavel Hofman
@ 2020-04-24  9:29   ` Alexander Tsoy
  2020-04-24  9:39     ` Pavel Hofman
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Tsoy @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Pavel Hofman, alsa-devel; +Cc: Takashi Iwai, Gregor Pintar, Roope Salmi

В Пт, 24/04/2020 в 11:19 +0200, Pavel Hofman пишет:
> Dne 24. 04. 20 v 4:24 Alexander Tsoy napsal(a):
> > For computation of the the next frame size current value of fs/fps
> > and
> > accumulated fractional parts of fs/fps are used, where values are
> > stored
> > in Q16.16 format. This is quite natural for computing frame size
> > for
> > asynchronous endpoints driven by explicit feedback, since in this
> > case
> > fs/fps is a value provided by the feedback endpoint and it's
> > already in
> > the Q format. If an error is accumulated over time, the device can
> > adjust fs/fps value to prevent buffer overruns/underruns.
> > 
> > But for synchronous endpoints the accuracy provided by these
> > computations
> > is not enough. Due to accumulated error the driver periodically
> > produces
> > frames with incorrect size (+/- 1 audio sample).
> > 
> > This patch fixes this issue by implementing a different algorithm
> > for
> > frame size computation. It is based on accumulating of the
> > remainders
> > from division fs/fps and it doesn't accumulate errors over time.
> > This
> > new method is enabled for synchronous and adaptive playback
> > endpoints.
> > 
> > Signed-off-by: Alexander Tsoy <alexander@tsoy.me>
> > ---
> >  sound/usb/card.h     |  4 ++++
> >  sound/usb/endpoint.c | 43 ++++++++++++++++++++++++++++++++++++++
> > -----
> >  sound/usb/endpoint.h |  1 +
> >  sound/usb/pcm.c      |  2 ++
> >  4 files changed, 45 insertions(+), 5 deletions(-)
> > 
> > diff --git a/sound/usb/card.h b/sound/usb/card.h
> > index 395403a2d33f..820e564656ed 100644
> > --- a/sound/usb/card.h
> > +++ b/sound/usb/card.h
> > @@ -84,6 +84,10 @@ struct snd_usb_endpoint {
> >  	dma_addr_t sync_dma;		/* DMA address of syncbuf
> > */
> >  
> >  	unsigned int pipe;		/* the data i/o pipe */
> > +	unsigned int framesize[2];	/* small/large frame sizes in
> > samples */
> > +	unsigned int sample_rem;	/* remainder from division fs/fps
> > */
> > +	unsigned int sample_accum;	/* sample accumulator */
> > +	unsigned int fps;		/* frames per second */
> >  	unsigned int freqn;		/* nominal sampling rate in fs/fps
> > in Q16.16 format */
> >  	unsigned int freqm;		/* momentary sampling rate in
> > fs/fps in Q16.16 format */
> >  	int	   freqshift;		/* how much to shift the feedback
> > value to get Q16.16 */
> > diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> > index 4a9a2f6ef5a4..d8dc7cb56d43 100644
> > --- a/sound/usb/endpoint.c
> > +++ b/sound/usb/endpoint.c
> > @@ -124,12 +124,12 @@ int
> > snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint
> > *ep)
> >  
> >  /*
> >   * For streaming based on information derived from sync endpoints,
> > - * prepare_outbound_urb_sizes() will call next_packet_size() to
> > + * prepare_outbound_urb_sizes() will call slave_next_packet_size()
> > to
> >   * determine the number of samples to be sent in the next packet.
> 
> Please should not this read
> 
> "For streaming based on information derived from async endpoints,"
> 
> or
> 
> "For streaming based on information derived from sync-master
> endpoints,"?

"sync endpoints" actually means "feedback endpoints" here. This is the
terminology used by the driver. So it is not the type of
synchronization of the endpoint for which this function is being
called. :)

Probably comment I made for snd_usb_endpoint_next_packet_size() is
slightly inaccurate, because this function will be also used for
asynchronous endpoints in the case feedback endpoint is not configured
for some reason.

> 
> Because the next method says:
> 
> For adaptive and synchronous endpoints,
> prepare_outbound_urb_sizes()...
> 
> Thanks for the great patch,
> 
> Pavel.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Improve frames size computation
  2020-04-24  9:29   ` Alexander Tsoy
@ 2020-04-24  9:39     ` Pavel Hofman
  0 siblings, 0 replies; 12+ messages in thread
From: Pavel Hofman @ 2020-04-24  9:39 UTC (permalink / raw)
  To: Alexander Tsoy, alsa-devel; +Cc: Takashi Iwai, Gregor Pintar, Roope Salmi



Dne 24. 04. 20 v 11:29 Alexander Tsoy napsal(a):
> 
> "sync endpoints" actually means "feedback endpoints" here. This is the
> terminology used by the driver. So it is not the type of
> synchronization of the endpoint for which this function is being
> called. :)
> 
> Probably comment I made for snd_usb_endpoint_next_packet_size() is
> slightly inaccurate, because this function will be also used for
> asynchronous endpoints in the case feedback endpoint is not configured
> for some reason.
> 

OK, thanks for the implementation. Being a noob I got confused. But
still maybe a few words like in your message could clear the noob
confusion, thanks for considering.

Regards,

Pavel.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Improve frames size computation
  2020-04-24  2:24 [PATCH 1/2] ALSA: usb-audio: Improve frames size computation Alexander Tsoy
                   ` (2 preceding siblings ...)
  2020-04-24  9:19 ` Pavel Hofman
@ 2020-04-24 15:02 ` Gregor Pintar
  2020-04-24 16:43   ` Alexander Tsoy
  3 siblings, 1 reply; 12+ messages in thread
From: Gregor Pintar @ 2020-04-24 15:02 UTC (permalink / raw)
  To: Alexander Tsoy; +Cc: Takashi Iwai, alsa-devel, Roope Salmi

On Fri, Apr 24, 2020 at 4:24 AM Alexander Tsoy <alexander@tsoy.me> wrote:
>
> This patch fixes this issue by implementing a different algorithm for
> frame size computation. It is based on accumulating of the remainders
> from division fs/fps and it doesn't accumulate errors over time. This
> new method is enabled for synchronous and adaptive playback endpoints.
>

Hm, I still sometimes get click usually in about first 2 seconds,
but it is hard to reproduce.

This will provide better out-of-box experience. Thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] ALSA: usb-audio: Remove async workaround for Scarlett 2nd gen
  2020-04-24  2:24 ` [PATCH 2/2] ALSA: usb-audio: Remove async workaround for Scarlett 2nd gen Alexander Tsoy
  2020-04-24  6:28   ` Takashi Iwai
@ 2020-04-24 15:21   ` Gregor Pintar
  1 sibling, 0 replies; 12+ messages in thread
From: Gregor Pintar @ 2020-04-24 15:21 UTC (permalink / raw)
  To: Alexander Tsoy; +Cc: Takashi Iwai, alsa-devel, Roope Salmi

On Fri, Apr 24, 2020 at 4:24 AM Alexander Tsoy <alexander@tsoy.me> wrote:
>
> Frame size computation has been fixed and the workaround is no longer
> needed.
>

It seems async is preferred and usually more reliable.

Would it be possible to check, if there is feedback endpoint and use async,
even if interface is reporting synchronous?

Maybe make it configurable so it doesn't break devices with broken feedback
endpoints.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Improve frames size computation
  2020-04-24 15:02 ` Gregor Pintar
@ 2020-04-24 16:43   ` Alexander Tsoy
  2020-04-25 16:50     ` Gregor Pintar
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Tsoy @ 2020-04-24 16:43 UTC (permalink / raw)
  To: Gregor Pintar; +Cc: Takashi Iwai, alsa-devel, Roope Salmi

В Пт, 24/04/2020 в 17:02 +0200, Gregor Pintar пишет:
> On Fri, Apr 24, 2020 at 4:24 AM Alexander Tsoy <alexander@tsoy.me>
> wrote:
> > This patch fixes this issue by implementing a different algorithm
> > for
> > frame size computation. It is based on accumulating of the
> > remainders
> > from division fs/fps and it doesn't accumulate errors over time.
> > This
> > new method is enabled for synchronous and adaptive playback
> > endpoints.
> > 
> 
> Hm, I still sometimes get click usually in about first 2 seconds,
> but it is hard to reproduce.

I wonder if it's because the driver doesn't honor wLockDelay. Anyway,
the second patch can be reverted if there are still issues with 2nd gen
Scarletts.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Improve frames size computation
  2020-04-24 16:43   ` Alexander Tsoy
@ 2020-04-25 16:50     ` Gregor Pintar
  2020-04-25 18:09       ` Gregor Pintar
  0 siblings, 1 reply; 12+ messages in thread
From: Gregor Pintar @ 2020-04-25 16:50 UTC (permalink / raw)
  To: Alexander Tsoy; +Cc: Takashi Iwai, alsa-devel, Roope Salmi

On Fri, Apr 24, 2020 at 6:44 PM Alexander Tsoy <alexander@tsoy.me> wrote:
>
> В Пт, 24/04/2020 в 17:02 +0200, Gregor Pintar пишет:
> > On Fri, Apr 24, 2020 at 4:24 AM Alexander Tsoy <alexander@tsoy.me>
> > wrote:
> > > This patch fixes this issue by implementing a different algorithm
> > > for
> > > frame size computation. It is based on accumulating of the
> > > remainders
> > > from division fs/fps and it doesn't accumulate errors over time.
> > > This
> > > new method is enabled for synchronous and adaptive playback
> > > endpoints.
> > >
> >
> > Hm, I still sometimes get click usually in about first 2 seconds,
> > but it is hard to reproduce.
>
> I wonder if it's because the driver doesn't honor wLockDelay. Anyway,
> the second patch can be reverted if there are still issues with 2nd gen
> Scarletts.
>

I just got click with async. I better stop testing before I get click
with 48kHz.
Could this wLockDelay affect async too?

Does anybody else still get clicks? I would totally think I'm mad, if
I would not
record them. Maybe it is something else.

I guess I will just switch back to 48kHz and try not too think about it.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Improve frames size computation
  2020-04-25 16:50     ` Gregor Pintar
@ 2020-04-25 18:09       ` Gregor Pintar
  0 siblings, 0 replies; 12+ messages in thread
From: Gregor Pintar @ 2020-04-25 18:09 UTC (permalink / raw)
  To: Alexander Tsoy; +Cc: Takashi Iwai, alsa-devel, Roope Salmi

On Sat, Apr 25, 2020 at 6:50 PM Gregor Pintar <grpintar@gmail.com> wrote:
>
> On Fri, Apr 24, 2020 at 6:44 PM Alexander Tsoy <alexander@tsoy.me> wrote:
> >
> > В Пт, 24/04/2020 в 17:02 +0200, Gregor Pintar пишет:
> > > On Fri, Apr 24, 2020 at 4:24 AM Alexander Tsoy <alexander@tsoy.me>
> > > wrote:
> > > > This patch fixes this issue by implementing a different algorithm
> > > > for
> > > > frame size computation. It is based on accumulating of the
> > > > remainders
> > > > from division fs/fps and it doesn't accumulate errors over time.
> > > > This
> > > > new method is enabled for synchronous and adaptive playback
> > > > endpoints.
> > > >
> > >
> > > Hm, I still sometimes get click usually in about first 2 seconds,
> > > but it is hard to reproduce.
> >
> > I wonder if it's because the driver doesn't honor wLockDelay. Anyway,
> > the second patch can be reverted if there are still issues with 2nd gen
> > Scarletts.
> >
>
> I just got click with async. I better stop testing before I get click
> with 48kHz.
> Could this wLockDelay affect async too?
>
> Does anybody else still get clicks? I would totally think I'm mad, if
> I would not
> record them. Maybe it is something else.
>
> I guess I will just switch back to 48kHz and try not too think about it.

Guess what 48kHz sync and async clicks too.
It could be related to running capture at the same time.
It seems fine with 44.1kHz so far with input muted.

Sorry for all this spam.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-24  2:24 [PATCH 1/2] ALSA: usb-audio: Improve frames size computation Alexander Tsoy
2020-04-24  2:24 ` [PATCH 2/2] ALSA: usb-audio: Remove async workaround for Scarlett 2nd gen Alexander Tsoy
2020-04-24  6:28   ` Takashi Iwai
2020-04-24 15:21   ` Gregor Pintar
2020-04-24  6:28 ` [PATCH 1/2] ALSA: usb-audio: Improve frames size computation Takashi Iwai
2020-04-24  9:19 ` Pavel Hofman
2020-04-24  9:29   ` Alexander Tsoy
2020-04-24  9:39     ` Pavel Hofman
2020-04-24 15:02 ` Gregor Pintar
2020-04-24 16:43   ` Alexander Tsoy
2020-04-25 16:50     ` Gregor Pintar
2020-04-25 18:09       ` Gregor Pintar

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git