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

Create a low-level interface for websocket transcription #82

Merged
merged 15 commits into from
Aug 12, 2024

Conversation

jcdyer
Copy link
Contributor

@jcdyer jcdyer commented Aug 7, 2024

  • Add low-level websocket handle to transcription api
  • fmt

@jcdyer jcdyer requested review from bd-g and DamienDeepgram August 7, 2024 20:27
bd-g
bd-g previously approved these changes Aug 7, 2024
Copy link
Contributor

@bd-g bd-g left a comment

Choose a reason for hiding this comment

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

Feels much cleaner.

CHANGELOG.md Show resolved Hide resolved
@bd-g bd-g self-requested a review August 7, 2024 21:18
@bd-g bd-g dismissed their stale review August 7, 2024 21:19

Accidentally clicked approve - meant to just comment, will review more in depth tomorrow

@jcdyer jcdyer force-pushed the websocket-handle branch from 8b02749 to a8bcae6 Compare August 8, 2024 02:36
Copy link
Contributor

@bd-g bd-g left a comment

Choose a reason for hiding this comment

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

The code itself looks clean, but something here is breaking the simple_stream example - I can get it to run successfully on main, but not this branch.

With that said, we also need to update examples/README.md. The filename is incorrect, we should suggest exporting an API key, and we should update the example names.


Running Examples

Setting Env Vars

export FILENAME=./examples/bueller.wav
# If using the hosted Deepgram API
export DEEPGRAM_API_KEY=<your_api_key_here>

Running the examples

cargo run --example prerecorded_from_file
cargo run --example callback
cargo run --example make_prerecorded_request_builder
cargo run --example prerecorded_from_url
cargo run --example simple_stream
cargo run --example microphone_stream
cargo run --example text_to_speech_to_file
cargo run --example text_to_speech_to_stream

@jcdyer jcdyer requested review from bd-g and DamienDeepgram August 9, 2024 21:58
@DamienDeepgram
Copy link
Contributor

LGTM

Copy link
Contributor

@bd-g bd-g left a comment

Choose a reason for hiding this comment

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

Paired with Cliff to review, committed requested changes.

One notable TODO that remains: the stream loop assumes a given future will interoperate with tokio::select_biased cleanly. When testing with futures::channels streams, the futures::select_biased does not properly bias towards the websocket channel, and can get stuck in an infinite loop receiving None messages from the closed audio channel.

@jcdyer jcdyer merged commit 4821b91 into main Aug 12, 2024
18 checks passed
@jcdyer jcdyer deleted the websocket-handle branch August 12, 2024 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants