oficina / meet Public

room.js: a tile follows the live track, not just the stream

The third thing `laterolls` found, and the one that was never a
counter: after a forced late collision, the polite side's tile for the
other camera sat at currentTime 0.1 for the rest of the scenario —
readyState 4, not paused, the connection stable and connected, and the
same connection decoding four hundred inbound frames nobody could see.
The counter was telling the truth. The element was rendering a track
that had ended.

Chromium rebuilds every stream on a connection when it rolls back an
offer that added an m-line, receive streams included, so the far side's
camera arrives again as a *new* track — sometimes carrying no stream of
its own, sometimes carried inside the very stream that still holds the
old, ended one. `onRemoteTrack` added the new track to that stream and
left the ended one in it, and a media element renders the first track
of its kind: a face frozen for the rest of the call on a connection
that is otherwise perfect.

Two halves. `adoptTrack` takes an ended track of the same kind out of
the stream a tile shows before putting the new one in, on both the
camera and the screen path. And `tile()` follows the stream's video
track as well as the stream object, nulling `srcObject` first when only
the track changed, because assigning the same MediaStream again is a
no-op and the element would keep what it had.

Four tests in room-share.test.mjs, mutation-verified: leaving the ended
track in place fails the rebuilt-camera test and only it. The audio
track beside it is not collateral, and two live video tracks are left
alone — telling those apart is classifyTrack's job, long before this.

node --check, 194 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 1 month ago fee4eb45b34180e025d833511facc6d1695e860b parent 2eadfd2

2 files changed, +122 −6

internal/meet/static/room.js +51 −6
test/browser/room-share.test.mjs +71 −0

diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js

index ccaea53..5b5b48d 100644

--- a/internal/meet/static/room.js

+++ b/internal/meet/static/room.js

@@ -3056,8 +3056,9 @@

if (what === 'share') {

entry.sharing = true;

entry.shareMid = mid || entry.shareMid;

- entry.share = { stream: stream || new MediaStream([track]), track };

- if (!stream) entry.share.stream.addTrack(track);

+ const held = (entry.share && entry.share.stream) || null;

+ entry.share = { stream: stream || held || new MediaStream(), track };

+ adoptTrack(entry.share.stream, track);

// §5.4's first ending: the sender stopped the transceiver, so

// this track ends rather than freezing. The listener is dropped

// with the connection, and dropShare is idempotent, so the `share

@@ -3078,11 +3079,44 @@

// Recorded from the first track of any kind, camera or microphone:

// it is what places a video track for a peer that has no camera.

if (stream && !entry.peerStream) entry.peerStream = stream.id;

- entry.stream = stream || entry.stream || new MediaStream([track]);

- if (!stream) entry.stream.addTrack(track);

+ entry.stream = stream || entry.stream || new MediaStream();

+ adoptTrack(entry.stream, track);

renderTiles();

}

+ // adoptTrack puts a newly arrived track into the stream a tile shows,

+ // and takes out any ended track of the same kind that was there

+ // before. Pure enough to lift by name, and gated in the node test.

+ //

+ // This is the receiving half of the rollback damage (Codex S4-5's

+ // other side, found by the gate's `laterolls`). Chromium rebuilds

+ // every stream on a connection when it rolls back an offer that added

+ // an m-line — receive streams included — so the far side's camera

+ // arrives again as a *new* track, sometimes carrying no stream of its

+ // own and sometimes carried inside the very stream that still holds

+ // the old, ended one. A media element renders the first track of its

+ // kind, so an ended track left in place is a face that never moves

+ // again on a connection that is otherwise perfect: connected, stable,

+ // sendrecv, and decoding hundreds of frames nobody can see.

+ function adoptTrack(stream, track) {

+ if (!stream || !track) return stream;

+ for (const old of stream.getTracks()) {

+ if (old !== track && old.kind === track.kind && old.readyState === 'ended') {

+ stream.removeTrack(old);

+ }

+ if (!stream.getTracks().includes(track)) stream.addTrack(track);

+ return stream;

+ }

+ // shownTrack is the video track a tile is actually rendering: the

+ // first one in the stream. The tile follows it, not just the stream

+ // object, because the two can change independently.

+ function shownTrack(stream) {

+ if (!stream || !stream.getVideoTracks) return null;

+ return stream.getVideoTracks()[0] || null;

+ }

// dropShare removes one peer's screen, from either ending. The frame

// count is deliberately left behind: it says the screen was really

// arriving before it went away, which is exactly what a gate watching

@@ -3477,7 +3511,10 @@

// `self` is kept on the record because rearmFrames reads it: the

// local preview is never counted, and only tile() knows which

// tile is ours.

- t = { figure, video, label, nameEl, note, retry, play, remove: null, stream: null, self: isSelf };

+ t = {

+ figure, video, label, nameEl, note, retry, play,

+ remove: null, stream: null, track: null, self: isSelf,

+ };

tiles.set(key, t);

if (!isSelf) applySink(video);

}

