[PATCH 0/5] rewrite snd_pcm_playback_silence() again

[PATCH 0/5] rewrite snd_pcm_playback_silence() again

[Jaroslav Kysela]

This is a split of changes for both patches (the first with the missing
top-up mode + later fix with mixups) proposed by Oswald. The aim was to
pick only real code changes.

Only the first two patches after revert fixes the current silencing issues.
The last two are just cleanups with the extra optimization in the last patch
moving the common code to a function.

Cc: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Cc: Jeff Chua <jeff.chua.linux@gmail.com>

Jaroslav Kysela (5):
  ALSA: pcm: Revert "ALSA: pcm: rewrite snd_pcm_playback_silence()"
  ALSA: pcm: fix playback silence - use the actual new_hw_ptr for the
    threshold mode
  ALSA: pcm: fix playback silence - correct the incremental silencing
  ALSA: pcm: playback silence - remove extra code
  ALSA: pcm: playback silence - move silence variables updates to
    separate function

 sound/core/pcm_lib.c    | 91 +++++++++++++++++++++++++----------------
 sound/core/pcm_local.h  |  3 +-
 sound/core/pcm_native.c |  6 +--
 3 files changed, 61 insertions(+), 39 deletions(-)

-- 
2.39.2

[PATCH 1/5] ALSA: pcm: Revert "ALSA: pcm: rewrite snd_pcm_playback_silence()"

[Jaroslav Kysela]

This reverts commit 9f656705c5faa18afb26d922cfc64f9fd103c38d.

There is a regression (the top-up mode). Unfortunately, the patch
provided from the author of this commit is not easy for the review.

Keep the update and new comments in headers.

Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 sound/core/pcm_lib.c    | 86 ++++++++++++++++++++++++-----------------
 sound/core/pcm_local.h  |  3 +-
 sound/core/pcm_native.c |  6 +--
 3 files changed, 55 insertions(+), 40 deletions(-)

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index d21c73944efd..af1eb136feb0 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -42,56 +42,70 @@ static int fill_silence_frames(struct snd_pcm_substream *substream,
  *
  * when runtime->silence_size >= runtime->boundary - fill processed area with silence immediately
  */
-void snd_pcm_playback_silence(struct snd_pcm_substream *substream)
+void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_uframes_t new_hw_ptr)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	snd_pcm_uframes_t appl_ptr = READ_ONCE(runtime->control->appl_ptr);
-	snd_pcm_sframes_t added, hw_avail, frames;
-	snd_pcm_uframes_t noise_dist, ofs, transfer;
+	snd_pcm_uframes_t frames, ofs, transfer;
 	int err;
 
-	added = appl_ptr - runtime->silence_start;
-	if (added) {
-		if (added < 0)
-			added += runtime->boundary;
-		if (added < runtime->silence_filled)
-			runtime->silence_filled -= added;
-		else
-			runtime->silence_filled = 0;
-		runtime->silence_start = appl_ptr;
-	}
-
-	// This will "legitimately" turn negative on underrun, and will be mangled
-	// into a huge number by the boundary crossing handling. The initial state
-	// might also be not quite sane. The code below MUST account for these cases.
-	hw_avail = appl_ptr - runtime->status->hw_ptr;
-	if (hw_avail < 0)
-		hw_avail += runtime->boundary;
-
-	noise_dist = hw_avail + runtime->silence_filled;
 	if (runtime->silence_size < runtime->boundary) {
-		frames = runtime->silence_threshold - noise_dist;
-		if (frames <= 0)
+		snd_pcm_sframes_t noise_dist, n;
+		snd_pcm_uframes_t appl_ptr = READ_ONCE(runtime->control->appl_ptr);
+		if (runtime->silence_start != appl_ptr) {
+			n = appl_ptr - runtime->silence_start;
+			if (n < 0)
+				n += runtime->boundary;
+			if ((snd_pcm_uframes_t)n < runtime->silence_filled)
+				runtime->silence_filled -= n;
+			else
+				runtime->silence_filled = 0;
+			runtime->silence_start = appl_ptr;
+		}
+		if (runtime->silence_filled >= runtime->buffer_size)
+			return;
+		noise_dist = snd_pcm_playback_hw_avail(runtime) + runtime->silence_filled;
+		if (noise_dist >= (snd_pcm_sframes_t) runtime->silence_threshold)
 			return;
+		frames = runtime->silence_threshold - noise_dist;
 		if (frames > runtime->silence_size)
 			frames = runtime->silence_size;
 	} else {
-		frames = runtime->buffer_size - noise_dist;
-		if (frames <= 0)
-			return;
+		if (new_hw_ptr == ULONG_MAX) {	/* initialization */
+			snd_pcm_sframes_t avail = snd_pcm_playback_hw_avail(runtime);
+			if (avail > runtime->buffer_size)
+				avail = runtime->buffer_size;
+			runtime->silence_filled = avail > 0 ? avail : 0;
+			runtime->silence_start = (runtime->status->hw_ptr +
+						  runtime->silence_filled) %
+						 runtime->boundary;
+		} else {
+			ofs = runtime->status->hw_ptr;
+			frames = new_hw_ptr - ofs;
+			if ((snd_pcm_sframes_t)frames < 0)
+				frames += runtime->boundary;
+			runtime->silence_filled -= frames;
+			if ((snd_pcm_sframes_t)runtime->silence_filled < 0) {
+				runtime->silence_filled = 0;
+				runtime->silence_start = new_hw_ptr;
+			} else {
+				runtime->silence_start = ofs;
+			}
+		}
+		frames = runtime->buffer_size - runtime->silence_filled;
 	}
