spatialaudio / python-sounddevice

:sound: Play and Record Sound with Python :snake:
https://python-sounddevice.readthedocs.io/
MIT License
1.02k stars 149 forks source link

Allow exact matches to include the hostapi's name in _get_device_id #360

Closed blattm closed 2 years ago

blattm commented 3 years ago

Hi there,

here is a suggestion for a change of _get_device_id: When a string of the form "device_name, host_api" name is queried, it should count as an exact match if it matches full_string.

This would solve the following problem, I currently encounter. I want to use strings in the specified format to store audio settings in configuration files. But this will happen:

>>> sd.query_devices("default, ALSA")
ValueError: Multiple input/output devices found for 'default, ALSA':
[6] sysdefault, ALSA
[14] default, ALSA

Without the hostapi part, the error does not occur. However, I cannot omit it, because on Windows, there are sometimes devices with identical names, but different APIs.

blattm commented 3 years ago

I just realized that the way I planned to craft identifying strings for config files is problematic. There are two reasons: 1) The names of the hostapis are not stable, if I correctly understand this comment https://github.com/spatialaudio/python-sounddevice/issues/84#issuecomment-297292987 . However, I don't think this is as much of a problem, because such changes will probably not happen that often. 2) ALSA device names of low-level hardware include a string such as "hw:1,0". Querying without that string might not work ( #279 ) and querying the full name including "hw:1,0" might not work, as these numbers could potentially change if external hardware gets connected in a different order.

So I'm not sure if this pull request would really help in its current form...

One possibility, that would also help with 2. and #279: What if we exclude strings like " (hw:1,0)" from the device_string, but still include it in the full_string for ALSA-devices. But this would not help if query_string contained the hostapi... So maybe one could introduce an additional parameter like hostapi=None in _get_device_id and query_devices..? Just bouncing ideas.

What do you think?

pep8speaks commented 3 years ago

Hello @blattm! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-07-24 12:07:21 UTC
blattm commented 3 years ago

Ok, I think I found a nice solution, that will not introduce changes to the public API, so I updated the PR :)

The new code would allow the exact matching of the device's name, if 1) the hostapi is not part of the query string, 2) the exact hostapi's name is at the end of the query string, or 3) a part of the hostapi's name is at the end of the query string

(The current implementation of 0.4.2 allows 1., the initial PR only allows 1. and 2.)

Also it solves #279, even when the query contains the hostapi's name.

With this change, it will be possible to craft strings for config files, that will always give you the correct device when queried, like this: sd._remove_alsa_id(device_name) + " " + hostapi_name

mgeier commented 3 years ago

Thanks for this PR!

When a string of the form "device_name, host_api" name is queried, it should count as an exact match if it matches full_string.

Yes, that sounds reasonable, I wasn't aware of this problem.

I don't really like the recent update, though.

I don't want to make any assumptions about the device string that's provided by PortAudio. If you want to do that, you should do that in your own code.

mgeier commented 2 years ago

I would like to merge the change in f1fcaaf387e22e2470254515a5c09e91dce0bebf, but I'm not convinced of the following commits.

@blattm I assume the first commit would already be helpful to you, right?

Are you OK with me just taking the first commit?

blattm commented 2 years ago

Hi,

I'm fine with that!

I don't want to make any assumptions about the device string that's provided by PortAudio. If you want to do that, you should do that in your own code.

I thought that was a reasonable boundary to draw, so I already solved the problem within my own code.

mgeier commented 2 years ago

Thanks!

I've cherry-picked your first commit into the master branch as a6248d50f3f6e99bcc10fad73f2489f13c6a39f2.