Correct stopping capture and playback substreams?

Correct stopping capture and playback substreams?

[Pavel Hofman]

Hi Takashi,

I am working on stopping alsa streams of audio USB gadget when USB host 
stops capture/playback/USB cable unplugged.

For capture I used code from AK4114 SPDIF receiver 
https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:

static void stop_substream(struct uac_rtd_params *prm)
{
	unsigned long _flags;
	struct snd_pcm_substream *substream;

	substream = prm->ss;
	if (substream) {
		snd_pcm_stream_lock_irqsave(substream, _flags);
		if (snd_pcm_running(substream))
			// TODO - correct handling for playback substream?
			snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
		snd_pcm_stream_unlock_irqrestore(substream, _flags);
	}
}

For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP) 
(https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63) 
  Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED 
(https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).

Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?

Please what is the recommended way?

Thanks a lot,

Pavel.

Re: Correct stopping capture and playback substreams?

[Pavel Hofman]

Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> Hi Takashi,
> 
> I am working on stopping alsa streams of audio USB gadget when USB host 
> stops capture/playback/USB cable unplugged.
> 
> For capture I used code from AK4114 SPDIF receiver 
> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590: 
> 
> 
> static void stop_substream(struct uac_rtd_params *prm)
> {
>      unsigned long _flags;
>      struct snd_pcm_substream *substream;
> 
>      substream = prm->ss;
>      if (substream) {
>          snd_pcm_stream_lock_irqsave(substream, _flags);
>          if (snd_pcm_running(substream))
>              // TODO - correct handling for playback substream?
>              snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>          snd_pcm_stream_unlock_irqrestore(substream, _flags);
>      }
> }
> 
> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP) 
> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63) 
>   Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED 
> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> 
> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
> 
> Please what is the recommended way?
> 

Please can I ask for expert view on this issue? E.g. in SoX stopping the 
stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop 
the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with 
non-recoverable status. I am considering implementing both methods and 
letting users choose their suitable snd_pcm_stop operation (none 
(default)/SETUP-DRAINING/DISCONNECTED) for the two events (host 
playback/capture stop, cable disconnection) with a configfs param. Would 
this make sense?

Thanks a lot in advance,

Pavel.

Re: Correct stopping capture and playback substreams?

[Pavel Hofman]

Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> Hi Takashi,
> 
> I am working on stopping alsa streams of audio USB gadget when USB host 
> stops capture/playback/USB cable unplugged.
> 
> For capture I used code from AK4114 SPDIF receiver 
> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590: 
> 
> 
> static void stop_substream(struct uac_rtd_params *prm)
> {
>      unsigned long _flags;
>      struct snd_pcm_substream *substream;
> 
>      substream = prm->ss;
>      if (substream) {
>          snd_pcm_stream_lock_irqsave(substream, _flags);
>          if (snd_pcm_running(substream))
>              // TODO - correct handling for playback substream?
>              snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>          snd_pcm_stream_unlock_irqrestore(substream, _flags);
>      }
> }
> 
> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP) 
> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63) 
>   Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED 
> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> 
> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
> 
> Please what is the recommended way?
> 

Please can I ask for expert view on this issue? E.g. in SoX stopping the 
stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop 
the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with 
non-recoverable status. I am considering implementing both methods and 
letting users choose their suitable snd_pcm_stop operation (none 
(default)/SETUP-DRAINING/DISCONNECTED) for the two events (host 
playback/capture stop, cable disconnection) with a configfs param. Would 
this make sense?

Thanks a lot in advance,

Pavel.

Re: Correct stopping capture and playback substreams?

[Jaroslav Kysela]

On 03. 01. 22 9:22, Pavel Hofman wrote:
> 
> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>> Hi Takashi,
>>
>> I am working on stopping alsa streams of audio USB gadget when USB host
>> stops capture/playback/USB cable unplugged.
>>
>> For capture I used code from AK4114 SPDIF receiver
>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
>>
>>
>> static void stop_substream(struct uac_rtd_params *prm)
>> {
>>       unsigned long _flags;
>>       struct snd_pcm_substream *substream;
>>
>>       substream = prm->ss;
>>       if (substream) {
>>           snd_pcm_stream_lock_irqsave(substream, _flags);
>>           if (snd_pcm_running(substream))
>>               // TODO - correct handling for playback substream?
>>               snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>           snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>       }
>> }
>>
>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
>>    Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
>>
>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
>>
>> Please what is the recommended way?
>>
> 
> Please can I ask for expert view on this issue? E.g. in SoX stopping the
> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
> non-recoverable status. I am considering implementing both methods and
> letting users choose their suitable snd_pcm_stop operation (none
> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
> playback/capture stop, cable disconnection) with a configfs param. Would
> this make sense?

The disconnection state is unrecoverable. It's expected that the device will 
be freed and cannot be reused.

If you expect to keep the PCM device, we should probably introduce a new 
function which puts the device to the SNDRV_PCM_STATE_OPEN state. In this 
state, all I/O routines will return -EBADFD for the applications, so they 
should close or re-initialize the PCM device completely.

https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: Correct stopping capture and playback substreams?

[Pavel Hofman]

Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
> On 03. 01. 22 9:22, Pavel Hofman wrote:
>>
>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>>> Hi Takashi,
>>>
>>> I am working on stopping alsa streams of audio USB gadget when USB host
>>> stops capture/playback/USB cable unplugged.
>>>
>>> For capture I used code from AK4114 SPDIF receiver
>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590: 
>>>
>>>
>>>
>>> static void stop_substream(struct uac_rtd_params *prm)
>>> {
>>>       unsigned long _flags;
>>>       struct snd_pcm_substream *substream;
>>>
>>>       substream = prm->ss;
>>>       if (substream) {
>>>           snd_pcm_stream_lock_irqsave(substream, _flags);
>>>           if (snd_pcm_running(substream))
>>>               // TODO - correct handling for playback substream?
>>>               snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>>           snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>>       }
>>> }
>>>
>>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63) 
>>>
>>>    Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
>>>
>>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
>>>
>>> Please what is the recommended way?
>>>
>>
>> Please can I ask for expert view on this issue? E.g. in SoX stopping the
>> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
>> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
>> non-recoverable status. I am considering implementing both methods and
>> letting users choose their suitable snd_pcm_stop operation (none
>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
>> playback/capture stop, cable disconnection) with a configfs param. Would
>> this make sense?
> 
> The disconnection state is unrecoverable. It's expected that the device 
> will be freed and cannot be reused.
> 
> If you expect to keep the PCM device, we should probably introduce a new 
> function which puts the device to the SNDRV_PCM_STATE_OPEN state. In 
> this state, all I/O routines will return -EBADFD for the applications, 
> so they should close or re-initialize the PCM device completely.
> 
> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
> 

The fact is that after closing the USB host can re-open the device with 
different samplerate (and perhaps later on with different channels 
count/sample size). That would hint at the need to re-initialize the 
gadget side before opening  anyway.

As of keeping the device - it's likely some use cases would prefer 
keeping the device, to minimize the operations needed to react to the 
host-side playback/capture start.

A function you describe would make sense for this. IMO from the gadget 
POW there is no difference  between the host stopping playback/capture 
and cable disconnection, in both cases the data stream is stopped and 
next stream can have entirely different parameters. Maybe the gadget 
configfs parameter could only toggle between no action (i.e. current 
situation) and the new alsa function stopping the stream.

Jaroslav, please can you draft such a function? Perhaps both changes 
could make it to 5.17.

Thanks a lot,

Pavel.

Re: Correct stopping capture and playback substreams?

[Takashi Iwai]

On Mon, 03 Jan 2022 12:32:53 +0100,
Pavel Hofman wrote:
> 
> 
> 
> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
> > On 03. 01. 22 9:22, Pavel Hofman wrote:
> >>
> >> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> >>> Hi Takashi,
> >>>
> >>> I am working on stopping alsa streams of audio USB gadget when USB host
> >>> stops capture/playback/USB cable unplugged.
> >>>
> >>> For capture I used code from AK4114 SPDIF receiver
> >>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590: 
> >>>
> >>>
> >>>
> >>> static void stop_substream(struct uac_rtd_params *prm)
> >>> {
> >>>       unsigned long _flags;
> >>>       struct snd_pcm_substream *substream;
> >>>
> >>>       substream = prm->ss;
> >>>       if (substream) {
> >>>           snd_pcm_stream_lock_irqsave(substream, _flags);
> >>>           if (snd_pcm_running(substream))
> >>>               // TODO - correct handling for playback substream?
> >>>               snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
> >>>           snd_pcm_stream_unlock_irqrestore(substream, _flags);
> >>>       }
> >>> }
> >>>
> >>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
> >>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63) 
> >>>
> >>>    Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
> >>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> >>>
> >>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
> >>>
> >>> Please what is the recommended way?
> >>>
> >>
> >> Please can I ask for expert view on this issue? E.g. in SoX stopping the
> >> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
> >> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
> >> non-recoverable status. I am considering implementing both methods and
> >> letting users choose their suitable snd_pcm_stop operation (none
> >> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
> >> playback/capture stop, cable disconnection) with a configfs param. Would
> >> this make sense?
> >
> > The disconnection state is unrecoverable. It's expected that the
> > device will be freed and cannot be reused.
> >
> > If you expect to keep the PCM device, we should probably introduce a
> > new function which puts the device to the SNDRV_PCM_STATE_OPEN
> > state. In this state, all I/O routines will return -EBADFD for the
> > applications, so they should close or re-initialize the PCM device
> > completely.
> >
> > https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
> >
> 
> The fact is that after closing the USB host can re-open the device
> with different samplerate (and perhaps later on with different
> channels count/sample size). That would hint at the need to
> re-initialize the gadget side before opening  anyway.
> 
> As of keeping the device - it's likely some use cases would prefer
> keeping the device, to minimize the operations needed to react to the
> host-side playback/capture start.
> 
> A function you describe would make sense for this. IMO from the gadget
> POW there is no difference  between the host stopping playback/capture
> and cable disconnection, in both cases the data stream is stopped and
> next stream can have entirely different parameters. Maybe the gadget
> configfs parameter could only toggle between no action (i.e. current
> situation) and the new alsa function stopping the stream.
> 
> Jaroslav, please can you draft such a function? Perhaps both changes
> could make it to 5.17.

(Sorry for the delayed response, as I've been on vacation and now
catching up the huge pile of backlogs...)

About the change to keep PCM OPEN state: I'm afraid that the
disconnection in the host side may happen at any time, and keeping the
state OPEN would confuse the things if the host is indeed
unrecoverable.  That said, from the safety POV, the complete
card-level disconnection would make sense, which has, of course, a
clear downside for the smooth transition in the application as you
described above.  But most applications should handle such a
disconnection in anyway for the normal USB-audio devices that face
more or less the same problem.


thanks,

Takashi

Re: Correct stopping capture and playback substreams?

[Takashi Iwai]