-
 	if (snd_BUG_ON(frames > runtime->buffer_size))
 		return;
-	ofs = (runtime->silence_start + runtime->silence_filled) % runtime->buffer_size;
-	do {
+	if (frames == 0)
+		return;
+	ofs = runtime->silence_start % runtime->buffer_size;
+	while (frames > 0) {
 		transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames;
 		err = fill_silence_frames(substream, ofs, transfer);
 		snd_BUG_ON(err < 0);
 		runtime->silence_filled += transfer;
 		frames -= transfer;
 		ofs = 0;
-	} while (frames > 0);
+	}
 	snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE);
 }
 
@@ -425,6 +439,10 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 		return 0;
 	}
 
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
+	    runtime->silence_size > 0)
+		snd_pcm_playback_silence(substream, new_hw_ptr);
+
 	if (in_interrupt) {
 		delta = new_hw_ptr - runtime->hw_ptr_interrupt;
 		if (delta < 0)
@@ -442,10 +460,6 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 		runtime->hw_ptr_wrap += runtime->boundary;
 	}
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
-	    runtime->silence_size > 0)
-		snd_pcm_playback_silence(substream);
-
 	update_audio_tstamp(substream, &curr_tstamp, &audio_tstamp);
 
 	return snd_pcm_update_state(substream, runtime);
diff --git a/sound/core/pcm_local.h b/sound/core/pcm_local.h
index 42fe3a4e9154..ecb21697ae3a 100644
--- a/sound/core/pcm_local.h
+++ b/sound/core/pcm_local.h
@@ -29,7 +29,8 @@ 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);
 
-void snd_pcm_playback_silence(struct snd_pcm_substream *substream);
+void snd_pcm_playback_silence(struct snd_pcm_substream *substream,
+			      snd_pcm_uframes_t new_hw_ptr);
 
 static inline snd_pcm_uframes_t
 snd_pcm_avail(struct snd_pcm_substream *substream)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 91c87cdb786e..39a65d1415ab 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -958,7 +958,7 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
 	if (snd_pcm_running(substream)) {
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 		    runtime->silence_size > 0)
-			snd_pcm_playback_silence(substream);
+			snd_pcm_playback_silence(substream, ULONG_MAX);
 		err = snd_pcm_update_state(substream, runtime);
 	}
 	snd_pcm_stream_unlock_irq(substream);
@@ -1455,7 +1455,7 @@ static void snd_pcm_post_start(struct snd_pcm_substream *substream,
 	__snd_pcm_set_state(runtime, state);
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 	    runtime->silence_size > 0)
-		snd_pcm_playback_silence(substream);
+		snd_pcm_playback_silence(substream, ULONG_MAX);
 	snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MSTART);
 }
 
@@ -1916,7 +1916,7 @@ static void snd_pcm_post_reset(struct snd_pcm_substream *substream,
 	runtime->control->appl_ptr = runtime->status->hw_ptr;
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 	    runtime->silence_size > 0)
-		snd_pcm_playback_silence(substream);
+		snd_pcm_playback_silence(substream, ULONG_MAX);
 	snd_pcm_stream_unlock_irq(substream);
 }
 
-- 
2.39.2

Re: [PATCH 1/5] ALSA: pcm: Revert "ALSA: pcm: rewrite snd_pcm_playback_silence()"

[Takashi Iwai]

