From: Alexander Tsoy <alexander@tsoy.me>
To: Takashi Iwai <tiwai@suse.de>
Cc: Gregor Pintar <grpintar@gmail.com>, alsa-devel@alsa-project.org
Subject: Re: [PATCH] ALSA: usb-audio: Apply async workaround for Scarlett 2i4 2nd gen
Date: Thu, 23 Apr 2020 02:45:01 +0300 [thread overview]
Message-ID: <c0cb78664e3acd94f07e59f3a4216c16ab9f497d.camel@tsoy.me> (raw)
In-Reply-To: <s5heesfxo2o.wl-tiwai@suse.de>
В Ср, 22/04/2020 в 22:28 +0200, Takashi Iwai пишет:
> On Wed, 22 Apr 2020 21:26:23 +0200,
> Alexander Tsoy wrote:
> > В Ср, 22/04/2020 в 20:55 +0200, Gregor Pintar пишет:
> > > On Wed, 22 Apr 2020 Alexander Tsoy wrote:
> > > > В Вт, 21/04/2020 в 21:31 +0200, Takashi Iwai пишет:
> > > > > I wonder, though, whether we can correct the rounding error
> > > > > in
> > > > > the
> > > > > driver code, too.
> > > >
> > > > I'm not sure if it's possible with currently used Q16.16
> > > > arithmetic.
> > >
> > > Maybe calculate fixed correction shifts (like it would be
> > > feedback)?
> > > Something like leap year.
> > >
> > > In endpoint.c:
> > > static inline unsigned get_usb_high_speed_rate(unsigned int rate)
> > > {
> > > return ((rate << 10) + 62) / 125;
> > > }
> > > I guess 62 tries to round it, but exact number is needed. So
> > > exact
> > > value for
> > > 44100 should be 361267.2. For 48000 it is 360448.
> > > If only we can deliver that 0.2 by shifting rate somehow?
> > >
> > > At least maybe it would be better to disable sample rates that do
> > > not
> > > divide
> > > by 1000 on SYNC playback endpoints, if there are others sample
> > > rates.
> > >
> > > But I'm not familar with the code or USB.
> >
> > I think instead of accumulating the fractional part of fs/fps in
> > Q16.16
> > format we should accumulating remainder of division fs/fps.
> >
> > So for 44100 Hz and High Speed USB the calculations would be:
> >
> > fs = 44100
> > fps = 8000
> > rem = 44100 % 8000 = 4100
> > accum = 0
> > packet_size_min = 44100 / 8000 = 5
> > packet_size_max = 44100 + (8000 - 1) / 8000 = 6
> >
> >
> > 1. accum += rem = 4100
> > accum < fps => packet_size = packet_size_min = 5
> >
> > 2. accum += rem = 8200
> > accum >= fps => {
> > packet_size = packet_size_max = 6
> > accum -= fps = 200
> > }
> >
> > 3. accum += rem = 4300
> > accum < fps => packet_size = packet_size_min = 5
> >
> > ...
> >
> > 80. accum += rem = 8000
> > accum >= fps => {
> > packet_size = packet_size_max = 6
> > accum -= fps = 0
> > }
> > ...
>
> Yeah, something like that is what I had in my mind.
> It'd be greatly appreciated if someone can experiment it.
> Unfortunately I have no proper USB-audio device now at hands...
OK, here is a quick hacky patch, that seems to work for me:
diff --git a/sound/usb/card.h b/sound/usb/card.h
index 395403a2d33f..fc5e3e391219 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]; /* min/max frame size in samples */
+ unsigned int sample_rem; /* remainder of division fs/fps */
+ unsigned int sample_accum; /* fs % fps 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..e0e4dc5cfe0e 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,32 @@ 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)
+{
+ unsigned long flags;
+ int ret;
+
+ if (ep->fill_max)
+ return ep->maxframesize;
+
+ spin_lock_irqsave(&ep->lock, flags);
+ 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];
+ }
+ spin_unlock_irqrestore(&ep->lock, flags);
+
+ return ret;
+}
+
static void retire_outbound_urb(struct snd_usb_endpoint *ep,
struct snd_urb_ctx *urb_ctx)
{
@@ -190,6 +216,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 +902,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;
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.25.3
next prev parent reply other threads:[~2020-04-22 23:46 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-21 19:09 [PATCH] ALSA: usb-audio: Apply async workaround for Scarlett 2i4 2nd gen Alexander Tsoy
2020-04-21 19:31 ` Takashi Iwai
2020-04-22 3:57 ` Alexander Tsoy
2020-04-22 18:55 ` Gregor Pintar
2020-04-22 19:26 ` Alexander Tsoy
2020-04-22 20:19 ` Alexander Tsoy
2020-04-22 20:28 ` Takashi Iwai
2020-04-22 23:45 ` Alexander Tsoy [this message]
2020-04-23 7:22 ` Takashi Iwai
2020-04-23 11:22 ` Takashi Iwai
2020-04-23 15:10 ` Alexander Tsoy
2020-04-23 16:57 ` Alexander Tsoy
2020-04-23 17:29 ` Takashi Iwai
2020-04-23 17:35 ` Takashi Iwai
2020-04-23 18:24 ` Alexander Tsoy
2020-04-26 17:12 ` Alexander Tsoy
2020-04-27 7:08 ` Takashi Iwai
2020-04-23 17:32 ` Takashi Iwai
2020-04-23 11:46 ` Gregor Pintar
2020-04-23 14:01 ` Alexander Tsoy
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=c0cb78664e3acd94f07e59f3a4216c16ab9f497d.camel@tsoy.me \
--to=alexander@tsoy.me \
--cc=alsa-devel@alsa-project.org \
--cc=grpintar@gmail.com \
--cc=tiwai@suse.de \
/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;
as well as URLs for NNTP newsgroup(s).