-
Notifications
You must be signed in to change notification settings - Fork 837
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
Resolve setup issues for macOS (Venture 13.4): pyaudiowpatch -> pyaud… #91
base: main
Are you sure you want to change the base?
Changes from all commits
7b91e2c
d2be01d
82ddd74
39b5e77
379db93
ab18d8f
a158b67
5c72389
9cb37fa
3fee4bd
fd27ea4
11ea194
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,15 @@ | ||
import custom_speech_recognition as sr | ||
import pyaudiowpatch as pyaudio | ||
import os | ||
from datetime import datetime | ||
|
||
try: | ||
import pyaudiowpatch as pyaudio | ||
except ImportError: | ||
if os.name != "nt": | ||
import pyaudio | ||
else: | ||
raise | ||
|
||
RECORD_TIMEOUT = 3 | ||
ENERGY_THRESHOLD = 1000 | ||
DYNAMIC_ENERGY_THRESHOLD = False | ||
|
@@ -37,23 +45,44 @@ def __init__(self): | |
self.adjust_for_noise("Default Mic", "Please make some noise from the Default Mic...") | ||
|
||
class DefaultSpeakerRecorder(BaseRecorder): | ||
def __init__(self): | ||
with pyaudio.PyAudio() as p: | ||
wasapi_info = p.get_host_api_info_by_type(pyaudio.paWASAPI) | ||
default_speakers = p.get_device_info_by_index(wasapi_info["defaultOutputDevice"]) | ||
|
||
if not default_speakers["isLoopbackDevice"]: | ||
for loopback in p.get_loopback_device_info_generator(): | ||
if default_speakers["name"] in loopback["name"]: | ||
default_speakers = loopback | ||
break | ||
else: | ||
|
||
# Different implementations of obtaining the info dict of a default speaker, for different platforms | ||
if os.name == "nt": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is what I mean, dependency injection seems appropriate here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dependency injection at the class level is not the best idea, since their scope will only be at the class declaration level (but not even at the class level). Minimal reproducible example: class A:
import os
print("os_0", os)
def __init__(self):
print("os_1", os)
A()
print("os_2", os) So it's better to leave the imports at the module level. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sry what I meant was I would like the code to be extendable where we would just have a function at the beginning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are talking about monkey patching, but putting it in a separate function is inappropriate. This will confuse the code and complicate its further maintenance. I suggest a more elegant option: group methods in one place, and replace them for a specific OS using a decorator. MRE for this: import sys
def os_dependent(target_class):
current_os = sys.platform
name_prefix = "_os_"
name_prefix_len = len(name_prefix)
for os_dep_name in (method_name for method_name in dir(target_class)
if method_name.startswith(name_prefix)):
method_platform_end_index = os_dep_name.find("_", name_prefix_len)
if os_dep_name[name_prefix_len:method_platform_end_index] == current_os:
# If this method is for the current system, replace.
setattr(target_class, os_dep_name[method_platform_end_index+1:],
getattr(target_class, os_dep_name))
return target_class
@os_dependent
class OSDependentClass:
def _os_linux_method(self):
print("_os_linux_method")
def _os_darwin_method(self):
print("_os_darwin_method")
def method(self):
# win32 method
print("method")
def _os_win32_method1(self):
...
def method1(self):
...
print(OSDependentClass.method.__name__)
print(OSDependentClass.method1.__name__)
pros:
cons:
And to perform OS-dependent actions at the module level, need to use the if-else block(or leave it as it is now - try-catch block). It will be also more appropriate than a standalone function. |
||
def _get_default_speaker(self): | ||
# Requires PyAudioWPatch >= 0.2.12.6 | ||
with pyaudio.PyAudio() as p: | ||
try: | ||
# Get loopback of default WASAPI speaker | ||
return p.get_default_wasapi_loopback() | ||
|
||
except OSError: | ||
print("[ERROR] Looks like WASAPI is not available on the system.") | ||
|
||
except LookupError: | ||
print("[ERROR] No loopback device found.") | ||
|
||
else: | ||
def _get_default_speaker(self): | ||
# At the moment, recording from speakers is only available under Windows | ||
# raise NotImplementedError("Recording from speakers is only available under Windows") | ||
|
||
# As far as I understand, now the code style does not provide | ||
# for error handling - only print them. | ||
print("[ERROR] Recording from speakers is only available under Windows.") | ||
|
||
|
||
def __init__(self): | ||
default_speaker = self._get_default_speaker() | ||
|
||
if not default_speaker: | ||
print("[ERROR] Something went wrong while trying to get the default speakers.") | ||
super().__init__(source=sr.Microphone(sample_rate=16000), source_name="Speaker") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't think its appropriate to duplicate default mic here. Might be better to just have source as None and extend code to allow none sources where we do nothing with that recorder...not sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what I originally wanted to do, BUT for this it will be necessary to make a number of changes to the Again, if you think this is a necessary change, we can discuss it. I thought that rewriting a significant part of the code to create a temporary (until the issue with the speakers is resolved) patch is unnecessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't think a rewrite is necessary, more of a extension of the code where we also allow no op recorder would be fine right? Only thing is AudioTranscriber init should be able to handle none source. Hopefully code is written where supporting MacOS with no op recording can be extended easily again to supporting MacOS with specific mac recording. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SevaSk Any update here? |
||
return | ||
|
||
source = sr.Microphone(speaker=True, | ||
device_index= default_speakers["index"], | ||
sample_rate=int(default_speakers["defaultSampleRate"]), | ||
device_index=default_speaker["index"], | ||
sample_rate=int(default_speaker["defaultSampleRate"]), | ||
chunk_size=pyaudio.get_sample_size(pyaudio.paInt16), | ||
channels=default_speakers["maxInputChannels"]) | ||
channels=default_speaker["maxInputChannels"]) | ||
super().__init__(source=source, source_name="Speaker") | ||
self.adjust_for_noise("Default Speaker", "Please make or play some noise from the Default Speaker...") | ||
self.adjust_for_noise("Default Speaker", "Please make or play some noise from the Default Speaker...") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you that a structure like this:
would look better. But since now the main platform is Windows, I thought that doing a check every time on an OS that is not fully supported would be redundant. And I thought the approach I chose was the best.
However, I don't mind changing that.