Re: Click after draining

[Takashi Iwai <tiwai@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

At Fri, 08 Oct 2004 17:34:45 +0200,
I wrote:
> 
> At Thu, 7 Oct 2004 20:58:01 +0200,
> Giuliano Pochini wrote:
> > 
> > On Tue, 21 Sep 2004 22:13:18 +0200
> > Giuliano Pochini <pochini@shiny.it> wrote:
> > 
> > > With both the powermac driver and my echoaudio driver, playback ends with a
> > > "click". With properly choosen buffer/period/file sizes I can make the card
> > > play almost a whole period of old data before the substream is actually
> > > stopped. It seems that periods after the last one are not silenced, but I'm
> > > not having luck at findind the cause. This is not a recent problem.
> > > [...]
> > 
> > I reply myself to add that I asked about this issue at one of the
> > betatesters and he can hear the click also. It's not a mac-only problem...
> > Any ideas ?
> 
> ALSA seems not putting silence properly unless silence_size > 0.
> How about the attached patch?
> (it's an untested and quick fix, as usual :)

And broken, as usual :)
The corrected on is below.


Takashi

[-- Attachment #2: Type: text/plain, Size: 4778 bytes --]

Index: alsa-kernel/include/pcm.h
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/include/pcm.h,v
retrieving revision 1.50
diff -u -r1.50 pcm.h
--- alsa-kernel/include/pcm.h	24 Sep 2004 13:55:55 -0000	1.50
+++ alsa-kernel/include/pcm.h	8 Oct 2004 15:15:07 -0000
@@ -875,6 +875,7 @@
 int snd_pcm_playback_xrun_asap(snd_pcm_substream_t *substream);
 int snd_pcm_capture_xrun_asap(snd_pcm_substream_t *substream);
 void snd_pcm_playback_silence(snd_pcm_substream_t *substream, snd_pcm_uframes_t new_hw_ptr);
+int snd_pcm_lib_fill_silence(snd_pcm_substream_t *substream, unsigned int ofs, snd_pcm_uframes_t frames);
 int snd_pcm_playback_ready(snd_pcm_substream_t *substream);
 int snd_pcm_capture_ready(snd_pcm_substream_t *substream);
 long snd_pcm_playback_ready_jiffies(snd_pcm_substream_t *substream);
Index: alsa-kernel/core/pcm_lib.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/core/pcm_lib.c,v
retrieving revision 1.57
diff -u -r1.57 pcm_lib.c
--- alsa-kernel/core/pcm_lib.c	24 Sep 2004 13:55:54 -0000	1.57
+++ alsa-kernel/core/pcm_lib.c	8 Oct 2004 15:33:31 -0000
@@ -95,32 +95,10 @@
 	ofs = (runtime->silence_start + runtime->silence_filled) % 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->silence) {
-				int err;
-				err = substream->ops->silence(substream, -1, ofs, transfer);
-				snd_assert(err >= 0, );
-			} else {
-				char *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->silence) {
-				for (c = 0; c < channels; ++c) {
-					int err;
-					err = substream->ops->silence(substream, c, ofs, transfer);
-					snd_assert(err >= 0, );
-				}
-			} else {
-				size_t dma_csize = runtime->dma_bytes / channels;
-				for (c = 0; c < channels; ++c) {
-					char *hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, ofs);
-					snd_pcm_format_set_silence(runtime->format, hwbuf, transfer);
-				}
-			}
+		
+		if (snd_pcm_lib_fill_silence(substream, ofs, transfer) < 0) {
+			snd_printk(KERN_ERR "fill_silence error\n");
+			break;
 		}
 		runtime->silence_filled += transfer;
 		frames -= transfer;
@@ -128,6 +106,42 @@
 	}
 }
 
+int snd_pcm_lib_fill_silence(snd_pcm_substream_t *substream, unsigned int ofs, snd_pcm_uframes_t frames)
+{
+	snd_pcm_runtime_t *runtime = substream->runtime;
+	int c, channels = runtime->channels;
+	char *hwbuf;
+	size_t dma_csize;
+
+	if (substream->ops->silence) {
+		int err = 0;
+		if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED ||
+		    runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED)
+			err = substream->ops->silence(substream, -1, ofs, frames);
+		else {
+			for (c = 0; c < channels; c++) {
+				if ((err = substream->ops->silence(substream, c, ofs, frames)) < 0)
+					break;
+			}
+		}
+		return err;
+	}
+
+	if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED ||
+	    runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) {
+		hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs);
+		snd_pcm_format_set_silence(runtime->format, hwbuf, frames * channels);
+	} else {
+		dma_csize = runtime->dma_bytes / channels;
+		hwbuf = runtime->dma_area + samples_to_bytes(runtime, ofs);
+		for (c = 0; c < channels; c++) {
+			snd_pcm_format_set_silence(runtime->format, hwbuf, frames);
+			hwbuf += dma_csize;
+		}
+	}
+	return 0;
+}
+
 static void xrun(snd_pcm_substream_t *substream)
 {
 	snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
Index: alsa-kernel/core/pcm_native.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/core/pcm_native.c,v
retrieving revision 1.84
diff -u -r1.84 pcm_native.c
--- alsa-kernel/core/pcm_native.c	24 Sep 2004 13:55:55 -0000	1.84
+++ alsa-kernel/core/pcm_native.c	8 Oct 2004 16:06:05 -0000
@@ -1296,7 +1296,16 @@
 			runtime->status->state = SNDRV_PCM_STATE_DRAINING;
 			break;
 		default:
-			break;
+			return 0; /* do nothing */
+		}
+		if (! runtime->silence_size) {
+			/* fill silence until the priod end */
+			snd_pcm_uframes_t appl_ptr = runtime->control->appl_ptr;
+			snd_pcm_uframes_t delta = appl_ptr % runtime->period_size;
+			if (delta > 0)
+				snd_pcm_lib_fill_silence(substream,
+							 appl_ptr % runtime->buffer_size,
+							 runtime->period_size - delta);
 		}
 	} else {
 		/* stop running stream */