| Message ID | 1402762571-6316-2-git-send-email-m.chehab@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
(CC stable dropped; this is not how to submit stable patches.) Mauro Carvalho Chehab wrote: > The Auvitek 0828 chip, used on HVR950Q actually need two > quirks and not just one. > > The first one, already implemented, enforces that it won't have > channel swaps at the transfers. > > However, for TV applications, like xawtv and tvtime, another quirk > is needed, in order to enforce that, at least 2 URB transfer > intervals will be needed to fill a buffer. > + period = 2 * MAX_URBS * fp->maxpacksize; > + min_period = period * 90 / 100; > + max_period = period * 110 / 100; I don't quite understand what you mean with "URB transfer interval". All USB audio devices transfer packets in intervals between 125 µs and 1000 µs. MAX_URBS is a somewhat random value that is not directly derived from either a hardware or software constraint. Are you trying to enforce two packets per URB? Why are you setting both a minimum and a maximum? Isn't this affected by the constraints of the playback device? > Without it, buffer underruns happen when trying to syncronize the > audio input from au0828 and the audio playback at the default audio > output device. This looks like a workaround for a userspace bug that would affect all USB audio devices. What period/buffer sizes are xawtv/tvtime trying to use? Regards, Clemens -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> This looks like a workaround for a userspace bug that would affect all > USB audio devices. What period/buffer sizes are xawtv/tvtime trying to > use? I have similar concerns, although I don't know what the right solution is. For example, the last time Mauro tweaked the latency in tvtime, it broke support for all cx231xx devices (note that tvtime and xawtv share essentially the same ALSA code): http://git.linuxtv.org/cgit.cgi/tvtime.git/commit/?id=3d58ba563bfcc350c180b59a94cec746ccad6ebe It seems like there is definitely something wrong with the latency/period selection in both applications, but we need some insight from people who are better familiar with the ALSA subsystem for advice on the "right" way to do low latency audio capture (i.e. properly negotiating minimal latency in a way that works with all devices). Devin
Em Mon, 16 Jun 2014 09:39:17 +0200 Clemens Ladisch <clemens@ladisch.de> escreveu: > (CC stable dropped; this is not how to submit stable patches.) > > Mauro Carvalho Chehab wrote: > > The Auvitek 0828 chip, used on HVR950Q actually need two > > quirks and not just one. > > > > The first one, already implemented, enforces that it won't have > > channel swaps at the transfers. > > > > However, for TV applications, like xawtv and tvtime, another quirk > > is needed, in order to enforce that, at least 2 URB transfer > > intervals will be needed to fill a buffer. > > > + period = 2 * MAX_URBS * fp->maxpacksize; > > + min_period = period * 90 / 100; > > + max_period = period * 110 / 100; > > I don't quite understand what you mean with "URB transfer interval". > > All USB audio devices transfer packets in intervals between 125 µs and > 1000 µs. In this case, it uses a 1000 µs, as defined at the USB descriptor for the au0828 devices (bInterval). FYI, those TV devices are too limited, in terms of audio: they only provide: - 48 kHz rate; - 16 bits/sample; - 2 channels; - maxumum URB size: 256 bytes. Its internal firmware is also too buggy. We needed to add several workarounds at both analog and digital stream support for some conditions that caused the chip to stop producing URBs, or made it to cause ESHUTDOWN errors while streaming. > MAX_URBS is a somewhat random value that is not directly derived from > either a hardware or software constraint. Yes, I noticed that. > Are you trying to enforce two packets per URB? No, I'm trying to enforce that it won't complain about underruns, while keeping the latency constrained. This is the same kind of fix we needed to do with em28xx-audio.c some time ago. In this case, I'm enforcing that the URB callback will receive 3072 samples, and that the PCM timer won't be triggered too early, e. g. it will wait for the needed time for the URB callback to be called twice. > Why are you setting both a minimum and a maximum? When I wrote em28xx patches, I did several tests with different max latency constraints. On some cases, when it selected an odd number of periods, we would still have some troubles. So, it sounds safer to keep the same type of logic here. Anyway, just setting the minimum is enough for xawtv/tvtime to work with the default -alsa-latency parameter. > Isn't this affected by the constraints of the playback device? Hard to tell without having a test hardware with different constraints. All playback hardware I currently have supports 48 kHz rate, and supports a period size in the range of this The application takes those constraints into account > > > Without it, buffer underruns happen when trying to syncronize the > > audio input from au0828 and the audio playback at the default audio > > output device. > > This looks like a workaround for a userspace bug that would affect all > USB audio devices. What period/buffer sizes are xawtv/tvtime trying to > use? Both xawtv and tvtime use the same code for audio: http://git.linuxtv.org/cgit.cgi/xawtv3.git/tree/common/alsa_stream.c There's an algorithm there that gets the period size form both the capture and the playback cards, trying to find a minimum period that would work properly for both. It tries to enforce a choice where the max latency would be constrained. The max latency is 30ms, by default, but the user can change it via -alsa-latency parameter. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
16.06.2014 20:21, Mauro Carvalho Chehab wrote: > Both xawtv and tvtime use the same code for audio: > http://git.linuxtv.org/cgit.cgi/xawtv3.git/tree/common/alsa_stream.c > > There's an algorithm there that gets the period size form both the > capture and the playback cards, trying to find a minimum period that > would work properly for both. I don't see any adaptive resampler (similar to what module-loopback does in pulseaudio) there. Without that, or dynamically controlling the audio capture clock PLL in the tuner, xruns are unavoidable when transferring data between two unrelated cards. So, until any further evidence appears, I think it is a common bug in these audio codes.
Em Mon, 16 Jun 2014 09:22:08 -0400 Devin Heitmueller <dheitmueller@kernellabs.com> escreveu: > > This looks like a workaround for a userspace bug that would affect all > > USB audio devices. What period/buffer sizes are xawtv/tvtime trying to > > use? > > I have similar concerns, although I don't know what the right solution > is. For example, the last time Mauro tweaked the latency in tvtime, > it broke support for all cx231xx devices (note that tvtime and xawtv > share essentially the same ALSA code): > > http://git.linuxtv.org/cgit.cgi/tvtime.git/commit/?id=3d58ba563bfcc350c180b59a94cec746ccad6ebe > > It seems like there is definitely something wrong with the > latency/period selection in both applications, but we need some > insight from people who are better familiar with the ALSA subsystem > for advice on the "right" way to do low latency audio capture (i.e. > properly negotiating minimal latency in a way that works with all > devices). Well, I suspect that the issue is at Kernel level. Let's see the au0828 case: 48 kHz, 2 bytes/sample, 2 channels, 256 maxpacksize, 1 ms URB interval (bInterval = 1). In this case, there is 192 bytes per 1ms period. Let's assume that the period was set to 3456, with corresponds to a latency of 18 ms. In this case, as NUM_URBS = 12, it means that the transfer buffer will be set to its maximum value of 3072 bytes per URB pack (12 * 256), and the URB transfer_callback will be called on every 16 ms. So, what happens is: - after 16 ms, the first 3072 bytes arrive. The next packet will take another 16ms to arrive; - after 2 ms, underrun, as the period_size was not filled yet. The thing is that any latency that between 16 ms and 32 ms are invalid, as the URB settings won't support it. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Mon, 16 Jun 2014 20:38:52 +0600 "Alexander E. Patrakov" <patrakov@gmail.com> escreveu: > 16.06.2014 20:21, Mauro Carvalho Chehab wrote: > > Both xawtv and tvtime use the same code for audio: > > http://git.linuxtv.org/cgit.cgi/xawtv3.git/tree/common/alsa_stream.c > > > > There's an algorithm there that gets the period size form both the > > capture and the playback cards, trying to find a minimum period that > > would work properly for both. > > I don't see any adaptive resampler (similar to what module-loopback does > in pulseaudio) there. Are you referring to changing the sample rate? This doesn't affect my test scenario, as the playback interface supports the only PCM format/rate used by the TV card (48kHz, 16 bits/sample, stereo): Codec: Realtek ALC269VC Default PCM: rates [0x5f0]: 32000 44100 48000 88200 96000 192000 bits [0xe]: 16 20 24 formats [0x1]: PCM > Without that, or dynamically controlling the audio > capture clock PLL in the tuner, xruns are unavoidable when transferring > data between two unrelated cards. What do you mean by dynamically controlling the audio capture clock PLL in the tuner? That doesn't make any sense to me. The xc5000 tuner used on this TV device doesn't provide any mechanism to control audio PLL. It just sends the audio samples to au0828 via a I2S bus. All the audio control is done by the USB bridge at au0828, and that is pretty much limited. The only control that au0828 accepts is the control of the URB buffers (e. g., number of URB packets and URB size). > > So, until any further evidence appears, I think it is a common bug in > these audio codes. >
16.06.2014 22:24, Mauro Carvalho Chehab wrote: > Em Mon, 16 Jun 2014 20:38:52 +0600 > "Alexander E. Patrakov" <patrakov@gmail.com> escreveu: > >> 16.06.2014 20:21, Mauro Carvalho Chehab wrote: >>> Both xawtv and tvtime use the same code for audio: >>> http://git.linuxtv.org/cgit.cgi/xawtv3.git/tree/common/alsa_stream.c >>> >>> There's an algorithm there that gets the period size form both the >>> capture and the playback cards, trying to find a minimum period that >>> would work properly for both. >> >> I don't see any adaptive resampler (similar to what module-loopback does >> in pulseaudio) there. > > Are you referring to changing the sample rate? This doesn't > affect my test scenario, as the playback interface supports the > only PCM format/rate used by the TV card (48kHz, 16 bits/sample, stereo): > > Codec: Realtek ALC269VC > Default PCM: > rates [0x5f0]: 32000 44100 48000 88200 96000 192000 > bits [0xe]: 16 20 24 > formats [0x1]: PCM No, it doesn't. The card only pretends to give out samples at 48000 Hz, but, due to the imperfect quartz, actually gives them out at something like 48010 or 47990 Hz (if we take the Realtek's idea of 48 kHz as the source of truth), and that even changes slightly due to thermal issues. The goal here is to measure the actual sample rate and to resample from it to 48 kHz. The "alsaloop" program (part of alsa-utils), when compiled with libsamplerate, does exactly that. If GPLv2+ is OK for you, you can copy the code. >> Without that, or dynamically controlling the audio >> capture clock PLL in the tuner, xruns are unavoidable when transferring >> data between two unrelated cards. > > What do you mean by dynamically controlling the audio capture clock PLL > in the tuner? That doesn't make any sense to me. Some chips (e.g. SAA7133) have a register that allows fine-tuning the actual rate at which they sample the sound (while applications still think that it is nominally at 32 kHz). This register is not exposed at the ALSA level, but exposed as the "audio_clock_tweak" parameter of the saa7134 module. Linux applications normally don't use this register, but Windows uses this register as follows. The official TV playback application, found on the CD with drivers, captures samples from the card into its buffer, and plays from the other end of the buffer concurrently. If there are, on average for a few seconds, too few samples in the buffer, it means that they are consumed faster than they arrive, and so the SAA chip is told to produce them a bit faster. If they accumulate too much, the SAA chip is told to produce them slower. That's it. > > The xc5000 tuner used on this TV device doesn't provide any mechanism > to control audio PLL. It just sends the audio samples to au0828 via a > I2S bus. All the audio control is done by the USB bridge at au0828, > and that is pretty much limited. The only control that au0828 accepts > is the control of the URB buffers (e. g., number of URB packets and > URB size). OK, as you can't tweak the PLL, you have to resample. The idea is, again, simple. Record samples to a buffer if you can, and play them through a variable-rate resampler concurrently if you can. You can use poll() to figure out the "if you can" part. If samples accumulate too much or if the buffer becomes too empty, change the resampling ratio slightly in order to compensate. As I said, the code is here: http://git.alsa-project.org/?p=alsa-utils.git;a=tree;f=alsaloop
Em Mon, 16 Jun 2014 23:16:02 +0600 "Alexander E. Patrakov" <patrakov@gmail.com> escreveu: > 16.06.2014 22:24, Mauro Carvalho Chehab wrote: > > Em Mon, 16 Jun 2014 20:38:52 +0600 > > "Alexander E. Patrakov" <patrakov@gmail.com> escreveu: > > > >> 16.06.2014 20:21, Mauro Carvalho Chehab wrote: > >>> Both xawtv and tvtime use the same code for audio: > >>> http://git.linuxtv.org/cgit.cgi/xawtv3.git/tree/common/alsa_stream.c > >>> > >>> There's an algorithm there that gets the period size form both the > >>> capture and the playback cards, trying to find a minimum period that > >>> would work properly for both. > >> > >> I don't see any adaptive resampler (similar to what module-loopback does > >> in pulseaudio) there. > > > > Are you referring to changing the sample rate? This doesn't > > affect my test scenario, as the playback interface supports the > > only PCM format/rate used by the TV card (48kHz, 16 bits/sample, stereo): > > > > Codec: Realtek ALC269VC > > Default PCM: > > rates [0x5f0]: 32000 44100 48000 88200 96000 192000 > > bits [0xe]: 16 20 24 > > formats [0x1]: PCM > > No, it doesn't. The card only pretends to give out samples at 48000 Hz, > but, due to the imperfect quartz, actually gives them out at something > like 48010 or 47990 Hz (if we take the Realtek's idea of 48 kHz as the > source of truth), and that even changes slightly due to thermal issues. > The goal here is to measure the actual sample rate and to resample from > it to 48 kHz. I see. Well, you're talking about a xrun that would happen after several seconds, and caused by a clock drift between capture and playback xtals. The issue we're facing happens dozen of times a second, and it is caused by something else: despiste what period_size says, with this board, the sampling period is not continuous. The reason for that is simple: 1ms is the minimal interval for this board can feed interrupts, and the minimal period size = 192 bytes (as anything lower than 192 bytes would cause data to be dropped). As the maximum limit per URB transfer is 3072 bytes (12 URB packs, with 256 bytes each), we have: Period time = (num_bytes + 191) / 192 ms For num_bytes between 192 and 3072. After 3072, the URB endpoint will always use 3072 bytes per URB transfer, with takes 16 ms to be filled. So, the period time is given by: Period time = (num_bytes / 3072) * 16 ms In other words, the period ranges from 1 to 16 ms, in 1ms step, up to 3072 bytes. After that, the period is always multiple of 16 ms. Trying to set it to any value between 17 ms and 31 ms causes xruns, because the buffer will only be filled the next time the URB callback is called (and this happens on every 16 ms). That's what happens with xawtv currently: while the hardware has a 16ms-multiple constraint for the period size, but as there's no constraints at the ALSA Kernel code enforcing a period size multiple of 16 ms, xawtv will set it to its default latency (30 ms). That works fine for the first transfer, but it gets an underrun before the second one. So, we have about 30 underruns per second. There's nothing that can be done on userspace, as the 16 ms multiple request is specific to this card. Other cards use different URB constraints, as different maxsize and/or different number of URBs would result on a different period size. So, changing it in userspace from one card breaks for the others. > The "alsaloop" program (part of alsa-utils), when compiled > with libsamplerate, does exactly that. If GPLv2+ is OK for you, you can > copy the code. After having this big xrun issue fixed, I'll look into it. I need to check if xawtv/tvtime license is GPLv2. > >> Without that, or dynamically controlling the audio > >> capture clock PLL in the tuner, xruns are unavoidable when transferring > >> data between two unrelated cards. > > > > What do you mean by dynamically controlling the audio capture clock PLL > > in the tuner? That doesn't make any sense to me. > > Some chips (e.g. SAA7133) have a register that allows fine-tuning the > actual rate at which they sample the sound (while applications still > think that it is nominally at 32 kHz). This register is not exposed at > the ALSA level, but exposed as the "audio_clock_tweak" parameter of the > saa7134 module. Linux applications normally don't use this register, but > Windows uses this register as follows. > > The official TV playback application, found on the CD with drivers, > captures samples from the card into its buffer, and plays from the other > end of the buffer concurrently. If there are, on average for a few > seconds, too few samples in the buffer, it means that they are consumed > faster than they arrive, and so the SAA chip is told to produce them a > bit faster. If they accumulate too much, the SAA chip is told to produce > them slower. That's it. Ok. Well, xc5000 (with does the audio sampling) doesn't have it, AFAIKT. > > > > The xc5000 tuner used on this TV device doesn't provide any mechanism > > to control audio PLL. It just sends the audio samples to au0828 via a > > I2S bus. All the audio control is done by the USB bridge at au0828, > > and that is pretty much limited. The only control that au0828 accepts > > is the control of the URB buffers (e. g., number of URB packets and > > URB size). > > OK, as you can't tweak the PLL, you have to resample. The idea is, > again, simple. Record samples to a buffer if you can, and play them > through a variable-rate resampler concurrently if you can. You can use > poll() to figure out the "if you can" part. If samples accumulate too > much or if the buffer becomes too empty, change the resampling ratio > slightly in order to compensate. As I said, the code is here: > > http://git.alsa-project.org/?p=alsa-utils.git;a=tree;f=alsaloop > Ok, I'll look into it after fixing this issue. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> The official TV playback application, found on the CD with drivers, >> captures samples from the card into its buffer, and plays from the other >> end of the buffer concurrently. If there are, on average for a few >> seconds, too few samples in the buffer, it means that they are consumed >> faster than they arrive, and so the SAA chip is told to produce them a >> bit faster. If they accumulate too much, the SAA chip is told to produce >> them slower. That's it. > > Ok. Well, xc5000 (with does the audio sampling) doesn't have it, AFAIKT. > >> > >> > The xc5000 tuner used on this TV device doesn't provide any mechanism >> > to control audio PLL. It just sends the audio samples to au0828 via a >> > I2S bus. All the audio control is done by the USB bridge at au0828, >> > and that is pretty much limited. The only control that au0828 accepts >> > is the control of the URB buffers (e. g., number of URB packets and >> > URB size). It's probably worth noting that Mauro's explanation here is incorrect - the xc5000 does *not* put out I2S. It outputs an SIF which is fed to the au8522. The au8522 has the audio decoder, and it's responsible for putting out I2S to the au0828. Hence the xc5000's PLL would have no role here. In fact, you should see the exact same behavior on the A/V input, since the au8522 is responsible for the I2S clock which drives the cs5503 (the 5503 is in slave mode). Devin
Em Mon, 16 Jun 2014 15:04:44 -0400 Devin Heitmueller <dheitmueller@kernellabs.com> escreveu: > >> The official TV playback application, found on the CD with drivers, > >> captures samples from the card into its buffer, and plays from the other > >> end of the buffer concurrently. If there are, on average for a few > >> seconds, too few samples in the buffer, it means that they are consumed > >> faster than they arrive, and so the SAA chip is told to produce them a > >> bit faster. If they accumulate too much, the SAA chip is told to produce > >> them slower. That's it. > > > > Ok. Well, xc5000 (with does the audio sampling) doesn't have it, AFAIKT. > > > >> > > >> > The xc5000 tuner used on this TV device doesn't provide any mechanism > >> > to control audio PLL. It just sends the audio samples to au0828 via a > >> > I2S bus. All the audio control is done by the USB bridge at au0828, > >> > and that is pretty much limited. The only control that au0828 accepts > >> > is the control of the URB buffers (e. g., number of URB packets and > >> > URB size). > > It's probably worth noting that Mauro's explanation here is incorrect > - the xc5000 does *not* put out I2S. It outputs an SIF which is fed > to the au8522. The au8522 has the audio decoder, and it's responsible > for putting out I2S to the au0828. > > Hence the xc5000's PLL would have no role here. > Ah, OK. I didn't notice that hvr950q was using SIF. In this case, then maybe au8522 may have some PLL fine tune register to adjust the sampling rate. > In fact, you should see the exact same behavior on the A/V input, > since the au8522 is responsible for the I2S clock which drives the > cs5503 (the 5503 is in slave mode). I suspect that the best is to use a resampling code to avoid the frequency drift clock issue, as it is generic enough to cover both cases, and won't require hardware-assisted support. Yet, as I posted on my previous email, this is not yet the kind of bug that we're facing right now. If I'm doing the calculus correct, a -10Hz difference at a 48 kHz sampling rate would take ~150 seconds to cause an underrun (a 16 ms period_bytes lost). Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab wrote: > Let's see the au0828 case: > 48 kHz, 2 bytes/sample, 2 channels, 256 maxpacksize, 1 ms URB > interval (bInterval = 1). > > In this case, there is 192 bytes per 1ms period. The device's clock and the bus clock are not synchronized, so there will be _approximately_ 192 bytes per USB frame. > Let's assume that the period was set to 3456, with corresponds to > a latency of 18 ms. > > In this case, as NUM_URBS = 12, There is no symbol named NUM_URBS. > it means that the transfer buffer will be set to its maximum value of > 3072 bytes per URB pack (12 * 256) The number of URBs is not the same as the number of packets per URB. > and the URB transfer_callback will be called on every 16 ms. It will be called once per millisecond. > So, what ... definitely not ... > happens is: > > - after 16 ms, the first 3072 bytes arrive. The next > packet will take another 16ms to arrive; > - after 2 ms, underrun, as the period_size was not > filled yet. Regards, Clemens -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab wrote: > Both xawtv and tvtime use the same code for audio: > http://git.linuxtv.org/cgit.cgi/xawtv3.git/tree/common/alsa_stream.c > > There's an algorithm there that gets the period size form both the > capture and the playback cards, trying to find a minimum period that > would work properly for both. Why are you trying to match period sizes? The sample clocks won't be synchronized anyway, so it is not possible to force them to happen at the same time. Please note that for playback devices, the latency is the same as the buffer length, while for capture device, the latency is the same as the _period_ length. Therefore, it does not make sense to put an upper limit on the size of the capture buffer. I do not think it is a good idea to stop the capture device when it overruns. Regards, Clemens -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/sound/usb/card.h b/sound/usb/card.h index 97acb906acc2..f6f2c7ca6ed4 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -122,6 +122,7 @@ struct snd_usb_substream { unsigned int buffer_periods; /* current periods per buffer */ unsigned int altset_idx; /* USB data format: index of alternate setting */ unsigned int txfr_quirk:1; /* allow sub-frame alignment */ + unsigned int media_sync_quirk:1;/* Enforce a min period_bytes big enough to handle 2 URB transfer periods */ unsigned int fmt_type; /* USB audio format type (1-3) */ unsigned int pkt_offset_adj; /* Bytes to drop from beginning of packets (for non-compliant devices) */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index c62a1659106d..e4a215b63d9f 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1102,6 +1102,8 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre unsigned int pt, ptmin; int param_period_time_if_needed; int err; + size_t period_bytes_min = UINT_MAX; + size_t period_bytes_max = 0; runtime->hw.formats = subs->formats; @@ -1129,7 +1131,39 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre } pt = 125 * (1 << fp->datainterval); ptmin = min(ptmin, pt); + + if (subs->media_sync_quirk) { + size_t period, min_period, max_period; + + /* + * multimedia streams have both Audio and Video + * URBs and limited contraints with regards to + * the period size. Also, their stream will be + * most likely synked with the playback card with + * different constraints. + * Due to that, we need to be sure that, at least + * 2 URB transfers will be needed to fill a period, + * in order to avoid buffer underruns on applications + * like tvtime and xawtv. + * A non-multiple of period_bytes_min can also be + * a problem, so the best is to also enfoce it. + */ + period = 2 * MAX_URBS * fp->maxpacksize; + min_period = period * 90 / 100; + max_period = period * 110 / 100; + + if (period_bytes_min > min_period) + period_bytes_min = min_period; + if (period_bytes_max < max_period) + period_bytes_max = max_period; + } } + + if (period_bytes_min != UINT_MAX && period_bytes_max != 0) { + runtime->hw.period_bytes_min = period_bytes_min; + runtime->hw.period_bytes_max = period_bytes_max; + } + err = snd_usb_autoresume(subs->stream->chip); if (err < 0) return err; diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index f652b10ce905..236adf61d27a 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -2757,7 +2757,7 @@ YAMAHA_DEVICE(0x7010, "UB99"), .vendor_name = "Hauppauge", .product_name = "HVR-950Q", .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, + .type = QUIRK_AUDIO_AUVITEK_0828, } }, { @@ -2771,7 +2771,7 @@ YAMAHA_DEVICE(0x7010, "UB99"), .vendor_name = "Hauppauge", .product_name = "HVR-950Q", .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, + .type = QUIRK_AUDIO_AUVITEK_0828, } }, { @@ -2785,7 +2785,7 @@ YAMAHA_DEVICE(0x7010, "UB99"), .vendor_name = "Hauppauge", .product_name = "HVR-950Q", .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, + .type = QUIRK_AUDIO_AUVITEK_0828, } }, { @@ -2799,7 +2799,7 @@ YAMAHA_DEVICE(0x7010, "UB99"), .vendor_name = "Hauppauge", .product_name = "HVR-950Q", .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, + .type = QUIRK_AUDIO_AUVITEK_0828, } }, { @@ -2813,7 +2813,7 @@ YAMAHA_DEVICE(0x7010, "UB99"), .vendor_name = "Hauppauge", .product_name = "HVR-950Q", .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, + .type = QUIRK_AUDIO_AUVITEK_0828, } }, { @@ -2827,7 +2827,7 @@ YAMAHA_DEVICE(0x7010, "UB99"), .vendor_name = "Hauppauge", .product_name = "HVR-950Q", .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, + .type = QUIRK_AUDIO_AUVITEK_0828, } }, { @@ -2841,7 +2841,7 @@ YAMAHA_DEVICE(0x7010, "UB99"), .vendor_name = "Hauppauge", .product_name = "HVR-850", .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, + .type = QUIRK_AUDIO_AUVITEK_0828, } }, { @@ -2855,7 +2855,7 @@ YAMAHA_DEVICE(0x7010, "UB99"), .vendor_name = "Hauppauge", .product_name = "HVR-950Q", .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, + .type = QUIRK_AUDIO_AUVITEK_0828, } }, { @@ -2869,7 +2869,7 @@ YAMAHA_DEVICE(0x7010, "UB99"), .vendor_name = "Hauppauge", .product_name = "HVR-950Q", .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, + .type = QUIRK_AUDIO_AUVITEK_0828, } }, diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 7c57f2268dd7..fea68eef5bdb 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -74,15 +74,21 @@ static int ignore_interface_quirk(struct snd_usb_audio *chip, /* - * Allow alignment on audio sub-slot (channel samples) rather than - * on audio slots (audio frames) + * There are actually two quirks needed by Auvitek 0828 audio: + * + * 1) Allow alignment on audio sub-slot (channel samples) rather than + * on audio slots (audio frames), enabled via chip->txfr_quirk + * + * 2) Enforce that the minimum period_bytes will be able to store, + * at least, 2 URB transfer intervals. */ -static int create_align_transfer_quirk(struct snd_usb_audio *chip, +static int create_auvitek_0828_quirk(struct snd_usb_audio *chip, struct usb_interface *iface, struct usb_driver *driver, const struct snd_usb_audio_quirk *quirk) { chip->txfr_quirk = 1; + chip->media_sync_quirk = 1; return 1; /* Continue with creating streams and mixer */ } @@ -529,7 +535,7 @@ int snd_usb_create_quirk(struct snd_usb_audio *chip, [QUIRK_AUDIO_STANDARD_INTERFACE] = create_standard_audio_quirk, [QUIRK_AUDIO_FIXED_ENDPOINT] = create_fixed_stream_quirk, [QUIRK_AUDIO_EDIROL_UAXX] = create_uaxx_quirk, - [QUIRK_AUDIO_ALIGN_TRANSFER] = create_align_transfer_quirk, + [QUIRK_AUDIO_AUVITEK_0828] = create_auvitek_0828_quirk, [QUIRK_AUDIO_STANDARD_MIXER] = create_standard_mixer_quirk, }; diff --git a/sound/usb/stream.c b/sound/usb/stream.c index 310a3822d2b7..2ea6516e5e34 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -92,6 +92,7 @@ static void snd_usb_init_substream(struct snd_usb_stream *as, subs->direction = stream; subs->dev = as->chip->dev; subs->txfr_quirk = as->chip->txfr_quirk; + subs->media_sync_quirk = as->chip->media_sync_quirk; subs->speed = snd_usb_get_speed(subs->dev); subs->pkt_offset_adj = 0; diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 91d0380431b4..a38e94554022 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -43,6 +43,7 @@ struct snd_usb_audio { unsigned int in_pm:1; unsigned int autosuspended:1; unsigned int txfr_quirk:1; /* Subframe boundaries on transfers */ + unsigned int media_sync_quirk:1;/* Enforce a min period_bytes big enough to handle 2 URB transfer periods */ int num_interfaces; int num_suspended_intf; @@ -97,7 +98,7 @@ enum quirk_type { QUIRK_AUDIO_STANDARD_INTERFACE, QUIRK_AUDIO_FIXED_ENDPOINT, QUIRK_AUDIO_EDIROL_UAXX, - QUIRK_AUDIO_ALIGN_TRANSFER, + QUIRK_AUDIO_AUVITEK_0828, QUIRK_AUDIO_STANDARD_MIXER, QUIRK_TYPE_COUNT
The Auvitek 0828 chip, used on HVR950Q actually need two quirks and not just one. The first one, already implemented, enforces that it won't have channel swaps at the transfers. However, for TV applications, like xawtv and tvtime, another quirk is needed, in order to enforce that, at least 2 URB transfer intervals will be needed to fill a buffer. Without it, buffer underruns happen when trying to syncronize the audio input from au0828 and the audio playback at the default audio output device. As the second quirk may be needed by some other media devices not based on au0828 chipset, keep it as a separate quirk. Cc: stable@vger.kernel.org Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> --- sound/usb/card.h | 1 + sound/usb/pcm.c | 34 ++++++++++++++++++++++++++++++++++ sound/usb/quirks-table.h | 18 +++++++++--------- sound/usb/quirks.c | 14 ++++++++++---- sound/usb/stream.c | 1 + sound/usb/usbaudio.h | 3 ++- 6 files changed, 57 insertions(+), 14 deletions(-)