On Fri, 26 May 2017 15:21:54 +0200, Takashi Sakamoto wrote:
On May 26 2017 04:17, Takashi Iwai wrote:
Use the existing silence helper codes for simplification.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/core/pcm_lib.c | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index c24a78e39b88..094447ae8486 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -42,6 +42,9 @@ #define trace_hw_ptr_error(substream, reason) #endif +static int fill_silence(struct snd_pcm_substream *substream, int channel,
unsigned long hwoff, void *buf, unsigned long bytes);- /*
- fill ring buffer with silence
- runtime->silence_start: starting pointer to silence area
@@ -55,8 +58,7 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t frames, ofs, transfer;
- char *hwbuf;
- int err;
- int c; if (runtime->silence_size < runtime->boundary) { snd_pcm_sframes_t noise_dist, n;
@@ -111,33 +113,17 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames; if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) {
if (substream->ops->fill_silence) {err = substream->ops->fill_silence(substream, 0,frames_to_bytes(runtime, ofs),frames_to_bytes(runtime, transfer));snd_BUG_ON(err < 0);} else {hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs);snd_pcm_format_set_silence(runtime->format, hwbuf, transfer * runtime->channels);}
fill_silence(substream, 0,frames_to_bytes(runtime, ofs), NULL,frames_to_bytes(runtime, transfer));
unsigned int c;unsigned int channels = runtime->channels;if (substream->ops->fill_silence) {for (c = 0; c < channels; ++c) {err = substream->ops->fill_silence(substream, c,samples_to_bytes(runtime, ofs),samples_to_bytes(runtime, transfer));snd_BUG_ON(err < 0);}} else {size_t dma_csize = runtime->dma_bytes / channels;for (c = 0; c < channels; ++c) {hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, ofs);snd_pcm_format_set_silence(runtime->format, hwbuf, transfer);}}
int byte_ofs = samples_to_bytes(runtime, ofs);int byte_xfer = samples_to_bytes(runtime, transfer);for (c = 0; c < runtime->channels; ++c)fill_silence(substream, c, byte_ofs, NULL,byte_xfer);- runtime->silence_filled += transfer; frames -= transfer; ofs = 0;
A part of the content inner the while loop can be replaced with added 'writer' functions.
Right, we can reuse that.
And the purpose of this patchset is a kind of refactoring, however this drops snd_BUG_ON(). It should be remained. Below patch can be squashed to your patch for the points.
Thanks. I prefer just creating a new function instead of calling the function pointer there. Basically it's easier to read/understand.
The revised patch is below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH v2] ALSA: pcm: Simplify snd_pcm_playback_silence()
Use the existing silence helper codes for simplification.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_lib.c | 50 ++++++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 30 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index f15460eaf8b5..a592d3308474 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -42,6 +42,9 @@ #define trace_hw_ptr_error(substream, reason) #endif
+static int fill_silence_frames(struct snd_pcm_substream *substream, + snd_pcm_uframes_t off, snd_pcm_uframes_t frames); + /* * fill ring buffer with silence * runtime->silence_start: starting pointer to silence area @@ -55,7 +58,6 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t frames, ofs, transfer; - char *hwbuf; int err;
if (runtime->silence_size < runtime->boundary) { @@ -109,35 +111,8 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram ofs = runtime->silence_start % runtime->buffer_size; while (frames > 0) { transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames; - if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || - runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) { - if (substream->ops->fill_silence) { - err = substream->ops->fill_silence(substream, 0, - frames_to_bytes(runtime, ofs), - frames_to_bytes(runtime, transfer)); - snd_BUG_ON(err < 0); - } else { - hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs); - snd_pcm_format_set_silence(runtime->format, hwbuf, transfer * runtime->channels); - } - } else { - unsigned int c; - unsigned int channels = runtime->channels; - if (substream->ops->fill_silence) { - for (c = 0; c < channels; ++c) { - err = substream->ops->fill_silence(substream, c, - samples_to_bytes(runtime, ofs), - samples_to_bytes(runtime, transfer)); - snd_BUG_ON(err < 0); - } - } else { - size_t dma_csize = runtime->dma_bytes / channels; - for (c = 0; c < channels; ++c) { - hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, ofs); - snd_pcm_format_set_silence(runtime->format, hwbuf, transfer); - } - } - } + err = fill_silence_frames(substream, ofs, transfer); + snd_BUG_ON(err < 0); runtime->silence_filled += transfer; frames -= transfer; ofs = 0; @@ -2101,6 +2076,21 @@ static int noninterleaved_copy(struct snd_pcm_substream *substream, return 0; }
+/* fill silence on the given buffer position; + * called from snd_pcm_playback_silence() + */ +static int fill_silence_frames(struct snd_pcm_substream *substream, + snd_pcm_uframes_t off, snd_pcm_uframes_t frames) +{ + if (substream->runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || + substream->runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) + return interleaved_copy(substream, off, NULL, 0, frames, + fill_silence); + else + return noninterleaved_copy(substream, off, NULL, 0, frames, + fill_silence); +} + /* sanity-check for read/write methods */ static int pcm_sanity_check(struct snd_pcm_substream *substream) {