Skip to content

Add audio encoder reconfiguration #1362

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.

Sign up for GitHub

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

Open
Qizot wants to merge 1 commit into
base: main
Choose a base branch
from
+40 −17
Open

Add audio encoder reconfiguration #1362

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 40 additions & 17 deletions js/publish/src/audio/encoder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ export class Encoder {
this.groupDuration = props?.groupDuration ?? (100 as Time.Milli); // Default is a group every 100ms

this.#signals.run(this.#runSource.bind(this));
this.#signals.run(this.#runConfig.bind(this));
this.#signals.run(this.#runGain.bind(this));
this.#signals.run(this.#runCatalog.bind(this));
}
Expand Down Expand Up @@ -102,14 +101,29 @@ export class Encoder {
await context.audioWorklet.addModule(CaptureWorklet);
if (context.state === "closed") return;

const channelCount = settings.channelCount ?? root.channelCount;
const worklet = new AudioWorkletNode(context, "capture", {
numberOfInputs: 1,
numberOfOutputs: 0,
channelCount: settings.channelCount ?? root.channelCount,
channelCount,
});

effect.set(this.#worklet, worklet);

// The information about channels count can be unreliable on different platforms (Apple's safari).
// Try to get the first audio frame and only then create the configuration.
worklet.port.onmessage = ({ data }: { data: Capture.AudioFrame }) => {
Copy link
Copy Markdown
Collaborator

@kixelated kixelated May 2, 2026

Choose a reason for hiding this comment

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

It's a little spooky setting onmessage in two places.

You shuld use

effect.event(worklet.port, "message", (data) => {

It'll unregister the event on cleanup too.

const channelCount = data.channels.length;
if (!channelCount) return;

this.#config.set(this.#createConfig(worklet, channelCount));
worklet.port.onmessage = null;
};
effect.cleanup(() => {
worklet.port.onmessage = null;
this.#config.set(undefined);
});
Comment on lines +122 to +125
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Clear the derived catalog when the config is reset.

This cleanup drops #config, but #runCatalog() never mirrors that back to #catalog. After source teardown or reinit, catalog will still expose the last rendition until another config arrives.

🩹 Suggested fix 
`#runCatalog`(effect: Effect): void {
	const config = effect.get(this.#config);
-	if (!config) return;
+	if (!config) {
+		effect.set(this.#catalog, undefined);
+		return;
+	}

	const catalog: Catalog.Audio = {
		renditions: { [Encoder.TRACK]: config },
	};
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@js/publish/src/audio/encoder.ts` around lines 122 - 125, The cleanup
currently clears the encoder config via this.#config.set(undefined) but leaves
the derived catalog stale; update the effect.cleanup block (the same place that
nulls worklet.port.onmessage and calls this.#config.set(undefined)) to also
reset the derived catalog produced by `#runCatalog` — e.g., call the catalog-reset
method/property (the internal `#catalog` setter/clear function) so that `#catalog`
no longer exposes the previous rendition after source teardown or reinit.

Copy link
Copy Markdown
Collaborator

@kixelated kixelated May 2, 2026

Choose a reason for hiding this comment

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

Can you make sure #runCatalog uses this #config signal?

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 2, 2026

Choose a reason for hiding this comment

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

@kixelated, the fix is straightforward — #runCatalog already calls effect.get(this.#config) so it re-runs whenever #config changes, but the early return on a falsy config leaves #catalog stale. Clearing it before returning keeps them in sync:

`#runCatalog`(effect: Effect): void {
    const config = effect.get(this.#config);
    if (!config) {
        effect.set(this.#catalog, undefined);
        return;
    }

    const catalog: Catalog.Audio = {
        renditions: { [Encoder.TRACK]: config },
    };

    effect.set(this.#catalog, catalog);
}

Now when #config is reset to undefined (e.g., during source cleanup), #catalog will immediately be cleared as well instead of exposing a stale rendition.

🧠 Learnings used 
Learnt from: kixelated
Repo: moq-dev/moq PR: 948
File: js/publish/vite.config.ts:14-16
Timestamp: 2026-02-13T18:36:03.315Z
Learning: In JS projects that publish web components (custom elements) implemented with a framework (e.g., Solid.js), ensure the framework is bundled into the library output rather than externalized. This means in build config (e.g., vite.config.ts) avoid marking the framework as external and configure the library build to include the framework so consumers do not need to install unused framework dependencies. Verify via build output: the published bundle contains framework code, and no externalized externals for the framework are present. This differs from libraries exporting framework components for direct consumption, where externalizing the framework can be appropriate.


gain.connect(worklet);
effect.cleanup(() => worklet.disconnect());

Expand All @@ -118,23 +132,17 @@ export class Encoder {
});
}

#runConfig(effect: Effect): void {
const values = effect.getAll([this.source, this.#worklet]);
if (!values) return;
const [_source, worklet] = values;

const config = {
#createConfig(worklet: AudioWorkletNode, channelCount: number): Catalog.AudioConfig {
return {
codec: "opus",
sampleRate: Catalog.u53(worklet.context.sampleRate),
numberOfChannels: Catalog.u53(worklet.channelCount),
bitrate: Catalog.u53(worklet.channelCount * 32_000),
numberOfChannels: Catalog.u53(channelCount),
bitrate: Catalog.u53(channelCount * 32_000),
container: { kind: "legacy" } as const,
// TODO parse the actual frame duration instead of assuming 20ms.
// Opus supports 2.5–60ms but 20ms is the real-time default.
jitter: Catalog.u53(20),
};

effect.set(this.#config, config);
}

#runGain(effect: Effect): void {
Expand All @@ -153,9 +161,9 @@ export class Encoder {
}

serve(track: Moq.Track, effect: Effect): void {
const values = effect.getAll([this.enabled, this.#worklet, this.#config]);
const values = effect.getAll([this.enabled, this.#worklet]);
if (!values) return;
const [_, worklet, config] = values;
const [_, worklet] = values;

effect.set(this.active, true, false);

Expand Down Expand Up @@ -190,11 +198,26 @@ export class Encoder {
});
effect.cleanup(() => encoder.close());

console.debug("encoding audio", config);
encoder.configure(config);
let config = this.#config.peek();
Copy link
Copy Markdown
Collaborator

@kixelated kixelated May 2, 2026

Choose a reason for hiding this comment

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

Peek is racey.

If we really need the config, we should break early if effect.get(this.#config) is undefined. This function will rerun when it's set, then we can properly configure the encoder and stuff.

if (config) {
console.debug("encoding audio", config);
encoder.configure(config);
}

worklet.port.onmessage = ({ data }: { data: Capture.AudioFrame }) => {
const channels = data.channels.slice(0, worklet.channelCount);
const channelCount = data.channels.length;
if (!channelCount) return;

if (!config || channelCount !== config.numberOfChannels) {
config = this.#createConfig(worklet, channelCount);
this.#config.set(config);
lastKeyframe = undefined;

console.debug("encoding audio", config);
encoder.configure(config);
}

const channels = data.channels;
const joinedLength = channels.reduce((a, b) => a + b.length, 0);
const joined = new Float32Array(joinedLength);

Expand Down
Loading