Skip to content

Conversation

@markbackman
Copy link
Contributor

@markbackman markbackman commented Oct 30, 2025

Please describe the changes in your PR. If it is addressing an issue, please reference that as well.

This is a refactored version of #2711.

I do see a significant reduction in latency—I see 200-300ms vs 1-1.75sec. I tripled checked that the TTFB is in the right place and empirically, the TTS is noticeably faster.

This is a breaking change though, as it requires an aiohttp_session now. There's no good way around that requirement though. We'd have to introduce a new DeepgramHttpTTSService class and deprecate this class. That is possible, but that's even more disruptive than having to add the additional arg.

If we want to move forward with this, ideally, we would make these changes in the original PR. I've notified the author. But, we should decide about the breaking change here.

@codecov
Copy link

codecov bot commented Oct 30, 2025

Codecov Report

❌ Patch coverage is 0% with 38 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pipecat/services/deepgram/tts.py 0.00% 38 Missing ⚠️
Files with missing lines Coverage Δ
src/pipecat/services/deepgram/tts.py 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aconchillo
Copy link
Contributor

This is a breaking change though, as it requires an aiohttp_session now. There's no good way around that requirement though. We'd have to introduce a new DeepgramHttpTTSService class and deprecate this class. That is possible, but that's even more disruptive than having to add the additional arg.

I think keeping the current class and adding DeepgramHttpTTSService feels better, it's what we have in other cases. If you want the fast version, just use the HTTP version, at least for now. I'm sure this will change in the future and then people can go back and use DeepgramTTSService.

@markbackman
Copy link
Contributor Author

markbackman commented Oct 31, 2025

I think keeping the current class and adding DeepgramHttpTTSService feels better, it's what we have in other cases. If you want the fast version, just use the HTTP version, at least for now. I'm sure this will change in the future and then people can go back and use DeepgramTTSService.

@aconchillo that makes sense. I've made this update. This is cleaner.

@markbackman markbackman changed the title Refactor DeepgramTTSService to use HTTP directly Add DeepgramHttpTTSService Oct 31, 2025
@aconchillo
Copy link
Contributor

LGTM!

@markbackman markbackman merged commit c6e12b9 into main Oct 31, 2025
6 checks passed
@markbackman markbackman deleted the mb/deepgram-http branch October 31, 2025 15:51
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