On Mon, 03 Jan 2022 12:32:53 +0100,
Pavel Hofman wrote:
> 
> 
> 
> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
> > On 03. 01. 22 9:22, Pavel Hofman wrote:
> >>
> >> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> >>> Hi Takashi,
> >>>
> >>> I am working on stopping alsa streams of audio USB gadget when USB host
> >>> stops capture/playback/USB cable unplugged.
> >>>
> >>> For capture I used code from AK4114 SPDIF receiver
> >>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590: 
> >>>
> >>>
> >>>
> >>> static void stop_substream(struct uac_rtd_params *prm)
> >>> {
> >>>       unsigned long _flags;
> >>>       struct snd_pcm_substream *substream;
> >>>
> >>>       substream = prm->ss;
> >>>       if (substream) {
> >>>           snd_pcm_stream_lock_irqsave(substream, _flags);
> >>>           if (snd_pcm_running(substream))
> >>>               // TODO - correct handling for playback substream?
> >>>               snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
> >>>           snd_pcm_stream_unlock_irqrestore(substream, _flags);
> >>>       }
> >>> }
> >>>
> >>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
> >>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63) 
> >>>
> >>>    Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
> >>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> >>>
> >>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
> >>>
> >>> Please what is the recommended way?
> >>>
> >>
> >> Please can I ask for expert view on this issue? E.g. in SoX stopping the
> >> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
> >> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
> >> non-recoverable status. I am considering implementing both methods and
> >> letting users choose their suitable snd_pcm_stop operation (none
> >> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
> >> playback/capture stop, cable disconnection) with a configfs param. Would
> >> this make sense?
> >
> > The disconnection state is unrecoverable. It's expected that the
> > device will be freed and cannot be reused.
> >
> > If you expect to keep the PCM device, we should probably introduce a
> > new function which puts the device to the SNDRV_PCM_STATE_OPEN
> > state. In this state, all I/O routines will return -EBADFD for the
> > applications, so they should close or re-initialize the PCM device
> > completely.
> >
> > https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
> >
> 
> The fact is that after closing the USB host can re-open the device
> with different samplerate (and perhaps later on with different
> channels count/sample size). That would hint at the need to
> re-initialize the gadget side before opening  anyway.
> 
> As of keeping the device - it's likely some use cases would prefer
> keeping the device, to minimize the operations needed to react to the
> host-side playback/capture start.
> 
> A function you describe would make sense for this. IMO from the gadget
> POW there is no difference  between the host stopping playback/capture
> and cable disconnection, in both cases the data stream is stopped and
> next stream can have entirely different parameters. Maybe the gadget
> configfs parameter could only toggle between no action (i.e. current
> situation) and the new alsa function stopping the stream.
> 
> Jaroslav, please can you draft such a function? Perhaps both changes
> could make it to 5.17.

(Sorry for the delayed response, as I've been on vacation and now
catching up the huge pile of backlogs...)

About the change to keep PCM OPEN state: I'm afraid that the
disconnection in the host side may happen at any time, and keeping the
state OPEN would confuse the things if the host is indeed
unrecoverable.  That said, from the safety POV, the complete
card-level disconnection would make sense, which has, of course, a
clear downside for the smooth transition in the application as you
described above.  But most applications should handle such a
disconnection in anyway for the normal USB-audio devices that face
more or less the same problem.


thanks,

Takashi

Re: Correct stopping capture and playback substreams?

[Jaroslav Kysela]

On 03. 01. 22 13:15, Takashi Iwai wrote:
> On Mon, 03 Jan 2022 12:32:53 +0100,
> Pavel Hofman wrote:
>>
>>
>>
>> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
>>> On 03. 01. 22 9:22, Pavel Hofman wrote:
>>>>
>>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>>>>> Hi Takashi,
>>>>>
>>>>> I am working on stopping alsa streams of audio USB gadget when USB host
>>>>> stops capture/playback/USB cable unplugged.
>>>>>
>>>>> For capture I used code from AK4114 SPDIF receiver
>>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
>>>>>
>>>>>
>>>>>
>>>>> static void stop_substream(struct uac_rtd_params *prm)
>>>>> {
>>>>>        unsigned long _flags;
>>>>>        struct snd_pcm_substream *substream;
>>>>>
>>>>>        substream = prm->ss;
>>>>>        if (substream) {
>>>>>            snd_pcm_stream_lock_irqsave(substream, _flags);
>>>>>            if (snd_pcm_running(substream))
>>>>>                // TODO - correct handling for playback substream?
>>>>>                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>>>>            snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>>>>        }
>>>>> }
>>>>>
>>>>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
>>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
>>>>>
>>>>>     Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
>>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
>>>>>
>>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
>>>>>
>>>>> Please what is the recommended way?
>>>>>
>>>>
>>>> Please can I ask for expert view on this issue? E.g. in SoX stopping the
>>>> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
>>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
>>>> non-recoverable status. I am considering implementing both methods and
>>>> letting users choose their suitable snd_pcm_stop operation (none
>>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
>>>> playback/capture stop, cable disconnection) with a configfs param. Would
>>>> this make sense?
>>>
>>> The disconnection state is unrecoverable. It's expected that the
>>> device will be freed and cannot be reused.
>>>
>>> If you expect to keep the PCM device, we should probably introduce a
>>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
>>> state. In this state, all I/O routines will return -EBADFD for the
>>> applications, so they should close or re-initialize the PCM device
>>> completely.
>>>
>>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
>>>
>>
>> The fact is that after closing the USB host can re-open the device
>> with different samplerate (and perhaps later on with different
>> channels count/sample size). That would hint at the need to
>> re-initialize the gadget side before opening  anyway.
>>
>> As of keeping the device - it's likely some use cases would prefer
>> keeping the device, to minimize the operations needed to react to the
>> host-side playback/capture start.
>>
>> A function you describe would make sense for this. IMO from the gadget
>> POW there is no difference  between the host stopping playback/capture
>> and cable disconnection, in both cases the data stream is stopped and
>> next stream can have entirely different parameters. Maybe the gadget
>> configfs parameter could only toggle between no action (i.e. current
>> situation) and the new alsa function stopping the stream.
>>
>> Jaroslav, please can you draft such a function? Perhaps both changes
>> could make it to 5.17.
> 
> (Sorry for the delayed response, as I've been on vacation and now
> catching up the huge pile of backlogs...)
> 
> About the change to keep PCM OPEN state: I'm afraid that the
> disconnection in the host side may happen at any time, and keeping the
> state OPEN would confuse the things if the host is indeed
> unrecoverable.

I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the 
application (in the PCM_OPEN state) and if the USB bus connection is no longer 
active, it may fail. We can distinguish between host -> device disconnection 
and device -> host one. It is not really a similar thing.

I think that the idea was to avoid to re-build the whole card / device 
structure for the fixed device allocation.

Pavel, if the USB host is not connected to the gadget, where the playback PCM 
device fails now ? Is the PCM device created or not ?

						Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: Correct stopping capture and playback substreams?

[Jaroslav Kysela]

On 03. 01. 22 13:15, Takashi Iwai wrote:
> On Mon, 03 Jan 2022 12:32:53 +0100,
> Pavel Hofman wrote:
>>
>>
>>
>> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
>>> On 03. 01. 22 9:22, Pavel Hofman wrote:
>>>>
>>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>>>>> Hi Takashi,
>>>>>
>>>>> I am working on stopping alsa streams of audio USB gadget when USB host
>>>>> stops capture/playback/USB cable unplugged.
>>>>>
>>>>> For capture I used code from AK4114 SPDIF receiver
>>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
>>>>>
>>>>>
>>>>>
>>>>> static void stop_substream(struct uac_rtd_params *prm)
>>>>> {
>>>>>        unsigned long _flags;
>>>>>        struct snd_pcm_substream *substream;
>>>>>
>>>>>        substream = prm->ss;
>>>>>        if (substream) {
>>>>>            snd_pcm_stream_lock_irqsave(substream, _flags);
>>>>>            if (snd_pcm_running(substream))
>>>>>                // TODO - correct handling for playback substream?
>>>>>                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>>>>            snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>>>>        }
>>>>> }
>>>>>
>>>>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
>>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
>>>>>
>>>>>     Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
>>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
>>>>>
>>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
>>>>>
>>>>> Please what is the recommended way?
>>>>>
>>>>
>>>> Please can I ask for expert view on this issue? E.g. in SoX stopping the
>>>> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
>>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
>>>> non-recoverable status. I am considering implementing both methods and
>>>> letting users choose their suitable snd_pcm_stop operation (none
>>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
>>>> playback/capture stop, cable disconnection) with a configfs param. Would
>>>> this make sense?
>>>
>>> The disconnection state is unrecoverable. It's expected that the
>>> device will be freed and cannot be reused.
>>>
>>> If you expect to keep the PCM device, we should probably introduce a
>>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
>>> state. In this state, all I/O routines will return -EBADFD for the
>>> applications, so they should close or re-initialize the PCM device
>>> completely.
>>>
>>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
>>>
>>
>> The fact is that after closing the USB host can re-open the device
>> with different samplerate (and perhaps later on with different
>> channels count/sample size). That would hint at the need to
>> re-initialize the gadget side before opening  anyway.
>>
>> As of keeping the device - it's likely some use cases would prefer
>> keeping the device, to minimize the operations needed to react to the
>> host-side playback/capture start.
>>
>> A function you describe would make sense for this. IMO from the gadget
>> POW there is no difference  between the host stopping playback/capture
>> and cable disconnection, in both cases the data stream is stopped and
>> next stream can have entirely different parameters. Maybe the gadget
>> configfs parameter could only toggle between no action (i.e. current
>> situation) and the new alsa function stopping the stream.
>>
>> Jaroslav, please can you draft such a function? Perhaps both changes
>> could make it to 5.17.
> 
> (Sorry for the delayed response, as I've been on vacation and now
> catching up the huge pile of backlogs...)
> 
> About the change to keep PCM OPEN state: I'm afraid that the
> disconnection in the host side may happen at any time, and keeping the
> state OPEN would confuse the things if the host is indeed
> unrecoverable.

I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the 
application (in the PCM_OPEN state) and if the USB bus connection is no longer 
active, it may fail. We can distinguish between host -> device disconnection 
and device -> host one. It is not really a similar thing.

I think that the idea was to avoid to re-build the whole card / device 
structure for the fixed device allocation.

Pavel, if the USB host is not connected to the gadget, where the playback PCM 
device fails now ? Is the PCM device created or not ?

						Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: Correct stopping capture and playback substreams?

[Takashi Iwai]

