[10/14] ALSA: pcm: Simplify snd_pcm_playback_silence()

Commit Message

Takashi Iwai May 26, 2017, 3:46 p.m. UTC

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 <[e-mail]>
> > ---
> >   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));
> >   		} 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);
> > -				}
> > -			}
> > +			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 <[e-mail]>
Subject: [PATCH v2] ALSA: pcm: Simplify snd_pcm_playback_silence()

Use the existing silence helper codes for simplification.

Signed-off-by: Takashi Iwai <[e-mail]>
---
 sound/core/pcm_lib.c | 50 ++++++++++++++++++++------------------------------
 1 file changed, 20 insertions(+), 30 deletions(-)

Patch

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)
 {