Properly cancel the video encoder.

[kixelated]

Summary by CodeRabbit

• Bug Fixes
  • Improved responsiveness when canceling video publishing/encoding, reducing delays or potential hangs when stopping a stream.
• Refactor
  • Removed inactive screen capture implementation, simplifying the broadcasting code without changing current functionality.

[coderabbitai]

Walkthrough

• In js/hang/src/publish/broadcast.ts, the private helper method for screen capture (#runScreen) and its associated logic (including getDisplayMedia handling, CaptureController usage, per-track binding to this.video.media and this.audio.media) were removed. The prior call to runScreen is now commented out. The catalog execution logic (#runCatalog) remains.
• In js/hang/src/publish/video/encoder.ts, the frame-reading loop now races reader.read() against a cancellation promise, allowing earlier loop exit when canceled. No other encoding flow changes were made.
• No exported or public API signatures were modified.

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "Properly cancel the video encoder." concisely and accurately describes the primary change in this PR (adding cancellation support to the video encoder's frame-reading loop in encoder.ts). It is specific, single-sentence, and not misleading even though the PR also removes a private screen-capture helper as a secondary change.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cancel-spawn

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

js/hang/src/publish/video/encoder.ts (3)

119-139: Avoid dropping trailing chunks on graceful stop: break + flush.

A return inside the loop skips flushing pending output. Prefer breaking the loop and flushing, racing with cancel to stay responsive.

Apply:

       encoder.encode(frame, { keyFrame });

-      next = await Promise.race([reader.read(), cancel]);
-      if (!next || !next.value) return;
+      next = await Promise.race([reader.read(), cancel]);
+      if (!next || !next.value) break;
       frame = next.value;
     }
+    // Drain any queued output unless we're canceled.
+    await Promise.race([encoder.flush(), cancel]).catch(() => {});
+    effect.set(this.#active, false, false);

---

131-133: Guard against encoder queue backpressure.

encode() can throw when encodeQueueSize grows. Throttle slightly to avoid spikes in software paths.

For example:

-    encoder.encode(frame, { keyFrame });
+    // Backpressure: keep queue small for realtime.
+    while (encoder.encodeQueueSize > 2) {
+      await Promise.race([new Promise(r => setTimeout(r, 1)), cancel]);
+    }
+    encoder.encode(frame, { keyFrame });

---

286-291: Nit: spelling in comments.

“nessisarily” → “necessarily” (twice).

-      // Annex-B allows changing the resolution without nessisarily updating the catalog (description).
+      // Annex-B allows changing the resolution without necessarily updating the catalog (description).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b05b9af and f59933c.

📒 Files selected for processing (2)

• js/hang/src/publish/broadcast.ts (0 hunks)
• js/hang/src/publish/video/encoder.ts (1 hunks)

💤 Files with no reviewable changes (1)

• js/hang/src/publish/broadcast.ts

🧰 Additional context used

📓 Path-based instructions (1)

{rs,js}/hang/**

📄 CodeRabbit inference engine (CLAUDE.md)

{rs,js}/hang/**: All media-specific logic (encoding/streaming, codec handling) must live in the hang layer.
Keep the hang layer generic: expose primitives (e.g., access to individual frames) and avoid application/UI-specific code.

Files:

• js/hang/src/publish/video/encoder.ts

🧠 Learnings (1)

📓 Common learnings

Learnt from: CR
PR: kixelated/moq#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-11T20:16:03.101Z
Learning: Applies to {rs,js}/hang/** : All media-specific logic (encoding/streaming, codec handling) must live in the hang layer.

🧬 Code graph analysis (1)

js/hang/src/publish/video/encoder.ts (1)

js/hang/src/frame.ts (1)

• cancel (87-108)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Check

🔇 Additional comments (1)

js/hang/src/publish/video/encoder.ts (1)

133-137: Nice: cancellation now exits the read loop promptly.

Racing reader.read() with cancel inside the loop prevents blocking on reads and brings this code in line with the pattern used in frame.ts. LGTM.