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

RFC: Accepting more format changes #342

Open
pavhofman opened this issue Apr 29, 2024 · 7 comments
Open

RFC: Accepting more format changes #342

pavhofman opened this issue Apr 29, 2024 · 7 comments

Comments

@pavhofman
Copy link
Contributor

pavhofman commented Apr 29, 2024

Currently the Capture/PlaybackFormatChange carries new samplerate integer. The alsa loopback device already does (and the usb gadget will do https://lore.kernel.org/all/CAB0kiBJm=Ya6a1mWRZ28p9=D_BesH55DFk4fd4wP0be4zKPR7w@mail.gmail.com/T/#mc2b68c2e74ed57e2998c1b0f42bfca82e07db181 ) offer a change in channel count and sample format too.

Rate can be read from the ctl. The sample format and channels params are not announced via some ctl, but would need to be read from the current hw params. These slaved devices always offer only one value as it's specified by their master side when activating the transmission.

Maybe it would be convenient to determine the newly=currently allowed params directly by the capture/playback threads when opening the device. Some params would be acceptable (e.g. sample format), some may be refused => exit (e.g. channel count not fitting the processing), some may require some adjustments (e.g. if the channel count could be accommodated to the current pipeline configuration somehow, or e.g. by selecting a different pipeline configuration for the given number of channels - just brainstorming).

When doing the current format params detection, maybe even the samplerate could be included. That would allow to kick in the resampler automatically even at first start, without the FormatChange message received. That could be usable for all backends. It would result in no parameter having to be carried by the FormatChange message.

In alsa all these format changes could be handled simply by using the plug plugin for the device (apart of samplerate conversion which is much more efficient in CDSP). In other backends which do not have the plug "luxury" they would need some CDSP support (probably just the format conversion which is already available anyway).

Just a thought for a bit of discussion, no real issue :-)

@pavhofman pavhofman changed the title RFC: RFC: Accepting more format changes Apr 29, 2024
@HEnquist
Copy link
Owner

It would be really nice if the gadget number of channels and format were exposed in controls too, since that would allow us to stop as soon as a change is announced. Or would changing these break capture anyway?

It would be easy to add auto detection of the sample format. In other backends this is controlled by setting the format to null to use the default. For alsa, this could be done by just picking the best available format. CamillaDSP sets the format as the last parameter, so doing this can't affect anything else.

Letting channels be autodetected would be much more difficult. I would at least wait with this one.
Sample rate is also a bit tricky but can probably be handled. I have added support for capturing from Wav, that can use the sample rate in the file. See here: https://github.com/HEnquist/camilladsp/blob/next30/README.md#file-rawfile-wavfile-stdin-stdout
A similar approach should work for other backends.

@pavhofman
Copy link
Contributor Author

pavhofman commented Apr 30, 2024

It would be really nice if the gadget number of channels and format were exposed in controls too, since that would allow us to stop as soon as a change is announced. Or would changing these break capture anyway?

IMO once the device is opened with some hw_params, the params cannot be changed within that session. So any change of any of the params would be preceded with the stop (rate -> 0) and start signals.

As of reading all the new params at the moment of start signal - the existing devices do not offer these controls. But IMO it would be superfluous because when configuring hw_params before opening the device the param checks need to be done anyway. I think there is no need to pass the new params in the FormatChange msg, at least for alsa.

It would be easy to add auto detection of the sample format. In other backends this is controlled by setting the format to null to use the default. For alsa, this could be done by just picking the best available format. CamillaDSP sets the format as the last parameter, so doing this can't affect anything else.

Letting channels be autodetected would be much more difficult. I would at least wait with this one. Sample rate is also a bit tricky but can probably be handled.

IMO in alsa a check for any params would be identical and is mostly already implemented - setting default hw by HwParams::any(&pcmdev) , then setting/checking individual parameters with near and checking what the method returns. Format could be read by hwp.get_format.

The slaved sides of these devices always offer only one value (locked by their master side first) which makes it easier than e.g. in the plug plugin which must work with ranges offered by the slave device.

I have added support for capturing from Wav, that can use the sample rate in the file. See here: https://github.com/HEnquist/camilladsp/blob/next30/README.md#file-rawfile-wavfile-stdin-stdout A similar approach should work for other backends.

To be honest I do not know for the other backends. Wasapi has only the isFormatSupported which requires checking all possible variants - I do that long looping in the REW Wasapi backend to get all supported combinations, very ugly. Input stream backend has no internal info, no idea for CoreAudio.

