-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fixes realtime example app stability issues #1905
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
Conversation
…nd sanitizing history items
bf8dfa0
to
9c9fa43
Compare
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
async ensurePlaybackNode() { | ||
if (this.playbackNode) { | ||
return; | ||
} | ||
|
||
this.isPlayingAudio = true; | ||
|
||
// Initialize audio context if needed | ||
if (!this.playbackAudioContext) { | ||
this.playbackAudioContext = new AudioContext({ sampleRate: 24000, latencyHint: 'interactive' }); | ||
} | ||
if (!this.playbackInitPromise) { | ||
this.playbackInitPromise = (async () => { | ||
if (!this.playbackAudioContext) { | ||
this.playbackAudioContext = new AudioContext({ sampleRate: 24000, latencyHint: 'interactive' }); | ||
} | ||
|
||
// Ensure context is running (autoplay policies can suspend it) | ||
if (this.playbackAudioContext.state === 'suspended') { | ||
try { await this.playbackAudioContext.resume(); } catch {} | ||
} | ||
if (this.playbackAudioContext.state === 'suspended') { | ||
try { await this.playbackAudioContext.resume(); } catch {} |
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.
Resume playback AudioContext after browser suspends it
When playAudio
pushes a chunk it calls ensurePlaybackNode
, but this helper immediately returns once this.playbackNode
has been created. That means the call to AudioContext.resume()
only happens the first time the node is built. Browsers (Chrome/Edge/Safari) automatically suspend idle audio contexts after a period of silence, so any later audio events will enqueue chunks into the worklet while the context remains suspended and nothing is heard. The previous implementation resumed the context before every playback, so this is a regression that makes audio permanently silent after the tab has idled. Consider resuming the context even when playbackNode
already exists or resuming inside playAudio
before posting chunks.
Useful? React with 👍 / 👎.
Can you fix this mypy error?
|
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.
I did a few manual tests and didn't see any issues. This improvement is fantastic 👍
history_updated
events to remove audio bytes from bloating the websocket events to the client