On Mon, 03 Jan 2022 13:28:17 +0100,
Jaroslav Kysela wrote:
> 
> On 03. 01. 22 13:15, Takashi Iwai wrote:
> > On Mon, 03 Jan 2022 12:32:53 +0100,
> > Pavel Hofman wrote:
> >>
> >>
> >>
> >> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
> >>> On 03. 01. 22 9:22, Pavel Hofman wrote:
> >>>>
> >>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> >>>>> Hi Takashi,
> >>>>>
> >>>>> I am working on stopping alsa streams of audio USB gadget when USB host
> >>>>> stops capture/playback/USB cable unplugged.
> >>>>>
> >>>>> For capture I used code from AK4114 SPDIF receiver
> >>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
> >>>>>
> >>>>>
> >>>>>
> >>>>> static void stop_substream(struct uac_rtd_params *prm)
> >>>>> {
> >>>>>        unsigned long _flags;
> >>>>>        struct snd_pcm_substream *substream;
> >>>>>
> >>>>>        substream = prm->ss;
> >>>>>        if (substream) {
> >>>>>            snd_pcm_stream_lock_irqsave(substream, _flags);
> >>>>>            if (snd_pcm_running(substream))
> >>>>>                // TODO - correct handling for playback substream?
> >>>>>                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
> >>>>>            snd_pcm_stream_unlock_irqrestore(substream, _flags);
> >>>>>        }
> >>>>> }
> >>>>>
> >>>>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
> >>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
> >>>>>
> >>>>>     Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
> >>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> >>>>>
> >>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
> >>>>>
> >>>>> Please what is the recommended way?
> >>>>>
> >>>>
> >>>> Please can I ask for expert view on this issue? E.g. in SoX stopping the
> >>>> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
> >>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
> >>>> non-recoverable status. I am considering implementing both methods and
> >>>> letting users choose their suitable snd_pcm_stop operation (none
> >>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
> >>>> playback/capture stop, cable disconnection) with a configfs param. Would
> >>>> this make sense?
> >>>
> >>> The disconnection state is unrecoverable. It's expected that the
> >>> device will be freed and cannot be reused.
> >>>
> >>> If you expect to keep the PCM device, we should probably introduce a
> >>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
> >>> state. In this state, all I/O routines will return -EBADFD for the
> >>> applications, so they should close or re-initialize the PCM device
> >>> completely.
> >>>
> >>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
> >>>
> >>
> >> The fact is that after closing the USB host can re-open the device
> >> with different samplerate (and perhaps later on with different
> >> channels count/sample size). That would hint at the need to
> >> re-initialize the gadget side before opening  anyway.
> >>
> >> As of keeping the device - it's likely some use cases would prefer
> >> keeping the device, to minimize the operations needed to react to the
> >> host-side playback/capture start.
> >>
> >> A function you describe would make sense for this. IMO from the gadget
> >> POW there is no difference  between the host stopping playback/capture
> >> and cable disconnection, in both cases the data stream is stopped and
> >> next stream can have entirely different parameters. Maybe the gadget
> >> configfs parameter could only toggle between no action (i.e. current
> >> situation) and the new alsa function stopping the stream.
> >>
> >> Jaroslav, please can you draft such a function? Perhaps both changes
> >> could make it to 5.17.
> >
> > (Sorry for the delayed response, as I've been on vacation and now
> > catching up the huge pile of backlogs...)
> >
> > About the change to keep PCM OPEN state: I'm afraid that the
> > disconnection in the host side may happen at any time, and keeping the
> > state OPEN would confuse the things if the host is indeed
> > unrecoverable.
> 
> I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the
> application (in the PCM_OPEN state) and if the USB bus connection is
> no longer active, it may fail. We can distinguish between host ->
> device disconnection and device -> host one. It is not really a
> similar thing.

Well, I don't know whether we have a proper protocol to downgrade the
state from RUNNING to OPEN.  Currently the valid state transition
seems to be RUNNING -> SETUP, XRUN, or DISCONNECTED.  So, if any, this
might be a two-step procedure.


thanks,

Takashi

Re: Correct stopping capture and playback substreams?

[Takashi Iwai]

On Mon, 03 Jan 2022 13:28:17 +0100,
Jaroslav Kysela wrote:
> 
> On 03. 01. 22 13:15, Takashi Iwai wrote:
> > On Mon, 03 Jan 2022 12:32:53 +0100,
> > Pavel Hofman wrote:
> >>
> >>
> >>
> >> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
> >>> On 03. 01. 22 9:22, Pavel Hofman wrote:
> >>>>
> >>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> >>>>> Hi Takashi,
> >>>>>
> >>>>> I am working on stopping alsa streams of audio USB gadget when USB host
> >>>>> stops capture/playback/USB cable unplugged.
> >>>>>
> >>>>> For capture I used code from AK4114 SPDIF receiver
> >>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
> >>>>>
> >>>>>
> >>>>>
> >>>>> static void stop_substream(struct uac_rtd_params *prm)
> >>>>> {
> >>>>>        unsigned long _flags;
> >>>>>        struct snd_pcm_substream *substream;
> >>>>>
> >>>>>        substream = prm->ss;
> >>>>>        if (substream) {
> >>>>>            snd_pcm_stream_lock_irqsave(substream, _flags);
> >>>>>            if (snd_pcm_running(substream))
> >>>>>                // TODO - correct handling for playback substream?
> >>>>>                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
> >>>>>            snd_pcm_stream_unlock_irqrestore(substream, _flags);
> >>>>>        }
> >>>>> }
> >>>>>
> >>>>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
> >>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
> >>>>>
> >>>>>     Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
> >>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> >>>>>
> >>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
> >>>>>
> >>>>> Please what is the recommended way?
> >>>>>
> >>>>
> >>>> Please can I ask for expert view on this issue? E.g. in SoX stopping the
> >>>> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
> >>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
> >>>> non-recoverable status. I am considering implementing both methods and
> >>>> letting users choose their suitable snd_pcm_stop operation (none
> >>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
> >>>> playback/capture stop, cable disconnection) with a configfs param. Would
> >>>> this make sense?
> >>>
> >>> The disconnection state is unrecoverable. It's expected that the
> >>> device will be freed and cannot be reused.
> >>>
> >>> If you expect to keep the PCM device, we should probably introduce a
> >>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
> >>> state. In this state, all I/O routines will return -EBADFD for the
> >>> applications, so they should close or re-initialize the PCM device
> >>> completely.
> >>>
> >>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
> >>>
> >>
> >> The fact is that after closing the USB host can re-open the device
> >> with different samplerate (and perhaps later on with different
> >> channels count/sample size). That would hint at the need to
> >> re-initialize the gadget side before opening  anyway.
> >>
> >> As of keeping the device - it's likely some use cases would prefer
> >> keeping the device, to minimize the operations needed to react to the
> >> host-side playback/capture start.
> >>
> >> A function you describe would make sense for this. IMO from the gadget
> >> POW there is no difference  between the host stopping playback/capture
> >> and cable disconnection, in both cases the data stream is stopped and
> >> next stream can have entirely different parameters. Maybe the gadget
> >> configfs parameter could only toggle between no action (i.e. current
> >> situation) and the new alsa function stopping the stream.
> >>
> >> Jaroslav, please can you draft such a function? Perhaps both changes
> >> could make it to 5.17.
> >
> > (Sorry for the delayed response, as I've been on vacation and now
> > catching up the huge pile of backlogs...)
> >
> > About the change to keep PCM OPEN state: I'm afraid that the
> > disconnection in the host side may happen at any time, and keeping the
> > state OPEN would confuse the things if the host is indeed
> > unrecoverable.
> 
> I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the
> application (in the PCM_OPEN state) and if the USB bus connection is
> no longer active, it may fail. We can distinguish between host ->
> device disconnection and device -> host one. It is not really a
> similar thing.

Well, I don't know whether we have a proper protocol to downgrade the
state from RUNNING to OPEN.  Currently the valid state transition
seems to be RUNNING -> SETUP, XRUN, or DISCONNECTED.  So, if any, this
might be a two-step procedure.


thanks,

Takashi

Re: Correct stopping capture and playback substreams?

[Pavel Hofman]

Dne 03. 01. 22 v 13:15 Takashi Iwai napsal(a):
> On Mon, 03 Jan 2022 12:32:53 +0100,
> Pavel Hofman wrote:
>>
>>
>>
>> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
>>> On 03. 01. 22 9:22, Pavel Hofman wrote:
>>>>
>>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>>>>> Hi Takashi,
>>>>>
>>>>> I am working on stopping alsa streams of audio USB gadget when USB host
>>>>> stops capture/playback/USB cable unplugged.
>>>>>
>>>>> For capture I used code from AK4114 SPDIF receiver
>>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
>>>>>
>>>>>
>>>>>
>>>>> static void stop_substream(struct uac_rtd_params *prm)
>>>>> {
>>>>>        unsigned long _flags;
>>>>>        struct snd_pcm_substream *substream;
>>>>>
>>>>>        substream = prm->ss;
>>>>>        if (substream) {
>>>>>            snd_pcm_stream_lock_irqsave(substream, _flags);
>>>>>            if (snd_pcm_running(substream))
>>>>>                // TODO - correct handling for playback substream?
>>>>>                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>>>>            snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>>>>        }
>>>>> }
>>>>>
>>>>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
>>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
>>>>>
>>>>>     Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
>>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
>>>>>
>>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
>>>>>
>>>>> Please what is the recommended way?
>>>>>
>>>>
>>>> Please can I ask for expert view on this issue? E.g. in SoX stopping the
>>>> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
>>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
>>>> non-recoverable status. I am considering implementing both methods and
>>>> letting users choose their suitable snd_pcm_stop operation (none
>>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
>>>> playback/capture stop, cable disconnection) with a configfs param. Would
>>>> this make sense?
>>>
>>> The disconnection state is unrecoverable. It's expected that the
>>> device will be freed and cannot be reused.
>>>
>>> If you expect to keep the PCM device, we should probably introduce a
>>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
>>> state. In this state, all I/O routines will return -EBADFD for the
>>> applications, so they should close or re-initialize the PCM device
>>> completely.
>>>
>>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
>>>
>>
>> The fact is that after closing the USB host can re-open the device
>> with different samplerate (and perhaps later on with different
>> channels count/sample size). That would hint at the need to
>> re-initialize the gadget side before opening  anyway.
>>
>> As of keeping the device - it's likely some use cases would prefer
>> keeping the device, to minimize the operations needed to react to the
>> host-side playback/capture start.
>>
>> A function you describe would make sense for this. IMO from the gadget
>> POW there is no difference  between the host stopping playback/capture
>> and cable disconnection, in both cases the data stream is stopped and
>> next stream can have entirely different parameters. Maybe the gadget
>> configfs parameter could only toggle between no action (i.e. current
>> situation) and the new alsa function stopping the stream.
>>
>> Jaroslav, please can you draft such a function? Perhaps both changes
>> could make it to 5.17.
> 
> (Sorry for the delayed response, as I've been on vacation and now
> catching up the huge pile of backlogs...)
> 
> About the change to keep PCM OPEN state: I'm afraid that the
> disconnection in the host side may happen at any time, and keeping the
> state OPEN would confuse the things if the host is indeed
> unrecoverable.  That said, from the safety POV, the complete
> card-level disconnection would make sense, which has, of course, a
> clear downside for the smooth transition in the application as you
> described above.  But most applications should handle such a
> disconnection in anyway for the normal USB-audio devices that face
> more or less the same problem.
> 

Thanks for your valuable insight. IMO a major difference between host 
and gadget side is that when the USB cable gets disconnected, the host 
alsa device disappears, while the (existing) gadget alsa device just 
stops delivering/consuming samples (with the blocking read/write timing 
out, eventually).

I sent a series of RFC patches to linux-usb which handle signalling that 
the host started or stopped (+ cable disconnection) to the gadget side 
via alsa ctl + snd_ctl_notify, as sort of a side channel. A user-space 
example implementation of handling the ctl events on the gadget side is 
https://github.com/pavhofman/gaudio_ctl . The feature of stopping the 
alsa substreams is a mechanism e.g. for setups where the application 
keeps polling the alsa device whether it can be opened. Also sending a 
signal (term, hup, usr1, ...) to an app which is blocked on alsa 
read/write is more difficult without interrupting the blocking wait with 
snd_pcm_close by the gadget device.

I am not sure the stop should "destroy" the whole device, it seems a bit 
of overkill to me. Really just closing the device cleanly so that it 
must be again initialized with new hw_params (which can change between 
starts) would do, perhaps.

Thanks a lot,

Pavel.

Re: Correct stopping capture and playback substreams?

[Pavel Hofman]

