Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subscribe ALSA Ctl and trigger CAPTUREFORMATCHANGE for rate changes. #202

Open
Wang-Yue opened this issue May 11, 2022 · 16 comments
Open

subscribe ALSA Ctl and trigger CAPTUREFORMATCHANGE for rate changes. #202

Wang-Yue opened this issue May 11, 2022 · 16 comments
Labels
enhancement New feature or request linux Only affects Linux
Milestone

Comments

@Wang-Yue
Copy link
Contributor

Wang-Yue commented May 11, 2022

The existing design already subscribes for CoreAudio and WASAPI format changes and triggers STOP and throws CAPTUREFORMATCHANGE.
Take CoreAudio as example, it has two logics: Here it gets the rate event from the system and trigger CAPTUREFORMATCHANGE, regardless of user defined stop_on_rate_change value. This detection is based on CoreAudio's rate listener. And here it check if the measured rate is off 4% and trigger CAPTUREFORMATCHANGE if stop_on_rate_change = true

On ALSA the latter test is implemented here, but no former equivalent.

@Wang-Yue
Copy link
Contributor Author

@pavhofman

@HEnquist
Copy link
Owner

This should definitely be added yes. But the reason I haven't spent time on it yet is that in practice there aren't very many cases where it's actually useful. On MacOs and windows it's needed to support rate changes of the virtual soundcard drivers used there. But on Linux the Alsa loopback never changes rate while it's opened because of the broken notify functionality.
What is the application you want to use this for? Spdif input?

@DeLub
Copy link

DeLub commented May 12, 2022

No, USB input. The USB Gadget driver that creates the alsa device does support rate changes from kernel 5.18.

@HEnquist
Copy link
Owner

No, USB input. The USB Gadget driver that creates the alsa device does support rate changes from kernel 5.18.

Ah yes of course. I haven't had time to play with the new multi-rate gadget yet.

@Wang-Yue
Copy link
Contributor Author

Yeah @DeLub and I were early testers of the gadget code and I found this piece is missing from alsa.

@pavhofman
Copy link
Contributor

IMO there are several steps which would be required.

  1. Advanced handling the StatusMessage::CaptureFormatChange in bin.rs. Currently the format change results in stopping and reporting StopReason::CaptureFormatChange. In order to support independent operation without any external utility the main layer would have to be able to reload new config appropriate for the new format (be it rate, sample size, etc.) and restart the chain. IMO StatusMessage::CaptureFormatChange should be renamed to CaptureRateChange and CaptureFormatChange would be kept for the actual sample format change (as reported by aloop now, by the gadget likely in the future).

  2. Adding support for monitoring reported rate change and activity (start/stop) to the alsadevice module. Currently there are two alsa devices applicable - the aloop and the gadget. I will assume aloop notifications already work, if not they can be fixed, I will test them in 5.18 likely next week. Each device reports status of their other side differently, each requires a different code (with at least one or more internal ctl-monitoring threads). Every rate/format change reported from the other side is always preceded by the stop event (either directly reported by the other side, or as a result of the rate change detection by the rate monitor running in CDSP now).

IMO the gadget requires the delayed start as in https://github.com/pavhofman/gaudio_ctl#debouncing , to avoid false starts. Its params would require adding to alsa device config (with some reasonable default).

For reverse chain (ADC -> aloop/gadget) the functionality would have to be added to the Playback part of alsadevice + corresponding handling of StatusMessage::PlaybackFormatChange/PlaybackRateChange in bin.rs.

@HEnquist
Copy link
Owner

My thought on this:

  1. This will be nearly impossible to make general enough to work well. There are several problems to solve. The first is that there often is no clear answer to how the config should be modified to accommodate a new rate. It could simply enable resampling, but many users will likely want to instead follow with the sample rate of the dsp and playback device instead. And here it gets tricky, especially when using an interface where the available number of channels depends on the sample rate. Another complication is that Wasapi only sends an event saying that either sample rate or format (or both) changed, but not which one, and doesn't tell the new value(s). And when using spdif input, the device often can't tell what the new rate is, and the measured rate becmes some random nonsense. IMO it's much better to keep this in a separate python script, that each user can customize to their needs.
  2. The Alsa module should get the ability to monitor the gadget rate for sure. Same for the loopback, if notifications actually works there (I could not get it to work on my last attempt a few months ago). As for planning, I have quite a long list of things already planned for 1.1. I prefer small updates (the 0.6.3 to 1.0.0 was too big and too way too long) so I think I'll push this to 1.2.

