Added index search for get_microphone() #125
Conversation
yuannan
changed the title
|
There is a minor bug with this implementation, sound card IDs change and are not static. This could lead to issues if cards are changed around such as unplugging and plugging a device. This could be fixed by redefining source_list() as a properly indexed list with the index matching up with the sink number. This could lead to more memory usage but this is at most a few array elements. I'm happy to make this patch as well if you don't want to. I'm just worried that changing this around would mess up your other code. |
soundcard/pulseaudio.py
Outdated
| @@ -369,7 +369,7 @@ def get_microphone(id, include_loopback=False, exclude_monitors=True): | |||
| include_loopback = not exclude_monitors | |||
|
|
|||
| microphones = _pulse.source_list | |||
| return _Microphone(id=_match_soundcard(id, microphones, include_loopback)['id']) | |||
| return _Microphone(id=_match_soundcard(str(id), microphones, include_loopback)['id']) | |||
There was a problem hiding this comment.
I think the case should not happen here, but later on in _match_soundcard. Unless I am misunderstanding something.
There was a problem hiding this comment.
You are correct. The crash happens on when it tries to pattern match the int to a str within the if statements.
This mitigation is actually completely pointless now. As with the "naive try return" it will return iff it's an int and it's indexed. Only strings or larger numbers will fail(#125 (comment)). I've removed it for now.
If this bug comes back in the future simply put "id = str(id)" at the start of _match_soundcard().
| try: | ||
| return soundcards[int(id)] | ||
| except (ValueError): | ||
| pass |
There was a problem hiding this comment.
This block should probably be something like if int(id) in soundcards instead of a try/catch.
There was a problem hiding this comment.
Yeah, but that adds a lot more complexity. Like I said I have to change a few thing around first. The current source and sink info doesn't even have numerical IDs (afiak anyway). This would take much more work to first update the info obtained from CFFI and then construct a new soundcards with proper info.
This was just a simply one stop shop hack I put together to test my project.
This should be done for readability and future maintenance however. Gimme the go ahead and I'll try and fix it up once and for all if I get some time this week.
There was a problem hiding this comment.
Absolutely go ahead and investigate it fully. But take it easy; there's absolutely no rush, so do it on your own time.
Let me know when you think it's ready!
as stated by the docs:
Parameters: | id (int or str) – can be an int index, a backend id, a substring of the speaker name, or a fuzzy-matched pattern for the speaker name.This is not true and caused me to bash my face on my computer for all of last night before I thought it could be an upstream issue. Please merge this. Thanks.