Maybe implementing just for alsa (where the majority of loopback/gadget use cases happen) for now would make sense.

@HEnquist
Copy link
Owner

HEnquist commented May 1, 2024

IMO once the device is opened with some hw_params, the params cannot be changed within that session. So any change of any of the params would be preceded with the stop (rate -> 0) and start signals.

As of reading all the new params at the moment of start signal - the existing devices do not offer these controls. But IMO it would be superfluous because when configuring hw_params before opening the device the param checks need to be done anyway. I think there is no need to pass the new params in the FormatChange msg, at least for alsa.

If format and channels were provided as controls, then they would provide the new values when the device is opened, and this info could be given in the StopReason. This has some advantages, since it makes it possible to know the new values before trying to start with a new config. Autodetecting format is easy to handle (I just implemented it) but a different number of channels pretty much needs a new config.
Adding these controls would also align the gadget better with the loopback. It's a bit weird that they are so similar and at the same time so different :)

IMO in alsa a check for any params would be identical and is mostly already implemented - setting default hw by HwParams::any(&pcmdev) , then setting/checking individual parameters with near and checking what the method returns. Format could be read by hwp.get_format.

Getting the values is not the problem IMO, it's what to do with it! Sample format is easy, sample rate can be handled by the resampler, but number of channels may need pretty large changes in the pipeline (depending on the config of course).

To be honest I do not know for the other backends. Wasapi has only the isFormatSupported which requires checking all possible variants - I do that long looping in the REW Wasapi backend to get all supported combinations, very ugly. Input stream backend has no internal info, no idea for CoreAudio.

CoreAudio is well behaved here, it provides methods to query for supported formats, and there are events to tell whenever sample rate changes. Wasapi is really behind here, it can only tell that something changed, not what parameter it was or what the new value is. And as you mention, there is no reasonable way to query for supported formats.

Maybe implementing just for alsa (where the majority of loopback/gadget use cases happen) for now would make sense.

I'm working on a little python helper script, that would make this work on both Mac and linux. That together with the control event stuff in the alsa_fd branch would then finally allow sample rate changes using the loopback. This will work for the gadget too.

@pavhofman
Copy link
Contributor Author

If format and channels were provided as controls, then they would provide the new values when the device is opened, and this info could be given in the StopReason. This has some advantages, since it makes it possible to know the new values before trying to start with a new config.

I am not sure if the design will necessarily be simplified if the controls are read by the handler of format change event. When opening the device, the new params need to be tested anyway that they fit (inherently by the alsa methods). It can happen that they do not fit anymore (e.g. alsa usb driver when doing its enumeration opens the device just for a few bytes of samples), pulseaudio also opens its new devices for a moment to check their capabilities and that they actually work. Now if the goal for the changes will be to design CDSP so that it by itself (without any helper wrapper) does some effort to stay running and adjusting to new incoming situation, it should accept the new formats offered by the device (or flip to waiting for stream activation if inactive). So IMO the code for accepting/refusing = deciding on what to do with current params (or just some of them, some subset, whatever gets supported) will need to be part of that too (so that it can be called again). Otherwise CDSP will just stop (as it would now), not knowing how to handle a format issue when filling hw params.

Maybe sth like this would allow operation without any wrapper which always adds startup/format adjustment latency. It would allow sending format change with parameters externally via websockets, e.g. for stdin stream filedevice or alsa device reading/writing to clock-slaved I2S which has no idea about the I2S clock params (samplerate, bclk ratio, how many channels to actually read from etc.). Please take it as a help for discussion, it's just a quick chart-up

flow

Autodetecting format is easy to handle (I just implemented it) but a different number of channels pretty much needs a new config. Adding these controls would also align the gadget better with the loopback. It's a bit weird that they are so similar and at the same time so different :)

The gadget may end up with these controls - when a client uses the plug plugin to avoid having to do all the conversions itself, the plug plugin hides the native params of the hw device which may be important for the client to decide optimally (e.g. to avoid useless samplerate/format conversions by the plugin).

If CDSP reads these params when building the FormatChanged msg, it may help but IMO the hw params still may have to re-read, so it kind of seems an extra delay which may (or may not) be avoided. Reading from restricted hw params (as already restricted by aloop and gadget) is probably faster than kernel callbacks for each ctl read (though the time difference likely not being an issue here, of course).

