Skip to content

Commit d7f5dd9

Browse files
perexgtiwai
perexg
authored and
tiwai
committed
ALSA: pcm: Revert "ALSA: pcm: rewrite snd_pcm_playback_silence()"
This reverts commit 9f65670. There was a regression (in the top-up mode). Unfortunately, the patch provided from the author of this commit is not easy to review. Keep the updated and new comments in headers. Also add a new comment that documents the missed API constraint which led to the regression. Reported-by: Jeff Chua <jeff.chua.linux@gmail.com> Link: https://lore.kernel.org/r/CAAJw_ZsbTVd3Es373x_wTNDF7RknGhCD0r+NKUSwAO7HpLAkYA@mail.gmail.com Signed-off-by: Jaroslav Kysela <perex@perex.cz> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> Link: https://lore.kernel.org/r/20230505155244.2312199-1-oswald.buddenhagen@gmx.de Signed-off-by: Takashi Iwai <tiwai@suse.de>
1 parent 56fc217 commit d7f5dd9

3 files changed

Lines changed: 59 additions & 40 deletions

File tree

‎sound/core/pcm_lib.c‎

Lines changed: 54 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -42,56 +42,74 @@ static int fill_silence_frames(struct snd_pcm_substream *substream,
4242
*
4343
* when runtime->silence_size >= runtime->boundary - fill processed area with silence immediately
4444
*/
45-
void snd_pcm_playback_silence(struct snd_pcm_substream *substream)
45+
void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_uframes_t new_hw_ptr)
4646
{
4747
struct snd_pcm_runtime *runtime = substream->runtime;
48-
snd_pcm_uframes_t appl_ptr = READ_ONCE(runtime->control->appl_ptr);
49-
snd_pcm_sframes_t added, hw_avail, frames;
50-
snd_pcm_uframes_t noise_dist, ofs, transfer;
48+
snd_pcm_uframes_t frames, ofs, transfer;
5149
int err;
5250

53-
added = appl_ptr - runtime->silence_start;
54-
if (added) {
55-
if (added < 0)
56-
added += runtime->boundary;
57-
if (added < runtime->silence_filled)
58-
runtime->silence_filled -= added;
59-
else
60-
runtime->silence_filled = 0;
61-
runtime->silence_start = appl_ptr;
62-
}
63-
64-
// This will "legitimately" turn negative on underrun, and will be mangled
65-
// into a huge number by the boundary crossing handling. The initial state
66-
// might also be not quite sane. The code below MUST account for these cases.
67-
hw_avail = appl_ptr - runtime->status->hw_ptr;
68-
if (hw_avail < 0)
69-
hw_avail += runtime->boundary;
70-
71-
noise_dist = hw_avail + runtime->silence_filled;
7251
if (runtime->silence_size < runtime->boundary) {
73-
frames = runtime->silence_threshold - noise_dist;
74-
if (frames <= 0)
52+
snd_pcm_sframes_t noise_dist, n;
53+
snd_pcm_uframes_t appl_ptr = READ_ONCE(runtime->control->appl_ptr);
54+
if (runtime->silence_start != appl_ptr) {
55+
n = appl_ptr - runtime->silence_start;
56+
if (n < 0)
57+
n += runtime->boundary;
58+
if ((snd_pcm_uframes_t)n < runtime->silence_filled)
59+
runtime->silence_filled -= n;
60+
else
61+
runtime->silence_filled = 0;
62+
runtime->silence_start = appl_ptr;
63+
}
64+
if (runtime->silence_filled >= runtime->buffer_size)
65+
return;
66+
noise_dist = snd_pcm_playback_hw_avail(runtime) + runtime->silence_filled;
67+
if (noise_dist >= (snd_pcm_sframes_t) runtime->silence_threshold)
7568
return;
69+
frames = runtime->silence_threshold - noise_dist;
7670
if (frames > runtime->silence_size)
7771
frames = runtime->silence_size;
7872
} else {
79-
frames = runtime->buffer_size - noise_dist;
80-
if (frames <= 0)
81-
return;
73+
/*
74+
* This filling mode aims at free-running mode (used for example by dmix),
75+
* which doesn't update the application pointer.
76+
*/
77+
if (new_hw_ptr == ULONG_MAX) { /* initialization */
78+
snd_pcm_sframes_t avail = snd_pcm_playback_hw_avail(runtime);
79+
if (avail > runtime->buffer_size)
80+
avail = runtime->buffer_size;
81+
runtime->silence_filled = avail > 0 ? avail : 0;
82+
runtime->silence_start = (runtime->status->hw_ptr +
83+
runtime->silence_filled) %
84+
runtime->boundary;
85+
} else {
86+
ofs = runtime->status->hw_ptr;
87+
frames = new_hw_ptr - ofs;
88+
if ((snd_pcm_sframes_t)frames < 0)
89+
frames += runtime->boundary;
90+
runtime->silence_filled -= frames;
91+
if ((snd_pcm_sframes_t)runtime->silence_filled < 0) {
92+
runtime->silence_filled = 0;
93+
runtime->silence_start = new_hw_ptr;
94+
} else {
95+
runtime->silence_start = ofs;
96+
}
97+
}
98+
frames = runtime->buffer_size - runtime->silence_filled;
8299
}
83-
84100
if (snd_BUG_ON(frames > runtime->buffer_size))
85101
return;
86-
ofs = (runtime->silence_start + runtime->silence_filled) % runtime->buffer_size;
87-
do {
102+
if (frames == 0)
103+
return;
104+
ofs = runtime->silence_start % runtime->buffer_size;
105+
while (frames > 0) {
88106
transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames;
89107
err = fill_silence_frames(substream, ofs, transfer);
90108
snd_BUG_ON(err < 0);
91109
runtime->silence_filled += transfer;
92110
frames -= transfer;
93111
ofs = 0;
94-
} while (frames > 0);
112+
}
95113
snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE);
96114
}
97115

