amadan

oficina / meet Public

room.js: a frame counter that repairs itself

The previous commit keyed the counter on the tile and its stream, and
the gate came back with the same failure: a camera tile playing
perfectly — readyState 4, not paused, its clock advancing, 453 frames
decoded — and a count frozen at 24.

Keying on the stream was not enough. Chromium hands a rebuilt camera
back inside the *same* MediaStream object, on a new receiver track, and
the element still resets around the swap: its clock goes back to zero
and the pending requestVideoFrameCallback is discarded. Same stream,
nothing to re-arm for.

So the identity is the tile, the stream and the track inside it — and
behind that a backstop for whatever else can drop a callback: a chain
that has counted nothing for two seconds *while the element's own clock
moved on* is treated as dropped and armed again. A frozen tile has an
idle chain and is left alone, which is the distinction the clock makes
and a timer alone could not. `pollFrames` asks the question of every
tile once a second, so a lost callback costs a second of counting
rather than the rest of the call.

Eight tests now, two mutation-verified against the shipped code:
dropping the track from the identity fails the rebuilt-track test and
only it, and dropping the staleness check fails the quiet-chain test
and only it.

node --check, 190 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 2eadfd2089ec7d0b7fe0621073d04519b2dfa6ad parent 3ecebb6
2 files changed, +154 −19
  • internal/meet/static/room.js +50 −4
  • test/browser/room-share.test.mjs +104 −15
diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js
index 9651507..ccaea53 100644
--- a/internal/meet/static/room.js
+++ b/internal/meet/static/room.js
@@ -3474,7 +3474,10 @@
label.append(nameEl, note, retry, play);
figure.append(video, label);
tilesEl.append(figure);
- t = { figure, video, label, nameEl, note, retry, play, remove: null, stream: null };
+ // `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 };
tiles.set(key, t);
if (!isSelf) applySink(video);
}
@@ -3762,27 +3765,66 @@
// same tile up. A chain that finds its element detached forgets
// itself, so the next render arms a fresh one rather than counting
// nothing for ever.
- const watched = new WeakMap(); // video -> {id, stream}
+ // The identity is the tile, the stream *and* the track inside it: a
+ // camera the far side rebuilt comes back on a new receiver track, and
+ // in Chromium it comes back inside the same MediaStream, so a stream
+ // alone cannot tell the two apart.
+ const watched = new WeakMap(); // video -> {id, stream, track, at, time}
+
+ // How long a chain may count nothing before it is treated as dropped
+ // rather than idle — but only if the element's own clock moved on
+ // meanwhile. A frozen tile is idle and must be left alone; a tile
+ // that is playing and not counting has lost its callback.
+ const FRAME_QUIET = 2000;
function watchFrames(id, video) {
if (!HAS_RVFC) return;
const stream = video.srcObject;
+ const track = (stream && stream.getVideoTracks && stream.getVideoTracks()[0]) || null;
const current = watched.get(video);
- if (current && current.id === id && current.stream === stream) return;
- const mine = { id, stream };
+ if (current && current.id === id && current.stream === stream &&
+ current.track === track && !chainDropped(current, video)) {
+ return;
+ }
+ const mine = { id, stream, track, at: Date.now(), time: video.currentTime };
watched.set(video, mine);
const step = () => {
if (watched.get(video) !== mine) return;
if (!video.isConnected) {
+ // The grid was rebuilt under this tile. Forget the chain rather
+ // than keeping a dead one on the books, so the next render arms
+ // a fresh one.
watched.delete(video);
return;
}
+ mine.at = Date.now();
frames.set(id, (frames.get(id) || 0) + 1);
video.requestVideoFrameCallback(step);
};
video.requestVideoFrameCallback(step);
}
+ // chainDropped answers whether this chain has stopped counting while
+ // its element played on, and remembers the clock it saw for next
+ // time. Asked on every render and once a second from pollFrames, so
+ // no cause of a lost callback — a replaced stream, a replaced track,
+ // a tile detached and put back — can leave a tile playing perfectly
+ // with a count that never moves again.
+ function chainDropped(current, video) {
+ const moved = video.currentTime !== current.time;
+ current.time = video.currentTime;
+ return moved && Date.now() - current.at > FRAME_QUIET;
+ }
+
+ // rearmFrames asks the question of every tile that has media, which
+ // is what makes the answer above a repair rather than a diagnosis.
+ function rearmFrames() {
+ if (!HAS_RVFC) return;
+ for (const [key, t] of tiles) {
+ if (t && t.video && t.stream && !t.self) watchFrames(key, t.video);
+ }
+ }
+
// frameKey places one inbound video report on one tile. A peer that is
// sharing sends two video streams, and the loop below used to write
// both under the peer's own key — so the camera's count was whatever
@@ -3916,6 +3958,10 @@
// what arrives, and a camera that stopped encoding is a fault in what
// leaves.
async function pollFrames() {
+ // Once a second, whatever else this walk is for: a tile whose frame
+ // callback was dropped is put back on the books (Codex S4-6's gate
+ // found one, and the counter is what every frames assertion reads).
+ rearmFrames();
for (const [id, entry] of pcs) {
try {
const stats = await entry.pc.getStats();
diff --git a/test/browser/room-share.test.mjs b/test/browser/room-share.test.mjs
index 00de110..4c45e81 100644
--- a/test/browser/room-share.test.mjs
+++ b/test/browser/room-share.test.mjs
@@ -390,40 +390,71 @@ test('signed out is read before the status, because it can be true of a 200', ()
// chain, and the WeakSet of elements it used to keep then refused to
// arm another one.
+const FRAME_QUIET = readNumber(src, 'FRAME_QUIET');
+
const framesBox = new Function(
- 'frames', 'HAS_RVFC',
- 'const watched = new WeakMap();\n' + lift(src, 'watchFrames') +
+ 'frames', 'HAS_RVFC', 'Date',
+ 'const watched = new WeakMap();\nconst FRAME_QUIET = ' + FRAME_QUIET + ';\n' +
+ [lift(src, 'watchFrames'), lift(src, 'chainDropped')].join('\n') +
'\nreturn { watchFrames };',
);
-// A video element as watchFrames sees one: a stream, a connectedness,
-// and one pending frame callback at a time.
+// A video element as watchFrames sees one: a stream, a connectedness, a
+// clock, and one pending frame callback at a time. `frame()` both
+// delivers a rendered frame and moves the clock, the way a playing
+// video does.
function fakeVideo(stream) {
return {
srcObject: stream,
isConnected: true,
+ currentTime: 0,
pending: null,
requestVideoFrameCallback(fn) {
this.pending = fn;
},
- // frame() delivers one rendered frame to whatever chain is armed.
frame() {
const fn = this.pending;
this.pending = null;
+ this.currentTime += 0.033;
if (fn) fn();
return this;
},
};
}
+// A stream as watchFrames sees one: its video track is the second half
+// of the identity, because a camera the far side rebuilt comes back on
+// a new track inside the same MediaStream.
+function fakeStream(id, track) {
+ const tracks = track === null ? [] : [track || { id: id + '-v' }];
+ return {
+ id,
+ getVideoTracks: () => tracks,
+ // What a rebuilt camera does to a stream that is kept: the same
+ // MediaStream object, carrying a different track.
+ swap(next) {
+ tracks[0] = next;
+ return this;
+ },
+ };
+}
+
+// counter builds the box over a clock the test moves, so staleness can
+// be reached without waiting two seconds for it.
function counter() {
const frames = new Map();
- return { frames, watchFrames: framesBox(frames, true).watchFrames };
+ let now = 1000;
+ const clock = { now: () => now };
+ return {
+ frames,
+ watchFrames: framesBox(frames, true, clock).watchFrames,
+ tick: (ms) => { now += ms; },
+ };
}
test('one chain per tile: arming twice does not count twice', () => {
const c = counter();
- const video = fakeVideo({ id: 's1' });
+ const video = fakeVideo(fakeStream('s1'));
c.watchFrames('p1', video);
c.watchFrames('p1', video);
video.frame().frame().frame();
@@ -431,16 +462,16 @@ test('one chain per tile: arming twice does not count twice', () => {
});
test('a tile handed a new stream starts counting again', () => {
- // The defect itself. The element is the same one; the stream is not.
+ // The element is the same one; the stream is not.
const c = counter();
- const video = fakeVideo({ id: 's1' });
+ const video = fakeVideo(fakeStream('s1'));
c.watchFrames('p1', video);
video.frame().frame();
assert.equal(c.frames.get('p1'), 2);
// What `srcObject = stream` does: the element resets, and the pending
// frame callback goes with it.
- video.srcObject = { id: 's2' };
+ video.srcObject = fakeStream('s2');
video.pending = null;
c.watchFrames('p1', video);
video.frame().frame().frame();
@@ -448,9 +479,33 @@ test('a tile handed a new stream starts counting again', () => {
'the tile stopped counting when its stream was replaced');
});
+test('a tile whose track was rebuilt inside the same stream counts again', () => {
+ // The defect the browser gate found, and the reason the stream alone
+ // is not the identity: Chromium hands a rebuilt camera back inside
+ // the *same* MediaStream, on a new receiver track. The element resets
+ // — its clock goes back and the pending callback is discarded — and a
+ // chain keyed on the stream would have seen nothing to re-arm for.
+ const c = counter();
+ const stream = fakeStream('s1', { id: 'v1' });
+ const video = fakeVideo(stream);
+ c.watchFrames('p1', video);
+ video.frame().frame();
+ assert.equal(c.frames.get('p1'), 2);
+
+ // The same stream object, a new track inside it, and an element that
+ // reset itself around the swap.
+ stream.swap({ id: 'v2' });
+ video.pending = null;
+ video.currentTime = 0;
+ c.watchFrames('p1', video);
+ video.frame().frame().frame();
+ assert.equal(c.frames.get('p1'), 5,
+ 'the tile stopped counting when its track was rebuilt under it');
+});
+
test('a chain whose tile was detached is armed again on the next render', () => {
const c = counter();
- const video = fakeVideo({ id: 's1' });
+ const video = fakeVideo(fakeStream('s1'));
c.watchFrames('p1', video);
video.frame();
assert.equal(c.frames.get('p1'), 1);
@@ -466,12 +521,46 @@ test('a chain whose tile was detached is armed again on the next render', () =>
assert.equal(c.frames.get('p1'), 3, 'the tile never started counting again');
});
+test('a counter that goes quiet while the video plays on is re-armed', () => {
+ // The backstop, for a cause nobody has thought of. Whatever drops a
+ // frame callback, the signature is the same: the element's clock
+ // moves and the count does not.
+ const c = counter();
+ const video = fakeVideo(fakeStream('s1'));
+ c.watchFrames('p1', video);
+ video.frame();
+ assert.equal(c.frames.get('p1'), 1);
+
+ // The callback is gone, but the video is playing: the clock advances
+ // with nothing to count it.
+ video.pending = null;
+ video.currentTime += 1;
+ c.tick(FRAME_QUIET + 1);
+ c.watchFrames('p1', video);
+ video.frame().frame();
+ assert.equal(c.frames.get('p1'), 3, 'a dropped chain was never noticed');
+});
+
+test('a frozen tile is idle, not dropped, and is left alone', () => {
+ // The other half of the same question, and the reason the clock is
+ // read at all: a tile receiving nothing has a chain that is perfectly
+ // alive and simply has nothing to count. Re-arming it every second
+ // would pile up callbacks on a still picture.
+ const c = counter();
+ const video = fakeVideo(fakeStream('s1'));
+ c.watchFrames('p1', video);
+ const armed = video.pending;
+ c.tick(FRAME_QUIET * 5);
+ c.watchFrames('p1', video);
+ assert.equal(video.pending, armed, 'a frozen tile was re-armed');
+});
+
test('a tile that changed peer counts under the new key only', () => {
const c = counter();
- const video = fakeVideo({ id: 's1' });
+ const video = fakeVideo(fakeStream('s1'));
c.watchFrames('p1', video);
video.frame();
- video.srcObject = { id: 's2' };
+ video.srcObject = fakeStream('s2');
video.pending = null;
c.watchFrames('p2', video);
video.frame().frame();
@@ -483,8 +572,8 @@ test('a browser without requestVideoFrameCallback counts nothing here', () => {
// pollFrames is that browser's counter, and arming this one would be a
// second one racing it.
const frames = new Map();
- const { watchFrames } = framesBox(frames, false);
- const video = fakeVideo({ id: 's1' });
+ const { watchFrames } = framesBox(frames, false, Date);
+ const video = fakeVideo(fakeStream('s1'));
watchFrames('p1', video);
assert.equal(video.pending, null);
assert.equal(frames.size, 0);