IMO in alsa a check for any params would be identical and is mostly already implemented - setting default hw by HwParams::any(&pcmdev) , then setting/checking individual parameters with near and checking what the method returns. Format could be read by hwp.get_format.

Getting the values is not the problem IMO, it's what to do with it! Sample format is easy, sample rate can be handled by the resampler, but number of channels may need pretty large changes in the pipeline (depending on the config of course).

I am afraid that is the case. But it may be required even for switching samplerates - e.g. some filters are fs-dependent and need different configs for different samplerates. But due to your ingenious design where filters and mixers are defined separately from the pipeline, the channels/samplerate variants may concern only the pipeline. It would be a major change but maintaining compatibility with legacy configs would be possible (just one samplerate/channels combination).

To be honest I do not know for the other backends. Wasapi has only the isFormatSupported which requires checking all possible variants - I do that long looping in the REW Wasapi backend to get all supported combinations, very ugly. Input stream backend has no internal info, no idea for CoreAudio.

CoreAudio is well behaved here, it provides methods to query for supported formats, and there are events to tell whenever sample rate changes. Wasapi is really behind here, it can only tell that something changed, not what parameter it was or what the new value is. And as you mention, there is no reasonable way to query for supported formats.

Maybe implementing just for alsa (where the majority of loopback/gadget use cases happen) for now would make sense.

I'm working on a little python helper script, that would make this work on both Mac and linux. That together with the control event stuff in the alsa_fd branch would then finally allow sample rate changes using the loopback. This will work for the gadget too.

IMO it would be ideal if the final goal (even if reached after several major versions) could do without any wrappers for this. If there was any chance to be heading there, it would be just fantastic.

Thanks a lot for your patience with my brainstorming :-)

@HEnquist
Copy link
Owner

The wrapper is here: https://github.com/HEnquist/camilladsp-controller
The current version handles rate changes from the alsa gadget and loopback, and coreaudio (tested with the blackhole loopback driver).
It has some debouncing, and re-reads the parameters before trying to make a new config for CamillaDSP. On Alsa it reads the gadget or loopback controls. It does not (yet) try reading hwparams. Is there any way to read those without opening (and thereby blocking) the device?

@pavhofman
Copy link
Contributor Author

pavhofman commented May 28, 2024

It does not (yet) try reading hwparams. Is there any way to read those without opening (and thereby blocking) the device?

I am afraid reading hw params cannot be done without opening the device first with snd_pcm_open which provides the device PCM handle required for further calls. Then snd_pcm_hw_params_any fills the params struct with pre-configured hw params space of the device. A device which accepts only some parameters should have this space already restricted to possible values https://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m___h_w___params.html#ga6e2dd8efbb7a4084bd05e6cc458d84f7

Example e.g. https://github.com/pavhofman/csjsound-alsapcm/blob/main/src/impl.c#L205

But I am not sure the python alsa wrappers provide these low-level methods. IIUC the more complex pyalsaaudio library offers lookup in the hw params space e.g. https://github.com/larsimmisch/pyalsaaudio/blob/3e360b1bb75ca888f1e4973e92882c54391eb5a7/alsaaudio.c#L1056 but it's quite inefficient - getting every param type requires a separate hw_params allocation, snd_pcm_hw_params_any call etc.

In rust it would be mapped to HwParams.any() https://github.com/diwic/alsa-rs/blob/4d9735152b1a37554fb4aed74f3cb164d93bcf03/src/pcm.rs#L772

The project https://github.com/marcoevang/camilladsp-setrate could probably be modified to do that, the features seem quite comparable (apart of the debouncing and core-audio support).

@HEnquist
Copy link
Owner

I am afraid reading hw params cannot be done without opening the device first with snd_pcm_open which provides the device PCM handle required for further calls.

Ok, yeah that's what I thought. Thanks for confirming.

But I am not sure the python alsa wrappers provide these low-level methods.

They don't, but this would not be a big deal to implement with CFFI for example (or just by using another library)

The project https://github.com/marcoevang/camilladsp-setrate could probably be modified to do that, the features seem quite comparable (apart of the debouncing and core-audio support).

There are quite some similarities between camilladsp-controller and camilladsp-setrate. The main difference is that camilladsp-controller is written in Python. The idea was that this should make it faster to add features, and to make it much easier for people to customise it in all kinds of ways. Then we'll see, maybe the functionality can be built into CamillaDSP itself at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants