Re: [PATCH] ALSA: pcm: update state after xrun

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ALSA: pcm: stale trigger_tstamp in snd_pcm_status64() after xrun
--

commit 0ae9f8a317c199a098237f1b0f0b5ebbc84be3f4
Author: Mikael Magnusson <mikachu@gmail.com>

ALSA: pcm: update state after xrun

This commit updates status->state and status->suspended_state in
snd_pcm_status64() after calling snd_pcm_update_hw_ptr(), so userspace
receives the updated state if snd_pcm_update_hw_ptr() triggers an xrun.

> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 35152fae74ca2..62a2bea043e38 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -1132,6 +1132,9 @@ int snd_pcm_status64(struct snd_pcm_substream *substream,
>  	status->trigger_tstamp_nsec = runtime->trigger_tstamp.tv_nsec;
>  	if (snd_pcm_running(substream)) {
>  		snd_pcm_update_hw_ptr(substream);
> +		/* update_hw_ptr may have put us in xrun state, update state */
> +		status->state = runtime->state;
> +		status->suspended_state = runtime->suspended_state;

[Severity: Medium]
Should status->trigger_tstamp_sec and status->trigger_tstamp_nsec also be
updated after snd_pcm_update_hw_ptr()?

In sound/core/pcm_native.c, snd_pcm_status64() copies runtime->trigger_tstamp
into status->trigger_tstamp_sec and status->trigger_tstamp_nsec before calling
snd_pcm_update_hw_ptr(substream):

sound/core/pcm_native.c:snd_pcm_status64()
	status->trigger_tstamp_sec = runtime->trigger_tstamp.tv_sec;
	status->trigger_tstamp_nsec = runtime->trigger_tstamp.tv_nsec;
	if (snd_pcm_running(substream)) {
		snd_pcm_update_hw_ptr(substream);

When snd_pcm_update_hw_ptr() stops a running stream due to an xrun via
__snd_pcm_xrun() -> snd_pcm_stop() or drain completion via
snd_pcm_drain_done(), snd_pcm_post_stop() updates runtime->trigger_tstamp to
the stop timestamp alongside runtime->state:

sound/core/pcm_native.c:snd_pcm_post_stop()
	if (runtime->state != state) {
		snd_pcm_trigger_tstamp(substream);
		__snd_pcm_set_state(runtime, state);
		snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MSTOP);
	}

Refreshing only status->state and status->suspended_state leaves
status->trigger_tstamp_sec and status->trigger_tstamp_nsec holding the old
start timestamp while reporting the new stopped or xrun state to userspace.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027131233.17692-1-mikachu@gmail.com?part=1