Repository navigation
Add AlsaSettings for opening an ALSA PCM by address - #644
whatisyourname0 wants to merge 4 commits into
Conversation
|
Thanks for this PR! It seems to add two (or more?) different things, though: a new ALSA setting and a whole new testing framework. I think both are valuable, but they should be discussed separately. Regarding tests, there are already some discussions in #14 and #131, and there has been a very recent PR that also comes with its own test framework: #645. |
|
Thanks for the review! I agree that adding I'll update this PR to focus on the ALSA PCM-address support and its documentation, removing the new test suite, test dependencies, and test-related CI changes from the diff. I'll preserve the regression tests on a separate branch and keep the validation results and reproduction instructions available for reviewing the feature. Thanks also for pointing me to #14, #131, and #645. I'll follow up on testing separately, building on those discussions before proposing a shared approach. |
|
Done in 8e0aeba: this PR now contains only the ALSA PCM-address support and its documentation. The tests and reproduction instructions are preserved on |
|
Thanks for the update! I like the
I think an explicit sentinel would be better, as it would make the implementation more straightforward and it would be closer to PortAudio's C API. Maybe something like DEVICE_FROM_EXTRA_SETTINGS = _lib.paUseHostApiSpecificDeviceSpecificationMaybe you can come up with a better name? Your example would then look something like this: with sd.RawInputStream(
device=sd.DEVICE_FROM_EXTRA_SETTINGS,
samplerate=48000,
channels=2,
dtype='int16',
blocksize=1024,
latency=4096 / 48000,
extra_settings=sd.AlsaSettings(device_string='null'),
) as stream:
data, overflowed = stream.read(1024)
print(len(data), overflowed)
It is kinda annoying, but I guess we don't really have a choice, right? However, I would also prefer the handling of this to be less host-API-specific. Whenever |
|
Thanks for the review, and that makes sense. Your proposed name communicates the intent clearly than mine. I'll also rework the shared parameter handling around whether device information is available, removing the I'll update the examples and the separately maintained regression tests to cover the changed behavior. |
|
Updated and pushed in ffcda44: device selection now uses |
mgeier
left a comment
There was a problem hiding this comment.
Here's a first round of review ... I must postpone the rest because I couldn't manage to run this yet, I always get a sounddevice.PortAudioError: Error querying device -2. Is there anything special I need to do for this to work?
| * Module-wide default settings: `default` | ||
|
|
||
| * Platform-specific settings: | ||
| `AsioSettings`, `CoreAudioSettings`, `WasapiSettings` |
There was a problem hiding this comment.
You could also add the new class here.
| Pass this as ``device`` to select a device through ``extra_settings``. | ||
| Device information is unavailable for this value, so channels and sample | ||
| rate must be set explicitly or through defaults, and stream latency must | ||
| be numeric. Format checks do not require latency. |
There was a problem hiding this comment.
Why are you adding this text here instead of sounddevice.py?
Also, this text seems a bit long ... what about linking to AlsaSettings (which is AFAIK the only host API that supports it) and having all the details documented there, avoiding some repetition?
| if prime_output_buffers_using_stream_callback: | ||
| stream_flags |= _lib.paPrimeOutputBuffersUsingStreamCallback | ||
|
|
||
| # Retain effective settings, including their C buffers, through close. |
There was a problem hiding this comment.
This comment is hard to understand. I guess it's about what in other places is called "keep alive"?
| if extra_settings is None: | ||
| extra_settings = tuple(default.extra_settings) | ||
| self._extra_settings = (tuple(_split(extra_settings)), | ||
| tuple(default.extra_settings)) |
There was a problem hiding this comment.
Can you explain how that combination of tuple and _split() and handling of default is supposed to work?
Note that _split() is also used later, and _get_stream_parameters() is supposed to handle default values.
| *extra_settings*, e.g. with `AlsaSettings`. In this case, | ||
| channels, sample rate and numeric latency must be supplied | ||
| explicitly or through defaults. |
There was a problem hiding this comment.
Again, I don't think this needs to be repeated here:
| *extra_settings*, e.g. with `AlsaSettings`. In this case, | |
| channels, sample rate and numeric latency must be supplied | |
| explicitly or through defaults. | |
| *extra_settings*, e.g. with `AlsaSettings`. |
| class AlsaSettings: | ||
|
|
||
| def __init__(self, device_string): | ||
| """ALSA-specific settings for opening a PCM by address. |
There was a problem hiding this comment.
The other classes use something like this:
| """ALSA-specific settings for opening a PCM by address. | |
| """ALSA-specific input/output settings. |
You can also use the next sentence from the other classes, to keep them consistent.
| Use as ``extra_settings`` with | ||
| ``device=sd.DEVICE_FROM_EXTRA_SETTINGS``. Supplying these settings | ||
| alone does not override `default.device`. Channels and sample rate | ||
| must be supplied explicitly or through defaults. Streams also | ||
| require a numeric latency in seconds. |
There was a problem hiding this comment.
I think this should be moved to the documentation of device_string, because theoretically other things might be added in the future, then this paragraph might not make sense anymore.
| Notes | ||
| ----- | ||
| The stream's device attribute reports PortAudio's special value | ||
| ``-2`` when `DEVICE_FROM_EXTRA_SETTINGS` is selected. | ||
| PortAudio rejects these settings with a regular ALSA device index. | ||
| For direct output with ``mapping=[1]`` in `play()` or `playrec()`, | ||
| use `OutputStream` with explicit channels instead; automatic | ||
| stereo detection requires enumerated device information. |
There was a problem hiding this comment.
I think these notes also belong into the device_string section?
| @@ -690,7 +693,7 @@ | |||
| """ | |||
| parameters, dtype, samplesize, samplerate = _get_stream_parameters( | |||
| 'input', device=device, channels=channels, dtype=dtype, latency=None, | |||
There was a problem hiding this comment.
This whole for_check argument seems unnecessarily complicated, why not just pass 0 as latency?
| 'input', device=device, channels=channels, dtype=dtype, latency=None, | |
| 'input', device=device, channels=channels, dtype=dtype, latency=0, |
Applications with an ALSA PCM address can open it directly through
AlsaSettings(device_string=...), using PortAudio'sPaAlsaStreamInfo.deviceString. Direct device selection is explicit: passdevice=sd.DEVICE_FROM_EXTRA_SETTINGS, corresponding to PortAudio'spaUseHostApiSpecificDeviceSpecification.This implements the ALSA device-string item discussed in #4 through the existing platform-specific settings interface. Related PR #85 proposes a broader host-API interface. This change does not resolve all of #4.
Replace
'null'with a hardware PCM address or named ALSA configuration as needed.API behavior
AlsaSettingsalone does not overridedefault.device. SelectDEVICE_FROM_EXTRA_SETTINGSexplicitly, either in the call or indefault.device. PortAudio rejects ALSA settings combined with a regular device index; the error is propagated.AlsaSettingstype. Device information is queried only when required for channel-count/sample-rate defaults or symbolic latency. Query errors propagate.'low'and'high'require enumerated device information and raise an error for the sentinel.stream.devicereports-2for a directly specified direction. Settings and their C buffers remain alive through stream close.mapping=[1]needs device information for automatic stereo detection, so it fails for direct devices. UseOutputStreamwith an explicit channel count instead.Validation and separate tests
The feature PR includes implementation, API documentation and the documentation-only PortAudio stub update needed to expose the constant. It adds no test infrastructure, dependencies, CI steps or lockfile changes.
The updated suite and standalone instructions are on
test/alsa-direct-device-regressions, pinned at1149e87. That branch differs from feature commitffcda44only by the five files undertests/.Current local validation:
ffcda44and the separate test suite. Tests use isolated ALSA null/file PCMs and require no physical devices.626fcb0) and candidate processes, using the same ALSA configuration. 72 outcomes match exactly, including index/name/default selection, raw input/output/duplex, format checks and float32 input callbacks. Two intentional differences are asserted explicitly: an invalid numeric device now fails during opening instead of device-info lookup, and format checks ignore invalid default latency. No other differences were observed in this Linux matrix.SOUNDDEVICE_BASELINE_PATHas documented in the test README. Without that environment variable, those cases are explicitly skipped and the 53 direct-ALSA cases run.git diff --check, anduv lock --checkpassed.To run the separate tests against this PR on Linux with PortAudio installed:
The fixture sets its isolated ALSA configuration before PortAudio initialization and cleans up its temporary PCM configuration/data. The extracted test files remain in
$validation_diruntil manually removed. Testing infrastructure can be discussed separately in the context of #14 and #131.Limits
The revised sentinel API has not been rerun across the earlier multi-version Python matrix or on physical hardware. Native Linux USB capture, hotplug and native macOS/Windows runtime behavior remain unverified. Local external link checking was not repeated; hosted checks are tracked on this PR.
An ALSA PCM address is not persistent hardware identity. Serial lookup, hotplug resolution and application recovery remain outside this patch. PortAudio initialization may still enumerate devices. No performance improvement or negotiated hardware buffer size is claimed.
Earlier validation of the pre-sentinel candidate (not rerun for this revision)
At original candidate
07f9935, CPython 3.7–3.14 passed the original 33 synthetic tests per version; PyPy 3.9/3.11 passed 32 with one optional NumPy skip. Python 3.7/3.8 used a wheel built with Python 3.14, so those results establish runtime compatibility only. The original suite and instructions remain available at that immutable commit.Earlier physical input check
A separate local run used a MacBook Pro built-in microphone through Apple Virtualization.framework and an experimental virtio-snd guest kernel. The Linux runtime was a chroot containing Python 3.14.7, PortAudio 19.6.0 and ALSA 1.2.14. The installed sounddevice source matched this candidate byte-for-byte.
arecordcompleted two 10-second captures. A C program usingPaAlsaStreamInfo.deviceStringcompleted 60 seconds and a 5-second reopen. Withquery_devices()forbidden,AlsaSettings('hw:0,0')completed 60 seconds (2,880,000 mono frames), two 5-second reopens and recovery after a nonexistent PCM failed. All captures had nonzero signal, with no reported read errors or overflows. The host input route was monitored as the built-in microphone. Audio samples were processed in memory; only aggregate measurements were saved.This establishes physical input through that particular VZ guest, including experimental capture compatibility changes. Native Linux USB devices, hotplug and stock virtio-snd remain unverified. The capture trial did not run the recorder or Docker Compose. Guest shutdown did not complete within the harness timeout and required a second launcher signal to force-stop the test VM; orderly VM shutdown was not established.