Dne 03. 01. 22 v 13:15 Takashi Iwai napsal(a):
> On Mon, 03 Jan 2022 12:32:53 +0100,
> Pavel Hofman wrote:
>>
>>
>>
>> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
>>> On 03. 01. 22 9:22, Pavel Hofman wrote:
>>>>
>>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>>>>> Hi Takashi,
>>>>>
>>>>> I am working on stopping alsa streams of audio USB gadget when USB host
>>>>> stops capture/playback/USB cable unplugged.
>>>>>
>>>>> For capture I used code from AK4114 SPDIF receiver
>>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
>>>>>
>>>>>
>>>>>
>>>>> static void stop_substream(struct uac_rtd_params *prm)
>>>>> {
>>>>>        unsigned long _flags;
>>>>>        struct snd_pcm_substream *substream;
>>>>>
>>>>>        substream = prm->ss;
>>>>>        if (substream) {
>>>>>            snd_pcm_stream_lock_irqsave(substream, _flags);
>>>>>            if (snd_pcm_running(substream))
>>>>>                // TODO - correct handling for playback substream?
>>>>>                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>>>>            snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>>>>        }
>>>>> }
>>>>>
>>>>> For setup I found calling snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP)
>>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
>>>>>
>>>>>     Or for both capture and playback using SNDRV_PCM_STATE_DISCONNECTED
>>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
>>>>>
>>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or snd_pcm_drop(substream)?
>>>>>
>>>>> Please what is the recommended way?
>>>>>
>>>>
>>>> Please can I ask for expert view on this issue? E.g. in SoX stopping the
>>>> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not stop
>>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits with
>>>> non-recoverable status. I am considering implementing both methods and
>>>> letting users choose their suitable snd_pcm_stop operation (none
>>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
>>>> playback/capture stop, cable disconnection) with a configfs param. Would
>>>> this make sense?
>>>
>>> The disconnection state is unrecoverable. It's expected that the
>>> device will be freed and cannot be reused.
>>>
>>> If you expect to keep the PCM device, we should probably introduce a
>>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
>>> state. In this state, all I/O routines will return -EBADFD for the
>>> applications, so they should close or re-initialize the PCM device
>>> completely.
>>>
>>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
>>>
>>
>> The fact is that after closing the USB host can re-open the device
>> with different samplerate (and perhaps later on with different
>> channels count/sample size). That would hint at the need to
>> re-initialize the gadget side before opening  anyway.
>>
>> As of keeping the device - it's likely some use cases would prefer
>> keeping the device, to minimize the operations needed to react to the
>> host-side playback/capture start.
>>
>> A function you describe would make sense for this. IMO from the gadget
>> POW there is no difference  between the host stopping playback/capture
>> and cable disconnection, in both cases the data stream is stopped and
>> next stream can have entirely different parameters. Maybe the gadget
>> configfs parameter could only toggle between no action (i.e. current
>> situation) and the new alsa function stopping the stream.
>>
>> Jaroslav, please can you draft such a function? Perhaps both changes
>> could make it to 5.17.
> 
> (Sorry for the delayed response, as I've been on vacation and now
> catching up the huge pile of backlogs...)
> 
> About the change to keep PCM OPEN state: I'm afraid that the
> disconnection in the host side may happen at any time, and keeping the
> state OPEN would confuse the things if the host is indeed
> unrecoverable.  That said, from the safety POV, the complete
> card-level disconnection would make sense, which has, of course, a
> clear downside for the smooth transition in the application as you
> described above.  But most applications should handle such a
> disconnection in anyway for the normal USB-audio devices that face
> more or less the same problem.
> 

Thanks for your valuable insight. IMO a major difference between host 
and gadget side is that when the USB cable gets disconnected, the host 
alsa device disappears, while the (existing) gadget alsa device just 
stops delivering/consuming samples (with the blocking read/write timing 
out, eventually).

I sent a series of RFC patches to linux-usb which handle signalling that 
the host started or stopped (+ cable disconnection) to the gadget side 
via alsa ctl + snd_ctl_notify, as sort of a side channel. A user-space 
example implementation of handling the ctl events on the gadget side is 
https://github.com/pavhofman/gaudio_ctl . The feature of stopping the 
alsa substreams is a mechanism e.g. for setups where the application 
keeps polling the alsa device whether it can be opened. Also sending a 
signal (term, hup, usr1, ...) to an app which is blocked on alsa 
read/write is more difficult without interrupting the blocking wait with 
snd_pcm_close by the gadget device.

I am not sure the stop should "destroy" the whole device, it seems a bit 
of overkill to me. Really just closing the device cleanly so that it 
must be again initialized with new hw_params (which can change between 
starts) would do, perhaps.

Thanks a lot,

Pavel.

Re: Correct stopping capture and playback substreams?

[Pavel Hofman]

Dne 03. 01. 22 v 13:28 Jaroslav Kysela napsal(a):
> On 03. 01. 22 13:15, Takashi Iwai wrote:
>> On Mon, 03 Jan 2022 12:32:53 +0100,
>> Pavel Hofman wrote:
>>>
>>>
>>>
>>> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
>>>> On 03. 01. 22 9:22, Pavel Hofman wrote:
>>>>>
>>>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>>>>>> Hi Takashi,
>>>>>>
>>>>>> I am working on stopping alsa streams of audio USB gadget when USB 
>>>>>> host
>>>>>> stops capture/playback/USB cable unplugged.
>>>>>>
>>>>>> For capture I used code from AK4114 SPDIF receiver
>>>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590: 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> static void stop_substream(struct uac_rtd_params *prm)
>>>>>> {
>>>>>>        unsigned long _flags;
>>>>>>        struct snd_pcm_substream *substream;
>>>>>>
>>>>>>        substream = prm->ss;
>>>>>>        if (substream) {
>>>>>>            snd_pcm_stream_lock_irqsave(substream, _flags);
>>>>>>            if (snd_pcm_running(substream))
>>>>>>                // TODO - correct handling for playback substream?
>>>>>>                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>>>>>            snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>>>>>        }
>>>>>> }
>>>>>>
>>>>>> For setup I found calling snd_pcm_stop(substream, 
>>>>>> SNDRV_PCM_STATE_SETUP)
>>>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63) 
>>>>>>
>>>>>>
>>>>>>     Or for both capture and playback using 
>>>>>> SNDRV_PCM_STATE_DISCONNECTED
>>>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103). 
>>>>>>
>>>>>>
>>>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or 
>>>>>> snd_pcm_drop(substream)?
>>>>>>
>>>>>> Please what is the recommended way?
>>>>>>
>>>>>
>>>>> Please can I ask for expert view on this issue? E.g. in SoX 
>>>>> stopping the
>>>>> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not 
>>>>> stop
>>>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits 
>>>>> with
>>>>> non-recoverable status. I am considering implementing both methods and
>>>>> letting users choose their suitable snd_pcm_stop operation (none
>>>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
>>>>> playback/capture stop, cable disconnection) with a configfs param. 
>>>>> Would
>>>>> this make sense?
>>>>
>>>> The disconnection state is unrecoverable. It's expected that the
>>>> device will be freed and cannot be reused.
>>>>
>>>> If you expect to keep the PCM device, we should probably introduce a
>>>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
>>>> state. In this state, all I/O routines will return -EBADFD for the
>>>> applications, so they should close or re-initialize the PCM device
>>>> completely.
>>>>
>>>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794 
>>>>
>>>>
>>>
>>> The fact is that after closing the USB host can re-open the device
>>> with different samplerate (and perhaps later on with different
>>> channels count/sample size). That would hint at the need to
>>> re-initialize the gadget side before opening  anyway.
>>>
>>> As of keeping the device - it's likely some use cases would prefer
>>> keeping the device, to minimize the operations needed to react to the
>>> host-side playback/capture start.
>>>
>>> A function you describe would make sense for this. IMO from the gadget
>>> POW there is no difference  between the host stopping playback/capture
>>> and cable disconnection, in both cases the data stream is stopped and
>>> next stream can have entirely different parameters. Maybe the gadget
>>> configfs parameter could only toggle between no action (i.e. current
>>> situation) and the new alsa function stopping the stream.
>>>
>>> Jaroslav, please can you draft such a function? Perhaps both changes
>>> could make it to 5.17.
>>
>> (Sorry for the delayed response, as I've been on vacation and now
>> catching up the huge pile of backlogs...)
>>
>> About the change to keep PCM OPEN state: I'm afraid that the
>> disconnection in the host side may happen at any time, and keeping the
>> state OPEN would confuse the things if the host is indeed
>> unrecoverable.
> 
> I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the 
> application (in the PCM_OPEN state) and if the USB bus connection is no 
> longer active, it may fail. We can distinguish between host -> device 
> disconnection and device -> host one. It is not really a similar thing.
> 
> I think that the idea was to avoid to re-build the whole card / device 
> structure for the fixed device allocation.
> 
> Pavel, if the USB host is not connected to the gadget, where the 
> playback PCM device fails now ? Is the PCM device created or not ?
> 

The gaudio PCM device is created when the gadget function is activated 
(module loaded), regardless whether the USB host is actually connected. 
The playback/capture fails after the blocking read/write times out. The 
data delivery/consumption method is simply not called when no usb 
requests get completed 
https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L147 
.

The current code does basically nothing to the alsa pcm stream at 
capture/playback start/stop by the host (called when altsetting changes 
in the gadget) 
https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L468 
https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L557

Best regards,

Pavel.

Re: Correct stopping capture and playback substreams?

[Pavel Hofman]

