-
Notifications
You must be signed in to change notification settings - Fork 426
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
Use socket send async func to wrap the various socket send function w… #911
Conversation
…hen send initial file. Update deepgram connect func, use .websocket instead of live
WalkthroughThe changes primarily revolve around the modification of data transmission over sockets in Changes
Uplevel your code reviews with Entelligence.AI ProEntelligence.AI ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, Entelligence.AI Pro is free for open source projects. |
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.
Files selected (2)
- backend/routers/transcribe.py (2)
- backend/utils/stt/streaming.py (4)
Review comments generated (6)
- Review: 4
- LGTM: 2
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AI
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignore
anywhere in the PR description to pause further reviews from the bot.
async def deepgram_socket_send(data): | ||
return deepgram_socket.send(data) | ||
await send_initial_file_path(file_path, deepgram_socket_send) |
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.
The change from directly using the deepgram_socket
to send data to wrapping it in a new function deepgram_socket_send
seems unnecessary. The send_initial_file_path
function is now expecting a function instead of a socket object, which could lead to confusion for future developers. If there's a specific reason for this change, consider adding comments to clarify.
- async def deepgram_socket_send(data):
- return deepgram_socket.send(data)
- await send_initial_file_path(file_path, deepgram_socket_send)
+ await send_initial_file_path(file_path, deepgram_socket)
@@ -231,7 +233,7 @@ def stream_audio(audio_buffer): | |||
stream_transcript, speech_profile_stream_id, sample_rate, language, preseconds=duration | |||
) | |||
if duration: | |||
await send_initial_file_path(file_path, speechmatics_socket) | |||
await send_initial_file_path(file_path, speechmatics_socket.send) |
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.
Similar to the previous comment, the change from directly using the speechmatics_socket
to send data to passing the send
method of the socket object to send_initial_file_path
could be confusing. If there's a specific reason for this change, consider adding comments to clarify.
- await send_initial_file_path(file_path, speechmatics_socket.send)
+ await send_initial_file_path(file_path, speechmatics_socket)
@@ -4,7 +4,7 @@ | |||
from typing import List | |||
|
|||
import websockets | |||
from deepgram import DeepgramClient, DeepgramClientOptions, LiveTranscriptionEvents | |||
from deepgram import DeepgramClient, DeepgramClientOptions, LiveTranscriptionEvents, ListenWebSocketClient |
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.
The import statement has been modified to include ListenWebSocketClient
but it is not used anywhere in the provided code. If it's not used, it should be removed to keep the code clean and maintainable.
- from deepgram import DeepgramClient, DeepgramClientOptions, LiveTranscriptionEvents, ListenWebSocketClient
+ from deepgram import DeepgramClient, DeepgramClientOptions, LiveTranscriptionEvents
dg_connection = deepgram.listen.websocket.v("1") | ||
dg_connection.on(LiveTranscriptionEvents.Transcript, on_message) | ||
dg_connection.on(LiveTranscriptionEvents.Error, on_error) | ||
|
||
def on_open(self, open, **kwargs): | ||
print("Connection Open") | ||
|
||
def on_metadata(self, metadata, **kwargs): | ||
print(f"Metadata: {metadata}") | ||
|
||
def on_speech_started(self, speech_started, **kwargs): | ||
print("Speech Started") | ||
|
||
def on_utterance_end(self, utterance_end, **kwargs): | ||
print("Utterance End") | ||
global is_finals | ||
if len(is_finals) > 0: | ||
utterance = " ".join(is_finals) | ||
print(f"Utterance End: {utterance}") | ||
is_finals = [] | ||
|
||
def on_close(self, close, **kwargs): | ||
print("Connection Closed") | ||
|
||
def on_unhandled(self, unhandled, **kwargs): | ||
print(f"Unhandled Websocket Message: {unhandled}") | ||
|
||
dg_connection.on(LiveTranscriptionEvents.Open, on_open) | ||
dg_connection.on(LiveTranscriptionEvents.Metadata, on_metadata) | ||
dg_connection.on(LiveTranscriptionEvents.SpeechStarted, on_speech_started) | ||
dg_connection.on(LiveTranscriptionEvents.UtteranceEnd, on_utterance_end) | ||
dg_connection.on(LiveTranscriptionEvents.Close, on_close) | ||
dg_connection.on(LiveTranscriptionEvents.Unhandled, on_unhandled) |
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.
The event handling functions on_open
, on_metadata
, on_speech_started
, on_utterance_end
, on_close
, on_unhandled
are defined within the connect_to_deepgram
function but they don't have access to the self
parameter as they are not part of a class. This will cause a runtime error when these functions are called.
Also, the variable is_finals
is referenced in the on_utterance_end
function but it is not defined anywhere in the provided code. This will also cause a runtime error.
Here's a possible fix: remove the self
parameter from the function definitions and define is_finals
before using it.
- def on_open(self, open, **kwargs):
+ def on_open(open, **kwargs):
print("Connection Open")
- def on_metadata(self, metadata, **kwargs):
+ def on_metadata(metadata, **kwargs):
print(f"Metadata: {metadata}")
- def on_speech_started(self, speech_started, **kwargs):
+ def on_speech_started(speech_started, **kwargs):
print("Speech Started")
- def on_utterance_end(self, utterance_end, **kwargs):
+ def on_utterance_end(utterance_end, **kwargs):
print("Utterance End")
+ is_finals = []
if len(is_finals) > 0:
utterance = " ".join(is_finals)
print(f"Utterance End: {utterance}")
is_finals = []
- def on_close(self, close, **kwargs):
+ def on_close(close, **kwargs):
print("Connection Closed")
- def on_unhandled(self, unhandled, **kwargs):
+ def on_unhandled(unhandled, **kwargs):
print(f"Unhandled Websocket Message: {unhandled}")
…hen send initial file. Update deepgram connect func, use .websocket instead of live
Summary by Entelligence.AI