From: Pavel Hofman <pavel.hofman@ivitera.com>
To: Chris Wulff <Chris.Wulff@biamp.com>, Chris Wulff <crwulff@gmail.com>
Cc: Takashi Iwai <tiwai@suse.de>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
Ruslan Bilovol <ruslan.bilovol@gmail.com>,
Felipe Balbi <balbi@kernel.org>,
Jerome Brunet <jbrunet@baylibre.com>,
Julian Scheel <julian@jusst.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
AKASH KUMAR <quic_akakum@quicinc.com>,
Jack Pham <jackp@codeaurora.org>
Subject: Re: usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations
Date: Thu, 2 May 2024 13:13:19 +0200 [thread overview]
Message-ID: <fff3893d-dfa8-87ca-abe1-63bae8a32626@ivitera.com> (raw)
In-Reply-To: <CO1PR17MB54198ECD1842EAF3CC79985FE11A2@CO1PR17MB5419.namprd17.prod.outlook.com>
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.
prev parent reply other threads:[~2024-05-02 11:13 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-17 11:07 usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations Pavel Hofman
2024-04-24 7:40 ` Pavel Hofman
2024-04-25 9:22 ` Takashi Iwai
2024-04-25 15:07 ` Pavel Hofman
2024-04-28 15:30 ` Chris Wulff
2024-04-28 16:38 ` Chris Wulff
2024-04-29 15:02 ` Pavel Hofman
2024-04-30 18:51 ` Chris Wulff
2024-05-02 11:13 ` Pavel Hofman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fff3893d-dfa8-87ca-abe1-63bae8a32626@ivitera.com \
--to=pavel.hofman@ivitera.com \
--cc=Chris.Wulff@biamp.com \
--cc=alsa-devel@alsa-project.org \
--cc=balbi@kernel.org \
--cc=crwulff@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=jackp@codeaurora.org \
--cc=jbrunet@baylibre.com \
--cc=julian@jusst.de \
--cc=linux-usb@vger.kernel.org \
--cc=quic_akakum@quicinc.com \
--cc=ruslan.bilovol@gmail.com \
--cc=tiwai@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).