On Fri, 05 May 2023 09:38:09 +0200,
Jaroslav Kysela wrote:
> 
> This reverts commit 9f656705c5faa18afb26d922cfc64f9fd103c38d.
> 
> There is a regression (the top-up mode). Unfortunately, the patch
> provided from the author of this commit is not easy for the review.
> 
> Keep the update and new comments in headers.
> 
> Signed-off-by: Jaroslav Kysela <perex@perex.cz>

Better to add Reported-by and the reference for the original thread
reporting the regression, as well as the Fixes tag.

I can cook up it by myself, too.


Takashi

Re: [PATCH 1/5] ALSA: pcm: Revert "ALSA: pcm: rewrite snd_pcm_playback_silence()"

[Oswald Buddenhagen]

On Fri, May 05, 2023 at 11:31:16AM +0200, Takashi Iwai wrote:
>On Fri, 05 May 2023 09:38:09 +0200,
>Jaroslav Kysela wrote:
>> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
>
>Better to add Reported-by and the reference for the original thread
>reporting the regression,
>
i'll post a slight rework of the series shortly, where i'll include 
this.

> as well as the Fixes tag.
>
that seems pointless for a revert, as all the info is already included 
anyway, no?

regards

Re: [PATCH 1/5] ALSA: pcm: Revert "ALSA: pcm: rewrite snd_pcm_playback_silence()"

[Takashi Iwai]

On Fri, 05 May 2023 11:38:45 +0200,
Oswald Buddenhagen wrote:
> 
> On Fri, May 05, 2023 at 11:31:16AM +0200, Takashi Iwai wrote:
> > On Fri, 05 May 2023 09:38:09 +0200,
> > Jaroslav Kysela wrote:
> >> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
> > 
> > Better to add Reported-by and the reference for the original thread
> > reporting the regression,
> > 
> i'll post a slight rework of the series shortly, where i'll include
> this.
> 
> > as well as the Fixes tag.
> > 
> that seems pointless for a revert, as all the info is already included
> anyway, no?

People tend to look at Fixes tag to judge whether the commit can fix
something real or not.  For example, we (at SUSE) regularly check
Fixes tag to pick up the missing fixing.
In the case of a revert, it doesn't mean always a fix.  So it's not
mandatory, but would be still helpful.


thanks,

Takashi

[PATCH 2/5] ALSA: pcm: fix playback silence - use the actual new_hw_ptr for the threshold mode

[Jaroslav Kysela]

The snd_pcm_playback_hw_avail() function uses runtime->status->hw_ptr.
Unfortunately, in case when we call this function from snd_pcm_update_hw_ptr0(),
this variable contains the previous hardware pointer. Use the new_hw_ptr
argument to calculate hw_avail (filled samples by the user space) to
correct the threshold comparison.

The new_hw_ptr argument may also be set to ULONG_MAX which means the
initialization phase. In this case, use runtime->status->hw_ptr.

Suggested-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 sound/core/pcm_lib.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index af1eb136feb0..8a01aeda2213 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -63,7 +63,15 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
 		}
 		if (runtime->silence_filled >= runtime->buffer_size)
 			return;
-		noise_dist = snd_pcm_playback_hw_avail(runtime) + runtime->silence_filled;
+		/* initialization outside pointer updates */
+		if (new_hw_ptr == ULONG_MAX)
+			new_hw_ptr = runtime->status->hw_ptr;
+		/* get hw_avail with the boundary crossing */
+		noise_dist = appl_ptr - new_hw_ptr;
+		if (noise_dist < 0)
+			noise_dist += runtime->boundary;
+		/* total noise distance */
+		noise_dist += runtime->silence_filled;
 		if (noise_dist >= (snd_pcm_sframes_t) runtime->silence_threshold)
 			return;
 		frames = runtime->silence_threshold - noise_dist;
-- 
2.39.2

[PATCH 3/5] ALSA: pcm: fix playback silence - correct the incremental silencing

[Jaroslav Kysela]

The incremental silencing was broken with the threshold mode. The silenced
area was smaller than expected in some cases. The updated area starts
at runtime->silence_start + runtime->silence_filled position not
only at runtime->silence_start in this mode.

Unify the runtime->silence_start use for all cases (threshold and top-up).