@HEnquist HEnquist added enhancement New feature or request linux Only affects Linux labels May 14, 2022
@HEnquist HEnquist added this to the 1.2 milestone May 14, 2022
@pavhofman
Copy link
Contributor

pavhofman commented May 24, 2022

I just tested the loopback notifications. What does not work is breaking the stream on capture side when playback closes the stream. But notifications via alsa ctls do work, the principle is basically identical to the gadget. Ctl "PCM Slave Active" (true/false) reports whether the playback side is playing. Ctls "PCM Slave Rate", "PCM Slave Channels", and "PCM Slave Format" inform about current params of the playback stream, to be used on the capture side.

pi@raspberrypi:~ $ alsactl -d monitor hw:Loopback
node hw:Loopback, #3 (3,0,0,PCM Slave Active,0) VALUE
node hw:Loopback, #3 (3,0,0,PCM Slave Active,0) VALUE
node hw:Loopback, #3 (3,0,0,PCM Slave Active,0) VALUE
node hw:Loopback, #3 (3,0,0,PCM Slave Active,0) VALUE
node hw:Loopback, #5 (3,0,0,PCM Slave Rate,0) VALUE
node hw:Loopback, #3 (3,0,0,PCM Slave Active,0) VALUE
node hw:Loopback, #3 (3,0,0,PCM Slave Active,0) VALUE
node hw:Loopback, #6 (3,0,0,PCM Slave Channels,0) VALUE
node hw:Loopback, #3 (3,0,0,PCM Slave Active,0) VALUE
node hw:Loopback, #3 (3,0,0,PCM Slave Active,0) VALUE

I.e. monitoring the PCM Slave Active ctl controls the operation, with specific parameters to be subsequently read from the other ctls.

For comparison - the USB gadget defines only one ctl for each direction - Playback/Capture Rate which flips to 0 when the USB host stops the operation, no channels/format is reported (they are configured as fixed in the kernel module params).

Yeah, I can imagine the alsadevice having a dedicated constantly running thread which monitors these ctls, sending respective StatusMessages to the top-level thread which can start/stop the slaved threads, reload config etc. The existing code can already override the samplerate, channels, format, therefore handling that would be quite simple. But reloading a different config with filter params specific for the given samplerate would be tricky.

But a plain loopback with no samplerate-specific DSP, just for the gadget async feedback rate control, would work out of the box.

In configurations with resampling to one fixed playback rate for any capture rate no config reload would be needed, if the rate reported by the capture thread changed only the capture_samplerate, not the overall samplerate. It would have to be clearly communicated in the documentation as many users would get confused.

@HEnquist
Copy link
Owner

Those controls on the loopback are very interesting!
I'll start experimenting a bit with pyalsaaudio, like here: https://github.com/larsimmisch/pyalsaaudio/blob/main/mixertest.py

I think a separate python script is the right place to at least start. If that turns out to work well, and when the most common simple cases are well handled, then I would consider including this functionality directly in camilladsp.

I'm a little worried about the use case where an application plays at one sample rate into the loopback, and then switches to another rate. If the app tries to reopen the device immediately after closing, then camilladsp probably hasn't managed to close the capture side of the loopback yet. Here the non-functional breaking of the stream would really help.

@pavhofman
Copy link
Contributor

I was thinking of a separate module, something like DeviceMonitor. Capture and playback could have one optionally (Some()). IMO it could be just some refactoring of key parts of my https://github.com/pavhofman/gaudio_ctl. The module could send messages to the main thread through the same channel as the capture/playback threads (crossbeam_channel is multiple-producer capable), simplifying the main thread. Basically the "new status" messages could be sent by the existing capture/playback threads (e.g. the change rate detected by the existing runtime samplerate monitor), as well optionally by the optional device monitor associated with respective source/sink. The main thread may not care whether the message was sent by the actual source/sink thread or by the associated device monitor.

