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

Add AssemblyAI Plugin #687

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

oconnoob
Copy link

Extended from PR 419 - #419

Copy link

changeset-bot bot commented Aug 30, 2024

⚠️ No Changeset found

Latest commit: dd8ec5c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ cch41
❌ ryan-assemblyai
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Author

@oconnoob oconnoob left a comment

Choose a reason for hiding this comment

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

@ploeber could you review when you get a chance?

Comment on lines 78 to 87
self._opts = STTOptions(
sample_rate=sample_rate,
word_boost=word_boost,
encoding=encoding,
disable_partial_transcripts=disable_partial_transcripts,
enable_extra_session_information=enable_extra_session_information,
buffer_size_seconds=buffer_size_seconds,
token_expires_in=token_expires_in,
end_utterance_silence_threshold=end_utterance_silence_threshold,
)
Copy link
Author

Choose a reason for hiding this comment

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

Why not use dependency injection? Because higher-level class than e.g. SpeechStream which does use DI?

Copy link

Choose a reason for hiding this comment

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

Not sure what you mean?

Copy link
Author

Choose a reason for hiding this comment

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

Why don't we just pass in STTOptions directly as we do in SpeechStream's init?

Copy link

@ploeber ploeber left a comment

Choose a reason for hiding this comment

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

We also need to update the base README.md and list our plugin there

livekit-plugins/livekit-plugins-assemblyai/README.md Outdated Show resolved Hide resolved
livekit-plugins/livekit-plugins-assemblyai/README.md Outdated Show resolved Hide resolved
tests/test_stt.py Outdated Show resolved Hide resolved
@@ -9,7 +9,7 @@

import pytest
from livekit import agents, rtc
from livekit.plugins import azure, deepgram, google, openai, silero
from livekit.plugins import assemblyai, azure, deepgram, google, openai, silero
Copy link

Choose a reason for hiding this comment

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

since we add this new dependency, we need to add ./livekit-plugins/livekit-plugins-assemblyai \ to the tests.yml file

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice work so far, care to join a shared slack channel to help get this over the finish line? Just need your emails

this code will run on server so no need to generate temporary token
The following parameters of `SpeechStream`'s init are redundant with the parameter `opts: STTOptions`

- buffer_size_seconds
- sample_rate
- end_utterance_silence_threshold

they have been removed and their corresponding instance attributes have been removed
`prerecorded_transcription_to_speech_event` appears to have been brought over from a copy-paste
@oconnoob
Copy link
Author

oconnoob commented Sep 5, 2024

@ploeber the STT interface requires that the abstract async method _main_task is implemented. Currently _main_task exists as an instance attribute of SpeechStream - I'm not sure how to reconcile this

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.

6 participants