@@ -3492,11 +3529,19 @@

t.remove.remove();

t.remove = null;

}

- if (t.stream !== stream) {

+ // The track as well as the stream: a camera the far side rebuilt

+ // can arrive inside the same MediaStream object, and a tile that

+ // watched only the object would keep an element pointed at a track

+ // that has ended. Nulling srcObject first is what makes the element

+ // let go of it — assigning the same object again is a no-op.

+ const shown = shownTrack(stream);

+ if (t.stream !== stream || t.track !== shown) {

// srcObject only: media-src in the page's CSP would block a blob:

// URL, and createObjectURL is the wrong tool here anyway.

+ if (t.stream === stream) t.video.srcObject = null;

t.video.srcObject = stream || null;

t.stream = stream || null;

+ t.track = shown;

if (stream && !isSelf) startPlaying(key, t);

}

// The frame counter follows the stream, not the element: a tile

diff --git a/test/browser/room-share.test.mjs b/test/browser/room-share.test.mjs

index 4c45e81..d737678 100644

--- a/test/browser/room-share.test.mjs

+++ b/test/browser/room-share.test.mjs

@@ -377,6 +377,77 @@ test('signed out is read before the status, because it can be true of a 200', ()

assert.equal(accepted('signedout'), false);

});

+// ---------------------------------------------------------------

+// adoptTrack: the receiving half of the rollback damage

+// ---------------------------------------------------------------

+//

+// Chromium rebuilds every stream on a connection when it rolls back an

+// offer that added an m-line, receive streams included, so the far

+// side's camera arrives again as a *new* track — sometimes with no

+// stream of its own, sometimes inside the very stream that still holds

+// the old, ended one. A media element renders the first track of its

+// kind, so an ended track left in place is a face that never moves

+// again on a connection that is otherwise perfect. The gate's

+// `laterolls` is where that was measured.

+const adoptTrack = build('adoptTrack');

+const shownTrack = build('shownTrack');

+function mediaStream(tracks) {

+ const held = tracks.slice();

+ return {

+ getTracks: () => held.slice(),

+ getVideoTracks: () => held.filter((t) => t.kind === 'video'),

+ addTrack: (t) => held.push(t),

+ removeTrack: (t) => {

+ const at = held.indexOf(t);

+ if (at !== -1) held.splice(at, 1);

+ },

+ };

+const videoTrack = (id, readyState) => ({ id, kind: 'video', readyState: readyState || 'live' });

+test('a rebuilt camera replaces the ended one it arrives beside', () => {

+ const dead = videoTrack('v1', 'ended');

+ const live = videoTrack('v2');

+ const stream = mediaStream([dead, { id: 'a1', kind: 'audio', readyState: 'live' }]);

+ adoptTrack(stream, live);

+ assert.deepEqual(stream.getVideoTracks(), [live],

+ 'the element would have gone on rendering the ended track');

+ assert.equal(stream.getTracks().length, 2, 'the audio track was not collateral');

+ assert.equal(shownTrack(stream), live);

+});

+test('a live track already there is left alone', () => {

+ // Two live video tracks on one stream is not this function's business

+ // to resolve: a camera and something else are told apart by

+ // classifyTrack long before this.

+ const first = videoTrack('v1');

+ const second = videoTrack('v2');

+ const stream = mediaStream([first]);

+ adoptTrack(stream, second);

+ assert.deepEqual(stream.getVideoTracks(), [first, second]);

+ assert.equal(shownTrack(stream), first);

+});

+test('adopting a track twice adds it once', () => {

+ const track = videoTrack('v1');

+ const stream = mediaStream([]);

+ adoptTrack(stream, track);

+ assert.deepEqual(stream.getTracks(), [track]);

+});

+test('adoptTrack survives what a browser can hand it', () => {

+ assert.equal(adoptTrack(null, videoTrack('v1')), null);

+ const stream = mediaStream([]);

+ assert.equal(adoptTrack(stream, null), stream);

+ assert.equal(shownTrack(null), null);

+ assert.equal(shownTrack({}), null);

+ assert.equal(shownTrack(mediaStream([])), null);

+});

// ---------------------------------------------------------------

// watchFrames: the counter the whole browser gate stands on

// ---------------------------------------------------------------