@@ -425,6 +443,10 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
425443
return 0;
426444
}
427445

446+
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
447+
runtime->silence_size > 0)
448+
snd_pcm_playback_silence(substream, new_hw_ptr);
449+
428450
if (in_interrupt) {
429451
delta = new_hw_ptr - runtime->hw_ptr_interrupt;
430452
if (delta < 0)
@@ -442,10 +464,6 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
442464
runtime->hw_ptr_wrap += runtime->boundary;
443465
}
444466

445-
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
446-
runtime->silence_size > 0)
447-
snd_pcm_playback_silence(substream);
448-
449467
update_audio_tstamp(substream, &curr_tstamp, &audio_tstamp);
450468

451469
return snd_pcm_update_state(substream, runtime);

‎sound/core/pcm_local.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream,
2929
struct snd_pcm_runtime *runtime);
3030
int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream);
3131

32-
void snd_pcm_playback_silence(struct snd_pcm_substream *substream);
32+
void snd_pcm_playback_silence(struct snd_pcm_substream *substream,
33+
snd_pcm_uframes_t new_hw_ptr);
3334

3435
static inline snd_pcm_uframes_t
3536
snd_pcm_avail(struct snd_pcm_substream *substream)

‎sound/core/pcm_native.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,7 @@ static int snd_pcm_sw_params(struct snd_pcm_substream *substream,
958958
if (snd_pcm_running(substream)) {
959959
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
960960
runtime->silence_size > 0)
961-
snd_pcm_playback_silence(substream);
961+
snd_pcm_playback_silence(substream, ULONG_MAX);
962962
err = snd_pcm_update_state(substream, runtime);
963963
}
964964
snd_pcm_stream_unlock_irq(substream);
@@ -1455,7 +1455,7 @@ static void snd_pcm_post_start(struct snd_pcm_substream *substream,
14551455
__snd_pcm_set_state(runtime, state);
14561456
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
14571457
runtime->silence_size > 0)
1458-
snd_pcm_playback_silence(substream);
1458+
snd_pcm_playback_silence(substream, ULONG_MAX);
14591459
snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MSTART);
14601460
}
14611461

@@ -1916,7 +1916,7 @@ static void snd_pcm_post_reset(struct snd_pcm_substream *substream,
19161916
runtime->control->appl_ptr = runtime->status->hw_ptr;
19171917
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
19181918
runtime->silence_size > 0)
1919-
snd_pcm_playback_silence(substream);
1919+
snd_pcm_playback_silence(substream, ULONG_MAX);
19201920
snd_pcm_stream_unlock_irq(substream);
19211921
}
19221922

0 commit comments

Comments
 (0)