Re: popping noise when switching tracks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/17/2013 11:03 PM, B. Zhang wrote:
On 05/17/2013 04:03 PM, B. Zhang wrote:
On 05/17/2013 03:04 PM, Torstein Hegge wrote:
There is quite a bit of changes both in sound/usb and drivers/usb
between 3.4 and 3.9. You could try to test with a 3.5 kernel and a 3.8
kernel to narrow down where the problem is coming from. Testing with 3.6
and 3.7 would require that you apply the fix for the "atomic DMA
coherent pool is too small" issue.
I'll try 3.6 (and 3.7). Maybe it is caused by a drivers/usb change in
kernel 3.6.
I tried 3.6.11 and 3.7.10.
3.6.11 OK.
3.7.10 has this noise problem ( buffers management problem? ) with my
USB interface.
It seems to me that there was a big ALSA updatefrom 3.6 to 3.7.

I did some more tests for manual change tacks.
- kernel 3.5.7 or 3.6.11: no noise except mp3 -STOP - flac
- kernel 3.7.10: noise (stop or not before the change, flac - flac or mp3 - flac) - Kernel 3.7.10 + reverse three files changes (attached): same behavior as kernel 3.6.10.

This has been tested with mpd. With logitechmedia server and squeezelite, il seems OK, no noise.

I think that these noises exist and its level depends on a particular association "kernel+ audio application + sound card", it is quite difficult to find the exact cause. A change in ALSA or kernel-usb canreduce or increase the noise.

I found this troubleshooting pdf: http://www.google.com/url?q=http://www.sound-pixel.com/files/product_pdf/hiFace%2520-%2520Official%2520Troubleshooting%2520and%2520FAQ%2520-%252025-3-13.pdf&ei=AVqWUbiKIeX40gWkpIC4Dw&sa=X&oi=unauthorizedredirect&ct=targetlink&ust=1368809737543917&usg=AFQjCNHqV7suIMxts2YnPGdWRfFjL0WlyQ
for my usb sound card:
Symptom: Often, when skipping tracks or when exiting pause, I hear loud clicks
Cause: It’s a hiFace typical behaviour related to buffers management
Remedy: Some players are less exposed to the problem (e.g.: Winamp, Jriver Media Center under Windows): use one of them

