Dne 27. 05. 21 v 11:52 Jerome Brunet napsal(a): > > On Thu 27 May 2021 at 08:52, Pavel Hofman <pavel.hofman@xxxxxxxxxxx> wrote: > >> Dne 24. 05. 21 v 18:09 Pavel Hofman napsal(a): >>> Dne 24. 05. 21 v 17:40 Jerome Brunet napsal(a): >>>> >>>> On Mon 24 May 2021 at 14:29, Pavel Hofman <pavel.hofman@xxxxxxxxxxx> >>>> wrote: >>>> >>>>> Dne 30. 04. 21 v 16:26 Jerome Brunet napsal(a): >>>>>> From: Ruslan Bilovol <ruslan.bilovol@xxxxxxxxx> >>>>>> >>>>>> 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 <ruslan.bilovol@xxxxxxxxx> >>>>>> Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx> >>>>> >>>>> >>>>> 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 ... Thanks for the patches, they look good. When you send your final explicit FB patchset, I will rebase your two playback patches and put all my changes (mainly rebased Julian's multiple samplerates patches) on top to test the whole suite. Pavel.
