Turns out that we cannot rely on the application pointer being updated
in top-up mode, as its primary purpose is to remain operational in
free-running mode as used by dmix.
So we logically revert some bits from commit 9f656705c5faa ("ALSA: pcm:
rewrite snd_pcm_playback_silence()"), but we retain the bug fixes and
try to make the code paths congruent.
Reported-by: Jeff Chua <jeff.chua.linux@xxxxxxxxx>
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@xxxxxx>
---
sound/core/pcm_lib.c | 82 +++++++++++++++++++++++++++--------------
sound/core/pcm_local.h | 3 +-
sound/core/pcm_native.c | 6 +--
3 files changed, 60 insertions(+), 31 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index d21c73944efd..cd5f2ef14ab4 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -42,41 +42,69 @@ 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_sframes_t hw_avail, frames;
snd_pcm_uframes_t noise_dist, 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) {
+ snd_pcm_uframes_t appl_ptr = READ_ONCE(runtime->control->appl_ptr);
+ snd_pcm_sframes_t 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;
+ }
+
+ if (new_hw_ptr == ULONG_MAX) // initialization
+ new_hw_ptr = runtime->status->hw_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 - new_hw_ptr;
+ if (hw_avail < 0)
+ hw_avail += runtime->boundary;
+
+ noise_dist = hw_avail + runtime->silence_filled;
frames = runtime->silence_threshold - noise_dist;
if (frames <= 0)
return;
if (frames > runtime->silence_size)
frames = runtime->silence_size;
} else {
- frames = runtime->buffer_size - noise_dist;
+ // This filling mode aims at free-running mode (used for example by dmix),
+ // which doesn't update the application pointer.
+ snd_pcm_uframes_t hw_ptr = runtime->status->hw_ptr;
+ if (new_hw_ptr == ULONG_MAX) { // initialization
+ // Usually, this is entered while stopped, before data is queued,
+ // so both pointers are expected to be zero.
+ hw_avail = runtime->control->appl_ptr - hw_ptr;
+ if (hw_avail < 0)
+ hw_avail += runtime->boundary;
+ // In free-running mode, appl_ptr will be zero even while running,
+ // so we end up with a huge number. There is no useful way to
+ // handle this, so we just clear the whole buffer.
+ runtime->silence_filled = hw_avail > runtime->buffer_size ? 0 : hw_avail;
+ runtime->silence_start = hw_ptr;
+ } else {
+ snd_pcm_sframes_t played = new_hw_ptr - hw_ptr;
+ if (played) {
+ if (played < 0)
+ played += runtime->boundary;
+ if (played < runtime->silence_filled)
+ runtime->silence_filled -= played;
+ else // This may happen due to a reset.
+ runtime->silence_filled = 0;
+ runtime->silence_start = new_hw_ptr;
+ }
+ }
+ frames = runtime->buffer_size - runtime->silence_filled;
if (frames <= 0)
return;
}
@@ -425,6 +453,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 +474,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 3d0c4a5b701b..94185267a7b9 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.40.0.152.g15d061e6df