Regards,
Bin
diff -Nur 3.7.10/sound/usb/endpoint.c 3.6.11/sound/usb/endpoint.c
--- 3.7.10/sound/usb/endpoint.c	2013-05-18 12:19:13.731756752 +0200
+++ 3.6.11/sound/usb/endpoint.c	2013-05-18 12:18:06.687757506 +0200
@@ -584,19 +584,20 @@
  * configure a data endpoint
  */
 static int data_ep_set_params(struct snd_usb_endpoint *ep,
-			      snd_pcm_format_t pcm_format,
-			      unsigned int channels,
-			      unsigned int period_bytes,
+			      struct snd_pcm_hw_params *hw_params,
 			      struct audioformat *fmt,
 			      struct snd_usb_endpoint *sync_ep)
 {
 	unsigned int maxsize, i, urb_packs, total_packs, packs_per_ms;
+	int period_bytes = params_period_bytes(hw_params);
+	int format = params_format(hw_params);
 	int is_playback = usb_pipeout(ep->pipe);
-	int frame_bits = snd_pcm_format_physical_width(pcm_format) * channels;
+	int frame_bits = snd_pcm_format_physical_width(params_format(hw_params)) *
+							params_channels(hw_params);
 
 	ep->datainterval = fmt->datainterval;
 	ep->stride = frame_bits >> 3;
-	ep->silence_value = pcm_format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0;
+	ep->silence_value = format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0;
 
 	/* calculate max. frequency */
 	if (ep->maxpacksize) {
@@ -709,6 +710,7 @@
  * configure a sync endpoint
  */
 static int sync_ep_set_params(struct snd_usb_endpoint *ep,
+			      struct snd_pcm_hw_params *hw_params,
 			      struct audioformat *fmt)
 {
 	int i;
@@ -751,10 +753,7 @@
  * snd_usb_endpoint_set_params: configure an snd_usb_endpoint
  *
  * @ep: the snd_usb_endpoint to configure
- * @pcm_format: the audio fomat.
- * @channels: the number of audio channels.
- * @period_bytes: the number of bytes in one alsa period.
- * @rate: the frame rate.
+ * @hw_params: the hardware parameters
  * @fmt: the USB audio format information
  * @sync_ep: the sync endpoint to use, if any
  *
@@ -763,10 +762,7 @@
  * An endpoint that is already running can not be reconfigured.
  */
 int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
-				snd_pcm_format_t pcm_format,
-				unsigned int channels,
-				unsigned int period_bytes,
-				unsigned int rate,
+				struct snd_pcm_hw_params *hw_params,
 				struct audioformat *fmt,
 				struct snd_usb_endpoint *sync_ep)
 {
@@ -786,9 +782,9 @@
 	ep->fill_max = !!(fmt->attributes & UAC_EP_CS_ATTR_FILL_MAX);
 
 	if (snd_usb_get_speed(ep->chip->dev) == USB_SPEED_FULL)
-		ep->freqn = get_usb_full_speed_rate(rate);
+		ep->freqn = get_usb_full_speed_rate(params_rate(hw_params));
 	else
-		ep->freqn = get_usb_high_speed_rate(rate);
+		ep->freqn = get_usb_high_speed_rate(params_rate(hw_params));
 
 	/* calculate the frequency in 16.16 format */
 	ep->freqm = ep->freqn;
@@ -798,11 +794,10 @@
 
 	switch (ep->type) {
 	case  SND_USB_ENDPOINT_TYPE_DATA:
-		err = data_ep_set_params(ep, pcm_format, channels,
-					 period_bytes, fmt, sync_ep);
+		err = data_ep_set_params(ep, hw_params, fmt, sync_ep);
 		break;
 	case  SND_USB_ENDPOINT_TYPE_SYNC:
-		err = sync_ep_set_params(ep, fmt);
+		err = sync_ep_set_params(ep, hw_params, fmt);
 		break;
 	default:
 		err = -EINVAL;
diff -Nur 3.7.10/sound/usb/endpoint.h 3.6.11/sound/usb/endpoint.h
--- 3.7.10/sound/usb/endpoint.h	2013-05-18 12:19:13.731756752 +0200
+++ 3.6.11/sound/usb/endpoint.h	2013-05-18 12:18:06.687757506 +0200
@@ -9,10 +9,7 @@
 					      int ep_num, int direction, int type);
 
 int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
-				snd_pcm_format_t pcm_format,
-				unsigned int channels,
-				unsigned int period_bytes,
-				unsigned int rate,
+				struct snd_pcm_hw_params *hw_params,
 				struct audioformat *fmt,
 				struct snd_usb_endpoint *sync_ep);
 
diff -Nur 3.7.10/sound/usb/pcm.c 3.6.11/sound/usb/pcm.c
--- 3.7.10/sound/usb/pcm.c	2013-05-18 12:19:13.731756752 +0200
+++ 3.6.11/sound/usb/pcm.c	2013-05-18 12:18:06.687757506 +0200
@@ -84,7 +84,8 @@
 /*
  * find a matching audio format
  */
-static struct audioformat *find_format(struct snd_usb_substream *subs)
+static struct audioformat *find_format(struct snd_usb_substream *subs, unsigned int format,
+				       unsigned int rate, unsigned int channels)
 {
 	struct list_head *p;
 	struct audioformat *found = NULL;
@@ -93,17 +94,16 @@
 	list_for_each(p, &subs->fmt_list) {
 		struct audioformat *fp;
 		fp = list_entry(p, struct audioformat, list);
-		if (!(fp->formats & (1uLL << subs->pcm_format)))
+		if (!(fp->formats & (1uLL << format)))
 			continue;
-		if (fp->channels != subs->channels)
+		if (fp->channels != channels)
 			continue;
-		if (subs->cur_rate < fp->rate_min ||
-		    subs->cur_rate > fp->rate_max)
+		if (rate < fp->rate_min || rate > fp->rate_max)
 			continue;
 		if (! (fp->rates & SNDRV_PCM_RATE_CONTINUOUS)) {
 			unsigned int i;
 			for (i = 0; i < fp->nr_rates; i++)
-				if (fp->rate_table[i] == subs->cur_rate)
+				if (fp->rate_table[i] == rate)
 					break;
 			if (i >= fp->nr_rates)
 				continue;
@@ -438,38 +438,6 @@
 }
 
 /*
- * configure endpoint params
- *
- * called  during initial setup and upon resume
- */
-static int configure_endpoint(struct snd_usb_substream *subs)
-{
-	int ret;
-
-	/* format changed */
-	stop_endpoints(subs, 0, 0, 0);
-	ret = snd_usb_endpoint_set_params(subs->data_endpoint,
-					  subs->pcm_format,
-					  subs->channels,
-					  subs->period_bytes,
-					  subs->cur_rate,
-					  subs->cur_audiofmt,
-					  subs->sync_endpoint);
-	if (ret < 0)
-		return ret;
-
-	if (subs->sync_endpoint)
-		ret = snd_usb_endpoint_set_params(subs->sync_endpoint,
-						  subs->pcm_format,
-						  subs->channels,
-						  subs->period_bytes,
-						  subs->cur_rate,
-						  subs->cur_audiofmt,
-						  NULL);
-	return ret;
-}
-
-/*
  * hw_params callback
  *
  * allocate a buffer and set the given audio format.
@@ -484,39 +452,68 @@
 {
 	struct snd_usb_substream *subs = substream->runtime->private_data;
 	struct audioformat *fmt;
-	int ret;
+	unsigned int channels, rate, format;
+	int ret, changed;
 
 	ret = snd_pcm_lib_alloc_vmalloc_buffer(substream,
 					       params_buffer_bytes(hw_params));
 	if (ret < 0)
 		return ret;
 
-	subs->pcm_format = params_format(hw_params);
-	subs->period_bytes = params_period_bytes(hw_params);
-	subs->channels = params_channels(hw_params);
-	subs->cur_rate = params_rate(hw_params);
-
-	fmt = find_format(subs);
+	format = params_format(hw_params);
+	rate = params_rate(hw_params);
+	channels = params_channels(hw_params);
+	fmt = find_format(subs, format, rate, channels);
 	if (!fmt) {
 		snd_printd(KERN_DEBUG "cannot set format: format = %#x, rate = %d, channels = %d\n",
-			   subs->pcm_format, subs->cur_rate, subs->channels);
+			   format, rate, channels);
 		return -EINVAL;
 	}
 
+	changed = subs->cur_audiofmt != fmt ||
+		subs->period_bytes != params_period_bytes(hw_params) ||
+		subs->cur_rate != rate;
+
 	down_read(&subs->stream->chip->shutdown_rwsem);
-	if (subs->stream->chip->shutdown)
+	if (subs->stream->chip->shutdown) {
 		ret = -ENODEV;
-	else
-		ret = set_format(subs, fmt);
-	up_read(&subs->stream->chip->shutdown_rwsem);
-	if (ret < 0)
-		return ret;
+		goto unlock;
+	}
+	if ((ret = set_format(subs, fmt)) < 0)
+		goto unlock;
 
-	subs->interface = fmt->iface;
-	subs->altset_idx = fmt->altset_idx;
-	subs->need_setup_ep = true;
+	if (subs->cur_rate != rate) {
+		struct usb_host_interface *alts;
+		struct usb_interface *iface;
+		iface = usb_ifnum_to_if(subs->dev, fmt->iface);
+		alts = &iface->altsetting[fmt->altset_idx];
+		ret = snd_usb_init_sample_rate(subs->stream->chip, fmt->iface, alts, fmt, rate);
+		if (ret < 0)
+			goto unlock;
+		subs->cur_rate = rate;
+	}
 
-	return 0;
+	if (changed) {
+		/* format changed */
+		stop_endpoints(subs, 0, 0, 0);
+		ret = snd_usb_endpoint_set_params(subs->data_endpoint, hw_params, fmt,
+						  subs->sync_endpoint);
+		if (ret < 0)
+			goto unlock;
+
+		if (subs->sync_endpoint)
+			ret = snd_usb_endpoint_set_params(subs->sync_endpoint,
+							  hw_params, fmt, NULL);
+	}
+
+	if (ret == 0) {
+		subs->interface = fmt->iface;
+		subs->altset_idx = fmt->altset_idx;
+	}
+
+unlock:
+	up_read(&subs->stream->chip->shutdown_rwsem);
+	return ret;
 }
 
 /*
@@ -549,9 +546,7 @@
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_usb_substream *subs = runtime->private_data;
-	struct usb_host_interface *alts;
-	struct usb_interface *iface;
-	int ret;
+	int ret = 0;
 
 	if (! subs->cur_audiofmt) {
 		snd_printk(KERN_ERR "usbaudio: no format is specified!\n");
@@ -571,27 +566,6 @@
 	snd_usb_endpoint_sync_pending_stop(subs->sync_endpoint);
 	snd_usb_endpoint_sync_pending_stop(subs->data_endpoint);
 
-	ret = set_format(subs, subs->cur_audiofmt);
-	if (ret < 0)
-		goto unlock;
-
-	iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface);
-	alts = &iface->altsetting[subs->cur_audiofmt->altset_idx];
-	ret = snd_usb_init_sample_rate(subs->stream->chip,
-				       subs->cur_audiofmt->iface,
-				       alts,
-				       subs->cur_audiofmt,
-				       subs->cur_rate);
-	if (ret < 0)
-		goto unlock;
-
-	if (subs->need_setup_ep) {
-		ret = configure_endpoint(subs);
-		if (ret < 0)
-			goto unlock;
-		subs->need_setup_ep = false;
-	}
-
 	/* some unit conversions in runtime */
 	subs->data_endpoint->maxframesize =
 		bytes_to_frames(runtime, subs->data_endpoint->maxpacksize);
------------------------------------------------------------------------------
AlienVault Unified Security Management (USM) platform delivers complete
security visibility with the essential security capabilities. Easily and
efficiently configure, manage, and operate all of your security controls
from a single console and one unified framework. Download a free trial.
http://p.sf.net/sfu/alienvault_d2d
_______________________________________________
Alsa-user mailing list
Alsa-user@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/alsa-user

[Index of Archives]     [ALSA Devel]     [Linux Audio Users]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]

  Powered by Linux