Hi,
On 08/25/2014 07:22 PM, Jassi Brar wrote:
> On Mon, Aug 25, 2014 at 9:30 PM, Daniel Mack <zonque@xxxxxxxxx> wrote:
>> The UAC2 function driver currently responds to all packets at all times
>> with wMaxPacketSize packets. That results in way too fast audio
>> playback as the function driver (which is in fact supposed to define
>> the audio stream pace) delivers as fast as it can.
>>
>> Fix this by pre-calculating the size of each packet to meet the
>> requested sample rate and format. This won't be 100% accurate, but
>> that's acceptable.
>>
> For rates like 44100 we will always hear clicks because we can not
> transfer 176400bytes by packets of equal size over duration of 1
> second.
How do you test to hear those clicks? If you do "arecord | aplay" on the
host, you will have underruns or overruns at some point, because the
internal sound interface of the host runs at a different speed than the
gadget. This, however, also happens when you use any other USB sound
card, and is hence it not surprising.
It doesn't really matter if we transfer 176000 or 176400 bytes per
second, measured by the host's time base. After all, the internal sound
card may also be off by some percentage, depending on how it is built.
We shouldn't be too far off though, as we currently are.
> The inaccuracy here is due to the way we program, and not due
> to system/bus load.
Sure, but rates across devices will never match, so it doesn't matter
really. Two clocks on two devices will always drift, even if they're
totally accurate by their own means. You have the same situation the
other way around on the playback endpoint: the host plays at whatever
speed it considers to be 176400 bytes/sec, which will never be exactly
176400 bytes/s measured by the gadget's clock, because there is no
feedback endpoint. Audio applications have to be aware of that, and if
they need to synchronize two devices with their own clock each, they
have to implement some sort of resampler.
> Have you tried the approach I suggested - {4x176, 1x178} pattern
> packets, and does that not work? Please let me know if I am
> overlooking something. Otherwise let us do the best we can (If you
> want me to give that a try, I can in a day or two).
That would only be necessary if you want the gadget's playback device to
run absolutely in sync with its system clock. And there's no need for
that IMO.
>> @@ -187,7 +189,7 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request *req)
>>
>> if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> src = prm->dma_area + prm->hw_ptr;
>> - req->actual = req->length;
>> + req->length = req->actual = uac2->c_pktsize;
>>
> This doesn't seem right.
> We asked req->length to be transmitted by the udc which shouldn't
> return until that is done. So at this point setting 'length' doesn't
> mean much.
That's right. I had it fixed already. Seems I staged the wrong version.
Will fix in v3, thanks!
> which should render the patch-4/4 needless, I hope because there is
> nowhere 512 in the code and neither did I assume any such fixed value.
The maxsize variables are initialized to the endpoint's wMaxPacketSize,
which is 512. So your audio packets will go in slices of 512, and so
they'll always fit nicely into the dma buffer, which has 64k.
> We simply alloc 2 usb requests of wMaxPacketSize each and copy data
> to/from the ALSA ring buffer. For you the wMaxPacketSize might be 512,
> but the code should work for any value.
Exactly, but with 65536 bytes in the DMA buffer, and 176 bytes in each
packet, you will have an uneven wrap around around each 372th packet, so
we need to address these cases.
Best regards,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