Dne 03. 01. 22 v 13:28 Jaroslav Kysela napsal(a):
> On 03. 01. 22 13:15, Takashi Iwai wrote:
>> On Mon, 03 Jan 2022 12:32:53 +0100,
>> Pavel Hofman wrote:
>>>
>>>
>>>
>>> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
>>>> On 03. 01. 22 9:22, Pavel Hofman wrote:
>>>>>
>>>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>>>>>> Hi Takashi,
>>>>>>
>>>>>> I am working on stopping alsa streams of audio USB gadget when USB 
>>>>>> host
>>>>>> stops capture/playback/USB cable unplugged.
>>>>>>
>>>>>> For capture I used code from AK4114 SPDIF receiver
>>>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590: 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> static void stop_substream(struct uac_rtd_params *prm)
>>>>>> {
>>>>>>        unsigned long _flags;
>>>>>>        struct snd_pcm_substream *substream;
>>>>>>
>>>>>>        substream = prm->ss;
>>>>>>        if (substream) {
>>>>>>            snd_pcm_stream_lock_irqsave(substream, _flags);
>>>>>>            if (snd_pcm_running(substream))
>>>>>>                // TODO - correct handling for playback substream?
>>>>>>                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>>>>>            snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>>>>>        }
>>>>>> }
>>>>>>
>>>>>> For setup I found calling snd_pcm_stop(substream, 
>>>>>> SNDRV_PCM_STATE_SETUP)
>>>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63) 
>>>>>>
>>>>>>
>>>>>>     Or for both capture and playback using 
>>>>>> SNDRV_PCM_STATE_DISCONNECTED
>>>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103). 
>>>>>>
>>>>>>
>>>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or 
>>>>>> snd_pcm_drop(substream)?
>>>>>>
>>>>>> Please what is the recommended way?
>>>>>>
>>>>>
>>>>> Please can I ask for expert view on this issue? E.g. in SoX 
>>>>> stopping the
>>>>> stream with SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not 
>>>>> stop
>>>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED SoX exits 
>>>>> with
>>>>> non-recoverable status. I am considering implementing both methods and
>>>>> letting users choose their suitable snd_pcm_stop operation (none
>>>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
>>>>> playback/capture stop, cable disconnection) with a configfs param. 
>>>>> Would
>>>>> this make sense?
>>>>
>>>> The disconnection state is unrecoverable. It's expected that the
>>>> device will be freed and cannot be reused.
>>>>
>>>> If you expect to keep the PCM device, we should probably introduce a
>>>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
>>>> state. In this state, all I/O routines will return -EBADFD for the
>>>> applications, so they should close or re-initialize the PCM device
>>>> completely.
>>>>
>>>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794 
>>>>
>>>>
>>>
>>> The fact is that after closing the USB host can re-open the device
>>> with different samplerate (and perhaps later on with different
>>> channels count/sample size). That would hint at the need to
>>> re-initialize the gadget side before opening  anyway.
>>>
>>> As of keeping the device - it's likely some use cases would prefer
>>> keeping the device, to minimize the operations needed to react to the
>>> host-side playback/capture start.
>>>
>>> A function you describe would make sense for this. IMO from the gadget
>>> POW there is no difference  between the host stopping playback/capture
>>> and cable disconnection, in both cases the data stream is stopped and
>>> next stream can have entirely different parameters. Maybe the gadget
>>> configfs parameter could only toggle between no action (i.e. current
>>> situation) and the new alsa function stopping the stream.
>>>
>>> Jaroslav, please can you draft such a function? Perhaps both changes
>>> could make it to 5.17.
>>
>> (Sorry for the delayed response, as I've been on vacation and now
>> catching up the huge pile of backlogs...)
>>
>> About the change to keep PCM OPEN state: I'm afraid that the
>> disconnection in the host side may happen at any time, and keeping the
>> state OPEN would confuse the things if the host is indeed
>> unrecoverable.
> 
> I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the 
> application (in the PCM_OPEN state) and if the USB bus connection is no 
> longer active, it may fail. We can distinguish between host -> device 
> disconnection and device -> host one. It is not really a similar thing.
> 
> I think that the idea was to avoid to re-build the whole card / device 
> structure for the fixed device allocation.
> 
> Pavel, if the USB host is not connected to the gadget, where the 
> playback PCM device fails now ? Is the PCM device created or not ?
> 

The gaudio PCM device is created when the gadget function is activated 
(module loaded), regardless whether the USB host is actually connected. 
The playback/capture fails after the blocking read/write times out. The 
data delivery/consumption method is simply not called when no usb 
requests get completed 
https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L147 
.

The current code does basically nothing to the alsa pcm stream at 
capture/playback start/stop by the host (called when altsetting changes 
in the gadget) 
https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L468 
https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L557

Best regards,

Pavel.

Re: Correct stopping capture and playback substreams?

[John Keeping]

