fix: incorrectly changed the base type in my last pull request for L… #1184
Conversation
…calAudioTransport
|
Good catch. Thank you. Can you either set up Ruff for linting or install pre-commit hooks? |
|
Ooops,my bad. 🤦♂️ |
| @@ -26,7 +25,7 @@ | |||
|
|
|||
|
|
|||
| async def main(): | |||
| transport = LocalAudioTransport(TransportParams(audio_out_enabled=True)) | |||
| transport = LocalAudioTransport(LocalTransportParams(audio_out_enabled=True)) | |||
There was a problem hiding this comment.
I don't think this is necessary. LocalTransportParams is a subclass of TransportParams. The actual audio_out_enabled is in TransportParams, so it seems fine to keep it.
There was a problem hiding this comment.
I think this change is correct.
There was a problem hiding this comment.
SInce we are at it, it would be nice to make this params=LocalTransportParams(...) and force keyword in the contructor.
def __init__(self, *, params: LocalTransportParams):
| @@ -41,7 +40,7 @@ async def say_something(): | |||
| await asyncio.sleep(1) | |||
| await task.queue_frames([TTSSpeakFrame("Hello there, how is it going!"), EndFrame()]) | |||
|
|
|||
| runner = PipelineRunner() | |||
| runner = PipelineRunner(handle_sigint=False if sys.platform == "win32" else True) | |||
There was a problem hiding this comment.
We can probably drop this too. In general, Pipecat doesn't support Windows. We probably need a higher level message than just something in this demo.
There was a problem hiding this comment.
transport = LocalAudioTransport(TransportParams(audio_out_enabled=True))Since the type of param LocalAudioTransport has changed, it's required to use the new class, since LocalAudioTransport now reads input_device_index and output_device_index to setup pyaudio.
TransportParams doesn't have the default value.
I could manage the case but seems inconsistent with the rest of the code where params are subclassed by service that requires specific parameters.
Any specific reason why pipecat doesnt support windows ?
There was a problem hiding this comment.
Currently, only daily-python doesn't really support Windows. I'd say everything else supports Windows, even though I haven't personally tried it.
| pipecat[whisper, openai] | ||
| textual==1.0.0 | ||
| pydantic-settings==2.7.1 | ||
| pyaudio==0.2.14 |
There was a problem hiding this comment.
you can remove pyaudio and specify local option in pipecat
| --extra-index-url https://download.pytorch.org/whl/cu124 | ||
| torch==2.5.0+cu124 | ||
| torchvision | ||
| torchaudio |
There was a problem hiding this comment.
why do we need all these torch imports?
|
@TheCodingLand This is awesome! |
Fix base class of LocalAudioTransport