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

[Takashi Iwai <tiwai@suse.de>]

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.)


thanks,

Takashi

-- 8< --
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 4d665b4148d7..a1b19d558234 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -206,8 +206,9 @@ void __snd_pcm_xrun(struct snd_pcm_substream *substream)
 
 #endif
 
-int snd_pcm_update_state(struct snd_pcm_substream *substream,
-			 struct snd_pcm_runtime *runtime)
+static int do_pcm_update_state(struct snd_pcm_substream *substream,
+			       struct snd_pcm_runtime *runtime,
+			       bool notify)
 {
 	snd_pcm_uframes_t avail;
 
@@ -216,12 +217,14 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream,
 		runtime->avail_max = avail;
 	if (runtime->state == SNDRV_PCM_STATE_DRAINING) {
 		if (avail >= runtime->buffer_size) {
-			snd_pcm_drain_done(substream);
+			if (notify)
+				snd_pcm_drain_done(substream);
 			return -EPIPE;
 		}
 	} else {
 		if (avail >= runtime->stop_threshold) {
-			__snd_pcm_xrun(substream);
+			if (notify)
+				__snd_pcm_xrun(substream);
 			return -EPIPE;
 		}
 	}
@@ -233,6 +236,12 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+int snd_pcm_update_state(struct snd_pcm_substream *substream,
+			 struct snd_pcm_runtime *runtime)
+{
+	return do_pcm_update_state(substream, runtime, true);
+}
+
 static void update_audio_tstamp(struct snd_pcm_substream *substream,
 				struct timespec64 *curr_tstamp,
 				struct timespec64 *audio_tstamp)
@@ -283,8 +292,8 @@ static void update_audio_tstamp(struct snd_pcm_substream *substream,
 	runtime->driver_tstamp = driver_tstamp;
 }
 
-static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
-				  unsigned int in_interrupt)
+int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
+			   bool in_interrupt, bool notify)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	snd_pcm_uframes_t pos;
@@ -490,13 +499,13 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 
 	update_audio_tstamp(substream, &curr_tstamp, &audio_tstamp);
 
-	return snd_pcm_update_state(substream, runtime);
+	return do_pcm_update_state(substream, runtime, notify);
 }
 
 /* CAUTION: call it with irq disabled */
 int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream)
 {
-	return snd_pcm_update_hw_ptr0(substream, 0);
+	return snd_pcm_update_hw_ptr0(substream, false, true);
 }
 
 /**
@@ -1906,7 +1915,7 @@ void snd_pcm_period_elapsed_under_stream_lock(struct snd_pcm_substream *substrea
 	runtime = substream->runtime;
 
 	if (!snd_pcm_running(substream) ||
-	    snd_pcm_update_hw_ptr0(substream, 1) < 0)
+	    snd_pcm_update_hw_ptr0(substream, true, true) < 0)
 		goto _end;
 
 #ifdef CONFIG_SND_PCM_TIMER
diff --git a/sound/core/pcm_local.h b/sound/core/pcm_local.h
index ecb21697ae3a..12bb6e75329a 100644
--- a/sound/core/pcm_local.h
+++ b/sound/core/pcm_local.h
@@ -28,6 +28,8 @@ int pcm_lib_apply_appl_ptr(struct snd_pcm_substream *substream,
 int snd_pcm_update_state(struct snd_pcm_substream *substream,
 			 struct snd_pcm_runtime *runtime);
 int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream);
+int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
+			   bool in_interrupt, bool notify);
 
 void snd_pcm_playback_silence(struct snd_pcm_substream *substream,
 			      snd_pcm_uframes_t new_hw_ptr);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index a845f242a163..76983e5f4d4e 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2094,6 +2094,8 @@ static int snd_pcm_do_drain_init(struct snd_pcm_substream *substream,
 				 snd_pcm_state_t state)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
+	int err;
+
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		switch (runtime->state) {
 		case SNDRV_PCM_STATE_PREPARED:
@@ -2119,8 +2121,12 @@ 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;
+			err = snd_pcm_update_hw_ptr0(substream, false, false);
+			if (err < 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);
 		}