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

Added index search for get_microphone() #125

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions soundcard/pulseaudio.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the case should not happen here, but later on in _match_soundcard. Unless I am misunderstanding something.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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().



def _match_soundcard(id, soundcards, include_loopback=False):
Expand All @@ -386,19 +386,29 @@ def _match_soundcard(id, soundcards, include_loopback=False):
else:
soundcards_by_id = {soundcard['id']: soundcard for soundcard in soundcards}
soundcards_by_name = {soundcard['name']: soundcard for soundcard in soundcards}

# index search
try:
return soundcards[int(id)]
except (ValueError):
pass
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block should probably be something like if int(id) in soundcards instead of a try/catch.

Copy link
Author

@yuannan yuannan Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!


# id search
if id in soundcards_by_id:
return soundcards_by_id[id]

# try substring match:
for name, soundcard in soundcards_by_name.items():
if id in name:
return soundcard

# try fuzzy match:
pattern = '.*'.join(id)
for name, soundcard in soundcards_by_name.items():
if re.match(pattern, name):
return soundcard
raise IndexError('no soundcard with id {}'.format(id))

raise IndexError('no soundcard with id {}'.format(id))

def get_name():
"""Get application name.
Expand Down