Re: [RFC PATCH 2/2] ALSA: pcm: refresh capture pointers before drain

[Takashi Iwai <tiwai@suse.de>]

On Wed, 14 Oct 2026 10:15:26 +0200,
Takashi Iwai wrote:
> 
> On Wed, 14 Oct 2026 05:30:57 +0200,
> Joonho Lee wrote:
> > 
> > Refresh active capture pointers before the drain transition so the final
> > captured tail is accounted before reads finish.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
> > ---
> >  sound/core/pcm_native.c | 32 +++++++++++++++++++++++++-------
> >  1 file changed, 25 insertions(+), 7 deletions(-)
> > 
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index defbb2977efe..622ae7be2d04 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -2176,14 +2176,32 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
> >  	} else if (substream->f_flags & O_NONBLOCK)
> >  		nonblock = 1;
> >  
> > -	snd_pcm_stream_lock_irq(substream);
> > -	/* resume pause */
> > -	if (runtime->state == SNDRV_PCM_STATE_PAUSED)
> > -		snd_pcm_pause(substream, false);
> > +	/* Keep linked runtimes alive until the initial drain action completes. */
> > +	scoped_guard(rwsem_read, &snd_pcm_link_rwsem) {
> > +		snd_pcm_stream_lock_irq(substream);
> > +		/* Resume before refreshing capture positions. */
> > +		if (runtime->state == SNDRV_PCM_STATE_PAUSED)
> > +			snd_pcm_pause(substream, false);
> > +		snd_pcm_stream_unlock_irq(substream);
> > +
> > +		/* A pointer error may stop the group: hold only this member's lock. */
> > +		snd_pcm_group_for_each_entry(s, substream) {
> > +			if (s->stream != SNDRV_PCM_STREAM_CAPTURE)
> > +				continue;
> > +			scoped_guard(pcm_stream_lock_irq, s) {
> > +				if (s->runtime->state == SNDRV_PCM_STATE_RUNNING)
> > +					result = snd_pcm_update_hw_ptr(s);
> > +			}
> > +			if (result < 0)
> > +				break;
> > +		}
> >  
> > -	/* pre-start/stop - all running streams are changed to DRAINING state */
> > -	result = snd_pcm_action(&snd_pcm_action_drain_init, substream,
> > -				ACTION_ARG_IGNORE);
> > +		snd_pcm_stream_lock_irq(substream);
> > +		/* Change the stable group to DRAINING only after all refreshes. */
> > +		if (result >= 0)
> > +			result = snd_pcm_action(&snd_pcm_action_drain_init,
> > +						substream, ACTION_ARG_IGNORE);
> > +	}
> >  	if (result < 0)
> >  		goto unlock;
> >  	/* in non-blocking, we don't wait in ioctl but let caller poll */
> 
> IIUC, basically what here we do is to just call
> snd_pcm_update_hw_ptr() for capture at drain init phase, so calling it
> inside snd_pcm_do_drain_init() would make more sense, I suppose.
> 
> But, the problem is that snd_pcm_update_hw_ptr() itself takes an
> action to stop the stream for XRUN.  So we'd need to fiddle along with 
> it, too, as something like below.  (Maybe we need more brush-ups.)

On the second thought, it should be fine to call snd_pcm_stop() & co
from that context, too.  Maybe a small change like below suffices.


Takashi

-- 8< --
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2119,8 +2119,11 @@ static int snd_pcm_do_drain_init(struct snd_pcm_substream *substream,
 		if (runtime->state == SNDRV_PCM_STATE_RUNNING) {
 			snd_pcm_state_t new_state;
 
-			new_state = snd_pcm_capture_avail(runtime) > 0 ?
-				SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP;
+			if (snd_pcm_update_hw_ptr(substream) < 0)
+				new_state = SNDRV_PCM_STATE_SETUP;
+			else
+				new_state = snd_pcm_capture_avail(runtime) > 0 ?
+					SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP;
 			snd_pcm_do_stop(substream, new_state);
 			snd_pcm_post_stop(substream, new_state);
 		}