Re: usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 30. 04. 24 20:51, Chris Wulff wrote:
>> Would it make sense to name the p/c_fu_name properties specifically
>> p/c_fu_volume_name, to leave name room for additional feature units?
>> Yeah, I think that makes sense. I will change it to be p/c_fu_volume_name.

Just a thought - maybe just p/c_fu_vol_name would be shorter without
loosing meaning, maybe.
> 
>> Please what does p/c_it_ch_name stand for?
> 
> This is the iChannelNames string from the Input Terminal descriptor
> "the name of the first logical channel" from the UAC1 spec.

Thanks!


>>
>> Maybe we could leave it to the userspace here too. In fact there are
>> already open-source clients which try to handle this master/slave
>> principle of the loopback and gadget basically in the same way
>> https://github.com/HEnquist/camilladsp/pull/341*issue-2267218348
>> https://github.com/HEnquist/camilladsp/issues/342
>>
> 
> I will take a look at the references you provided. I may have solved this problem
> in a different way and maybe there is a better way to handle this.
> 
> The primary use case for the USB gadget interface of the products that I am
> working on is to interface with UC clients (like Microsoft Teams or Google Meet).
> 
> My basic problem is that I'm using alsaloop to connect the UAC gadget to a
> second ALSA interface. I ended up making modifications to alsaloop that
> keeps it running correctly when the host starts/stops audio on the USB interface.
> Without doing that I was having trouble with dropping the start of speaking
> or high latency when the USB host decides to start streaming audio because of
> the time required to get alsaloop running again after getting notified that a
> different alt mode was selected.

I see, this is a common problem. In fact that linked CamillaDSP
discussion has partly the same motivation - to avoid having to restart
the whole process, to keep the latency down and loose as few initial
samples as possible.

> I do still have the plug plugin in the pipeline
> as well, so this does result in a double conversion with how I have the UAC
> gadget driver currently doing sample size conversion.
> 
> If we allow userspace to handle all the rate/channel conversion (which does
> seem like a cleaner approach and is what I initially was trying to do), I think that
> would mean advertising multiple bit depths/channel counts in the hw_params.
> That part is pretty easy.
> 
> Then the userspace program can pick which to use, but what should we do
> with the sample data to/from USB if the userspace program picks a different
> combination than the alt mode currently selected by the host?

This is a very good point! I did not think about it. Using the plug
plugin provides all the conversions necessary but hides the information
about the original hw values.

That would perhaps suggest to really add the alsa read-only controls
informing about required (and only allowed) channels and format, just
like aloop does
https://github.com/torvalds/linux/blob/0106679839f7c69632b3b9833c3268c316c0a9fc/sound/drivers/aloop.c#L1588-L1611


> 
> Perhaps just discarding audio when the ALSA and USB formats differ would
> be the right thing to do here.

Can the USB format (incl. samplerate) change without going through
altset 0 first, i.e. without explicit stop of the stream? A client
subscribed to the Capture/Playback Rate ctl notifications will learn
about this event first.

Currently the u_audio.c code just sends the ctl notification in
set_active(). There are cases in alsa drivers where change in incoming
spdif rate stops the stream too, e.g.
https://github.com/torvalds/linux/blob/0106679839f7c69632b3b9833c3268c316c0a9fc/sound/i2c/other/ak4114.c#L589-L597
. I vaguely remember this was discussed when implementing the current
u_audio solution and no support for stopping the stream was given. Maybe
it could be optionally enabled with some config parameter. It may make
it easier for the clients, otherwise they get stuck and wait for timeout.



> I might be able to solve my latency/data chopping
> issues instead by reconfiguring the ALSA pipeline in response to the change
> of the ssize ALSA control (or uevent). I think a fair bit of my time delay was
> re-launching alsaloop. I will experiment a bit with this and see what I can get.

IMO the final solution to the latency/data chopping issue should avoid
the restart of the loopback software (be it simple alsaloop or complex
CamillaDSP). Nevertheless IMO the device will need to be re-opened
anyway because hw params were changed. The gadget should facilitate this
goal, ideally.

Maybe a reasonable way would really be to offer the alsa ctls with
required params and let user decide if the conversions will be provided
by the plug plugin or by his client internally. E.g. a client natively
capable of 48k-multiples could accept these rates directly and let the
permanently-inserted plug do automatic rate conversion for the other
rates, no change in client configuration needed -> no client restart.

The question is (if this path was chosen) whether all of the controls
should notify (like they do in aloop), or only the rate one (which
always gets changed at any format change as the gadget must go through
set_active(false), IMO. I do not know what overhead the extra
notifications bring but every reduction counts :-)

>>
>> Are capture and playback alt modes dependent? I thought they were
>> separate configurations but I may be wrong.
>>
>> If they are separate, perhaps p_alt.X and c_alt.X dirs would make sense
>> instead, with using non-prefixed properties within them (ssize, ch_mask,
>> etc.). I.e. p/c_xxx on the main level (setting the defaults and default
>> alt1 for each direction) and non-prefixed properties in the actual
>> p/c_alt.X subdirs.
> 
> They are indeed separate. I like this idea. I will separate these into c_alt.x and p_alt.x
> which can be created separately.

Again - hats off to your fantastic effort.

Thanks a lot,

Pavel.



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux