On Thu 27 May 2021 at 08:52, Pavel Hofman [off-list ref] wrote:

Dne 24. 05. 21 v 18:09 Pavel Hofman napsal(a):

quoted

Dne 24. 05. 21 v 17:40 Jerome Brunet napsal(a):

quoted

On Mon 24 May 2021 at 14:29, Pavel Hofman [off-list ref]
wrote:

quoted

Dne 30. 04. 21 v 16:26 Jerome Brunet napsal(a):

quoted

From: Ruslan Bilovol <redacted>

This adds interface between userspace and feedback endpoint to report
real
feedback frequency to the Host.

Current implementation adds new userspace interface ALSA mixer control
"Capture Pitch 1000000" (similar to aloop driver's "PCM Rate Shift
100000"
mixer control)

Value in PPM is chosen to have correction value agnostic of the actual
HW
rate, which the application is not necessarily dealing with, while
still
retaining a good enough precision to allow smooth clock correction on
the
playback side, if necessary.

Similar to sound/usb/endpoint.c, a slow down is allowed up to 25%. This
has no impact on the required bandwidth. Speedup correction has an
impact
on the bandwidth reserved for the isochronous endpoint. The default
allowed speedup is 500ppm. This seems to be more than enough but, if
necessary, this is configurable through a module parameter. The
reserved
bandwidth is rounded up to the next packet size.

Usage of this new control is easy to implement in existing userspace
tools
like alsaloop from alsa-utils.

Signed-off-by: Ruslan Bilovol <redacted>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Hi, the existing patches solve the Host -> Gadget -> capturing
application direction, controlling the host playback rate. The other
direction (playback app -> gadget -> capturing host) is still paced by
the host USB controller. Packet size is pre-calculated in
u_audio_start_playback  as rate/p_interval
https://github.com/pavhofman/linux-rpi/blob/rpi-5.10.y/drivers/usb/gadget/function/u_audio.c#L441 
and this fixed value is used for copying the audio data in
u_audio_iso_complete
https://github.com/pavhofman/linux-rpi/blob/rpi-5.10.y/drivers/usb/gadget/function/u_audio.c#L124 
.

That means if the gadget has a physical duplex audio device with single
clock and runs a duplex operation, the path gadget-> host  will not run
synchronously with the physical audio device (the host -> gadget has
already the feedback control implemented).

How about "duplicating" the existing ALSA mixer control
"Capture Pitch 1000000" to "Playback Pitch 1000000" and using
pitch-adjusted p_srate in the above-linked calculations? That should
make the playback side run at the playback pitch requested by gadget
userspace, IIUC.

For the duplex operation with single clock, the capture pitch value
determined by the userspace chain (alsaloop, CamillaDSP, etc.) would be
used for setting both the capture and playback pitch controls, making
both directions run synchronously.

I can prepare patches based on Jerome's patchset should you find this
solution acceptable.

Well I have experimented with pitch on the playback path.
It does work but I'm not sure if it actually makes sense which is why I
have not not included it in RFC

If you need the playback and capture to run synchronously, you'd be
better off with implicit feedback (In which the host will provide the
same number of samples it received from the device)

With explicit feedback, we are (supposed) to tell the host to speed up
or slow down to match the device clock. This means the device run
asynchronously, and the host does the adaptation (if necessary). In such
case, I'm not sure adding pitch control on the playback path is a good
idea.

Having pitch control on the playback side allows to forward the audio
captured by the physical interface of the device (like I2S) to USB
without performing any resampling between the two (when USB and I2S are
not in sync). It's nice and convenient ... but I wonder if this is a
feature or a hack ;)

Jerome, thanks a lot for your reply. The current implementation of the 
EP IN audio direction is timed by the USB host controller. Let's consider
48Khz samplerate and bInterval=1 fullspeed (8k packets per second). Every
IN packet will always carry 6 audio frames. No matter how fast the real
master clock (i.e. e.g. I2S of the gadget) runs. Until an xrun occurs,
unfortunately. Even if the gadget configuration used implicit feedback,
the incoming stream would still provide no synchronization information
from the I2S hardware clock as the data stream is clocked by the USB host
which controls the timing on the USB bus (and thus the moment of iso
completion). Plus the stock usb-audio driver in Windows 10 does not
support implicit feedback, according to
https://docs.microsoft.com/en-us/windows-hardware/drivers/audio/usb-2-0-audio-drivers#audio-streaming 
.

The problem is the fixed "slicing" of the samples into the packets which
cannot be controlled. The same situation was on the host side before the 
feedback EP was introduced. Here the one preparing the slices (the
"transmitter") is the gadget now. And the slicing pace must be 
controllable just like the slicing pace on the host is via the feedback EP.
Because the gadget supports different playback and capture rates (which 
I find nice), I suggest a separate control element (Playback Pitch,
Capture Pitch).
Of course the motivation is to avoid adaptive resampling in the gadget 
in the direction I2S -> gadget interface -> USB host. But the very same
motivation lead to implementation of the async feedback EP in the 
opposite direction. IMO it is not a hack, but an indispensable feature
making the gadget usable for duplex operation with hardware (i.e. the 
real master) clock at the backend (all the other "clocks" are just
software-generated slices/blocks of audio frames).
With regards,
Pavel.

Jerome, please do you still have your playback-side patches available or
should I prepare them? Thanks a lot,

Yes, this is what I have tested:

https://gitlab.com/jbrunet/linux/-/commit/43f1930ba548e137a5d20b1801790fae83eaa1e0
https://gitlab.com/jbrunet/linux/-/commit/acb2ed9d9219488184cd2eb592fcdbf78b042a9e

It probably requires a rebase and cleaning before it is actually ready
for prime time but it does work.

For now, I'd like to focus on getting this initial explicit feedback
support in. There was no major complain on it, so I just have a minor
tweak to do before I send the patchset again. Hopefully soon ...

Pavel.