Skip to content

@moq/watch: expose AudioContext on the audio backend#1365

Merged
kixelated merged 2 commits into
moq-dev:mainfrom
skirsten:feat/audio-context-getter
May 1, 2026
Merged

@moq/watch: expose AudioContext on the audio backend#1365
kixelated merged 2 commits into
moq-dev:mainfrom
skirsten:feat/audio-context-getter

Conversation

@skirsten
Copy link
Copy Markdown
Contributor

The WebCodecs decoder owns its own AudioContext but doesn't surface it past the Decoder class. Browsers create the context in suspended state when there's no user gesture, and applications need a handle on it to prompt the user (e.g. a "click to enable audio" button) and call resume() from within a real gesture handler.

Add context: Getter<AudioContext | undefined> to Audio.Backend and proxy it from the WebCodecs path in MultiBackend. The MSE backend plays through the <video> element rather than WebAudio, so its context signal stays undefined.

The WebCodecs decoder owns its own AudioContext but doesn't surface it
past the Decoder class. Browsers create the context in `suspended` state
when there's no user gesture, and applications need a handle on it to
prompt the user (e.g. a "click to enable audio" button) and call
`resume()` from within a real gesture handler.

Add `context: Getter<AudioContext | undefined>` to `Audio.Backend` and
proxy it from the WebCodecs path in `MultiBackend`. The MSE backend
plays through the `<video>` element rather than WebAudio, so its
`context` signal stays undefined.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 273540c1-d58c-4ec6-81ca-3324ac2d8321

📥 Commits

Reviewing files that changed from the base of the PR and between 2b67ef0 and d8c7e16.

📒 Files selected for processing (1)
  • js/watch/src/backend.ts

Walkthrough

A context property was added to the audio backend API. The Backend interface now exposes a context getter returning AudioContext | undefined. The Mse backend implements this getter and currently returns undefined (it uses a <video> element rather than WebAudio). The AudioBackend class introduces a reactive context signal, and MultiBackend proxies that signal from the selected audio source in both WebCodecs and MSE paths.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: exposing AudioContext on the audio backend interface.
Description check ✅ Passed The description clearly explains the motivation and implementation of exposing AudioContext, including why it's needed and how different backends handle it.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@js/watch/src/backend.ts`:
- Line 186: The call to effect.proxy(this.audio.context, audioSource.context)
can leave this.audio.context bound to a stale/closed AudioContext when switching
to the MSE (video) path because `#runMse` never rebinds or clears it; update
`#runMse` (or the MSE initialization path) to explicitly reset/unproxy
this.audio.context (e.g., call effect.proxy(undefined, this.audio.context) or
set this.audio.context = undefined before/after entering the MSE flow) so
consumers don’t see an old AudioContext, and ensure any existing bindings
between effect.proxy and audioSource.context are removed when switching to MSE.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fd20bfff-bb06-49ae-b33d-ff368d36478d

📥 Commits

Reviewing files that changed from the base of the PR and between 019716f and 2b67ef0.

📒 Files selected for processing (3)
  • js/watch/src/audio/backend.ts
  • js/watch/src/audio/mse.ts
  • js/watch/src/backend.ts

Comment thread js/watch/src/backend.ts
Copy link
Copy Markdown
Collaborator

@kixelated kixelated left a comment

Choose a reason for hiding this comment

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

One thing code rabbit called out

Comment thread js/watch/src/backend.ts
@kixelated kixelated merged commit 5063dbf into moq-dev:main May 1, 2026
1 check passed
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.

2 participants