Suggested-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 sound/core/pcm_lib.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 8a01aeda2213..cfdb4aa5f6fa 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -78,26 +78,24 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
 		if (frames > runtime->silence_size)
 			frames = runtime->silence_size;
 	} else {
-		if (new_hw_ptr == ULONG_MAX) {	/* initialization */
+		if (new_hw_ptr == ULONG_MAX) {
+			/* initialization, fill silence to whole unused buffer */
 			snd_pcm_sframes_t avail = snd_pcm_playback_hw_avail(runtime);
 			if (avail > runtime->buffer_size)
 				avail = runtime->buffer_size;
 			runtime->silence_filled = avail > 0 ? avail : 0;
-			runtime->silence_start = (runtime->status->hw_ptr +
-						  runtime->silence_filled) %
-						 runtime->boundary;
+			runtime->silence_start = runtime->status->hw_ptr;
 		} else {
-			ofs = runtime->status->hw_ptr;
-			frames = new_hw_ptr - ofs;
+			/* top-up mode (appl_ptr is not required) */
+			/* silence the played area immediately */
+			frames = new_hw_ptr - runtime->status->hw_ptr;
 			if ((snd_pcm_sframes_t)frames < 0)
 				frames += runtime->boundary;
-			runtime->silence_filled -= frames;
-			if ((snd_pcm_sframes_t)runtime->silence_filled < 0) {
+			if ((snd_pcm_uframes_t)frames < runtime->silence_filled)
+				runtime->silence_filled -= frames;
+			else
 				runtime->silence_filled = 0;
-				runtime->silence_start = new_hw_ptr;
-			} else {
-				runtime->silence_start = ofs;
-			}
+			runtime->silence_start = new_hw_ptr;
 		}
 		frames = runtime->buffer_size - runtime->silence_filled;
 	}
@@ -105,7 +103,7 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
 		return;
 	if (frames == 0)
 		return;
-	ofs = runtime->silence_start % runtime->buffer_size;
+	ofs = (runtime->silence_start + runtime->silence_filled) % runtime->buffer_size;
 	while (frames > 0) {
 		transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames;
 		err = fill_silence_frames(substream, ofs, transfer);
-- 
2.39.2

Re: [PATCH 3/5] ALSA: pcm: fix playback silence - correct the incremental silencing

[Takashi Iwai]

On Fri, 05 May 2023 09:38:11 +0200,
Jaroslav Kysela wrote:
> 
> The incremental silencing was broken with the threshold mode. The silenced
> area was smaller than expected in some cases. The updated area starts
> at runtime->silence_start + runtime->silence_filled position not
> only at runtime->silence_start in this mode.
> 
> Unify the runtime->silence_start use for all cases (threshold and top-up).
> 
> Suggested-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> Signed-off-by: Jaroslav Kysela <perex@perex.cz>

While this change itself follows the original code, and is fine from
the code-refactoring POV.

But, the difficulty of the code (even after this patch) is that the
filling behavior is completely different between the threshold and the
fill-up modes, and we still try to use the similar code.
When reconsidering what we actually need, you can notice that, in the
fill-up mode, we don't have to keep tracking silence_start and
silence_size at all.

Namely, in the fill-up mode, what we need are:
- at init, fill silence in the unused buffer:

	ofs = runtime->control->appl_ptr % runtime->buffer_size;
	frames = snd_pcm_playback_avail(runtime);
	fill_silence_in_loop(ofs, frames);

- at each incremental hw_ptr update, fill the area with silence:

	ofs = runtime->status->hw_ptr % runtime->buffer_size;
	frames = new_hw_ptr - runtime->status->hw_ptr;
	if (frames < 0)
		frames += runtime->boundary;
	fill_silence_in_loop(ofs, frames);

That's all, and far simpler than keeping silence_start and
silence_filled.  (I really had hard time to understand why filling at
silence_start + silence_filled in the incremental mode works
correctly...)

I might have overlooked something and there can be a bit more room for
optimization, but the point is that unifying the code for two behavior
isn't always good.  Treating separately can be sometimes easier.


thanks,

Takashi

Re: [PATCH 3/5] ALSA: pcm: fix playback silence - correct the incremental silencing

[Oswald Buddenhagen]

On Fri, May 05, 2023 at 11:57:00AM +0200, Takashi Iwai wrote:
>But, the difficulty of the code (even after this patch) is that the
>filling behavior is completely different between the threshold and the
>fill-up modes, and we still try to use the similar code.
>
what is slightly confusing is that we're kinda abusing silence_filled to 
mean silence_filled + real_samples.
i can add a comment to that effect, but i don't think it's worth tearing 
apart the paths.

regards

[PATCH 4/5] ALSA: pcm: playback silence - remove extra code

[Jaroslav Kysela]

The removed condition handles de facto only one situation where
runtime->silence_filled variable is equal to runtime->buffer_size,
because this variable cannot go over the buffer size. This case is
catched also with the next condition for the noise distance and
required threshold comparison.

Suggested-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 sound/core/pcm_lib.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index cfdb4aa5f6fa..952f0d807124 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -61,8 +61,6 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
 				runtime->silence_filled = 0;
 			runtime->silence_start = appl_ptr;
 		}
-		if (runtime->silence_filled >= runtime->buffer_size)
-			return;
 		/* initialization outside pointer updates */
 		if (new_hw_ptr == ULONG_MAX)
 			new_hw_ptr = runtime->status->hw_ptr;
-- 
2.39.2

[PATCH 5/5] ALSA: pcm: playback silence - move silence variables updates to separate function

[Jaroslav Kysela]

There is a common code in the threshold and top-up mode tracking
the added (already silenced) samples. Move this code to one place
to enhance the readability.

Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 sound/core/pcm_lib.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 952f0d807124..6b0601fec832 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -33,6 +33,25 @@
 static int fill_silence_frames(struct snd_pcm_substream *substream,
 			       snd_pcm_uframes_t off, snd_pcm_uframes_t frames);
 
+
+static inline void silence_update(struct snd_pcm_runtime *runtime,
+				  snd_pcm_uframes_t ptr,
+				  snd_pcm_uframes_t new_ptr)
+{
+	snd_pcm_sframes_t n;
+
+	if (ptr == new_ptr)
+		return;
+	n = new_ptr - ptr;
+	if (n < 0)
+		n += runtime->boundary;
+	if ((snd_pcm_uframes_t)n < runtime->silence_filled)
+		runtime->silence_filled -= n;
+	else
+		runtime->silence_filled = 0;
+	runtime->silence_start = new_ptr;
+}
+
 /*
  * fill ring buffer with silence
  * runtime->silence_start: starting pointer to silence area
@@ -49,18 +68,9 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
 	int err;
 
 	if (runtime->silence_size < runtime->boundary) {
-		snd_pcm_sframes_t noise_dist, n;
+		snd_pcm_sframes_t noise_dist;
 		snd_pcm_uframes_t appl_ptr = READ_ONCE(runtime->control->appl_ptr);
-		if (runtime->silence_start != appl_ptr) {
-			n = appl_ptr - runtime->silence_start;
-			if (n < 0)
-				n += runtime->boundary;
-			if ((snd_pcm_uframes_t)n < runtime->silence_filled)
-				runtime->silence_filled -= n;
-			else
-				runtime->silence_filled = 0;
-			runtime->silence_start = appl_ptr;
-		}
+		silence_update(runtime, runtime->silence_start, appl_ptr);
 		/* initialization outside pointer updates */
 		if (new_hw_ptr == ULONG_MAX)
 			new_hw_ptr = runtime->status->hw_ptr;
@@ -86,14 +96,7 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
 		} else {
 			/* top-up mode (appl_ptr is not required) */
 			/* silence the played area immediately */
-			frames = new_hw_ptr - runtime->status->hw_ptr;
-			if ((snd_pcm_sframes_t)frames < 0)
-				frames += runtime->boundary;
-			if ((snd_pcm_uframes_t)frames < runtime->silence_filled)
-				runtime->silence_filled -= frames;
-			else
-				runtime->silence_filled = 0;
-			runtime->silence_start = new_hw_ptr;
+			silence_update(runtime, runtime->status->hw_ptr, new_hw_ptr);
 		}
 		frames = runtime->buffer_size - runtime->silence_filled;
 	}
