diff --git a/doc/lib/js/publish.md b/doc/lib/js/publish.md index fa9f23a1f23..078e97afc4f 100644 --- a/doc/lib/js/publish.md +++ b/doc/lib/js/publish.md @@ -199,7 +199,9 @@ new Publish.Audio.Encoder("audio", { broadcast, capture: audioCapture, enabled: Standalone components start enabled unless you pass `enabled: false` (or a signal). An enabled camera or microphone prompts for permission on construction, and a screen source must be built inside the user gesture that -authorizes it. Audio waits for the page's first click or keypress, since +authorizes it. A screen source's `out.state` is `off`, `prompting`, `live`, or +`ended` with a reason (`stopped`, `dismissed`, or `error`); an ended share stays +ended until the next enable, which prompts again. Audio waits for the page's first click or keypress, since browsers suspend Web Audio until then. Every input and output is a signal from [`@moq/signals`](/lib/js/signals). A failed encoder exposes the cause through `out.error` and disables only its rendition; siblings and the broadcast keep diff --git a/js/moq-boy/src/element.test.ts b/js/moq-boy/src/element.test.ts index 9c34a16df3b..66ed4b2dea0 100644 --- a/js/moq-boy/src/element.test.ts +++ b/js/moq-boy/src/element.test.ts @@ -54,3 +54,46 @@ for (const attribute of ["prefix", "prefix-game"] as const) { } }); } + +// A page's explicit disable wins: correcting a refused path must not turn the element back on. +test("an explicit disable survives an invalid path being corrected", async () => { + const el = new MoqBoy(); + try { + el.connectedCallback(); + await settle(); + expect(el.connection.enabled.peek()).toBe(true); + + el.connection.enabled.set(false); + expect(() => el.attributeChangedCallback("prefix", null, "/boy")).toThrow(); + await settle(); + el.attributeChangedCallback("prefix", "/boy", "boy"); + await settle(); + expect(el.connection.enabled.peek()).toBe(false); + + // A valid edit leaves the page's choice alone too. + el.attributeChangedCallback("prefix", "boy", "other"); + await settle(); + expect(el.connection.enabled.peek()).toBe(false); + } finally { + el.disconnectedCallback(); + } +}); + +// The element is already off while a path is refused, so the page's disable there must still count. +test("an explicit disable while a path is refused survives the fix", async () => { + const el = new MoqBoy(); + try { + el.connectedCallback(); + expect(() => el.attributeChangedCallback("prefix", null, "/boy")).toThrow(); + el.connection.enabled.set(false); + el.attributeChangedCallback("prefix", "/boy", "boy"); + await settle(); + expect(el.connection.enabled.peek()).toBe(false); + + el.connection.enabled.set(true); + await settle(); + expect(el.connection.enabled.peek()).toBe(true); + } finally { + el.disconnectedCallback(); + } +}); diff --git a/js/moq-boy/src/element.tsx b/js/moq-boy/src/element.tsx index b0ae90dc1a2..6fc891cbc03 100644 --- a/js/moq-boy/src/element.tsx +++ b/js/moq-boy/src/element.tsx @@ -30,6 +30,31 @@ function viewerId(): string { const cleanup = new FinalizationRegistry((signals) => signals.close()); +/** + * The element's `connection.enabled`, which the page may also write. The page's writes set what it + * wants and the element sets whether it can run (connected to a document, every path valid); it + * reads as on only while both hold. Kept apart, a page's disable survives the element suspending and + * resuming, whenever the page writes it: an explicit page choice wins. + */ +class Enabled extends Moq.Signals.Signal { + #wanted = true; + #running = false; + + constructor() { + super(false); + } + + override set(value: boolean, notify?: boolean): void { + this.#wanted = value; + super.set(value && this.#running, notify); + } + + run(running: boolean): void { + this.#running = running; + super.set(this.#wanted && running); + } +} + /** * `` web component - discovers and manages Game Boy streaming sessions, * and renders the interactive UI in a Shadow DOM. @@ -58,11 +83,11 @@ export default class MoqBoy extends HTMLElement { readonly #signals = new Moq.Signals.Effect(); // Set when the element is connected to the DOM. - readonly #connected = new Moq.Signals.Signal(false); + #connected = false; // The path attributes holding an invalid path, which idle the element until each is fixed. - readonly #refused = new Moq.Signals.Signal>(new Set()); - // Whether to run: connected, with nothing refused. - readonly #enabled = new Moq.Signals.Signal(false); + readonly #refused = new Set(); + // Whether to run: `#update` gates it on the element's state, and the page may turn it off. + readonly #enabled = new Enabled(); readonly #prefix = new Moq.Signals.Signal(DEFAULT_PREFIX); readonly #gamePrefixOverride = new Moq.Signals.Signal(undefined); readonly #sessions = new Map(); @@ -72,10 +97,6 @@ export default class MoqBoy extends HTMLElement { super(); cleanup.register(this, this.#signals); - this.#signals.run((effect) => { - this.#enabled.set(effect.get(this.#connected) && effect.get(this.#refused).size === 0); - }); - // One origin, both directions: the viewer's endpoint is published into it and the // relay's announced games and endpoints arrive in it, with no risk of echoing either back. this.connection = new Moq.Connection({ @@ -94,7 +115,8 @@ export default class MoqBoy extends HTMLElement { /** Called by the browser when the element joins a document. */ connectedCallback() { - this.#connected.set(true); + this.#connected = true; + this.#update(); // Render the UI into a Shadow DOM (reuse existing on reconnect). const shadow = this.shadowRoot ?? this.attachShadow({ mode: "open" }); @@ -103,7 +125,8 @@ export default class MoqBoy extends HTMLElement { /** Called by the browser when the element leaves its document. */ disconnectedCallback() { - this.#connected.set(false); + this.#connected = false; + this.#update(); this.#dispose?.(); this.#dispose = undefined; } @@ -115,9 +138,14 @@ export default class MoqBoy extends HTMLElement { return; } - // Refused until parsed, so an invalid path leaves the element idle and the error rethrown. - this.#refuse(name, true); - const path = newValue === null ? undefined : Moq.Path.from(newValue); + // An invalid path leaves the element idle until fixed, with the error rethrown. + let path: Moq.Path.Valid | undefined; + try { + path = newValue === null ? undefined : Moq.Path.from(newValue); + } catch (error) { + this.#refuse(name, true); + throw error; + } this.#refuse(name, false); if (name === "prefix") { @@ -128,10 +156,13 @@ export default class MoqBoy extends HTMLElement { } #refuse(name: Observed, refused: boolean) { - const next = new Set(this.#refused.peek()); - if (refused) next.add(name); - else next.delete(name); - this.#refused.set(next); + if (refused) this.#refused.add(name); + else this.#refused.delete(name); + this.#update(); + } + + #update() { + this.#enabled.run(this.#connected && this.#refused.size === 0); } /** The relay URL, mirrored by the `url` attribute. */ diff --git a/js/moq-boy/src/game.test.ts b/js/moq-boy/src/game.test.ts index 2157d32d651..b9a28810284 100644 --- a/js/moq-boy/src/game.test.ts +++ b/js/moq-boy/src/game.test.ts @@ -160,3 +160,57 @@ test("input opens a session that carries commands up and latency down until deac else Reflect.deleteProperty(globalThis, "document"); } }); + +// Input given while offline would land late, out of context; a reset pressed then must not fire on reconnect. +test("a command sent while disconnected is dropped, not replayed on reconnect", async () => { + const originalDocument = Object.getOwnPropertyDescriptor(globalThis, "document"); + Object.defineProperty(globalThis, "document", { configurable: true, value: new EventTarget() }); + + const connection = new Moq.Connection({ enabled: false }); + const status = new Moq.Signals.Signal("connecting"); + Object.defineProperty(connection, "status", { value: status }); + const origin = new Moq.Origin.Producer(); + const emulator = new Moq.Socket.Endpoint(origin, Moq.Path.from("game"), "test"); + const endpoint = new Moq.Socket.Endpoint(origin, Moq.Path.from("game"), "viewer-test"); + const game = new Game({ + sessionId: "test", + connection, + origin, + endpoint, + expanded: new Moq.Signals.Signal(undefined), + gamePrefix: "game", + }); + + try { + game.hovered.set(true); + game.sendCommand({ type: "reset" }); + await settle(); + + let dialed = false; + const incoming = emulator.acceptTrack().then((track) => { + dialed = true; + return track; + }); + status.set("connected"); + await settle(50); + expect(dialed).toBe(false); + + game.heldButtons.add("a"); + game.sendButtons(); + const session = await (await incoming).accept(); + const commands = session.consumer.ordered(); + expect(await (await commands.nextGroup())?.readJson()).toEqual({ + type: "buttons", + buttons: ["a"], + timestamps: [], + }); + } finally { + game.close(); + endpoint.close(); + emulator.close(); + origin.close(); + connection.close(); + if (originalDocument) Object.defineProperty(globalThis, "document", originalDocument); + else Reflect.deleteProperty(globalThis, "document"); + } +}); diff --git a/js/moq-boy/src/game.ts b/js/moq-boy/src/game.ts index 57bb4c5ae5b..115618b1fad 100644 --- a/js/moq-boy/src/game.ts +++ b/js/moq-boy/src/game.ts @@ -115,8 +115,9 @@ export class Game { /** The Game Boy buttons currently held down. */ readonly heldButtons = new Set(); - // Internal command publishing state. + // The command waiting for the session to open, consumed once written. #command = new Moq.Signals.Signal(undefined); + readonly #connection: Moq.Connection; #feedbackActive = new Moq.Signals.Signal(false); constructor(config: GameConfig) { @@ -124,6 +125,7 @@ export class Game { this.sessionId = sessionId; this.expanded = expanded; this.#endpoint = config.endpoint; + this.#connection = connection; // Derive active state from expanded + hovered. this.#signals.run(this.#runActive.bind(this)); @@ -213,8 +215,10 @@ export class Game { this.sendCommand({ type: "buttons", buttons: [...this.heldButtons] }); } - /** Send a command to the emulator. */ + /** Send a command to the emulator. Dropped while disconnected, since it would land late and out of context. */ sendCommand(cmd: Record) { + if (this.#connection.status.peek() !== "connected") return; + // Open the session on input, with idle timeout. this.#feedbackActive.set(true); @@ -306,11 +310,9 @@ export class Game { } #runSession(connection: Moq.Connection, effect: Moq.Signals.Effect) { - // Gate on a live connection: a dial is answered only through the relay. - if (effect.get(connection.status) !== "connected") return; - - if (!effect.get(this.active)) { - // Clear feedback state when deactivating. + // Gate on a live connection, since a dial is answered only through the relay. Losing it or + // deactivating drops pending input, which would otherwise replay on the next session. + if (effect.get(connection.status) !== "connected" || !effect.get(this.active)) { this.#feedbackActive.set(false); this.#command.set(undefined); return; @@ -364,6 +366,7 @@ export class Game { // One command per group, so a late one never holds up the next. const value = { ...command.cmd, timestamps: command.timestamps }; open.producer.writeFrame({ payload: new TextEncoder().encode(JSON.stringify(value)) }); + this.#command.set(undefined); } // Read the emulator's replies until the session ends. diff --git a/js/publish/api/element.api.md b/js/publish/api/element.api.md index e624c896f52..393ca4b62b1 100644 --- a/js/publish/api/element.api.md +++ b/js/publish/api/element.api.md @@ -17,7 +17,7 @@ export type SourceType = "camera" | "screen" | "file"; export default class extends HTMLElement { constructor(); - get announce(): AnnounceMode | undefined; + get announce(): AnnounceMode; set announce(value: AnnounceMode); attributeChangedCallback(name: "source" | "url" | "stats" | "announce" | "name" | "capture" | "backend" | "versions" | "muted" | "invisible" | "preview" | "echo-path", oldValue: string | null, newValue: string | null): void; readonly audio: Audio.Encoder; @@ -29,7 +29,7 @@ export default class extends HTMLElement { connectedCallback(): void; readonly connection: Net.Connection; readonly controls: { - announce: Signals.Signal; + announce: Signals.Signal; backend: Signals.Signal; capture: Signals.Signal; echoPath: Signals.Signal; diff --git a/js/publish/api/index.api.md b/js/publish/api/index.api.md index a8115cc38bf..e6651a3e727 100644 --- a/js/publish/api/index.api.md +++ b/js/publish/api/index.api.md @@ -318,16 +318,35 @@ export namespace Source { readonly out: Signals.Readonlys; video: Signals.Signal; } + export type ScreenEnd = { + reason: "stopped"; + } | { + reason: "dismissed"; + } | { + error: Error; + reason: "error"; + }; export type ScreenInput = { enabled: Signals.Getter; }; export type ScreenOutput = { - source: Signals.Signal; + source: Signals.Getter; + state: Signals.Getter; }; export interface ScreenProps extends Signals.Inputs { audio?: Audio.Constraints | boolean | Signals.Signal; video?: Video.Constraints | boolean | Signals.Signal; } + export type ScreenState = { + kind: "off"; + } | { + kind: "prompting"; + } | { + kind: "live"; + media: Source.Media; + } | { + kind: "ended"; + } & Source.ScreenEnd; } export namespace Video { diff --git a/js/publish/src/announce.test.ts b/js/publish/src/announce.test.ts index e27e66923be..8e24e09eee2 100644 --- a/js/publish/src/announce.test.ts +++ b/js/publish/src/announce.test.ts @@ -26,7 +26,7 @@ function setup(initial: SourceType = "camera") { const announcing = new Signal(false); const connected = new Signal(true); const source = new Signal(initial); - const mode = new Signal("source"); + const mode = new Signal("source"); const broadcast = new Broadcast({ enabled: connected, origin: new Origin.Producer(), @@ -247,12 +247,9 @@ test.each(Object.keys(boundaries) as (keyof typeof boundaries)[])( }, ); -test("an invalid mode stops a latched announcement while always needs no selection", async () => { +test("always needs no selection", async () => { using env = setup(); await start(env); - env.mode.set(undefined); - await settle(); - expect(env.announcing.peek()).toBe(false); env.source.set(undefined); env.mode.set("always"); await settle(); diff --git a/js/publish/src/announce.ts b/js/publish/src/announce.ts index 53fea761622..14d49afd7ec 100644 --- a/js/publish/src/announce.ts +++ b/js/publish/src/announce.ts @@ -10,8 +10,7 @@ interface Track { } export interface Props { - // Undefined refuses to announce. - mode: Getter; + mode: Getter; // The selected source while the element is connected; it stays set across capture gaps. source: Getter; video: Track; diff --git a/js/publish/src/audio/encoder.test.ts b/js/publish/src/audio/encoder.test.ts index 4b251bd96ef..8d42adeb4b5 100644 --- a/js/publish/src/audio/encoder.test.ts +++ b/js/publish/src/audio/encoder.test.ts @@ -627,7 +627,8 @@ test("muting sends nothing and marks where the audio ends, keeping the rendition }); // With `capture="release"` a mute closes the microphone, taking its format away. The rendition stays -// advertised, still enabled, through the mute and while the reopened microphone resolves. +// advertised, still enabled, through the mute. Once unmuted it is disabled until the microphone +// reopens, so one that never does isn't advertised as sending. test("a muted rendition keeps its config enabled while its capture is released", async () => { using _webcodecs = installFakeWebCodecs(); using env = await setup(); @@ -642,7 +643,15 @@ test("a muted rendition keeps its config enabled while its capture is released", encoder.muted.set(false); await settle(); + expect(encoder.out.catalog.peek()).toEqual({ ...before, enabled: false } as never); + + // Muting again while the reopen is pending is muted, not disabled. + encoder.muted.set(true); + await settle(); expect(encoder.out.catalog.peek()).toEqual(before); + encoder.muted.set(false); + await settle(); + expect(encoder.out.catalog.peek()).toEqual({ ...before, enabled: false } as never); capture.out.format.set(format); await settle(10); diff --git a/js/publish/src/audio/encoder.ts b/js/publish/src/audio/encoder.ts index 70e8081681a..15071e1e995 100644 --- a/js/publish/src/audio/encoder.ts +++ b/js/publish/src/audio/encoder.ts @@ -268,13 +268,10 @@ export class Encoder { #slot = new Signal | undefined>(undefined); // The slot while no network broadcast is serving the rendition. #local = new Slot(); - // Whether the rendition was disabled since a config last resolved, so it keeps advertising its last - // config as disabled until the re-enabled capture resolves a new one. A capture that never reopens - // leaves it disabled, which is accurate: no frames are coming. - #paused = false; - // Whether it was muted since a config last resolved, so it keeps advertising its last config, - // enabled, while a released capture is closed and until the reopened one resolves. - #muted = false; + // Whether the rendition was disabled or muted since a config last resolved, so it keeps advertising + // its last config until a new one resolves. Whether that config reads as enabled follows the + // current knobs alone, never how the gap began. + #held = false; constructor(name: string, props?: EncoderProps) { // `source` moved to Audio.Capture, which renditions share. TypeScript catches this, but a @@ -629,23 +626,24 @@ export class Encoder { effect.cleanup(() => this.#failures.update((n) => n - 1)); } - // Publish the probed config before the first frame reaches the track. A disabled rendition keeps its - // last config, since a disable may also release the capture, and stays disabled after re-enabling - // until the reopened capture reports its format. A muted one keeps it enabled throughout. + // Publish the probed config before the first frame reaches the track. A disabled or muted rendition + // keeps its last config, since either may also release the capture. It reads as enabled only while + // muted, which keeps subscribers attached for the next word; otherwise nothing is coming until the + // reopened capture reports its format, so a capture that never reopens leaves it disabled. #runCatalog(effect: Effect): void { const estimate = effect.get(this.#estimate); const enabled = effect.get(this.in.enabled) && !effect.get(this.#out.error); + const muted = effect.get(this.muted); const slot = this.#current(effect); - if (!enabled) this.#paused = true; - if (effect.get(this.muted)) this.#muted = true; + if (!enabled || muted) this.#held = true; const config = enabled ? effect.get(this.#config)?.catalog : undefined; if (!config) { - const last = this.#paused || this.#muted ? slot.last : undefined; - effect.set(this.#out.catalog, last && { ...last, ...estimate, ...(this.#paused && { enabled: false }) }); + const last = this.#held ? slot.last : undefined; + const stillEnabled = enabled && muted; + effect.set(this.#out.catalog, last && { ...last, ...estimate, ...(!stillEnabled && { enabled: false }) }); return; } - this.#paused = false; - this.#muted = false; + this.#held = false; // The catalog bitrate is the most this track has been configured for, so lowering the knob // retunes the encoder without lowering what the catalog advertises. diff --git a/js/publish/src/element.test.ts b/js/publish/src/element.test.ts index 4bb15fa21f6..df8ac341228 100644 --- a/js/publish/src/element.test.ts +++ b/js/publish/src/element.test.ts @@ -1,23 +1,114 @@ -import { expect, mock, test } from "bun:test"; +import { afterAll, expect, mock, test } from "bun:test"; +import { yieldNow } from "@moq/clock"; mock.module("./audio/capture-worklet.ts?worklet", () => ({ default: async () => "blob:capture" })); mock.module("./video/capture-worker.ts?worklet", () => ({ default: async () => "blob:worker" })); -// Native attribute reactions and their errors are covered by publish-ui.browser.ts. -test("importing the publish entrypoint registers the element", async () => { - const originalElement = globalThis.HTMLElement; - const originalRegistry = globalThis.customElements; - const registry = new Map(); - globalThis.HTMLElement = class {} as unknown as typeof HTMLElement; - globalThis.customElements = { - define: (name: string, ctor: CustomElementConstructor) => registry.set(name, ctor), - } as unknown as CustomElementRegistry; +// Just enough of the DOM to import the element, construct it and drive its callbacks. Native attribute +// reactions and their errors are covered by publish-ui.browser.ts. +const originalElement = globalThis.HTMLElement; +const originalRegistry = globalThis.customElements; +const originalObserver = globalThis.MutationObserver; +const originalVideo = globalThis.HTMLVideoElement; +const registry = new Map(); +globalThis.HTMLElement = class { + getRootNode() { + return {}; + } + getAttribute() { + return null; + } + setAttribute() {} + removeAttribute() {} + querySelector() { + return null; + } +} as unknown as typeof HTMLElement; +globalThis.customElements = { + define: (name: string, ctor: CustomElementConstructor) => registry.set(name, ctor), +} as unknown as CustomElementRegistry; +globalThis.MutationObserver = class { + observe() {} + disconnect() {} +} as unknown as typeof MutationObserver; +globalThis.HTMLVideoElement = class {} as unknown as typeof HTMLVideoElement; +afterAll(() => { + globalThis.HTMLElement = originalElement; + globalThis.customElements = originalRegistry; + globalThis.MutationObserver = originalObserver; + globalThis.HTMLVideoElement = originalVideo; +}); + +const { default: MoqPublish } = await import("./element"); + +// Effects rerun in a later task, so each check waits one out. +const settle = () => yieldNow(); + +test("importing the publish entrypoint registers the element", () => { + expect(registry.get("moq-publish")).toBe(MoqPublish); +}); + +test("a corrected attribute resumes the element", async () => { + const el = new MoqPublish(); + try { + el.connectedCallback(); + await settle(); + expect(el.connection.enabled.peek()).toBe(true); + + expect(() => el.attributeChangedCallback("backend", null, "bad")).toThrow(); + await settle(); + expect(el.connection.enabled.peek()).toBe(false); + + el.attributeChangedCallback("backend", "bad", null); + await settle(); + expect(el.connection.enabled.peek()).toBe(true); + } finally { + el.signals.close(); + } +}); + +// A page's explicit disable wins: correcting a refused attribute must not turn the element back on. +test("an explicit disable survives an invalid attribute being corrected", async () => { + const el = new MoqPublish(); + try { + el.connectedCallback(); + await settle(); + + el.connection.enabled.set(false); + expect(() => el.attributeChangedCallback("backend", null, "bad")).toThrow(); + await settle(); + el.attributeChangedCallback("backend", "bad", null); + await settle(); + expect(el.connection.enabled.peek()).toBe(false); + + // Nor does moving the element to another place in the document. + el.disconnectedCallback(); + el.connectedCallback(); + await settle(); + expect(el.connection.enabled.peek()).toBe(false); + + // The page turning it back on resumes it. + el.connection.enabled.set(true); + await settle(); + expect(el.connection.enabled.peek()).toBe(true); + } finally { + el.signals.close(); + } +}); + +// The element is already off while detached, so the page's disable there must still count. +test("an explicit disable while detached survives reattaching", async () => { + const el = new MoqPublish(); try { - await import("./element"); - expect(registry.has("moq-publish")).toBe(true); + el.connectedCallback(); + await settle(); + el.disconnectedCallback(); + el.connection.enabled.set(false); + el.connectedCallback(); + await settle(); + expect(el.connection.enabled.peek()).toBe(false); } finally { - globalThis.HTMLElement = originalElement; - globalThis.customElements = originalRegistry; + el.signals.close(); } }); diff --git a/js/publish/src/element.ts b/js/publish/src/element.ts index d266a01807e..769254f1bbb 100644 --- a/js/publish/src/element.ts +++ b/js/publish/src/element.ts @@ -72,8 +72,8 @@ export type AnnounceMode = "always" | "source"; * What a muted or invisible track does with its capture. * * `hold` keeps the device open, so resuming is instant but the browser's capture indicator stays lit. - * `release` closes it, so the indicator goes dark and resuming reopens it. A screen share or file - * stops only once both tracks are off. Defaults to `hold`. + * `release` closes it, so the indicator goes dark and resuming reopens it; the track reads as disabled + * in the catalog until it has. A screen share or file stops only once both tracks are off. Defaults to `hold`. */ export type CaptureMode = "hold" | "release"; @@ -123,6 +123,31 @@ function parseBackend(value: string | null): Preview.Backend { throw invalidAttribute("backend", value, '"auto", "webgpu", or "2d"'); } +/** + * The element's `connection.enabled`, which the page may also write. The page's writes set what it + * wants and the element sets whether it can run (connected to a document, every attribute valid); it + * reads as on only while both hold. Kept apart, a page's disable survives the element suspending and + * resuming, whenever the page writes it: an explicit page choice wins. + */ +class Enabled extends Signal { + #wanted = true; + #running = false; + + constructor() { + super(false); + } + + override set(value: boolean, notify?: boolean): void { + this.#wanted = value; + super.set(value && this.#running, notify); + } + + run(running: boolean): void { + this.#running = running; + super.set(this.#wanted && running); + } +} + /** The `` element: captures a source and publishes it as a broadcast. */ export default class MoqPublish extends HTMLElement { /** The attributes the browser reports changes to. */ @@ -141,9 +166,8 @@ export default class MoqPublish extends HTMLElement { preview: new Signal("source"), // Which graphics API draws a preview. backend: new Signal("auto"), - // When to advertise the broadcast: always, or only once a source is live. Undefined while the - // `announce` attribute is invalid, which never announces. - announce: new Signal("source"), + // When to advertise the broadcast: always, or only once a source is live. + announce: new Signal("source"), // Whether to publish a stats track named in the catalog. stats: new Signal(false), // Where viewers may publish feedback, relative to the broadcast, or undefined to solicit none. @@ -197,7 +221,8 @@ export default class MoqPublish extends HTMLElement { #preview = new Signal(undefined); // Whether each track publishes: connected and allowed by `muted` and `invisible`. Video encodes only - // while it does; muted audio stays served and sends nothing, so an unmute needs no catalog update. + // while it does; muted audio stays served and sends nothing, so an unmute with its capture held needs + // no catalog update. #videoEnabled = new Signal(false); #audioEnabled = new Signal(false); @@ -207,13 +232,12 @@ export default class MoqPublish extends HTMLElement { #audioCapture = new Signal(false); #eitherCapture = new Signal(false); - // Set when the element is connected to the DOM. - #enabled = new Signal(false); + // The document the element is connected to, if any. #root = new Signal(undefined); + // The attributes holding a refused value, which suspend the element until corrected. #invalid = new Signal>(new Set()); - // Whether clearing the last invalid attribute re-enables media: false when the page had - // already disabled the connection, so recovery does not override that. - #resume = false; + // Whether to connect and capture: `#update` gates it on the element's state, and the page may turn it off. + #enabled = new Enabled(); #reflecting = false; // Whether to advertise the broadcast, driven by the `announce` mode. @@ -445,14 +469,13 @@ export default class MoqPublish extends HTMLElement { /** Called by the browser when the element joins a document. */ connectedCallback() { this.#root.set(this.getRootNode() as Document | ShadowRoot); - if (this.#invalid.peek().size) this.#resume = true; - else this.#enabled.set(true); + this.#update(); } /** Called by the browser when the element leaves its document. */ disconnectedCallback() { this.#root.set(undefined); - this.#enabled.set(false); + this.#update(); } /** Called by the browser when an observed attribute changes. */ @@ -476,25 +499,24 @@ export default class MoqPublish extends HTMLElement { throw new Error(`Invalid attribute: ${exhaustive}`); } } catch (error) { - if (this.#invalid.peek().size === 0) this.#resume = this.#enabled.peek(); - if (name === "announce") this.controls.announce.set(undefined); this.#invalid.update((previous) => new Set(previous).add(name)); - this.#enabled.set(false); + this.#update(); throw error; } } #accept(name: string) { - // Ordinary edits must preserve an explicitly disabled connection. if (!this.#invalid.peek().has(name)) return; this.#invalid.update((previous) => { const next = new Set(previous); next.delete(name); return next; }); - if (this.#invalid.peek().size || !this.#resume) return; - this.#resume = false; - this.#enabled.set(this.#root.peek() !== undefined); + this.#update(); + } + + #update() { + this.#enabled.run(this.#root.peek() !== undefined && this.#invalid.peek().size === 0); } #reflect(effect: Effect, name: (typeof OBSERVED)[number], value: string | undefined) { @@ -697,8 +719,8 @@ export default class MoqPublish extends HTMLElement { this.#accept("backend"); } - /** When to announce the broadcast, mirrored by the `announce` attribute; undefined while that attribute is invalid. */ - get announce(): AnnounceMode | undefined { + /** When to announce the broadcast, mirrored by the `announce` attribute. */ + get announce(): AnnounceMode { return this.controls.announce.peek(); } diff --git a/js/publish/src/source/retry.ts b/js/publish/src/source/retry.ts index 23602404fe2..b8cd0555a20 100644 --- a/js/publish/src/source/retry.ts +++ b/js/publish/src/source/retry.ts @@ -33,15 +33,8 @@ export class Retry { readonly #rerun = new Signal(0); // Deliberately plain fields: effect reruns must not unwind them, or the budget never runs out. - #failures = 0; #settings: unknown[] | undefined; - #delay = Retry.DELAY.initial; - - // How long the next attempt still owes the backoff, set by `failed` and paid by `begin`. - #wait: DOMHighResTimeStamp | undefined; - - // The cause of the latest failure. Only surfaced once the budget is spent, so a retry stays quiet. - #error: Error | undefined; + #state: State = fresh(); /** * Subscribe the capture effect and report whether an attempt is worth making right now. @@ -60,14 +53,15 @@ export class Retry { if (settings.some((setting, i) => setting !== this.#settings?.[i])) { this.#settings = settings; - this.#clear(); + this.#state = fresh(); } - if (this.#failures > Retry.LIMIT) return false; + const state = this.#state; + if (state.kind === "spent") return false; - const wait = this.#wait; + const wait = state.wait; if (wait !== undefined) { - this.#wait = undefined; + this.#state = { ...state, wait: undefined }; effect.timer(() => this.#rerun.update((rerun) => rerun + 1), Nano.fromMilli(Milli(wait))); return false; } @@ -77,14 +71,17 @@ export class Retry { /** Spends budget and reruns the effect, returning whether another attempt is allowed. */ failed(error: Error): boolean { - this.#error = error; - this.#failures += 1; - const retry = this.#failures <= Retry.LIMIT; - // Equal jitter, so a page with several captures doesn't reopen them all on the same tick. - this.#wait = retry ? this.#delay * (0.5 + Math.random() / 2) : undefined; - this.#delay = Math.min(this.#delay * Retry.DELAY.multiplier, Retry.DELAY.max); + const state = this.#state; + if (state.kind === "trying" && state.failures < Retry.LIMIT) { + // Equal jitter, so a page with several captures doesn't reopen them all on the same tick. + const wait = state.delay * (0.5 + Math.random() / 2); + const delay = Math.min(state.delay * Retry.DELAY.multiplier, Retry.DELAY.max); + this.#state = { kind: "trying", failures: state.failures + 1, delay, wait }; + } else { + this.#state = { kind: "spent", error }; + } this.#rerun.update((rerun) => rerun + 1); - return retry; + return this.#state.kind === "trying"; } /** @@ -92,8 +89,7 @@ export class Retry { * Undefined while another attempt is allowed, including during backoff. */ spent(): Error | undefined { - if (this.#failures <= Retry.LIMIT) return undefined; - return this.#error; + return this.#state.kind === "spent" ? this.#state.error : undefined; } /** Spend budget because the track ended. Reported via {@link spent} once nothing remains. */ @@ -112,23 +108,17 @@ export class Retry { if (cause.name === "NotReadableError" || cause.name === "AbortError") { return this.failed(cause); } - this.#error = cause; - this.terminal(); - return false; - } - - /** Stop attempting this capture until its settings, device list, or permission changes. */ - terminal(): void { - this.#failures = Retry.LIMIT + 1; - this.#wait = undefined; + // Stop attempting this capture until its settings, device list, or permission changes. + this.#state = { kind: "spent", error: cause }; this.#rerun.update((rerun) => rerun + 1); + return false; } /** The attempt produced a live track. Reruns the effect if it dies. */ succeeded(effect: Effect, track: MediaStreamTrack): void { effect.timer( () => { - this.#clear(); + this.#state = fresh(); }, Nano.fromMilli(Milli(Retry.SETTLED)), ); @@ -138,17 +128,23 @@ export class Retry { /** Refund the budget, because something changed that makes another attempt worth trying. */ refund(): void { - if (this.#failures === 0 && this.#wait === undefined) return; + const state = this.#state; + if (state.kind === "trying" && state.failures === 0 && state.wait === undefined) return; - this.#clear(); + this.#state = fresh(); this.#rerun.update((rerun) => rerun + 1); } +} - /** Forget the failures so far, along with the pause they earned. */ - #clear(): void { - this.#failures = 0; - this.#wait = undefined; - this.#delay = Retry.DELAY.initial; - this.#error = undefined; - } +// Where a capture's budget stands. Its cause is kept only once spent, so a retry stays quiet. +type State = + // Attempts allowed: the failures so far, the next backoff, and the pause the next attempt still + // owes, set by `failed` and paid by `begin`. + | { kind: "trying"; failures: number; delay: number; wait?: DOMHighResTimeStamp } + // Out of budget until the settings, device list, or permission change. + | { kind: "spent"; error: Error }; + +// No failures, and none of the pause they earn. +function fresh(): State { + return { kind: "trying", failures: 0, delay: Retry.DELAY.initial }; } diff --git a/js/publish/src/source/screen.test.ts b/js/publish/src/source/screen.test.ts index 5b569fe2a50..a83878b592d 100644 --- a/js/publish/src/source/screen.test.ts +++ b/js/publish/src/source/screen.test.ts @@ -6,34 +6,176 @@ const settle = async () => { for (let i = 0; i < 20; i++) await Promise.resolve(); }; +class FakeTrack extends EventTarget { + readyState: MediaStreamTrackState = "live"; + readonly stop = mock(() => { + this.readyState = "ended"; + }); + getSettings() { + return {}; + } + // What the browser's own "Stop sharing" control does. + end() { + this.readyState = "ended"; + this.dispatchEvent(new Event("ended")); + } +} + +function stream(video = new FakeTrack(), audio = new FakeTrack()): MediaStream { + return { + getTracks: () => [video, audio], + getVideoTracks: () => [video], + getAudioTracks: () => [audio], + } as unknown as MediaStream; +} + +// Each call to getDisplayMedia opens a picker the test answers. +function install() { + const pickers: PromiseWithResolvers[] = []; + const originalNavigator = Object.getOwnPropertyDescriptor(globalThis, "navigator"); + const originalSelf = Object.getOwnPropertyDescriptor(globalThis, "self"); + Object.defineProperty(globalThis, "navigator", { + configurable: true, + value: { + mediaDevices: { + getDisplayMedia: () => { + const picker = Promise.withResolvers(); + pickers.push(picker); + return picker.promise; + }, + }, + }, + }); + Object.defineProperty(globalThis, "self", { configurable: true, value: {} }); + return { + pickers, + [Symbol.dispose]() { + if (originalNavigator) Object.defineProperty(globalThis, "navigator", originalNavigator); + else Reflect.deleteProperty(globalThis, "navigator"); + if (originalSelf) Object.defineProperty(globalThis, "self", originalSelf); + else Reflect.deleteProperty(globalThis, "self"); + }, + }; +} + for (const cancel of ["close", "disable"] as const) { test(`a screen picked after ${cancel} releases every track`, async () => { - const pending = Promise.withResolvers(); - const originalNavigator = Object.getOwnPropertyDescriptor(globalThis, "navigator"); - const originalSelf = Object.getOwnPropertyDescriptor(globalThis, "self"); - Object.defineProperty(globalThis, "navigator", { - configurable: true, - value: { mediaDevices: { getDisplayMedia: () => pending.promise } }, - }); - Object.defineProperty(globalThis, "self", { configurable: true, value: {} }); + using env = install(); const enabled = new Signal(true); const screen = new Screen({ enabled }); - const tracks = [{ stop: mock(() => {}) }, { stop: mock(() => {}) }]; + const tracks = [new FakeTrack(), new FakeTrack()]; try { await settle(); if (cancel === "close") screen.close(); else enabled.set(false); await settle(); - pending.resolve({ getTracks: () => tracks } as unknown as MediaStream); + env.pickers[0]?.resolve(stream(...tracks)); await settle(); for (const track of tracks) expect(track.stop).toHaveBeenCalledTimes(1); expect(screen.out.source.peek()).toBeUndefined(); + expect(screen.out.state.peek()).toEqual({ kind: "off" }); } finally { screen.close(); - if (originalNavigator) Object.defineProperty(globalThis, "navigator", originalNavigator); - else Reflect.deleteProperty(globalThis, "navigator"); - if (originalSelf) Object.defineProperty(globalThis, "self", originalSelf); - else Reflect.deleteProperty(globalThis, "self"); } }); } + +// A rejected picker is over, not still prompting, and nothing but a new enable prompts again. +for (const [name, reason] of [ + ["NotAllowedError", "dismissed"], + ["NotFoundError", "error"], +] as const) { + test(`a ${name} from the picker ends the share as ${reason} without prompting again`, async () => { + using env = install(); + const enabled = new Signal(true); + const screen = new Screen({ enabled }); + try { + await settle(); + expect(screen.out.state.peek()).toEqual({ kind: "prompting" }); + + const error = new DOMException("no", name); + env.pickers[0]?.reject(error); + await settle(); + expect(screen.out.state.peek()).toEqual( + reason === "error" ? { kind: "ended", reason, error } : { kind: "ended", reason }, + ); + expect(screen.out.source.peek()).toBeUndefined(); + + screen.video.set({ frameRate: 15 }); + screen.audio.set(false); + await settle(); + expect(env.pickers).toHaveLength(1); + + // Switching off keeps the reason; switching back on is the gesture that prompts again. + enabled.set(false); + await settle(); + expect(screen.out.state.peek().kind).toBe("ended"); + enabled.set(true); + await settle(); + expect(env.pickers).toHaveLength(2); + expect(screen.out.state.peek()).toEqual({ kind: "prompting" }); + } finally { + screen.close(); + } + }); +} + +test("a share the user stops releases both tracks and stays ended", async () => { + using env = install(); + const screen = new Screen(); + const video = new FakeTrack(); + const audio = new FakeTrack(); + try { + await settle(); + env.pickers[0]?.resolve(stream(video, audio)); + await settle(); + const state = screen.out.state.peek(); + expect(state.kind).toBe("live"); + expect(screen.out.state.peek()).toMatchObject({ kind: "live", media: { video: { track: video } } }); + + video.end(); + await settle(); + expect(screen.out.state.peek()).toEqual({ kind: "ended", reason: "stopped" }); + expect(screen.out.source.peek()).toBeUndefined(); + expect(audio.stop).toHaveBeenCalled(); + + screen.video.set({ frameRate: 15 }); + await settle(); + expect(env.pickers).toHaveLength(1); + } finally { + screen.close(); + } +}); + +test("a share picked dead is stopped, not live", async () => { + using env = install(); + const screen = new Screen(); + const video = new FakeTrack(); + video.readyState = "ended"; + try { + await settle(); + env.pickers[0]?.resolve(stream(video)); + await settle(); + expect(screen.out.state.peek()).toEqual({ kind: "ended", reason: "stopped" }); + expect(screen.out.source.peek()).toBeUndefined(); + } finally { + screen.close(); + } +}); + +// The constraints are read when the picker opens, so editing them never prompts on its own. +test("changing constraints while live does not prompt", async () => { + using env = install(); + const screen = new Screen({ video: { frameRate: 30 } }); + try { + await settle(); + env.pickers[0]?.resolve(stream()); + await settle(); + screen.video.set({ frameRate: 15 }); + await settle(); + expect(env.pickers).toHaveLength(1); + expect(screen.out.state.peek().kind).toBe("live"); + } finally { + screen.close(); + } +}); diff --git a/js/publish/src/source/screen.ts b/js/publish/src/source/screen.ts index 119749b7a47..606304f630d 100644 --- a/js/publish/src/source/screen.ts +++ b/js/publish/src/source/screen.ts @@ -1,53 +1,72 @@ -import { Effect, type Getter, getter, type Inputs, type Readonlys, readonlys, Signal } from "@moq/signals"; +import { Derived, Effect, type Getter, getter, type Inputs, type Readonlys, readonlys, Signal } from "@moq/signals"; import type * as Audio from "../audio"; import type * as Video from "../video"; import type { Media } from "./types"; /** Signals the screen capture reads. */ export type ScreenInput = { - // Whether to hold the capture open. Defaults to true. Enabling prompts the user to pick a surface, + // Whether to hold the capture open. Defaults to true. Each enable prompts the user to pick a surface, // so construct an enabled screen source during a user gesture or start it later with a live input. enabled: Getter; }; /** Constructor options: the wired inputs plus the live-editable track constraints. */ export interface ScreenProps extends Inputs { - /** Seed the video constraints; also live-editable via `screen.video`. */ + /** Seed the video constraints; also editable via `screen.video` for the next picker. */ video?: Video.Constraints | boolean | Signal; - /** Seed the audio constraints; also live-editable via `screen.audio`. */ + /** Seed the audio constraints; also editable via `screen.audio` for the next picker. */ audio?: Audio.Constraints | boolean | Signal; } +/** Why a screen share ended. */ +export type ScreenEnd = + // The user stopped sharing through the browser, or the surface went away. + | { reason: "stopped" } + // The user dismissed the picker or refused permission. + | { reason: "dismissed" } + // The browser could not capture. + | { reason: "error"; error: Error }; + +/** + * The phase of a screen share. `ended` holds until the next enable, so it says why sharing stopped + * even after the app switches it off in response. + */ +export type ScreenState = + | { kind: "off" } + | { kind: "prompting" } + | { kind: "live"; media: Media } + | ({ kind: "ended" } & ScreenEnd); + /** Signals the screen capture derives. */ export type ScreenOutput = { - // The captured surface, or undefined while disabled or dismissed. - source: Signal; + // The share's phase. + state: Getter; + // The captured surface while live, otherwise undefined. + source: Getter; }; +const OFF: ScreenState = { kind: "off" }; + /** Captures a screen, window, or tab that the user picks. */ export class Screen { /** The wired inputs, read-only. */ readonly in: Readonlys; - /** The live-editable video constraints, or false to capture audio only. */ + /** The video constraints the next picker uses. Browsers require video, so false makes the picker fail. */ video: Signal; - /** The live-editable audio constraints, or false to capture video only. */ + /** The audio constraints the next picker uses, or false to capture video only. */ audio: Signal; - readonly #out: ScreenOutput = { - source: new Signal(undefined), - }; + readonly #state = new Signal(OFF); + /** The derived outputs, read-only. */ - readonly out = readonlys(this.#out); + readonly out: Readonlys = readonlys({ + state: this.#state, + source: new Derived([this.#state], (state) => (state.kind === "live" ? state.media : undefined)), + }); #signals = new Effect(); - // Reruns #run once a track ends, so the run itself releases the share. - readonly #rerun = new Signal(0); - - // Deliberately a plain field: effect reruns must not unwind it, or the share reopens itself. - #stopped = false; - constructor(props?: ScreenProps) { this.in = { enabled: getter(props?.enabled ?? true), @@ -58,22 +77,14 @@ export class Screen { this.#signals.run(this.#run.bind(this)); } + // Reruns only when `enabled` changes, so each enable opens exactly one picker. The constraints + // are read rather than tracked: a picker needs a user gesture, and editing them is not one. #run(effect: Effect): void { - const enabled = effect.get(this.in.enabled); - if (!enabled) { - // Being switched off is the app's reset, so a later enable prompts again. - this.#stopped = false; - return; - } - - effect.get(this.#rerun); - - // The share is over and cannot be reopened without another user gesture. Returning here is - // what releases it: the previous run's cleanup stops every track and clears out.source. - if (this.#stopped) return; + if (!effect.get(this.in.enabled)) return; - const video = effect.get(this.video); - const audio = effect.get(this.audio); + this.#state.set({ kind: "prompting" }); + // Switching off or closing releases the share. An ended one keeps its reason. + effect.cleanup(() => this.#state.update((state) => (state.kind === "ended" ? state : OFF))); // TODO Expose these to the application. // @ts-expect-error TypeScript's DOM lib lacks the Chrome-only `CaptureController`. @@ -85,10 +96,12 @@ export class Screen { controller.setFocusBehavior("no-focus-change"); } - effect.spawn(async () => { - const pending = navigator.mediaDevices.getDisplayMedia({ - video, - audio, + // Called synchronously, so it still runs inside the gesture that enabled it; a synchronous throw + // becomes a rejection like any other refusal. + const pending = (async () => + navigator.mediaDevices.getDisplayMedia({ + video: this.video.peek(), + audio: this.audio.peek(), // @ts-expect-error TypeScript's DOM lib lacks the Chrome-only `CaptureController`. controller, preferCurrentTab: false, @@ -96,57 +109,71 @@ export class Screen { surfaceSwitching: "include", // TODO We should try to get system audio, but need to be careful about feedback. // systemAudio: "exclude", - }); - // The picker may outlive this run; its eventual stream still belongs to the run. - effect.cleanup(() => - pending.then( - (stream) => { - for (const track of stream.getTracks()) track.stop(); - }, - () => {}, - ), + }))(); + + // The picker may outlive this run; its eventual stream still belongs to the run. + const release = () => + pending.then( + (stream) => { + for (const track of stream.getTracks()) track.stop(); + }, + () => {}, ); - const media = await effect.race(pending.catch(() => undefined)); - if (!media) return; + effect.cleanup(release); + + // Closing can land between the picker settling and this task resuming; the cleanup's state wins. + const abort = effect.abort; + effect.spawn(async () => { + let media: MediaStream | undefined; + try { + media = await effect.race(pending); + } catch (error) { + if (abort.aborted) return; + // The spec reports a refused or dismissed picker as NotAllowedError, and nothing else does. + const end: ScreenEnd = + error instanceof DOMException && error.name === "NotAllowedError" + ? { reason: "dismissed" } + : { reason: "error", error: error instanceof Error ? error : new Error(String(error)) }; + this.#state.set({ kind: "ended", ...end }); + return; + } + if (!media || abort.aborted) return; const v = media.getVideoTracks().at(0) as Video.StreamTrack | undefined; const a = media.getAudioTracks().at(0) as Audio.StreamTrack | undefined; - // A track that arrives dead already fired "ended", so the listener below would never run - // and we would publish a corpse. - if (v?.readyState === "ended" || a?.readyState === "ended") return this.#stop(); + const stop = () => { + this.#state.set({ kind: "ended", reason: "stopped" }); + void release(); + }; + + // A track that arrives dead already fired "ended", so the listener below would never run. + if (v?.readyState === "ended" || a?.readyState === "ended") return stop(); - // The browser's own "Stop sharing" control ends a track. Either one ending means the - // share is over, and the tracks can end independently, so rerun into the stopped branch - // rather than clearing out.source here: that runs the cleanup above and releases the - // sibling track too. + // The browser's own "Stop sharing" control ends a track. Either one ending ends the share, + // releasing its sibling too. for (const track of [v, a]) { - if (!track) continue; - effect.event(track, "ended", () => this.#stop()); + if (track) effect.event(track, "ended", stop); } - effect.set(this.#out.source, { - video: v - ? { - track: v, - get scale() { - return (v.getSettings() as MediaTrackSettings & { screenPixelRatio?: number }) - .screenPixelRatio; - }, - } - : undefined, - audio: a ? { track: a, kind: "music" } : undefined, + this.#state.set({ + kind: "live", + media: { + video: v + ? { + track: v, + get scale() { + return (v.getSettings() as MediaTrackSettings & { screenPixelRatio?: number }) + .screenPixelRatio; + }, + } + : undefined, + audio: a ? { track: a, kind: "music" } : undefined, + }, }); }); } - // End the share. The rerun is what releases it: the run's own cleanup stops every track and - // clears out.source, so this never has to touch either. - #stop(): void { - this.#stopped = true; - this.#rerun.update((rerun) => rerun + 1); - } - /** Stop the capture and release the surface. */ close() { this.#signals.close(); diff --git a/js/publish/src/video/encoder.ts b/js/publish/src/video/encoder.ts index 746050103dd..741a3ad16e3 100644 --- a/js/publish/src/video/encoder.ts +++ b/js/publish/src/video/encoder.ts @@ -680,10 +680,11 @@ export class Encoder { // place meanwhile, so a re-probe swaps the catalog entry in one update, or not at all when nothing // changed. #runCodec(effect: Effect): void { - if (effect.get(this.#out.error)) return; - const enabled = effect.get(this.in.enabled); - const dimensions = effect.get(this.#coded); - const target = effect.get(this.#target); + // A failure drops the probe too, since it may be what failed, so a recovery waits on a new one. + const failed = effect.get(this.#out.error) !== undefined; + const enabled = !failed && effect.get(this.in.enabled); + const dimensions = enabled ? effect.get(this.#coded) : undefined; + const target = enabled ? effect.get(this.#target) : undefined; if (!enabled || !dimensions || !target) { this.#codec.set(undefined); return; diff --git a/js/room/src/local.test.ts b/js/room/src/local.test.ts index 77d609cbd8b..102e332d130 100644 --- a/js/room/src/local.test.ts +++ b/js/room/src/local.test.ts @@ -5,7 +5,7 @@ import { Signal } from "@moq/signals"; const sources: FakeSource[] = []; const encoders: Record[] = []; class FakeSource { - out = { source: new Signal(undefined) }; + out = { source: new Signal(undefined), state: new Signal({ kind: "off" }) }; constructor() { sources.push(this); } @@ -35,27 +35,51 @@ async function flush() { for (let i = 0; i < 30; i++) await Promise.resolve(); } -test("screen capture stays enabled while pending and resets after a live share ends", async () => { +function setup() { + // Each test counts only its own Local's sources and encoders. + sources.length = 0; + encoders.length = 0; const bandwidth = new Signal(undefined); const local = new Local({ - connection: { - origin: new Signal(undefined), - allocator: bandwidth, - } as never, + connection: { origin: new Signal(undefined), allocator: bandwidth } as never, identity: Path.from("alice"), }); + return { local, bandwidth, screen: sources.at(-1) as FakeSource }; +} + +test("screen capture stays enabled while prompting and resets after a live share ends", async () => { + const { local, bandwidth, screen } = setup(); try { await flush(); expect(encoders).toHaveLength(6); expect(encoders.every((props) => props.bandwidth === bandwidth)).toBe(true); local.screenEnabled.set(true); + screen.out.state.set({ kind: "prompting" }); await flush(); expect(local.screenEnabled.peek()).toBe(true); - const screen = sources.at(-1); - screen?.out.source.set({ video: {} }); + expect(local.preview.peek().screen).toBe(false); + + screen.out.state.set({ kind: "live", media: { video: {} } }); await flush(); expect(local.preview.peek().screen).toBe(true); - screen?.out.source.set(undefined); + + screen.out.state.set({ kind: "ended", reason: "stopped" }); + await flush(); + expect(local.screenEnabled.peek()).toBe(false); + expect(local.preview.peek().screen).toBe(false); + } finally { + local.close(); + } +}); + +// The share never went live, so nothing but its state says the picker is gone. +test("a dismissed screen picker turns sharing off", async () => { + const { local, screen } = setup(); + try { + local.screenEnabled.set(true); + screen.out.state.set({ kind: "prompting" }); + await flush(); + screen.out.state.set({ kind: "ended", reason: "dismissed" }); await flush(); expect(local.screenEnabled.peek()).toBe(false); expect(local.preview.peek().screen).toBe(false); diff --git a/js/room/src/local.ts b/js/room/src/local.ts index 9380b5a0066..8b773331396 100644 --- a/js/room/src/local.ts +++ b/js/room/src/local.ts @@ -22,7 +22,7 @@ export interface LocalProps { cameraEnabled?: GetterInit; /** Capture the microphone. Pass a Signal to share it with the app. */ microphoneEnabled?: GetterInit; - /** Prompt for and capture a screen. Pass a Signal to share it with the app. */ + /** Prompt for and capture a screen; turns off when the share ends. Pass a Signal to share it with the app. */ screenEnabled?: GetterInit; /** Seed the published user.json fields. */ user?: UserProps; @@ -46,7 +46,7 @@ export class Local { readonly cameraEnabled: Signal; /** Capture the microphone. */ readonly microphoneEnabled: Signal; - /** Prompt for and capture a screen. Unannounce when the share ends. */ + /** Prompt for and capture a screen. Turns off when the share ends, and `share.out.state` says why. */ readonly screenEnabled: Signal; /** True while the local participant is composing a chat message. */ @@ -271,15 +271,14 @@ export class Local { }; this.#signals.run((effect) => { - const source = effect.get(this.share.out.source); - effect.set(this.#screenVideo, source?.video); - effect.set(this.#screenAudioSource, source?.audio); - const live = !!source?.video || !!source?.audio; - const wasLive = this.#screenLive.peek(); - this.#screenLive.set(live); - if (!live && wasLive) { - this.screenEnabled.set(false); - } + const state = effect.get(this.share.out.state); + const media = state.kind === "live" ? state.media : undefined; + effect.set(this.#screenVideo, media?.video); + effect.set(this.#screenAudioSource, media?.audio); + this.#screenLive.set(media !== undefined); + // A share that ended for any reason, including a dismissed picker, turns sharing off, so the + // next share takes a new toggle. `share.out.state` keeps the reason. + if (state.kind === "ended") this.screenEnabled.set(false); }); this.#signals.run((effect) => { diff --git a/js/watch/src/element.ts b/js/watch/src/element.ts index 08caabb94e9..ee5e5b345f6 100644 --- a/js/watch/src/element.ts +++ b/js/watch/src/element.ts @@ -136,6 +136,31 @@ function parseBoolean(value: string | null, defaultValue: boolean): boolean { return normalized !== "false" && normalized !== "0"; } +/** + * The element's `connection.enabled`, which the page may also write. The page's writes set what it + * wants and the element sets whether it can run (connected to a document, every attribute valid); it + * reads as on only while both hold. Kept apart, a page's disable survives the element suspending and + * resuming, whenever the page writes it: an explicit page choice wins. + */ +class Enabled extends Signal { + #wanted = true; + #running = false; + + constructor() { + super(false); + } + + override set(value: boolean, notify?: boolean): void { + this.#wanted = value; + super.set(value && this.#running, notify); + } + + run(running: boolean): void { + this.#running = running; + super.set(this.#wanted && running); + } +} + /** The `` element: plays a broadcast into a nested . */ export default class MoqWatch extends HTMLElement { /** The attributes the browser reports changes to. */ @@ -223,13 +248,12 @@ export default class MoqWatch extends HTMLElement { #captionsOverlay = new Signal(undefined); #captionsOverlayEl?: HTMLDivElement; - // Set when the element is connected to the DOM. - #enabled = new Signal(false); + // Whether the element is connected to a document. #connected = false; + // The attributes holding a refused value, which suspend the element until corrected. #invalid = new Signal>(new Set()); - // Whether clearing the last invalid attribute re-enables media: false when the page had - // already disabled the connection, so recovery does not override that. - #resume = false; + // Whether to connect and play: `#update` gates it on the element's state, and the page may turn it off. + #enabled = new Enabled(); #reflecting = false; /** @@ -381,8 +405,7 @@ export default class MoqWatch extends HTMLElement { /** Called by the browser when the element joins a document. */ connectedCallback() { this.#connected = true; - if (this.#invalid.peek().size) this.#resume = true; - else this.#enabled.set(true); + this.#update(); this.style.display = "block"; this.style.position = "relative"; @@ -404,7 +427,7 @@ export default class MoqWatch extends HTMLElement { disconnectedCallback() { // Stop everything but don't actually cleanup just in case we get added back to the DOM. this.#connected = false; - this.#enabled.set(false); + this.#update(); } /** Called by the browser when an observed attribute changes. */ @@ -436,24 +459,24 @@ export default class MoqWatch extends HTMLElement { throw new Error(`Invalid attribute: ${exhaustive}`); } } catch (error) { - if (this.#invalid.peek().size === 0) this.#resume = this.#enabled.peek(); this.#invalid.update((previous) => new Set(previous).add(name)); - this.#enabled.set(false); + this.#update(); throw error; } } #accept(name: string) { - // Ordinary edits must preserve an explicitly disabled connection. if (!this.#invalid.peek().has(name)) return; this.#invalid.update((previous) => { const next = new Set(previous); next.delete(name); return next; }); - if (this.#invalid.peek().size || !this.#resume) return; - this.#resume = false; - this.#enabled.set(this.#connected); + this.#update(); + } + + #update() { + this.#enabled.run(this.#connected && this.#invalid.peek().size === 0); } #reflect(effect: Effect, name: (typeof OBSERVED)[number], value: string | undefined) { diff --git a/quest/m1/explicit-state/README.md b/quest/m1/explicit-state/README.md index f672dab642d..29889688da4 100644 --- a/quest/m1/explicit-state/README.md +++ b/quest/m1/explicit-state/README.md @@ -69,7 +69,6 @@ which collapses that state and waits on [Drop set](/quest/m1/drop-set.md). ## Required -- [Publish capture phases](/quest/m1/explicit-state/publish-capture.md) - dismissing the screen-share picker turns sharing off - [js/qmux close](/quest/m1/explicit-state/qmux-close.md) - a graceful remote close ends qmux streams cleanly - [FFI handle state](/quest/m1/explicit-state/ffi-state.md) - FFI handles hold one state each: video follow under one lock, requests answered once, drivers Cancelled after shutdown - [moq-net phase data](/quest/m1/explicit-state/net-phases.md) - moq-net's session and subscription state machines keep each phase's data in its variant diff --git a/quest/m1/explicit-state/publish-capture.md b/quest/m1/explicit-state/publish-capture.md deleted file mode 100644 index ebabc5d5ca5..00000000000 --- a/quest/m1/explicit-state/publish-capture.md +++ /dev/null @@ -1,57 +0,0 @@ -# [M] Publish capture phases - -## Goal - -In `@moq/publish` and `@moq/room`, each capture source and encoder slot is in -one explicit phase, and what is advertised follows from that phase. A screen -share is off, prompting, live, or ended, and the room's `screenEnabled` -follows it: dismissing the browser's picker, a capture error, or the user -stopping the share turns sharing off and says why. Nothing re-prompts without -a new user gesture. An encoder whose capture didn't reopen isn't advertised -as enabled. - -## Plan - -The shared decisions are in the [epic](/quest/m1/explicit-state/README.md). - -Bugs to reproduce and fix: - -- **Screen-share dismiss**: - - `js/publish/src/source/screen.ts` swallows a rejected `getDisplayMedia` - (`pending.catch(() => undefined)`). A dismissed picker then looks the same - as one still prompting: enabled, no source, `#stopped` false, and no - error. - - `js/room/src/local.ts` resets `screenEnabled` only on `!live && wasLive`, - and `wasLive` was never true, so the toggle stays on with nothing shared. - - Because `#stopped` stays false, a later change to `share.video` or - `share.audio` re-runs the effect and prompts again outside a user - gesture. - - Recommended: `{ kind: "off" } | { kind: "prompting" } | { kind: "live", - media } | { kind: "ended", reason: "stopped" | "dismissed" | "error" }`. -- **Audio `#muted` latch** (`js/publish/src/audio/encoder.ts`, reproduce - first): - - With `capture="release"`, mute, unmute, and then a failed reopen leave - `#muted` set, so the last config stays listed as enabled. - - The video encoder's `#paused` gives the correct `enabled: false` in the - same situation. - -Also sweep: - -- `js/publish/src/source/retry.ts`: `#failures = LIMIT + 1` is used as a - terminal sentinel. -- The publication entry lifecycle (`js/publish/src/publication.ts` and - `broadcast.ts`), which is resolved by an ordered if-chain. -- The video encoder's `#runCodec`, which keeps `#codec` after an error. -- The preview `Transcode.#active`. -- The `` and `` enable latches (`#enabled`, - `#connected`, `#invalid`, `#resume`). Derive enabled as connected with no - invalid attributes. ``'s `announce` getter returns - `AnnounceMode`, and an invalid `announce` attribute suspends announcing - through `#invalid` like every other attribute, instead of the getter - returning `undefined` (added 2026-11-07, maintainer, /quest-plan from the - API shape audit). An invalid attribute stays detectable the way every - other one already is: its parse error is thrown from - `attributeChangedCallback` and `enabled` turns false. The getter's type - narrows, so TypeScript readers still compile; say so in the PR. -- `js/moq-boy/src/game.ts`: a command sent while disconnected replays on - reconnect. diff --git a/quest/m1/publish-stream-end.md b/quest/m1/publish-stream-end.md index ad1f90b3860..a3712939908 100644 --- a/quest/m1/publish-stream-end.md +++ b/quest/m1/publish-stream-end.md @@ -41,4 +41,4 @@ Public API: none. Wire: none. ## Related -- [Publish capture phases](/quest/m1/explicit-state/publish-capture.md) - [moq-dev/moq#8262](https://github.com/moq-dev/moq/pull/8262) reworks capture phases nearby, but touches none of these lines +- [Publish capture phases](https://github.com/moq-dev/moq/pull/8262) reworks capture phases nearby, but touches none of these lines diff --git a/test/interop/clients/js/publish-ui.browser.ts b/test/interop/clients/js/publish-ui.browser.ts index 2a69c46ef11..8187d26e1df 100644 --- a/test/interop/clients/js/publish-ui.browser.ts +++ b/test/interop/clients/js/publish-ui.browser.ts @@ -1138,24 +1138,33 @@ test("invalid attributes and properties refuse, retain bad markup, and recover", return { stillBlocked, recovered }; }); expect(blocked).toEqual({ stillBlocked: true, recovered: true }); - const disabled = await page.evaluate(() => - (["moq-watch", "moq-publish"] as const).map((tag) => { + const disabled = await page.evaluate(async () => { + // Effects rerun in a later task, so each step waits one out. + const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + const results = []; + for (const tag of ["moq-watch", "moq-publish"] as const) { const element = document.createElement(tag); document.body.append(element); + await settle(); // Recovery must not override a connection the page disabled first. element.connection.enabled.set(false); element.setAttribute("backend", "bad"); + await settle(); element.removeAttribute("backend"); + await settle(); const kept = element.connection.enabled.peek(); element.connection.enabled.set(true); element.setAttribute("backend", "bad"); + await settle(); element.removeAttribute("backend"); + await settle(); const resumed = element.connection.enabled.peek(); element.remove(); element.signals.close(); - return { tag, kept, resumed }; - }), - ); + results.push({ tag, kept, resumed }); + } + return results; + }); expect(disabled).toEqual([ { tag: "moq-watch", kept: false, resumed: true }, { tag: "moq-publish", kept: false, resumed: true },