Re: [PATCH] ALSA: pcm: Release paused streams before suspend if resume is not supported

[Takashi Iwai <tiwai@suse.de>]

On Wed, 02 Apr 2025 11:28:55 +0200,
Péter Ujfalusi wrote:
> 
> 
> On 02/04/2025 11:52, Jaroslav Kysela wrote:
> >>>> Actually, this patch follows the same pattern, too.  It calls
> >>>> snd_pcm_pause(false) to set the state to RUNNING again, then proceed
> >>>> to the suspend action immediately that sets to SUSPENDED.
> >>>
> >>> The previous state (suspended_state) will be confusing from the
> >>> application with the proposed patch, because there will be RUNNING
> >>> state instead PAUSED. This previous state is exported via API.
> >>
> >> No, it won't happen.  The condition of the new behavior is only when
> >> SNDRV_PCM_INFO_RESUME isn't set -- that is, no resume is possible,
> >> hence no state recovery happens after resume.  From the application
> >> POV, it won't change.
> > 
> > The suspended_state can be obtained in snd_pcm_status64(). With Peter's
> > patch, there will be RUNNING instead PAUSED, don't?
> 
> Yes, that is true, but it does not matter.
> if the SNDRV_PCM_INFO_RESUME is not set, on resume nothing is going to
> be done, the state remains SUSPENDED and the suspended_substate also
> retained, so:
> 
> The stream was in RUNNING, after suspend:
> state = SUSPENDED, suspended_state = RUNNING
> After resume:
> state = SUSPENDED, suspended_state = RUNNING
> The the stream goes to xrun and got restarted.
> 
> The stream was in RUNNING, after suspend:
> Without my patch / with my patch:
> state = SUSPENDED, suspended_state = PAUSED / RUNNING
> After resume:
> state = SUSPENDED, suspended_state = RUNNING / RUNNING
> 
> The stream remains "not running" as resume is not supported and
> application knows that it had left the stream paused.
> 
> When it tries to PAUSE_RELEASE the stream, in both case the core will
> look at the state, which is SUSPENDED and thus the pause release fails
> (it is not in paused state), so in both cases we go under a restart
> (suspended_state is reset, ignored).

Well, the suspended_state itself is set no matter whether the resume
is supported or not.  The difference is that it's evaluated in the
driver only by the suspended state, but it's free for user-space to
read this out from the output of snd_pcm_status ioctl.
How seriously it needs to be a different question, but there is a
possibility of breakage.

Though, looking through the code, the suspended_state isn't updated at
all except for the suspend, and it means this can keep a bogus state,
too.  We should fix the code to clear the suspended_state at init
phase.  It's another corner case.


thanks,

Takashi