-
Notifications
You must be signed in to change notification settings - Fork 0
Add OpenAI TTS provider #3
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,83 @@ | ||||||||||||||||||||||||||||||||||
| 'use strict'; | ||||||||||||||||||||||||||||||||||
| const crypto = require('crypto'); | ||||||||||||||||||||||||||||||||||
| const fs = require('fs'); | ||||||||||||||||||||||||||||||||||
| const http = require('http'); | ||||||||||||||||||||||||||||||||||
| const https = require('https'); | ||||||||||||||||||||||||||||||||||
| const path = require('path'); | ||||||||||||||||||||||||||||||||||
| const fileDuration = require('../helpers/file-duration'); | ||||||||||||||||||||||||||||||||||
| const settings = require('../../settings'); | ||||||||||||||||||||||||||||||||||
| const logger = require('sonos-discovery/lib/helpers/logger'); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| function openai(phrase, language, voice = 'alloy', model = 'tts-1') { | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| function openai(phrase, language, voice = 'alloy', model = 'tts-1') { | |
| function openai(phrase, language, voice = 'alloy', model = 'tts-1') { | |
| if (!settings.openaiKey) { | |
| logger.warn('OpenAI TTS disabled: settings.openaiKey is not configured'); | |
| return Promise.resolve(); | |
| } |
Copilot
AI
Mar 5, 2026
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 cache filename includes language (e.g., openai-${phraseHash}-${language}.mp3), but as documented in the README, the language code has no effect on the OpenAI TTS output — OpenAI determines the language from the text content itself. Including language in the cache key means the same phrase will be cached multiple times when called with different language codes (or without a language code vs. with 'en'), wasting disk space and defeating the cache. The filename should only include the phrase hash, voice, and model, as those are the actual parameters that affect the output.
| const filename = `openai-${phraseHash}-${language}.mp3`; | |
| const filename = `openai-${phraseHash}-${voice}-${model}.mp3`; |
Copilot
AI
Mar 5, 2026
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.
Content-Length is set using postData.length, which returns the number of JavaScript characters (UTF-16 code units), not the byte length of the UTF-8 encoded string. When phrase contains multi-byte characters (e.g., accented characters, CJK characters, emoji), postData.length will under-count the actual byte size sent on the wire. This should use Buffer.byteLength(postData) to get the correct byte count.
| 'Content-Length': postData.length | |
| 'Content-Length': Buffer.byteLength(postData) |
Copilot
AI
Mar 5, 2026
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.
When the response status code is not in the 2xx range, the response body is never consumed (neither read nor destroyed). This can cause the underlying TCP socket to remain open and stall, potentially preventing further requests. The response stream should be consumed and drained (e.g., by calling res.resume()) before rejecting the promise, and ideally the error body from OpenAI should be included in the rejection error for better diagnostics.
| reject(new Error(`Download from OpenAI TTS failed with status ${res.statusCode}, ${res.statusMessage}`)); | |
| const chunks = []; | |
| res.on('data', (chunk) => { | |
| chunks.push(chunk); | |
| }); | |
| res.on('end', () => { | |
| const body = chunks.length ? Buffer.concat(chunks).toString('utf8') : ''; | |
| const message = `Download from OpenAI TTS failed with status ${res.statusCode}, ${res.statusMessage}` + | |
| (body ? `, body: ${body}` : ''); | |
| reject(new Error(message)); | |
| }); | |
| res.on('error', (err) => { | |
| // Ensure the stream is not left hanging on error | |
| res.resume(); | |
| reject(err); | |
| }); |
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
httpmodule is imported on this line but is never used anywhere inopenai.js. Onlyhttpsis used. This unused import should be removed to avoid confusion.