I can take a look at the implementation, in the following weeks, if you are OK with this direction.

If the app tries to reopen the device immediately after closing, then camilladsp probably hasn't managed to close the capture side of the loopback yet. Here the non-functional breaking of the stream would really help.

The serial stream of events on the CDSP side will always be in correct order - the stream stop message will always precede the new-rate message. Having the other side open is not a problem for the gadget, the USB side does not care about status of the gadget alsa device. For the loopback - the slaved side does need to be closed first to allow re-opening the master side with different hw_params. You are right this could be a problem if the master side is very fast. But I believe fixing the loopback should not be a major issue, just nobody has really looked into the problem thoroughly yet, IMO.

@HEnquist
Copy link
Owner

I have started prototyping a little in python: https://github.com/HEnquist/camilladsp-controller/blob/main/pyalsa_class.py
It's only for the loopback for now.

I had to switch to the official Alsa python binding pyalsa since pyalsaaudio is too limited. Unfortunately there is absolutely zero documentation for pyalsa, makes it a little hard to work with!

The Alsa backend should get a DeviceMonitor yes, to allow it to stop and report properly on rate/format changes. The monitor should send its messages to the capture or playback thread. Then that thread can stop and send a message to the main thread (that is how it's done in the Wasapi and CoreAudio backends). It would simplify things a lot if it can be done without adding more threads. Maybe by opening the hctl in non-blocking mode and letting the playback and capture loops call snd_hctl_handle_events(). The events could then be collected by registering callbacks that add the values to queues.

@pavhofman
Copy link
Contributor

Theoretically alsa should be able to wait for both pcm and ctl in a single poll(fds) call. That would minimize the latency of processing the start/stop notifications which IMO we need in this usecase. I asked about the combined wait at alsa-devel mailing list.

@pavhofman
Copy link
Contributor

It looks like polling multiple fds is the recommended method for this case https://mailman.alsa-project.org/pipermail/alsa-devel/2022-June/201479.html

https://github.com/alsa-project/alsa-utils/blob/a566f8a0ed6d7ca5fd6ae2d224f3f28654a2f3be/alsaloop/pcmjob.c#L1727
https://github.com/alsa-project/alsa-utils/blob/a566f8a0ed6d7ca5fd6ae2d224f3f28654a2f3be/alsaloop/pcmjob.c#L1738

https://github.com/alsa-project/alsa-utils/blob/a566f8a0ed6d7ca5fd6ae2d224f3f28654a2f3be/alsaloop/alsaloop.c#L762

Alsaloop is even polling all playback & capture pcm & ctl file descriptors in one poll, so all the processing runs in a single thread https://github.com/alsa-project/alsa-utils/blob/a566f8a0ed6d7ca5fd6ae2d224f3f28654a2f3be/alsaloop/alsaloop.c#L759

The alsa crate has full support for this method, and diwic even recommends using it (e.g. diwic/alsa-rs#33 (comment) )

The change would not be large, "just" initializing the pcm and hctl file descriptors using pcm.fill/hctl.fill, replacing pcm.wait with the poll, and analyzing the wait results with pcm.revents/hctl.revents.

Maybe the non-cached ctl would be faster than the cached hctl for receiving events, it should be easy to compare once the code for existing hctl works, the APIs are very similar.

@HEnquist I will take a look at this feature now, should you agree. It would make the CDSP framework easier to use than my gaudio_ctl + CDSP combo, also latencies of reaction to ctl change should be lower.

@HEnquist
Copy link
Owner

HEnquist commented Jun 7, 2022

Sorry I won't be able to take a look right now. We are moving to a new apartment. I should have more time soon!

@HEnquist
Copy link
Owner

@pavhofman Sorry it took a while! I agree that this seems like a good approach. Highly appreciated if you want to work on it!

@HEnquist HEnquist modified the milestones: 2.1, 4.0 Apr 18, 2024
@HEnquist
Copy link
Owner

A very early version of this can be found here: #341

@HEnquist HEnquist modified the milestones: 4.0, 3.0 Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request linux Only affects Linux
Projects
None yet
Development

No branches or pull requests

4 participants