amadan

oficina / meet Public

room.js: the repair keeps the role's stream, and a dead screen keeps its hands off the live one

Two lifecycle defects around the same replacement, both from Codex
round 3.

S4-13 (P2). The render watch's repair handed the element a fresh
MediaStream and left the role writing to the old one, so the next
microphone to arrive or be replaced went into a stream nothing was
showing: authoritative [video, new mic], element [video, old mic], and
nobody to hear. The fresh object is the role's own now — `freshStream`
puts it back where the role keeps it — which restores the one rule this
file has about tiles: the element holds the role's stream, and every
track the role adopts afterwards reaches it. A screen's repair replaces
the screen's stream and leaves the camera's alone, which is what
passing the role in is for.

S4-15 (P2). A screen can be replaced on its own mid — the far side
rebuilding it, or a rollback doing it for them — and the predecessor
ends afterwards, as a superseded track does. Its `ended` listener was
still on the connection and checked only that the connection was the
same one, so the dead screen's ending took the live replacement's tile
away with it. It drops the share only if the ending track is still the
one being shown; the screen that *is* showing still ends its tile when
it ends.

Six tests, each mutation-verified: restoring the element-only fresh
stream fails the three replay tests and nothing else, and dropping the
identity check on the ending fails the replacement test and nothing
else.

node --check, 217 node tests, make ci.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CzvnuKhLzudvq7jZ7SvVt2
Paul Campbell pushed by paul@keymail.dev 5a0be1eadcdec4273dce62452bf6d4b22440d7e7 parent ad13608
2 files changed, +157 −11
  • internal/meet/static/room.js +43 −11
  • test/browser/room-share.test.mjs +114 −0
diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js
index ea00181..234d5f8 100644
--- a/internal/meet/static/room.js
+++ b/internal/meet/static/room.js
@@ -3181,8 +3181,16 @@
// this track ends rather than freezing. The listener is dropped
// with the connection, and dropShare is idempotent, so the `share
// {on:false}` that usually arrives alongside it costs nothing.
+ //
+ // Only for the screen that is still showing, though (Codex
+ // S4-15). A screen can be *replaced* on its own mid — the far
+ // side rebuilding it, or a rollback doing it for them — and the
+ // predecessor ends afterwards, as a superseded track does. Its
+ // listener is still on the connection, and without this it took
+ // the live replacement's tile away with it.
track.addEventListener('ended', () => {
if (pcs.get(id) !== entry) return;
+ if (!entry.share || entry.share.track !== track) return;
dropShare(entry);
});
renderTiles();
@@ -4244,24 +4252,48 @@
return !isSelf && !!t.stream && (!t.play || t.play.hidden);
}
- // replayTile hands an element its media again: the stream taken away
- // and given back, and play() asked for once more. Cheaper than a
+ // freshStream gives a role a *new* MediaStream over the tracks it
+ // holds, and puts it back where the role keeps it — which is the half
+ // the first draft of the replay missed (Codex S4-13). A fresh object
+ // handed to the element alone left the role still writing to the old
+ // one, so the next microphone to arrive or be replaced went into a
+ // stream nothing was showing: authoritative [video, new mic], element
+ // [video, old mic], and nobody to hear.
+ //
+ // Making the fresh object the role's own keeps the one rule this file
+ // has about tiles: the element holds the role's stream, and every
+ // track the role adopts afterwards reaches it.
+ function freshStream(entry, share) {
+ const held = share ? entry.share && entry.share.stream : entry.stream;
+ if (!held || !held.getTracks) return null;
+ const next = new MediaStream(held.getTracks());
+ if (share) entry.share.stream = next;
+ else entry.stream = next;
+ return next;
+ }
+
+ // replayTile hands an element its media again: a fresh stream over
+ // the same tracks, and play() asked for once more. Cheaper than a
// renegotiation and enough for a decoder pipeline that has stopped
// painting what it is still producing.
- function replayTile(key, t) {
+ //
+ // Fresh rather than the object the element was already holding:
+ // assigning the same one back is where an earlier draft stopped, and
+ // it bought a frame or two before the picture stalled again. A
+ // different object is a different source as far as the element is
+ // concerned, and it rebuilds the pipeline rather than nudging it.
+ function replayTile(key, t, entry, share) {
+ const next = freshStream(entry, share);
+ if (!next) return;
try {
t.video.srcObject = null;
- // A *new* MediaStream over the same tracks, not the one the
- // element was already holding: assigning the same object back is
- // where the first draft of this stopped, and it bought a frame or
- // two before the picture stalled again. A different object is a
- // different source as far as the element is concerned, and it
- // rebuilds the whole pipeline rather than nudging it.
- t.video.srcObject = new MediaStream(t.stream.getTracks());
+ t.video.srcObject = next;
} catch {
// An element that has gone: the next render builds another.
return;
}
+ t.stream = next;
+ t.track = shownTrack(next);
if (shouldPlay(t, false)) startPlaying(key, t);
}
@@ -4303,7 +4335,7 @@
const t = tiles.get(key);
if (!t || t.self) continue;
const verdict = judgeRender(t, count);
- if (verdict === 'replay') replayTile(key, t);
+ if (verdict === 'replay') replayTile(key, t, entry, key !== id);
else if (verdict === 'reset') requestReset(id);
}
const tx = localCameraTx(entry);
diff --git a/test/browser/room-share.test.mjs b/test/browser/room-share.test.mjs
index 67c5055..9b257db 100644
--- a/test/browser/room-share.test.mjs
+++ b/test/browser/room-share.test.mjs
@@ -554,6 +554,120 @@ test("a peer's screen is bound to its own track, on its own stream", () => {
assert.deepEqual(r.entry.stream.getVideoTracks(), [camera]);
});
+test('a screen that was replaced cannot take its replacement away', () => {
+ // Codex S4-15. A screen can be replaced on its own mid — the far side
+ // rebuilding it, or a rollback doing it for them — and the
+ // predecessor ends afterwards, as a superseded track does. Its
+ // `ended` listener is still on the connection, and it checked only
+ // that the connection was the same one: so the predecessor's ending
+ // dropped the live replacement's tile.
+ const r = receiving();
+ r.onRemoteTrack('p1', r.entry, arriving(videoTrack('cam1'), { mid: '1' }));
+ const first = videoTrack('scr1');
+ r.onRemoteTrack('p1', r.entry, arriving(first, { mid: '2' }));
+ const second = videoTrack('scr2');
+ r.onRemoteTrack('p1', r.entry, arriving(second, { mid: '2' }));
+ assert.equal(r.entry.share.track, second);
+
+ first.end();
+ assert.equal(r.entry.sharing, true, 'the predecessor took the live screen with it');
+ assert.equal(r.entry.share && r.entry.share.track, second);
+
+ // And the screen that *is* showing still ends the tile when it ends.
+ second.end();
+ assert.equal(r.entry.sharing, false);
+ assert.equal(r.entry.share, null);
+});
+
+// ---------------------------------------------------------------
+// freshStream and replayTile (Codex S4-13)
+// ---------------------------------------------------------------
+//
+// The render watch's repair hands the element a new MediaStream over
+// the same tracks. The first draft handed it to the element alone and
+// left the role writing to the old object, so the next microphone to
+// arrive or be replaced went somewhere nothing was showing.
+
+const replayBox = new Function(
+ 'MediaStream', 'shownTrack', 'shouldPlay', 'startPlaying',
+ [lift(src, 'freshStream'), lift(src, 'replayTile')].join('\n') +
+ '\nreturn { freshStream, replayTile };',
+);
+
+function replaying() {
+ const played = [];
+ const box = replayBox(
+ function FakeMediaStream(tracks) { return mediaStream(tracks || []); },
+ (stream) => (stream && stream.getVideoTracks ? stream.getVideoTracks()[0] || null : null),
+ () => true,
+ (key) => played.push(key),
+ );
+ return { played, freshStream: box.freshStream, replayTile: box.replayTile };
+}
+
+// A tile record as replayTile sees one, over an element that remembers
+// what it was last handed.
+function replayTileRecord(stream) {
+ return { video: { srcObject: stream }, stream, track: null, play: null };
+}
+
+test('a replayed camera keeps hearing the microphone that arrives after it', () => {
+ // The reproduction: replay, then a microphone arrives. The
+ // authoritative stream has [video, mic]; the element must too.
+ const r = receiving();
+ const camera = videoTrack('cam1');
+ r.onRemoteTrack('p1', r.entry, arriving(camera, { mid: '1' }));
+ const t = replayTileRecord(r.entry.stream);
+
+ const box = replaying();
+ box.replayTile('p1', t, r.entry, false);
+ assert.notEqual(t.video.srcObject, null);
+ assert.equal(t.video.srcObject, r.entry.stream, 'the role kept writing to the old stream');
+ assert.equal(t.stream, r.entry.stream);
+ assert.equal(t.track, camera);
+
+ const mic = { id: 'mic1', kind: 'audio', readyState: 'live' };
+ r.onRemoteTrack('p1', r.entry, arriving(mic, { mid: '0' }));
+ assert.deepEqual(t.video.srcObject.getTracks().map((x) => x.id), ['cam1', 'mic1'],
+ 'the element cannot hear a microphone that arrived after the repair');
+});
+
+test('a replayed camera follows a microphone that is replaced after it', () => {
+ const r = receiving();
+ r.onRemoteTrack('p1', r.entry, arriving({ id: 'mic1', kind: 'audio', readyState: 'live' }, { mid: '0' }));
+ r.onRemoteTrack('p1', r.entry, arriving(videoTrack('cam1'), { mid: '1' }));
+ const t = replayTileRecord(r.entry.stream);
+
+ replaying().replayTile('p1', t, r.entry, false);
+ r.onRemoteTrack('p1', r.entry, arriving({ id: 'mic2', kind: 'audio', readyState: 'live' }, { mid: '0' }));
+
+ assert.deepEqual(t.video.srcObject.getTracks().map((x) => x.id), ['cam1', 'mic2'],
+ 'the element is still holding the microphone that was replaced');
+ assert.deepEqual(r.entry.stream.getTracks().map((x) => x.id), ['cam1', 'mic2']);
+});
+
+test('a replayed screen replaces the screen and leaves the camera alone', () => {
+ const r = receiving();
+ r.onRemoteTrack('p1', r.entry, arriving(videoTrack('cam1'), { mid: '1' }));
+ r.onRemoteTrack('p1', r.entry, arriving(videoTrack('scr1'), { mid: '2' }));
+ const camera = r.entry.stream;
+ const t = replayTileRecord(r.entry.share.stream);
+
+ replaying().replayTile('p1#share', t, r.entry, true);
+ assert.equal(t.video.srcObject, r.entry.share.stream);
+ assert.notEqual(r.entry.share.stream, camera);
+ assert.equal(r.entry.stream, camera, 'the camera tile was handed the screen repair');
+});
+
+test('a replay with nothing to hand over does nothing at all', () => {
+ const box = replaying();
+ const t = replayTileRecord(null);
+ assert.doesNotThrow(() => box.replayTile('p1', t, { stream: null }, false));
+ assert.equal(t.video.srcObject, null);
+ assert.equal(box.freshStream({ stream: null }, false), null);
+ assert.equal(box.freshStream({ share: null }, true), null);
+});
+
test('adoptTrack survives what a browser can hand it', () => {
assert.equal(adoptTrack(null, videoTrack('v1')), null);
const stream = mediaStream([]);