[alsa-devel] [PATCH v4 14/15] ALSA: usb-audio: always wait in start_endpoints

[Takashi Iwai]

At Mon, 7 Oct 2013 21:26:57 +0200 (CEST),
Eldad Zack wrote:
> 
> 
> 
> On Mon, 7 Oct 2013, Takashi Iwai wrote:
> 
> > At Sun,  6 Oct 2013 22:31:19 +0200,
> > Eldad Zack wrote:
> > > 
> > > Start the endpoints at prepare also for capture endpoints,
> > > since it might be needed to wait for the URBs to be unlinked.
> > > 
> > > If an implicit feedback source endpoint stops being used by its
> > > sink endpoint, but immediately used as a data endpoint, usb_submit_urb
> > > will return -EBUSY.
> > > 
> > > Merge two trigger cases since they are now the same.
> > This change worries me about the timing.  This change means that the
> > capture stream isn't started at the moment the trigger callback is
> > called but at the next urb handling.  It means a possible regression
> > in the case of realtime usage.
> 
> I'm not sure I understand. Do you mean it might cause the delay between 
> capture and playback to vary at each startup?

No, I mean that the actual begin of the recording will be delayed in
comparison with the driver without your patch.  The delay can't be
avoided in this style.

> > Is there any reason to do this except for clean up?  IOW, does this
> > fix any problem by itself?
> 
> Yes, I only became aware of it since I bumped into it with my test tool.
> Attached here - I hope the mailing list accepts attachments.

I understand that this will be required for the implicit feedback
case.  But why applying this to *all* other cases, too, although we
know that this may regress slightly with respect to the latency?
This is my biggest concern.


thanks,

Takashi

> I reverted the patch right now (mainline rc4 + this series) and this is 
> the exact sequence:
> $ ./atest --device hw:2 stream --first 0x0038f0
> atest v1
> Testing device hw:2, rate 96000, range 38f0:0
>  * Test stream, Steps:  [pcm_open] [pcm_hw_params] [pcm_prepare] [pcm_start] [waitsleep] [pcm_stop] [pcm_close]
> ++ Test: stream, permutation: 0x0038f0
>   * [1/14] playback => pcm_open
>   * [2/14] playback => pcm_hw_params
>   * [3/14] playback => pcm_prepare
>   * [4/14] playback => pcm_start
>   * [5/14] capture => pcm_open
>   * [6/14] capture => pcm_hw_params
>   * [7/14] capture => pcm_prepare
>   * [8/14] capture => pcm_start
>   * [9/14] playback => waitsleep
>   * [10/14] playback => pcm_stop
>   ++ Frames [playback]: 58560
>   * [11/14] playback => pcm_close
>   * [12/14] capture => waitsleep
>   * [13/14] capture => pcm_stop
>   ++ Frames [capture]: 5760
>   * [14/14] capture => pcm_close
> ++ permutation: 0x0038f0, runtime: 0.228940 sec
> ++ Test: stream, permutation: 0x003907
>   * [1/14] capture => pcm_open
>   * [2/14] capture => pcm_hw_params
>   * [3/14] capture => pcm_prepare
>   * [4/14] playback => pcm_open
>   * [5/14] playback => pcm_hw_params
>   * [6/14] playback => pcm_prepare
>   * [7/14] playback => pcm_start
>   * [8/14] playback => waitsleep
>   * [9/14] capture => pcm_start
>   * [10/14] playback => pcm_stop
>   ++ Frames [playback]: 58560
>   * [11/14] playback => pcm_close
> capture_thread:209: error -32 on readi 0
>  ** IO Error (capture)
> ++ permutation: 0x003907, runtime: 0.142909 sec
> !! Test stream permutation 0x003907 failed !!
> 
> ...and dmesg shows:
> 
> snd_usb_endpoint_start: cannot submit urb 0, error -16: device busy
> 
> After I removed the revert, no test fail.
> 
> Cheers,
> Eldad