On Wed, 24 Apr 2019 16:05:53 +0200, Takashi Iwai wrote: > > On Mon, 22 Apr 2019 12:50:15 +0200, > Jonathan Liu wrote: > > > > On Wed, 24 Oct 2018 at 18:13, Takashi Iwai <tiwai@xxxxxxx> wrote: > > > > > > On Tue, 23 Oct 2018 16:08:22 +0200, > > > Pierre-Louis Bossart wrote: > > > > > > > > > > > > >>>> Linux 4.17.14, Class Compliant Mode (snd-usb-audio, ALSA backend): > > > > >>>> 16/2 32 + 80 ~ 2.333 ms > > > > >> What are these numbers? Are these lines supposed to in the format > > > > >> expressed by the first formula above? If they are, how come > > > > >> "block_size/periods" shows up as a pair of numbers "16/2" but > > > > >> "block_size*periods" shows up as a single number "32"? > > > > >> > > > > > To interpret "16/2 32 + 80 ~ 2.333 ms" > > > > > Block size: 16 samples > > > > > Periods: 2 (one period for playback + one period for recording when > > > > > determining round trip latency) > > > > > The minimum round trip latency is: 16 * 2 = 32 samples > > > > > However, I measured 112 samples round trip latency which is an > > > > > additional delay of 80 samples (32 + 80 = 112). > > > > > 112 samples at 48000 Hz is 112 / 48000 * 1000 is approximately 2.333 > > > > > ms measured round trip latency. > > > > > > > > ok, so what problem are you trying to fix? > > > > > > > > Are you concerned about the latency numbers (but then they seem lower > > > > on Linux and latency concerns with large buffers are a self-negating > > > > proposition)? are you concerned about the variable delay that doesn't > > > > seem to exist on MacOS or Windows? Are you trying to match the > > > > performance of the RME driver on MacOS? > > > > > > > > I am not sure how this comparison is done btw, the delay includes both > > > > buffering on the device side before reaching the analog parts as well > > > > as buffering on the OS side. While the former should be constant, the > > > > latter depends a great deal on implementation, not sure there are > > > > direct lessons to be applied to ALSA. I also see > > > > inconsistent/non-linear results where with a larger block size the > > > > delay is smaller, e.g. > > > > > > > > 256/2 512 + 650 ~ 24.208 ms > > > > 2048/3 6144 + 633 ~ 141.188 ms > > > > > > > > Independently from the measurement done in this thread, actually, > > > there is a known latency source in the playback path in USB-audio > > > driver code -- which I mentioned in the audio mini conf in the last > > > year: namely, the USB-audio driver starts streaming at prepare time > > > for playback, not at the trigger-START time. This is a sort of > > > workaround to make the device looking similar to the standard > > > ring-buffer behavior. > > > > > > Maybe moving the start at trigger (like the capture direction) would > > > reduce this artificial latency, but it makes the driver behaving in an > > > unexpected manner. Then it may wake up for period_elapsed soon after > > > the stream start with a large runtime->delay value, as the data in > > > in-flight URBs are seen as already "processed". > > > > I observed that snd_usb_pcm_prepare calls start_endpoints which ends > > up submitting silent urbs (prepared by prepare_silent_urb) until > > ep->prepare_data_urb is set by SNDRV_PCM_TRIGGER_START in > > snd_usb_substream_playback_trigger. > > > > I tried to moving the start_endpoints call from snd_usb_pcm_prepare to > > snd_usb_substream_playback trigger's SNDRV_PCM_TRIGGER_START case (see > > https://github.com/net147/linux/commit/276eae5481653a2d4034fbae56f0d5bc579ecf67 > > - it is enabled using start_playback_on_prepare=0 module option for > > snd-usb-audio) but I get a kernel stall in some cases with the > > following call trace: > > _raw_spin_lock+0x2c/0x30 > > _snd_pcm_stream_lock_irqsave+0x31/0x60 [snd_pcm] > > snd_pcm_period_elapsed+0x26/0xb0 [snd_pcm] > > prepare_playback_urb+0x368/0x640 [snd_usb_audio] > > ? usb_submit_urb+0x3cb/0x590 > > snd_usb_endpoint_start+0x148/0x300 [snd_usb_audio] > > start_endpoints+0x36/0x160 [snd_usb_audio] > > snd_usb_substream_playback_trigger+0x152/0x1a0 [snd_usb_audio] > > snd_pcm_action+0x117/0x150 [snd_pcm] > > snd_pcm_common_ioctl+0x588/0xdb0 [snd_pcm] > > ? mprotect_fixup+0x1ec/0x2f0 > > snd_pcm_ioctl+0x23/0x30 [snd_pcm] > > do_vfs_ioctl+0xa6/0x760 > > ? syscall_trace_enter+0x1be/0x2b0 > > __x64_sys_ioctl+0x62/0x90 > > do_syscall_64+0x5b/0x170 > > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > Any ideas? > > This is because snd_pcm_period_elapsed() is called from > prepare_data_urb callback that is called also at start_endpoints(). > > I guess we'd need to move the hwptr accounting and > snd_pcm_period_elapsed() call into retire_data_urb callback in the > case of start-at-trigger for playback. I meant something like below. Only lightly tested. Takashi --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -84,6 +84,7 @@ static int pid[SNDRV_CARDS] = { [0 ... (SNDRV_CARDS-1)] = -1 }; static int device_setup[SNDRV_CARDS]; /* device parameter for this card */ static bool ignore_ctl_error; static bool autoclock = true; +static bool lowlatency; static char *quirk_alias[SNDRV_CARDS]; bool snd_usb_use_vmalloc = true; @@ -105,6 +106,8 @@ MODULE_PARM_DESC(ignore_ctl_error, "Ignore errors from USB controller for mixer interfaces."); module_param(autoclock, bool, 0444); MODULE_PARM_DESC(autoclock, "Enable auto-clock selection for UAC2 devices (default: yes)."); +module_param(lowlatency, bool, 0444); +MODULE_PARM_DESC(lowlatency, "Low latency playback"); module_param_array(quirk_alias, charp, NULL, 0444); MODULE_PARM_DESC(quirk_alias, "Quirk aliases, e.g. 0123abcd:5678beef."); module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444); @@ -487,6 +490,7 @@ static int snd_usb_audio_create(struct usb_interface *intf, chip->card = card; chip->setup = device_setup[idx]; chip->autoclock = autoclock; + chip->lowlatency = lowlatency; atomic_set(&chip->active, 1); /* avoid autopm during probing */ atomic_set(&chip->usage_count, 0); atomic_set(&chip->shutdown, 0); diff --git a/sound/usb/card.h b/sound/usb/card.h index 79fa2a19fb7b..244c80ff8e33 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -48,6 +48,7 @@ struct snd_urb_ctx { int index; /* index for urb array */ int packets; /* number of packets per urb */ int packet_size[MAX_PACKS_HS]; /* size of packets for next submission */ + bool period_elapsed; struct list_head ready_list; }; diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 056af0a57b22..165bf2de6a37 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -927,7 +927,8 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) /* for playback, submit the URBs now; otherwise, the first hwptr_done * updates for all URBs would happen at the same time when starting */ - if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK) + if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK && + !subs->stream->chip->lowlatency) ret = start_endpoints(subs); unlock: @@ -1542,7 +1543,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, struct snd_usb_endpoint *ep = subs->data_endpoint; struct snd_urb_ctx *ctx = urb->context; unsigned int counts, frames, bytes; - int i, stride, period_elapsed = 0; + int i, stride; unsigned long flags; stride = runtime->frame_bits >> 3; @@ -1551,6 +1552,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, urb->number_of_packets = 0; spin_lock_irqsave(&subs->lock, flags); subs->frame_limit += ep->max_urb_frames; + ctx->period_elapsed = 0; for (i = 0; i < ctx->packets; i++) { if (ctx->packet_size[i]) counts = ctx->packet_size[i]; @@ -1566,7 +1568,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, if (subs->transfer_done >= runtime->period_size) { subs->transfer_done -= runtime->period_size; subs->frame_limit = 0; - period_elapsed = 1; + ctx->period_elapsed = 1; if (subs->fmt_type == UAC_FORMAT_TYPE_II) { if (subs->transfer_done > 0) { /* FIXME: fill-max mode is not @@ -1589,7 +1591,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, } } /* finish at the period boundary or after enough frames */ - if ((period_elapsed || + if ((ctx->period_elapsed || subs->transfer_done >= subs->frame_limit) && !snd_usb_endpoint_implicit_feedback_sink(ep)) break; @@ -1640,7 +1642,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, spin_unlock_irqrestore(&subs->lock, flags); urb->transfer_buffer_length = bytes; - if (period_elapsed) + if (!subs->stream->chip->lowlatency && ctx->period_elapsed) snd_pcm_period_elapsed(subs->pcm_substream); } @@ -1654,6 +1656,7 @@ static void retire_playback_urb(struct snd_usb_substream *subs, unsigned long flags; struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime; struct snd_usb_endpoint *ep = subs->data_endpoint; + struct snd_urb_ctx *ctx = urb->context; int processed = urb->transfer_buffer_length / ep->stride; int est_delay; @@ -1695,12 +1698,16 @@ static void retire_playback_urb(struct snd_usb_substream *subs, out: spin_unlock_irqrestore(&subs->lock, flags); + + if (subs->stream->chip->lowlatency && ctx->period_elapsed) + snd_pcm_period_elapsed(subs->pcm_substream); } static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substream, int cmd) { struct snd_usb_substream *subs = substream->runtime->private_data; + int err; switch (cmd) { case SNDRV_PCM_TRIGGER_START: @@ -1709,6 +1716,14 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: subs->data_endpoint->prepare_data_urb = prepare_playback_urb; subs->data_endpoint->retire_data_urb = retire_playback_urb; + if (subs->stream->chip->lowlatency) { + err = start_endpoints(subs); + if (err < 0) { + subs->data_endpoint->prepare_data_urb = NULL; + subs->data_endpoint->retire_data_urb = NULL; + return err; + } + } subs->running = 1; return 0; case SNDRV_PCM_TRIGGER_STOP: diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index b9faeca645fd..71bc58b11ca0 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -64,6 +64,7 @@ struct snd_usb_audio { bool keep_iface; /* keep interface/altset after closing * or parameter change */ + bool lowlatency; struct usb_host_interface *ctrl_intf; /* the audio control interface */ }; _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