-- 
2.39.2

Re: [PATCH 0/5] rewrite snd_pcm_playback_silence() again

[Takashi Iwai]

On Fri, 05 May 2023 09:38:08 +0200,
Jaroslav Kysela wrote:
> 
> This is a split of changes for both patches (the first with the missing
> top-up mode + later fix with mixups) proposed by Oswald. The aim was to
> pick only real code changes.
> 
> Only the first two patches after revert fixes the current silencing issues.
> The last two are just cleanups with the extra optimization in the last patch
> moving the common code to a function.
> 
> Cc: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> Cc: Jeff Chua <jeff.chua.linux@gmail.com>
> 
> Jaroslav Kysela (5):
>   ALSA: pcm: Revert "ALSA: pcm: rewrite snd_pcm_playback_silence()"
>   ALSA: pcm: fix playback silence - use the actual new_hw_ptr for the
>     threshold mode
>   ALSA: pcm: fix playback silence - correct the incremental silencing
>   ALSA: pcm: playback silence - remove extra code
>   ALSA: pcm: playback silence - move silence variables updates to
>     separate function

Thanks, this makes it much easier to digest the whole changes!

I watch out for a while whether any objection comes up, then apply
patches in today -- or if we didn't reach to consensus, I'll pick up
only the first revert patch for 6.4-rc1, at least.


Takashi