On Mon, Jan 03, 2022 at 01:54:13PM +0100, Pavel Hofman wrote:
> 
> 
> Dne 03. 01. 22 v 13:28 Jaroslav Kysela napsal(a):
> > On 03. 01. 22 13:15, Takashi Iwai wrote:
> > > On Mon, 03 Jan 2022 12:32:53 +0100,
> > > Pavel Hofman wrote:
> > > > 
> > > > 
> > > > 
> > > > Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
> > > > > On 03. 01. 22 9:22, Pavel Hofman wrote:
> > > > > > 
> > > > > > Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> > > > > > > Hi Takashi,
> > > > > > > 
> > > > > > > I am working on stopping alsa streams of audio USB
> > > > > > > gadget when USB host
> > > > > > > stops capture/playback/USB cable unplugged.
> > > > > > > 
> > > > > > > For capture I used code from AK4114 SPDIF receiver
> > > > > > > https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > static void stop_substream(struct uac_rtd_params *prm)
> > > > > > > {
> > > > > > >        unsigned long _flags;
> > > > > > >        struct snd_pcm_substream *substream;
> > > > > > > 
> > > > > > >        substream = prm->ss;
> > > > > > >        if (substream) {
> > > > > > >            snd_pcm_stream_lock_irqsave(substream, _flags);
> > > > > > >            if (snd_pcm_running(substream))
> > > > > > >                // TODO - correct handling for playback substream?
> > > > > > >                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
> > > > > > >            snd_pcm_stream_unlock_irqrestore(substream, _flags);
> > > > > > >        }
> > > > > > > }
> > > > > > > 
> > > > > > > For setup I found calling snd_pcm_stop(substream,
> > > > > > > SNDRV_PCM_STATE_SETUP)
> > > > > > > (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
> > > > > > > 
> > > > > > > 
> > > > > > >     Or for both capture and playback using
> > > > > > > SNDRV_PCM_STATE_DISCONNECTED
> > > > > > > (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> > > > > > > 
> > > > > > > 
> > > > > > > Or perhaps using snd_pcm_dev_disconnect(dev) or
> > > > > > > snd_pcm_drop(substream)?
> > > > > > > 
> > > > > > > Please what is the recommended way?
> > > > > > > 
> > > > > > 
> > > > > > Please can I ask for expert view on this issue? E.g. in
> > > > > > SoX stopping the
> > > > > > stream with
> > > > > > SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not
> > > > > > stop
> > > > > > the application, while with SNDRV_PCM_STATE_DISCONNECTED
> > > > > > SoX exits with
> > > > > > non-recoverable status. I am considering implementing both methods and
> > > > > > letting users choose their suitable snd_pcm_stop operation (none
> > > > > > (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
> > > > > > playback/capture stop, cable disconnection) with a
> > > > > > configfs param. Would
> > > > > > this make sense?
> > > > > 
> > > > > The disconnection state is unrecoverable. It's expected that the
> > > > > device will be freed and cannot be reused.
> > > > > 
> > > > > If you expect to keep the PCM device, we should probably introduce a
> > > > > new function which puts the device to the SNDRV_PCM_STATE_OPEN
> > > > > state. In this state, all I/O routines will return -EBADFD for the
> > > > > applications, so they should close or re-initialize the PCM device
> > > > > completely.
> > > > > 
> > > > > https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
> > > > > 
> > > > > 
> > > > 
> > > > The fact is that after closing the USB host can re-open the device
> > > > with different samplerate (and perhaps later on with different
> > > > channels count/sample size). That would hint at the need to
> > > > re-initialize the gadget side before opening  anyway.
> > > > 
> > > > As of keeping the device - it's likely some use cases would prefer
> > > > keeping the device, to minimize the operations needed to react to the
> > > > host-side playback/capture start.
> > > > 
> > > > A function you describe would make sense for this. IMO from the gadget
> > > > POW there is no difference  between the host stopping playback/capture
> > > > and cable disconnection, in both cases the data stream is stopped and
> > > > next stream can have entirely different parameters. Maybe the gadget
> > > > configfs parameter could only toggle between no action (i.e. current
> > > > situation) and the new alsa function stopping the stream.
> > > > 
> > > > Jaroslav, please can you draft such a function? Perhaps both changes
> > > > could make it to 5.17.
> > > 
> > > (Sorry for the delayed response, as I've been on vacation and now
> > > catching up the huge pile of backlogs...)
> > > 
> > > About the change to keep PCM OPEN state: I'm afraid that the
> > > disconnection in the host side may happen at any time, and keeping the
> > > state OPEN would confuse the things if the host is indeed
> > > unrecoverable.
> > 
> > I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the
> > application (in the PCM_OPEN state) and if the USB bus connection is no
> > longer active, it may fail. We can distinguish between host -> device
> > disconnection and device -> host one. It is not really a similar thing.
> > 
> > I think that the idea was to avoid to re-build the whole card / device
> > structure for the fixed device allocation.
> > 
> > Pavel, if the USB host is not connected to the gadget, where the
> > playback PCM device fails now ? Is the PCM device created or not ?
> > 
> 
> The gaudio PCM device is created when the gadget function is activated
> (module loaded), regardless whether the USB host is actually connected. The
> playback/capture fails after the blocking read/write times out. The data
> delivery/consumption method is simply not called when no usb requests get
> completed https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L147
> .
> 
> The current code does basically nothing to the alsa pcm stream at
> capture/playback start/stop by the host (called when altsetting changes in
> the gadget) https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L468 https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L557

Thinking about it, I think the current behaviour is probably correct.

It's not 100% possible to detect when the host stops data transfer - we
can detect two scenarios:

	- Cable disconnected
	- Interface alt 0 selected

but it's equally possible to just leave the device configured as it was
and stop sending data.

While resetting state may be necessary when the cable is disconnected,
if the host is just stopping and restarting the stream then I don't see
why the gadget application should have to reconfigure the PCM device.

It's clearly useful to have some indication of host state, but I'm not
at all convinced the PCM state is the best way to provide that.


John

Re: Correct stopping capture and playback substreams?

[John Keeping]

On Mon, Jan 03, 2022 at 01:54:13PM +0100, Pavel Hofman wrote:
> 
> 
> Dne 03. 01. 22 v 13:28 Jaroslav Kysela napsal(a):
> > On 03. 01. 22 13:15, Takashi Iwai wrote:
> > > On Mon, 03 Jan 2022 12:32:53 +0100,
> > > Pavel Hofman wrote:
> > > > 
> > > > 
> > > > 
> > > > Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
> > > > > On 03. 01. 22 9:22, Pavel Hofman wrote:
> > > > > > 
> > > > > > Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> > > > > > > Hi Takashi,
> > > > > > > 
> > > > > > > I am working on stopping alsa streams of audio USB
> > > > > > > gadget when USB host
> > > > > > > stops capture/playback/USB cable unplugged.
> > > > > > > 
> > > > > > > For capture I used code from AK4114 SPDIF receiver
> > > > > > > https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > static void stop_substream(struct uac_rtd_params *prm)
> > > > > > > {
> > > > > > >        unsigned long _flags;
> > > > > > >        struct snd_pcm_substream *substream;
> > > > > > > 
> > > > > > >        substream = prm->ss;
> > > > > > >        if (substream) {
> > > > > > >            snd_pcm_stream_lock_irqsave(substream, _flags);
> > > > > > >            if (snd_pcm_running(substream))
> > > > > > >                // TODO - correct handling for playback substream?
> > > > > > >                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
> > > > > > >            snd_pcm_stream_unlock_irqrestore(substream, _flags);
> > > > > > >        }
> > > > > > > }
> > > > > > > 
> > > > > > > For setup I found calling snd_pcm_stop(substream,
> > > > > > > SNDRV_PCM_STATE_SETUP)
> > > > > > > (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
> > > > > > > 
> > > > > > > 
> > > > > > >     Or for both capture and playback using
> > > > > > > SNDRV_PCM_STATE_DISCONNECTED
> > > > > > > (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> > > > > > > 
> > > > > > > 
> > > > > > > Or perhaps using snd_pcm_dev_disconnect(dev) or
> > > > > > > snd_pcm_drop(substream)?
> > > > > > > 
> > > > > > > Please what is the recommended way?
> > > > > > > 
> > > > > > 
> > > > > > Please can I ask for expert view on this issue? E.g. in
> > > > > > SoX stopping the
> > > > > > stream with
> > > > > > SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not
> > > > > > stop
> > > > > > the application, while with SNDRV_PCM_STATE_DISCONNECTED
> > > > > > SoX exits with
> > > > > > non-recoverable status. I am considering implementing both methods and
> > > > > > letting users choose their suitable snd_pcm_stop operation (none
> > > > > > (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
> > > > > > playback/capture stop, cable disconnection) with a
> > > > > > configfs param. Would
> > > > > > this make sense?
> > > > > 
> > > > > The disconnection state is unrecoverable. It's expected that the
> > > > > device will be freed and cannot be reused.
> > > > > 
> > > > > If you expect to keep the PCM device, we should probably introduce a
> > > > > new function which puts the device to the SNDRV_PCM_STATE_OPEN
> > > > > state. In this state, all I/O routines will return -EBADFD for the
> > > > > applications, so they should close or re-initialize the PCM device
> > > > > completely.
> > > > > 
> > > > > https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
> > > > > 
> > > > > 
> > > > 
> > > > The fact is that after closing the USB host can re-open the device
> > > > with different samplerate (and perhaps later on with different
> > > > channels count/sample size). That would hint at the need to
> > > > re-initialize the gadget side before opening  anyway.
> > > > 
> > > > As of keeping the device - it's likely some use cases would prefer
> > > > keeping the device, to minimize the operations needed to react to the
> > > > host-side playback/capture start.
> > > > 
> > > > A function you describe would make sense for this. IMO from the gadget
> > > > POW there is no difference  between the host stopping playback/capture
> > > > and cable disconnection, in both cases the data stream is stopped and
> > > > next stream can have entirely different parameters. Maybe the gadget
> > > > configfs parameter could only toggle between no action (i.e. current
> > > > situation) and the new alsa function stopping the stream.
> > > > 
> > > > Jaroslav, please can you draft such a function? Perhaps both changes
> > > > could make it to 5.17.
> > > 
> > > (Sorry for the delayed response, as I've been on vacation and now
> > > catching up the huge pile of backlogs...)
> > > 
> > > About the change to keep PCM OPEN state: I'm afraid that the
> > > disconnection in the host side may happen at any time, and keeping the
> > > state OPEN would confuse the things if the host is indeed
> > > unrecoverable.
> > 
> > I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the
> > application (in the PCM_OPEN state) and if the USB bus connection is no
> > longer active, it may fail. We can distinguish between host -> device
> > disconnection and device -> host one. It is not really a similar thing.
> > 
> > I think that the idea was to avoid to re-build the whole card / device
> > structure for the fixed device allocation.
> > 
> > Pavel, if the USB host is not connected to the gadget, where the
> > playback PCM device fails now ? Is the PCM device created or not ?
> > 
> 
> The gaudio PCM device is created when the gadget function is activated
> (module loaded), regardless whether the USB host is actually connected. The
> playback/capture fails after the blocking read/write times out. The data
> delivery/consumption method is simply not called when no usb requests get
> completed https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L147
> .
> 
> The current code does basically nothing to the alsa pcm stream at
> capture/playback start/stop by the host (called when altsetting changes in
> the gadget) https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L468 https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L557

Thinking about it, I think the current behaviour is probably correct.

It's not 100% possible to detect when the host stops data transfer - we
can detect two scenarios:

	- Cable disconnected
	- Interface alt 0 selected

but it's equally possible to just leave the device configured as it was
and stop sending data.

While resetting state may be necessary when the cable is disconnected,
if the host is just stopping and restarting the stream then I don't see
why the gadget application should have to reconfigure the PCM device.

It's clearly useful to have some indication of host state, but I'm not
at all convinced the PCM state is the best way to provide that.


John

Re: Correct stopping capture and playback substreams?

[John Keeping]

On Tue, Jan 04, 2022 at 03:57:09PM +0000, John Keeping wrote:
> On Mon, Jan 03, 2022 at 01:54:13PM +0100, Pavel Hofman wrote:
> > 
> > 
> > Dne 03. 01. 22 v 13:28 Jaroslav Kysela napsal(a):
> > > On 03. 01. 22 13:15, Takashi Iwai wrote:
> > > > On Mon, 03 Jan 2022 12:32:53 +0100,
> > > > Pavel Hofman wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
> > > > > > On 03. 01. 22 9:22, Pavel Hofman wrote:
> > > > > > > 
> > > > > > > Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> > > > > > > > Hi Takashi,
> > > > > > > > 
> > > > > > > > I am working on stopping alsa streams of audio USB
> > > > > > > > gadget when USB host
> > > > > > > > stops capture/playback/USB cable unplugged.
> > > > > > > > 
> > > > > > > > For capture I used code from AK4114 SPDIF receiver
> > > > > > > > https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > static void stop_substream(struct uac_rtd_params *prm)
> > > > > > > > {
> > > > > > > >        unsigned long _flags;
> > > > > > > >        struct snd_pcm_substream *substream;
> > > > > > > > 
> > > > > > > >        substream = prm->ss;
> > > > > > > >        if (substream) {
> > > > > > > >            snd_pcm_stream_lock_irqsave(substream, _flags);
> > > > > > > >            if (snd_pcm_running(substream))
> > > > > > > >                // TODO - correct handling for playback substream?
> > > > > > > >                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
> > > > > > > >            snd_pcm_stream_unlock_irqrestore(substream, _flags);
> > > > > > > >        }
> > > > > > > > }
> > > > > > > > 
> > > > > > > > For setup I found calling snd_pcm_stop(substream,
> > > > > > > > SNDRV_PCM_STATE_SETUP)
> > > > > > > > (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
> > > > > > > > 
> > > > > > > > 
> > > > > > > >     Or for both capture and playback using
> > > > > > > > SNDRV_PCM_STATE_DISCONNECTED
> > > > > > > > (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> > > > > > > > 
> > > > > > > > 
> > > > > > > > Or perhaps using snd_pcm_dev_disconnect(dev) or
> > > > > > > > snd_pcm_drop(substream)?
> > > > > > > > 
> > > > > > > > Please what is the recommended way?
> > > > > > > > 
> > > > > > > 
> > > > > > > Please can I ask for expert view on this issue? E.g. in
> > > > > > > SoX stopping the
> > > > > > > stream with
> > > > > > > SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not
> > > > > > > stop
> > > > > > > the application, while with SNDRV_PCM_STATE_DISCONNECTED
> > > > > > > SoX exits with
> > > > > > > non-recoverable status. I am considering implementing both methods and
> > > > > > > letting users choose their suitable snd_pcm_stop operation (none
> > > > > > > (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
> > > > > > > playback/capture stop, cable disconnection) with a
> > > > > > > configfs param. Would
> > > > > > > this make sense?
> > > > > > 
> > > > > > The disconnection state is unrecoverable. It's expected that the
> > > > > > device will be freed and cannot be reused.
> > > > > > 
> > > > > > If you expect to keep the PCM device, we should probably introduce a
> > > > > > new function which puts the device to the SNDRV_PCM_STATE_OPEN
> > > > > > state. In this state, all I/O routines will return -EBADFD for the
> > > > > > applications, so they should close or re-initialize the PCM device
> > > > > > completely.
> > > > > > 
> > > > > > https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
> > > > > > 
> > > > > > 
> > > > > 
> > > > > The fact is that after closing the USB host can re-open the device
> > > > > with different samplerate (and perhaps later on with different
> > > > > channels count/sample size). That would hint at the need to
> > > > > re-initialize the gadget side before opening  anyway.
> > > > > 
> > > > > As of keeping the device - it's likely some use cases would prefer
> > > > > keeping the device, to minimize the operations needed to react to the
> > > > > host-side playback/capture start.
> > > > > 
> > > > > A function you describe would make sense for this. IMO from the gadget
> > > > > POW there is no difference  between the host stopping playback/capture
> > > > > and cable disconnection, in both cases the data stream is stopped and
> > > > > next stream can have entirely different parameters. Maybe the gadget
> > > > > configfs parameter could only toggle between no action (i.e. current
> > > > > situation) and the new alsa function stopping the stream.
> > > > > 
> > > > > Jaroslav, please can you draft such a function? Perhaps both changes
> > > > > could make it to 5.17.
> > > > 
> > > > (Sorry for the delayed response, as I've been on vacation and now
> > > > catching up the huge pile of backlogs...)
> > > > 
> > > > About the change to keep PCM OPEN state: I'm afraid that the
> > > > disconnection in the host side may happen at any time, and keeping the
> > > > state OPEN would confuse the things if the host is indeed
> > > > unrecoverable.
> > > 
> > > I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the
> > > application (in the PCM_OPEN state) and if the USB bus connection is no
> > > longer active, it may fail. We can distinguish between host -> device
> > > disconnection and device -> host one. It is not really a similar thing.
> > > 
> > > I think that the idea was to avoid to re-build the whole card / device
> > > structure for the fixed device allocation.
> > > 
> > > Pavel, if the USB host is not connected to the gadget, where the
> > > playback PCM device fails now ? Is the PCM device created or not ?
> > > 
> > 
> > The gaudio PCM device is created when the gadget function is activated
> > (module loaded), regardless whether the USB host is actually connected. The
> > playback/capture fails after the blocking read/write times out. The data
> > delivery/consumption method is simply not called when no usb requests get
> > completed https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L147
> > .
> > 
> > The current code does basically nothing to the alsa pcm stream at
> > capture/playback start/stop by the host (called when altsetting changes in
> > the gadget) https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L468 https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L557
> 
> Thinking about it, I think the current behaviour is probably correct.
> 
> It's not 100% possible to detect when the host stops data transfer - we
> can detect two scenarios:
> 
> 	- Cable disconnected
> 	- Interface alt 0 selected
> 
> but it's equally possible to just leave the device configured as it was
> and stop sending data.
> 
> While resetting state may be necessary when the cable is disconnected,
> if the host is just stopping and restarting the stream then I don't see
> why the gadget application should have to reconfigure the PCM device.
> 
> It's clearly useful to have some indication of host state, but I'm not
> at all convinced the PCM state is the best way to provide that.

D'oh, I totally forgot about the case of changing sample rate here,
which is the new feature in Pavel's proposed patches.

I still think the existing behaviour is correct when the sample rate
hasn't changed.  But if the sample rate changes, maybe we have to force
the gadget application to reconfigure the PCM as the available sample
rate is now different.  And I guess changing the PCM state is necessary
in that case.

But I'd really like to avoid it for a gadget with only one available
sample rate (and ideally in the case where a gadget supporting multiple
sample rates is stopped and re-started at the same rate).


John

Re: Correct stopping capture and playback substreams?

[John Keeping]

On Tue, Jan 04, 2022 at 03:57:09PM +0000, John Keeping wrote:
> On Mon, Jan 03, 2022 at 01:54:13PM +0100, Pavel Hofman wrote:
> > 
> > 
> > Dne 03. 01. 22 v 13:28 Jaroslav Kysela napsal(a):
> > > On 03. 01. 22 13:15, Takashi Iwai wrote:
> > > > On Mon, 03 Jan 2022 12:32:53 +0100,
> > > > Pavel Hofman wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
> > > > > > On 03. 01. 22 9:22, Pavel Hofman wrote:
> > > > > > > 
> > > > > > > Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
> > > > > > > > Hi Takashi,
> > > > > > > > 
> > > > > > > > I am working on stopping alsa streams of audio USB
> > > > > > > > gadget when USB host
> > > > > > > > stops capture/playback/USB cable unplugged.
> > > > > > > > 
> > > > > > > > For capture I used code from AK4114 SPDIF receiver
> > > > > > > > https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > static void stop_substream(struct uac_rtd_params *prm)
> > > > > > > > {
> > > > > > > >        unsigned long _flags;
> > > > > > > >        struct snd_pcm_substream *substream;
> > > > > > > > 
> > > > > > > >        substream = prm->ss;
> > > > > > > >        if (substream) {
> > > > > > > >            snd_pcm_stream_lock_irqsave(substream, _flags);
> > > > > > > >            if (snd_pcm_running(substream))
> > > > > > > >                // TODO - correct handling for playback substream?
> > > > > > > >                snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
> > > > > > > >            snd_pcm_stream_unlock_irqrestore(substream, _flags);
> > > > > > > >        }
> > > > > > > > }
> > > > > > > > 
> > > > > > > > For setup I found calling snd_pcm_stop(substream,
> > > > > > > > SNDRV_PCM_STATE_SETUP)
> > > > > > > > (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
> > > > > > > > 
> > > > > > > > 
> > > > > > > >     Or for both capture and playback using
> > > > > > > > SNDRV_PCM_STATE_DISCONNECTED
> > > > > > > > (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
> > > > > > > > 
> > > > > > > > 
> > > > > > > > Or perhaps using snd_pcm_dev_disconnect(dev) or
> > > > > > > > snd_pcm_drop(substream)?
> > > > > > > > 
> > > > > > > > Please what is the recommended way?
> > > > > > > > 
> > > > > > > 
> > > > > > > Please can I ask for expert view on this issue? E.g. in
> > > > > > > SoX stopping the
> > > > > > > stream with
> > > > > > > SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not
> > > > > > > stop
> > > > > > > the application, while with SNDRV_PCM_STATE_DISCONNECTED
> > > > > > > SoX exits with
> > > > > > > non-recoverable status. I am considering implementing both methods and
> > > > > > > letting users choose their suitable snd_pcm_stop operation (none
> > > > > > > (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
> > > > > > > playback/capture stop, cable disconnection) with a
> > > > > > > configfs param. Would
> > > > > > > this make sense?
> > > > > > 
> > > > > > The disconnection state is unrecoverable. It's expected that the
> > > > > > device will be freed and cannot be reused.
> > > > > > 
> > > > > > If you expect to keep the PCM device, we should probably introduce a
> > > > > > new function which puts the device to the SNDRV_PCM_STATE_OPEN
> > > > > > state. In this state, all I/O routines will return -EBADFD for the
> > > > > > applications, so they should close or re-initialize the PCM device
> > > > > > completely.
> > > > > > 
> > > > > > https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
> > > > > > 
> > > > > > 
> > > > > 
> > > > > The fact is that after closing the USB host can re-open the device
> > > > > with different samplerate (and perhaps later on with different
> > > > > channels count/sample size). That would hint at the need to
> > > > > re-initialize the gadget side before opening  anyway.
> > > > > 
> > > > > As of keeping the device - it's likely some use cases would prefer
> > > > > keeping the device, to minimize the operations needed to react to the
> > > > > host-side playback/capture start.
> > > > > 
> > > > > A function you describe would make sense for this. IMO from the gadget
> > > > > POW there is no difference  between the host stopping playback/capture
> > > > > and cable disconnection, in both cases the data stream is stopped and
> > > > > next stream can have entirely different parameters. Maybe the gadget
> > > > > configfs parameter could only toggle between no action (i.e. current
> > > > > situation) and the new alsa function stopping the stream.
> > > > > 
> > > > > Jaroslav, please can you draft such a function? Perhaps both changes
> > > > > could make it to 5.17.
> > > > 
> > > > (Sorry for the delayed response, as I've been on vacation and now
> > > > catching up the huge pile of backlogs...)
> > > > 
> > > > About the change to keep PCM OPEN state: I'm afraid that the
> > > > disconnection in the host side may happen at any time, and keeping the
> > > > state OPEN would confuse the things if the host is indeed
> > > > unrecoverable.
> > > 
> > > I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the
> > > application (in the PCM_OPEN state) and if the USB bus connection is no
> > > longer active, it may fail. We can distinguish between host -> device
> > > disconnection and device -> host one. It is not really a similar thing.
> > > 
> > > I think that the idea was to avoid to re-build the whole card / device
> > > structure for the fixed device allocation.
> > > 
> > > Pavel, if the USB host is not connected to the gadget, where the
> > > playback PCM device fails now ? Is the PCM device created or not ?
> > > 
> > 
> > The gaudio PCM device is created when the gadget function is activated
> > (module loaded), regardless whether the USB host is actually connected. The
> > playback/capture fails after the blocking read/write times out. The data
> > delivery/consumption method is simply not called when no usb requests get
> > completed https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L147
> > .
> > 
> > The current code does basically nothing to the alsa pcm stream at
> > capture/playback start/stop by the host (called when altsetting changes in
> > the gadget) https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L468 https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L557
> 
> Thinking about it, I think the current behaviour is probably correct.
> 
> It's not 100% possible to detect when the host stops data transfer - we
> can detect two scenarios:
> 
> 	- Cable disconnected
> 	- Interface alt 0 selected
> 
> but it's equally possible to just leave the device configured as it was
> and stop sending data.
> 
> While resetting state may be necessary when the cable is disconnected,
> if the host is just stopping and restarting the stream then I don't see
> why the gadget application should have to reconfigure the PCM device.
> 
> It's clearly useful to have some indication of host state, but I'm not
> at all convinced the PCM state is the best way to provide that.

D'oh, I totally forgot about the case of changing sample rate here,
which is the new feature in Pavel's proposed patches.

I still think the existing behaviour is correct when the sample rate
hasn't changed.  But if the sample rate changes, maybe we have to force
the gadget application to reconfigure the PCM as the available sample
rate is now different.  And I guess changing the PCM state is necessary
in that case.

But I'd really like to avoid it for a gadget with only one available
sample rate (and ideally in the case where a gadget supporting multiple
sample rates is stopped and re-started at the same rate).


John

Re: Correct stopping capture and playback substreams?

[Pavel Hofman]

Dne 04. 01. 22 v 17:21 John Keeping napsal(a):
> On Tue, Jan 04, 2022 at 03:57:09PM +0000, John Keeping wrote:
>> On Mon, Jan 03, 2022 at 01:54:13PM +0100, Pavel Hofman wrote:
>>>
>>>
>>> Dne 03. 01. 22 v 13:28 Jaroslav Kysela napsal(a):
>>>> On 03. 01. 22 13:15, Takashi Iwai wrote:
>>>>> On Mon, 03 Jan 2022 12:32:53 +0100,
>>>>> Pavel Hofman wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
>>>>>>> On 03. 01. 22 9:22, Pavel Hofman wrote:
>>>>>>>>
>>>>>>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>>>>>>>>> Hi Takashi,
>>>>>>>>>
>>>>>>>>> I am working on stopping alsa streams of audio USB
>>>>>>>>> gadget when USB host
>>>>>>>>> stops capture/playback/USB cable unplugged.
>>>>>>>>>
>>>>>>>>> For capture I used code from AK4114 SPDIF receiver
>>>>>>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> static void stop_substream(struct uac_rtd_params *prm)
>>>>>>>>> {
>>>>>>>>>         unsigned long _flags;
>>>>>>>>>         struct snd_pcm_substream *substream;
>>>>>>>>>
>>>>>>>>>         substream = prm->ss;
>>>>>>>>>         if (substream) {
>>>>>>>>>             snd_pcm_stream_lock_irqsave(substream, _flags);
>>>>>>>>>             if (snd_pcm_running(substream))
>>>>>>>>>                 // TODO - correct handling for playback substream?
>>>>>>>>>                 snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>>>>>>>>             snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>>>>>>>>         }
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> For setup I found calling snd_pcm_stop(substream,
>>>>>>>>> SNDRV_PCM_STATE_SETUP)
>>>>>>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>      Or for both capture and playback using
>>>>>>>>> SNDRV_PCM_STATE_DISCONNECTED
>>>>>>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or
>>>>>>>>> snd_pcm_drop(substream)?
>>>>>>>>>
>>>>>>>>> Please what is the recommended way?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Please can I ask for expert view on this issue? E.g. in
>>>>>>>> SoX stopping the
>>>>>>>> stream with
>>>>>>>> SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not
>>>>>>>> stop
>>>>>>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED
>>>>>>>> SoX exits with
>>>>>>>> non-recoverable status. I am considering implementing both methods and
>>>>>>>> letting users choose their suitable snd_pcm_stop operation (none
>>>>>>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
>>>>>>>> playback/capture stop, cable disconnection) with a
>>>>>>>> configfs param. Would
>>>>>>>> this make sense?
>>>>>>>
>>>>>>> The disconnection state is unrecoverable. It's expected that the
>>>>>>> device will be freed and cannot be reused.
>>>>>>>
>>>>>>> If you expect to keep the PCM device, we should probably introduce a
>>>>>>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
>>>>>>> state. In this state, all I/O routines will return -EBADFD for the
>>>>>>> applications, so they should close or re-initialize the PCM device
>>>>>>> completely.
>>>>>>>
>>>>>>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> The fact is that after closing the USB host can re-open the device
>>>>>> with different samplerate (and perhaps later on with different
>>>>>> channels count/sample size). That would hint at the need to
>>>>>> re-initialize the gadget side before opening  anyway.
>>>>>>
>>>>>> As of keeping the device - it's likely some use cases would prefer
>>>>>> keeping the device, to minimize the operations needed to react to the
>>>>>> host-side playback/capture start.
>>>>>>
>>>>>> A function you describe would make sense for this. IMO from the gadget
>>>>>> POW there is no difference  between the host stopping playback/capture
>>>>>> and cable disconnection, in both cases the data stream is stopped and
>>>>>> next stream can have entirely different parameters. Maybe the gadget
>>>>>> configfs parameter could only toggle between no action (i.e. current
>>>>>> situation) and the new alsa function stopping the stream.
>>>>>>
>>>>>> Jaroslav, please can you draft such a function? Perhaps both changes
>>>>>> could make it to 5.17.
>>>>>
>>>>> (Sorry for the delayed response, as I've been on vacation and now
>>>>> catching up the huge pile of backlogs...)
>>>>>
>>>>> About the change to keep PCM OPEN state: I'm afraid that the
>>>>> disconnection in the host side may happen at any time, and keeping the
>>>>> state OPEN would confuse the things if the host is indeed
>>>>> unrecoverable.
>>>>
>>>> I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the
>>>> application (in the PCM_OPEN state) and if the USB bus connection is no
>>>> longer active, it may fail. We can distinguish between host -> device
>>>> disconnection and device -> host one. It is not really a similar thing.
>>>>
>>>> I think that the idea was to avoid to re-build the whole card / device
>>>> structure for the fixed device allocation.
>>>>
>>>> Pavel, if the USB host is not connected to the gadget, where the
>>>> playback PCM device fails now ? Is the PCM device created or not ?
>>>>
>>>
>>> The gaudio PCM device is created when the gadget function is activated
>>> (module loaded), regardless whether the USB host is actually connected. The
>>> playback/capture fails after the blocking read/write times out. The data
>>> delivery/consumption method is simply not called when no usb requests get
>>> completed https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L147
>>> .
>>>
>>> The current code does basically nothing to the alsa pcm stream at
>>> capture/playback start/stop by the host (called when altsetting changes in
>>> the gadget) https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L468 https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L557
>>
>> Thinking about it, I think the current behaviour is probably correct.
>>
>> It's not 100% possible to detect when the host stops data transfer - we
>> can detect two scenarios:
>>
>> 	- Cable disconnected
>> 	- Interface alt 0 selected
>>
>> but it's equally possible to just leave the device configured as it was
>> and stop sending data.
>>
>> While resetting state may be necessary when the cable is disconnected,
>> if the host is just stopping and restarting the stream then I don't see
>> why the gadget application should have to reconfigure the PCM device.
>>
>> It's clearly useful to have some indication of host state, but I'm not
>> at all convinced the PCM state is the best way to provide that.
> 
> D'oh, I totally forgot about the case of changing sample rate here,
> which is the new feature in Pavel's proposed patches.
> 
> I still think the existing behaviour is correct when the sample rate
> hasn't changed.  But if the sample rate changes, maybe we have to force
> the gadget application to reconfigure the PCM as the available sample
> rate is now different.  And I guess changing the PCM state is necessary
> in that case.
> 
> But I'd really like to avoid it for a gadget with only one available
> sample rate (and ideally in the case where a gadget supporting multiple
> sample rates is stopped and re-started at the same rate).
> 

IMO there are several points:

1) Breaking the blocking wait in alsa read/write as soon as the gadget 
learns no data will arrive/be consumed by the host (altsetting to zero, 
cable disconnected). For me this is important as I do not want the app 
thread to be stuck waiting, unable to exit and especially to restart 
quickly (with new samplerate config).

2) Deciding what PCM state to set within the PCM stop call.
For the single-samplerate config we know the next open will use the same 
rate, therefore the less "intrusive" status/call can be used. But for 
multiple samplerates I do not know what samplerate the host will use for 
the next opening. Also we should consider that future patches can add 
multiple altsettings with different channel counts (and also 
corresponding different samplerate sets to fit the bandwidth).


3) Compatibility with the existing behaviour for existing installations.
I think the feature requires a dedicated configfs param. The param 
should at least disallow the PCM stop (as default). Perhaps a bitmask 
could be used to configure USB host stop (altsetting -> zero) and cable 
disconnection events separately. Maybe we could assign multiple bits to 
each event to allow configuring more reactions to each event 
(0000=default - no PCM stop, 0001 - no-reconfig PCM status, 0010 - 
reconfig PCM status, 0011 - DISCONNECTED status, etc.)

That way different requirements could be catered for.

I can prepare all currently-relevant patches should they be found 
heading in the right direction.

Thanks a lot,

Pavel.

Re: Correct stopping capture and playback substreams?

[Pavel Hofman]

Dne 04. 01. 22 v 17:21 John Keeping napsal(a):
> On Tue, Jan 04, 2022 at 03:57:09PM +0000, John Keeping wrote:
>> On Mon, Jan 03, 2022 at 01:54:13PM +0100, Pavel Hofman wrote:
>>>
>>>
>>> Dne 03. 01. 22 v 13:28 Jaroslav Kysela napsal(a):
>>>> On 03. 01. 22 13:15, Takashi Iwai wrote:
>>>>> On Mon, 03 Jan 2022 12:32:53 +0100,
>>>>> Pavel Hofman wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Dne 03. 01. 22 v 10:10 Jaroslav Kysela napsal(a):
>>>>>>> On 03. 01. 22 9:22, Pavel Hofman wrote:
>>>>>>>>
>>>>>>>> Dne 23. 12. 21 v 9:18 Pavel Hofman napsal(a):
>>>>>>>>> Hi Takashi,
>>>>>>>>>
>>>>>>>>> I am working on stopping alsa streams of audio USB
>>>>>>>>> gadget when USB host
>>>>>>>>> stops capture/playback/USB cable unplugged.
>>>>>>>>>
>>>>>>>>> For capture I used code from AK4114 SPDIF receiver
>>>>>>>>> https://elixir.bootlin.com/linux/latest/source/sound/i2c/other/ak4114.c#L590:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> static void stop_substream(struct uac_rtd_params *prm)
>>>>>>>>> {
>>>>>>>>>         unsigned long _flags;
>>>>>>>>>         struct snd_pcm_substream *substream;
>>>>>>>>>
>>>>>>>>>         substream = prm->ss;
>>>>>>>>>         if (substream) {
>>>>>>>>>             snd_pcm_stream_lock_irqsave(substream, _flags);
>>>>>>>>>             if (snd_pcm_running(substream))
>>>>>>>>>                 // TODO - correct handling for playback substream?
>>>>>>>>>                 snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
>>>>>>>>>             snd_pcm_stream_unlock_irqrestore(substream, _flags);
>>>>>>>>>         }
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> For setup I found calling snd_pcm_stop(substream,
>>>>>>>>> SNDRV_PCM_STATE_SETUP)
>>>>>>>>> (https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c#L63)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>      Or for both capture and playback using
>>>>>>>>> SNDRV_PCM_STATE_DISCONNECTED
>>>>>>>>> (https://elixir.bootlin.com/linux/latest/source/sound/core/pcm.c#L1103).
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Or perhaps using snd_pcm_dev_disconnect(dev) or
>>>>>>>>> snd_pcm_drop(substream)?
>>>>>>>>>
>>>>>>>>> Please what is the recommended way?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Please can I ask for expert view on this issue? E.g. in
>>>>>>>> SoX stopping the
>>>>>>>> stream with
>>>>>>>> SNDRV_PCM_STATE_SETUP/SNDRV_PCM_STATE_DRAINING does not
>>>>>>>> stop
>>>>>>>> the application, while with SNDRV_PCM_STATE_DISCONNECTED
>>>>>>>> SoX exits with
>>>>>>>> non-recoverable status. I am considering implementing both methods and
>>>>>>>> letting users choose their suitable snd_pcm_stop operation (none
>>>>>>>> (default)/SETUP-DRAINING/DISCONNECTED) for the two events (host
>>>>>>>> playback/capture stop, cable disconnection) with a
>>>>>>>> configfs param. Would
>>>>>>>> this make sense?
>>>>>>>
>>>>>>> The disconnection state is unrecoverable. It's expected that the
>>>>>>> device will be freed and cannot be reused.
>>>>>>>
>>>>>>> If you expect to keep the PCM device, we should probably introduce a
>>>>>>> new function which puts the device to the SNDRV_PCM_STATE_OPEN
>>>>>>> state. In this state, all I/O routines will return -EBADFD for the
>>>>>>> applications, so they should close or re-initialize the PCM device
>>>>>>> completely.
>>>>>>>
>>>>>>> https://elixir.bootlin.com/linux/latest/source/sound/core/pcm_native.c#L794
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> The fact is that after closing the USB host can re-open the device
>>>>>> with different samplerate (and perhaps later on with different
>>>>>> channels count/sample size). That would hint at the need to
>>>>>> re-initialize the gadget side before opening  anyway.
>>>>>>
>>>>>> As of keeping the device - it's likely some use cases would prefer
>>>>>> keeping the device, to minimize the operations needed to react to the
>>>>>> host-side playback/capture start.
>>>>>>
>>>>>> A function you describe would make sense for this. IMO from the gadget
>>>>>> POW there is no difference  between the host stopping playback/capture
>>>>>> and cable disconnection, in both cases the data stream is stopped and
>>>>>> next stream can have entirely different parameters. Maybe the gadget
>>>>>> configfs parameter could only toggle between no action (i.e. current
>>>>>> situation) and the new alsa function stopping the stream.
>>>>>>
>>>>>> Jaroslav, please can you draft such a function? Perhaps both changes
>>>>>> could make it to 5.17.
>>>>>
>>>>> (Sorry for the delayed response, as I've been on vacation and now
>>>>> catching up the huge pile of backlogs...)
>>>>>
>>>>> About the change to keep PCM OPEN state: I'm afraid that the
>>>>> disconnection in the host side may happen at any time, and keeping the
>>>>> state OPEN would confuse the things if the host is indeed
>>>>> unrecoverable.
>>>>
>>>> I don't think so. The SNDRV_PCM_IOCTL_HW_PARAMS must be issued by the
>>>> application (in the PCM_OPEN state) and if the USB bus connection is no
>>>> longer active, it may fail. We can distinguish between host -> device
>>>> disconnection and device -> host one. It is not really a similar thing.
>>>>
>>>> I think that the idea was to avoid to re-build the whole card / device
>>>> structure for the fixed device allocation.
>>>>
>>>> Pavel, if the USB host is not connected to the gadget, where the
>>>> playback PCM device fails now ? Is the PCM device created or not ?
>>>>
>>>
>>> The gaudio PCM device is created when the gadget function is activated
>>> (module loaded), regardless whether the USB host is actually connected. The
>>> playback/capture fails after the blocking read/write times out. The data
>>> delivery/consumption method is simply not called when no usb requests get
>>> completed https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L147
>>> .
>>>
>>> The current code does basically nothing to the alsa pcm stream at
>>> capture/playback start/stop by the host (called when altsetting changes in
>>> the gadget) https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L468 https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/function/u_audio.c#L557
>>
>> Thinking about it, I think the current behaviour is probably correct.
>>
>> It's not 100% possible to detect when the host stops data transfer - we
>> can detect two scenarios:
>>
>> 	- Cable disconnected
>> 	- Interface alt 0 selected
>>
>> but it's equally possible to just leave the device configured as it was
>> and stop sending data.
>>
>> While resetting state may be necessary when the cable is disconnected,
>> if the host is just stopping and restarting the stream then I don't see
>> why the gadget application should have to reconfigure the PCM device.
>>
>> It's clearly useful to have some indication of host state, but I'm not
>> at all convinced the PCM state is the best way to provide that.
> 
> D'oh, I totally forgot about the case of changing sample rate here,
> which is the new feature in Pavel's proposed patches.
> 
> I still think the existing behaviour is correct when the sample rate
> hasn't changed.  But if the sample rate changes, maybe we have to force
> the gadget application to reconfigure the PCM as the available sample
> rate is now different.  And I guess changing the PCM state is necessary
> in that case.
> 
> But I'd really like to avoid it for a gadget with only one available
> sample rate (and ideally in the case where a gadget supporting multiple
> sample rates is stopped and re-started at the same rate).
> 

IMO there are several points:

1) Breaking the blocking wait in alsa read/write as soon as the gadget 
learns no data will arrive/be consumed by the host (altsetting to zero, 
cable disconnected). For me this is important as I do not want the app 
thread to be stuck waiting, unable to exit and especially to restart 
quickly (with new samplerate config).

2) Deciding what PCM state to set within the PCM stop call.
For the single-samplerate config we know the next open will use the same 
rate, therefore the less "intrusive" status/call can be used. But for 
multiple samplerates I do not know what samplerate the host will use for 
the next opening. Also we should consider that future patches can add 
multiple altsettings with different channel counts (and also 
corresponding different samplerate sets to fit the bandwidth).


3) Compatibility with the existing behaviour for existing installations.
I think the feature requires a dedicated configfs param. The param 
should at least disallow the PCM stop (as default). Perhaps a bitmask 
could be used to configure USB host stop (altsetting -> zero) and cable 
disconnection events separately. Maybe we could assign multiple bits to 
each event to allow configuring more reactions to each event 
(0000=default - no PCM stop, 0001 - no-reconfig PCM status, 0010 - 
reconfig PCM status, 0011 - DISCONNECTED status, etc.)

That way different requirements could be catered for.

I can prepare all currently-relevant patches should they be found 
heading in the right direction.

Thanks a lot,

Pavel.