Addressing audio quality problem.
In sound/usb/usbaudio.c, for the Hauppage HVR-950Q only, change retire_capture_urb to copy the entire byte stream while still counting entire audio frames. urbs unaligned on channel sample boundaries are still truncated to the next lowest stride (audio slot) size to try to retain channel alignment in cases of data loss over usb.
With the HVR950Q the left and right channel samples can be split between two different urbs. Throwing away extra channel samples causes a sound quality problem for stereo streams as the left and right channels are swapped repeatedly.
BugLink: https://bugs.launchpad.net/ubuntu/+bug/495745
Signed-off-by: John S. Gruber JohnSGruber@gmail.com --- usb/usbaudio.c | 53 ++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/usb/usbaudio.c b/usb/usbaudio.c index b074a59..d450b23 100644 --- a/usb/usbaudio.c +++ b/usb/usbaudio.c @@ -108,6 +108,7 @@ MODULE_PARM_DESC(ignore_ctl_error, #define MAX_URBS 8 #define SYNC_URBS 4 /* always four urbs for sync */ #define MAX_QUEUE 24 /* try not to exceed this queue length, in ms */ +#define ALLOW_SUBSLOT_BOUNDARIES 0x01 /* quirk */
struct audioformat { struct list_head list; @@ -127,6 +128,7 @@ struct audioformat { unsigned int rate_min, rate_max; /* min/max rates */ unsigned int nr_rates; /* number of rate table entries */ unsigned int *rate_table; /* rate table */ + unsigned int txfr_quirks; /* transfer quirks */ };
struct snd_usb_substream; @@ -175,6 +177,8 @@ struct snd_usb_substream { unsigned int running: 1; /* running status */
unsigned int hwptr_done; /* processed frame position in the buffer */ + unsigned int byteptr; /* position, in bytes, of next move */ + unsigned int txfr_quirks; /* substream transfer quirks */ unsigned int transfer_done; /* processed frames since last period update */ unsigned long active_mask; /* bitmask of active urbs */ unsigned long unlink_mask; /* bitmask of unlinked urbs */ @@ -343,7 +347,7 @@ static int retire_capture_urb(struct snd_usb_substream *subs, unsigned long flags; unsigned char *cp; int i; - unsigned int stride, len, oldptr; + unsigned int stride, len, bytelen, oldbyteptr; int period_elapsed = 0;
stride = runtime->frame_bits >> 3; @@ -354,29 +358,39 @@ static int retire_capture_urb(struct snd_usb_substream *subs, snd_printd(KERN_ERR "frame %d active: %d\n", i, urb->iso_frame_desc[i].status); // continue; } - len = urb->iso_frame_desc[i].actual_length / stride; - if (! len) + bytelen = (urb->iso_frame_desc[i].actual_length); + if (!bytelen) continue; + if (bytelen%(runtime->sample_bits>>3) != 0) { + int oldbytelen = bytelen; + bytelen = ((bytelen/stride)*stride); + snd_printd(KERN_ERR "Corrected urb data len. %d->%d\n", + oldbytelen, bytelen); + } + if (!(subs->txfr_quirks & ALLOW_SUBSLOT_BOUNDARIES)) + bytelen = (bytelen/stride)*stride; /* update the current pointer */ spin_lock_irqsave(&subs->lock, flags); - oldptr = subs->hwptr_done; - subs->hwptr_done += len; - if (subs->hwptr_done >= runtime->buffer_size) - subs->hwptr_done -= runtime->buffer_size; + len = (bytelen + (subs->byteptr % stride)) / stride; subs->transfer_done += len; if (subs->transfer_done >= runtime->period_size) { subs->transfer_done -= runtime->period_size; period_elapsed = 1; } + oldbyteptr = subs->byteptr; + subs->byteptr += bytelen; + if (subs->byteptr >= runtime->buffer_size*stride) + subs->byteptr -= runtime->buffer_size*stride; + subs->hwptr_done = subs->byteptr/stride; spin_unlock_irqrestore(&subs->lock, flags); /* copy a data chunk */ - if (oldptr + len > runtime->buffer_size) { - unsigned int cnt = runtime->buffer_size - oldptr; - unsigned int blen = cnt * stride; - memcpy(runtime->dma_area + oldptr * stride, cp, blen); - memcpy(runtime->dma_area, cp + blen, len * stride - blen); + if ((oldbyteptr + bytelen) > (runtime->buffer_size*stride)) { + unsigned int blen; + blen = (runtime->buffer_size*stride) - oldbyteptr; + memcpy(runtime->dma_area + oldbyteptr, cp, blen); + memcpy(runtime->dma_area, cp + blen, bytelen - blen); } else { - memcpy(runtime->dma_area + oldptr * stride, cp, len * stride); + memcpy(runtime->dma_area + oldbyteptr, cp, bytelen); } } if (period_elapsed) @@ -1363,6 +1377,7 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) subs->syncpipe = subs->syncinterval = 0; subs->maxpacksize = fmt->maxpacksize; subs->fill_max = 0; + subs->txfr_quirks = fmt->txfr_quirks;
/* we need a sync pipe in async OUT or adaptive IN mode */ /* check the number of EP, since some devices have broken @@ -1532,6 +1547,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) /* reset the pointer */ subs->hwptr_done = 0; subs->transfer_done = 0; + subs->byteptr = 0; subs->phase = 0; runtime->delay = 0;
@@ -2776,6 +2792,7 @@ static int parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no) fp->maxpacksize = (((fp->maxpacksize >> 11) & 3) + 1) * (fp->maxpacksize & 0x7ff); fp->attributes = csep ? csep[3] : 0; + fp->txfr_quirks = 0;
/* some quirks for attributes here */
@@ -2804,6 +2821,16 @@ static int parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no) else fp->ep_attr |= EP_ATTR_SYNC; break; + case USB_ID(0x2040, 0x7200): /* Hauppage hvr950Q */ + case USB_ID(0x2040, 0x7221): /* Hauppage hvr950Q */ + case USB_ID(0x2040, 0x7222): /* Hauppage hvr950Q */ + case USB_ID(0x2040, 0x7223): /* Hauppage hvr950Q */ + case USB_ID(0x2040, 0x7224): /* Hauppage hvr950Q */ + case USB_ID(0x2040, 0x7225): /* Hauppage hvr950Q */ + case USB_ID(0x2040, 0x7230): /* Hauppage hvr850 */ + case USB_ID(0x2040, 0x7250): /* Hauppage hvr950Q */ + fp->txfr_quirks |= ALLOW_SUBSLOT_BOUNDARIES; + break; }
/* ok, let's parse further... */