amadan

oficina / meet Public

room.js: a yield that knows its own negotiation, and a camera watch

Codex S4-4 and S4-5 (both P2), and with them the Task 4 re-review's
carried items 1 and 2 and the whole-branch review's W4-3 and W4-5.

S4-4. `politeYield` asked only whether the connection was stable *now*,
which cannot tell uninterrupted stability from a whole remote exchange
that arrived, was answered and completed while the yield slept — and
`negotiate` did not stop a second invocation starting while the first
one slept. Three parts to the fix, all per connection: a negotiation
generation, moved by this side's own offer and by every exchange it
answers, which a yield captures and re-checks on waking; a
`negotiating` flag, so one local offer attempt is in flight at a time;
and `yields`, a small counter in place of the boolean, spent by giving
way, zeroed by this side's own offer, and handed back one at a time by
`answeredExchange` — a return to stable through somebody else's turn.
That last is the carried reset defect (item 1) and W4-4: the allowance
is no longer a per-connection budget one glare can spend for good.

S4-5. The yield prevents the rollback that costs a camera its encoder,
but it is a duration and not a guarantee — a round trip over 300 ms, a
429 retry, a slow offer — and nothing detected or repaired a camera
that had already been rebuilt. `pollFrames` now runs in every browser
rather than only where requestVideoFrameCallback is missing, and its
getStats walk reads outbound-rtp as well: two polls in a row with no
new framesEncoded on a sending m-line with a live track is a repair
(replaceTrack(null), replaceTrack(track)); two more is requestReset;
after that it stops asking, because a reset that did not take is not a
reason to ask again every two seconds.

W4-5: the comment claiming the yield "ends early because the remote
offer has arrived" was false — the sleep is unconditional and the check
is state-based — and now says so. W4-3: the node tests read every
constant out of the source, and one test pins the values themselves, so
POLITE_YIELD set to zero fails rather than passing green.

Sixteen node tests, four of them mutation-verified against the shipped
code: dropping the serializer fails the overlapping-invocation test,
dropping the generation check fails both exchange tests, dropping the
hand-back fails the protected-again test, and dropping the sending
guard fails the camera test.

node --check, 179 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 41deb52c676dab28d867dc1e186b99fbb6de54b7 parent a3a722f
3 files changed, +513 −56
  • internal/meet/static/room.js +232 −38
  • test/browser/negotiation.test.mjs +11 −1
  • test/browser/room-lane.test.mjs +270 −17
diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js
index 38507b4..231bdf0 100644
--- a/internal/meet/static/room.js
+++ b/internal/meet/static/room.js
@@ -147,10 +147,25 @@
// was already on its way arrives while this side is still stable and
// is answered rather than collided with, and no rollback happens at
// all. The cost is this much delay before the polite side offers, and
- // only when the other side has nothing to offer — at every moment when
- // both sides do, which is the case this exists for, the yield ends
- // early because the remote offer has arrived.
+ // only when the other side has nothing to offer. The wait itself is
+ // unconditional — the check that reads the connection happens after
+ // it, not during — so what a remote offer arriving buys is not a
+ // shorter wait but a negotiation this side no longer has to make.
const POLITE_YIELD = 300;
+ // How many times in a row the polite side may give way without an
+ // offer of its own going out. A boolean could not answer both "give
+ // the allowance back when the exchange that took our turn completes"
+ // and "never let giving way become a habit"; a small counter answers
+ // both (Codex S4-4, and the Task 4 re-review's item 1).
+ const POLITE_YIELDS = 2;
+ // Polls in a row that must report no new encoded frames before a
+ // camera counts as stopped, and repairs on one connection before the
+ // pair is reset instead. At one poll a second that is two seconds of
+ // a live track on a sending m-line encoding nothing, then a
+ // replaceTrack cycle, then two more seconds, then a reset (Codex
+ // S4-5, and the Task 4 re-review's item 2).
+ const CAMERA_STALL_POLLS = 2;
+ const CAMERA_REPAIRS = 1;
// §5.4: the tile key a peer's screen gets, built by shareKey.
const SHARE_SUFFIX = '#share';
const HAS_DISPLAY_MEDIA = typeof navigator !== 'undefined' &&
@@ -985,9 +1000,29 @@
// the outside — signalingState is still have-local-offer — and is
// not.
isSettingRemoteAnswerPending: false,
- // §5.1: whether this negotiation has already had its polite
- // yield. One per offer, so giving way can never become a habit.
- yielded: false,
+ // §5.1's polite yield, in three parts (Codex S4-4).
+ //
+ // `yields` is how many times in a row this side has given way
+ // without an offer of its own going out, capped at POLITE_YIELDS
+ // and handed back one at a time by an exchange this side did not
+ // offer. `negotiation` is the generation: it moves whenever this
+ // connection's negotiation moves on, so a yield that captured it
+ // and then slept can tell "nothing happened" from "an exchange
+ // came and went while I was asleep". `negotiating` is the
+ // serializer: one local offer attempt at a time, because two
+ // `negotiationneeded` events while one of them sleeps used to
+ // produce two offers, the second of them skipping the yield
+ // entirely because the first had already spent it.
+ yields: 0,
+ negotiation: 0,
+ negotiating: false,
+ // §5.1's camera watch (Codex S4-5): the last framesEncoded read
+ // off this connection's own camera m-line, how many polls in a
+ // row have reported that same number, and how many times the
+ // encoder has been rebuilt before giving up and resetting.
+ cameraFrames: -1,
+ cameraStalls: 0,
+ cameraRepairs: 0,
pending: [],
descSeq: 0,
gen: g,
@@ -1154,15 +1189,39 @@
return entry;
}
- // politeYield waits POLITE_YIELD and then answers whether this
- // connection is still ours and still stable — that is, whether no
- // remote offer arrived while we waited. False means one did: it is
- // being answered, and this negotiation belongs to the stable moment
- // after that rather than to a collision now.
- async function politeYield(id, entry) {
+ // politeYield waits POLITE_YIELD and then answers whether the
+ // negotiation it gave way for is still this side's to make.
+ //
+ // Three things can have happened while it slept. The connection can
+ // have gone, or the call ended: nothing to offer to. A remote offer
+ // can have arrived and still be being answered: signalingState says
+ // so. Or — the case a state check alone cannot see, and Codex S4-4's
+ // first half — a whole exchange can have arrived, been answered and
+ // completed, leaving the connection back at `stable` looking exactly
+ // as it did when this yield began. The generation is what tells those
+ // two apart: it moves on every exchange, so a yield whose generation
+ // has changed is a yield whose negotiation somebody else already
+ // made. Offering anyway is the extra offer that produced.
+ async function politeYield(id, entry, generation) {
await sleep(POLITE_YIELD);
- return pcs.get(id) === entry && !state.stopped &&
- entry.pc.signalingState === 'stable';
+ if (pcs.get(id) !== entry || state.stopped) return false;
+ if (entry.negotiation !== generation) return false;
+ return entry.pc.signalingState === 'stable';
+ }
+
+ // answeredExchange records one offer/answer exchange this side did
+ // not offer: a return to `stable` through somebody else's turn.
+ //
+ // It moves the generation, which invalidates any yield sleeping
+ // through it, and it hands back one spent yield — one, never more
+ // than was spent, so a pair renegotiating in turn cannot talk this
+ // side into giving way for ever. Without the hand-back, a yield lost
+ // to an exchange that happened to satisfy this side's own change left
+ // `yields` spent and the *next* independent negotiation unprotected,
+ // which is the reset defect the Task 4 re-review carried (item 1).
+ function answeredExchange(entry) {
+ entry.negotiation += 1;
+ if (entry.yields > 0) entry.yields -= 1;
}
// negotiate is §5.1's offering half: one offer per negotiation, with
@@ -1178,28 +1237,44 @@
// it offers instead, and the offer that was already on its way is
// answered from a stable state with no rollback anywhere.
//
- // Once per negotiation, which is what `yielded` is for: a side that
- // gave way and came back to find another offer waiting would give way
- // again, and two peers renegotiating in turn would keep one of them
- // permanently a beat behind. The second time round it offers and takes
- // the ordinary collision path, which is what the pattern is for. The
- // flag is cleared by the offer that actually goes out.
+ // Bounded, which is what `yields` is for: a side that gave way and
+ // came back to find another offer waiting would give way again, and
+ // two peers renegotiating in turn would keep one of them permanently a
+ // beat behind. Past POLITE_YIELDS in a row it offers and takes the
+ // ordinary collision path, which is what the pattern is for. The count
+ // goes back to zero on this side's own offer, and back by one on an
+ // exchange this side answered instead.
+ //
+ // One attempt at a time, too (Codex S4-4). `negotiationneeded` can
+ // fire again while the first attempt is asleep in its yield — a
+ // remote exchange completing is exactly the thing that fires it — and
+ // the second attempt used to skip the yield, because the first had
+ // already spent it, and then both of them offered.
async function negotiate(id, entry) {
- if (pcs.get(id) !== entry) return;
- if (entry.polite && !entry.yielded) {
- entry.yielded = true;
- if (!(await politeYield(id, entry))) return;
- }
+ if (pcs.get(id) !== entry || entry.negotiating) return;
+ entry.negotiating = true;
try {
- entry.makingOffer = true;
- await entry.pc.setLocalDescription();
- entry.yielded = false;
- send(id, 'offer', { description: describe(entry.pc.localDescription) });
- } catch {
- // A failed local description is not fatal: the pair recovers
- // through a reset if it never connects.
+ if (entry.polite && entry.yields < POLITE_YIELDS) {
+ entry.yields += 1;
+ if (!(await politeYield(id, entry, entry.negotiation))) return;
+ }
+ try {
+ entry.makingOffer = true;
+ await entry.pc.setLocalDescription();
+ // This side's own offer: the negotiation is ours, the
+ // generation moves with it, and the next one starts with the
+ // whole allowance again.
+ entry.negotiation += 1;
+ entry.yields = 0;
+ send(id, 'offer', { description: describe(entry.pc.localDescription) });
+ } catch {
+ // A failed local description is not fatal: the pair recovers
+ // through a reset if it never connects.
+ } finally {
+ entry.makingOffer = false;
+ }
} finally {
- entry.makingOffer = false;
+ entry.negotiating = false;
}
}
@@ -1321,8 +1396,11 @@
entry.descSeq = msg.seq || entry.descSeq;
await drainCandidates(entry);
await entry.pc.setLocalDescription();
- // One exchange completed, on the answering side (§5.3 step 2).
+ // One exchange completed, on the answering side (§5.3 step 2) —
+ // and an exchange this side did not offer, which is what moves
+ // the negotiation generation and hands a spent yield back.
entry.desc += 1;
+ answeredExchange(entry);
send(msg.from, 'answer', { description: describe(entry.pc.localDescription) });
} catch {
// A description that will not apply leaves the pair to recover
@@ -3670,14 +3748,122 @@
return '';
}
+ // localCameraTx is this connection's own camera transceiver: the one
+ // whose sender is carrying the local stream's live video track. The
+ // screen has a transceiver and a track of its own and is excluded by
+ // identity, not by mid, so a tab that is sharing is still watched on
+ // the right m-line.
+ function localCameraTx(entry) {
+ if (!entry.pc.getTransceivers || !local || !local.getVideoTracks) return null;
+ const wanted = local.getVideoTracks()[0];
+ if (!wanted || wanted.readyState === 'ended') return null;
+ for (const tx of entry.pc.getTransceivers()) {
+ if (tx && tx.sender && tx.sender.track === wanted) return tx;
+ }
+ return null;
+ }
+
+ // cameraReport picks the outbound video report that belongs to the
+ // camera. By mid when both sides of the comparison have one; with no
+ // mid to compare, only a single unambiguous report counts — the same
+ // refusal to guess between two video streams that frameKey makes.
+ // Pure, and lifted by name in the node test.
+ function cameraReport(reports, mid) {
+ if (mid) {
+ for (const r of reports) if (r.mid === mid) return r;
+ return reports.length === 1 && !reports[0].mid ? reports[0] : null;
+ }
+ return reports.length === 1 ? reports[0] : null;
+ }
+
+ // judgeCamera watches one connection's own camera encoder and says
+ // what to do about it: '' to leave it alone, 'repair' to rebuild the
+ // encoder, 'reset' to rebuild the whole connection.
+ //
+ // This is Codex S4-5, and the Task 4 re-review's item 2. POLITE_YIELD
+ // prevents the rollback that costs a camera its encoder, but it is a
+ // duration and not a guarantee: a signalling round trip longer than
+ // 300 ms, a 429 retry, or a slow offer can still land the remote offer
+ // late, and then the polite side rolls back and Chromium rebuilds
+ // every send stream on the connection. What is left looks entirely
+ // healthy — `connected`, the m-line `sendrecv`, the track `live` —
+ // and encodes nothing, ever. No connection-state handler fires,
+ // because nothing failed.
+ //
+ // The one thing that does say so is framesEncoded standing still, so
+ // that is what this reads, off the getStats walk that already runs.
+ // Two polls in a row with no new frames on a sending m-line with a
+ // live track is the signature; the repair is the cheapest thing that
+ // makes the browser build a new encoder for the same sender, and if
+ // two more polls are still frozen the pair is reset instead.
+ //
+ // The counters live on the entry, so a connection that is not sending
+ // video — no camera, an ended track, a recvonly m-line — resets them
+ // rather than accumulating a stall it could never repair.
+ function judgeCamera(entry, tx, report) {
+ const track = tx && tx.sender && tx.sender.track;
+ const direction = (tx && (tx.currentDirection || tx.direction)) || '';
+ const sends = direction === 'sendrecv' || direction === 'sendonly';
+ if (!tx || !sends || !track || track.readyState === 'ended' ||
+ !report || typeof report.framesEncoded !== 'number') {
+ entry.cameraFrames = -1;
+ entry.cameraStalls = 0;
+ return '';
+ }
+ if (report.framesEncoded !== entry.cameraFrames) {
+ // Encoding. Whatever was wrong is not wrong now, and a repair that
+ // worked is forgotten rather than counted against the next stall.
+ entry.cameraFrames = report.framesEncoded;
+ entry.cameraStalls = 0;
+ entry.cameraRepairs = 0;
+ return '';
+ }
+ entry.cameraStalls += 1;
+ if (entry.cameraStalls < CAMERA_STALL_POLLS) return '';
+ entry.cameraStalls = 0;
+ entry.cameraRepairs += 1;
+ if (entry.cameraRepairs <= CAMERA_REPAIRS) return 'repair';
+ if (entry.cameraRepairs === CAMERA_REPAIRS + 1) return 'reset';
+ // A reset has already been asked for and the frames are still
+ // frozen. The reset rebuilds the connection and these counters with
+ // it, so being here at all means it did not take — and asking for
+ // another one every two seconds would be a loop, not a recovery.
+ return '';
+ }
+
+ // repairCamera takes the track off its sender and puts it straight
+ // back. Neither call renegotiates — the kind is unchanged — and the
+ // second one is what makes the browser build an encoder for a sender
+ // whose own was torn down and never rebuilt.
+ async function repairCamera(tx) {
+ const sender = tx && tx.sender;
+ const track = sender && sender.track;
+ if (!sender || !track || typeof sender.replaceTrack !== 'function') return;
+ try {
+ await sender.replaceTrack(null);
+ await sender.replaceTrack(track);
+ } catch {
+ // A connection that is closing. The reset is behind this anyway.
+ }
+ }
+
+ // pollFrames is one getStats walk per connection, once a second, and
+ // it does two jobs. The inbound half is the frame counter's fallback
+ // for a browser without requestVideoFrameCallback. The outbound half
+ // is the camera watch, and it runs in every browser — rVFC counts
+ // what arrives, and a camera that stopped encoding is a fault in what
+ // leaves.
async function pollFrames() {
- // The fallback for a browser without requestVideoFrameCallback:
- // framesDecoded off the inbound video stats.
- if (HAS_RVFC) return;
for (const [id, entry] of pcs) {
try {
const stats = await entry.pc.getStats();
+ const outbound = [];
stats.forEach((report) => {
+ if (report.type === 'outbound-rtp' && report.kind === 'video') {
+ outbound.push(report);
+ return;
+ }
+ if (HAS_RVFC) return;
if (report.type !== 'inbound-rtp' || report.kind !== 'video' ||
typeof report.framesDecoded !== 'number') {
return;
@@ -3685,6 +3871,10 @@
const key = frameKey(id, entry, report.mid || '');
if (key) frames.set(key, report.framesDecoded);
});
+ const tx = localCameraTx(entry);
+ const verdict = judgeCamera(entry, tx, cameraReport(outbound, (tx && tx.mid) || ''));
+ if (verdict === 'repair') await repairCamera(tx);
+ else if (verdict === 'reset') requestReset(id);
} catch {
// getStats on a closing connection: nothing to report.
}
@@ -3822,7 +4012,11 @@
renderQueue();
await startLocal();
renderTiles();
- if (!HAS_RVFC) statsTimer = setInterval(pollFrames, 1000);
+ // Once a second, in every browser. rVFC counts arriving frames
+ // better than getStats can, but nothing but getStats can see a
+ // camera that stopped encoding (Codex S4-5), so the walk runs
+ // either way and does whichever halves apply.
+ statsTimer = setInterval(pollFrames, 1000);
connectStream();
}
diff --git a/test/browser/negotiation.test.mjs b/test/browser/negotiation.test.mjs
index 945b2dc..5097f5f 100644
--- a/test/browser/negotiation.test.mjs
+++ b/test/browser/negotiation.test.mjs
@@ -88,10 +88,16 @@ test('a remote offer already applied is glare', () => {
// which the six cases above would pass without.
const handlers = new Function(
'accept', 'send', 'describe', 'drainCandidates',
- [lift(src, 'offerCollision'), lift(src, 'onOffer'), lift(src, 'onAnswer')].join('\n') +
+ [lift(src, 'offerCollision'), lift(src, 'onOffer'), lift(src, 'onAnswer'),
+ lift(src, 'answeredExchange')].join('\n') +
'\nreturn { onOffer, onAnswer };',
);
+// answeredExchange is lifted with them because onOffer calls it: an
+// exchange this side answered is what moves the negotiation generation
+// and hands the polite side its yield back (Codex S4-4). Its own
+// behaviour is gated in room-lane.test.mjs, next to `negotiate`.
+
// fakePC is the part of RTCPeerConnection these two handlers touch. The
// answer's setRemoteDescription is deliberately slow, and the state it
// leaves behind while it is in flight is the real one: signalingState
@@ -133,6 +139,8 @@ test('an offer arriving while an answer is being applied is answered, not discar
makingOffer: false,
ignoreOffer: false,
isSettingRemoteAnswerPending: false,
+ yields: 0,
+ negotiation: 0,
pending: [],
descSeq: 0,
};
@@ -217,6 +225,8 @@ test('the answer handler clears its own flag', async () => {
makingOffer: false,
ignoreOffer: false,
isSettingRemoteAnswerPending: false,
+ yields: 0,
+ negotiation: 0,
pending: [],
descSeq: 0,
};
diff --git a/test/browser/room-lane.test.mjs b/test/browser/room-lane.test.mjs
index f2776fa..8e6319a 100644
--- a/test/browser/room-lane.test.mjs
+++ b/test/browser/room-lane.test.mjs
@@ -472,16 +472,31 @@ test('chatEnabled is silent with no panel to enable', () => {
// `mutualshare`; what is here is the decision that keeps it from
// happening: the side that would roll back gives way before it offers.
+// Every number these tests assert against is read out of room.js
+// rather than retyped: a test carrying a constant of its own proves its
+// own harness (the Task 4 re-review's item 8).
+function pin(name) {
+ const m = new RegExp('const ' + name + ' = (\\d+);').exec(src);
+ assert.ok(m, name + ' is not a number in room.js any more — this test is stale, not passing');
+ return Number(m[1]);
+}
+
+const POLITE_YIELD = pin('POLITE_YIELD');
+const POLITE_YIELDS = pin('POLITE_YIELDS');
+
const negotiator = new Function(
- 'pcs', 'send', 'describe', 'sleep', 'state', 'POLITE_YIELD', 'console',
- [lift(src, 'negotiate'), lift(src, 'politeYield')].join('\n') +
- '\nreturn { negotiate };',
+ 'pcs', 'send', 'describe', 'sleep', 'state', 'POLITE_YIELD', 'POLITE_YIELDS', 'console',
+ [lift(src, 'negotiate'), lift(src, 'politeYield'), lift(src, 'answeredExchange')].join('\n') +
+ '\nreturn { negotiate, answeredExchange };',
);
// A connection as `negotiate` sees one, over a fake pc whose
// setLocalDescription can be watched and whose signalingState a test
// moves under it — which is what a remote offer arriving during the
-// yield actually looks like.
+// yield actually looks like. `duringYield` is handed the whole box, so
+// a test can also complete an exchange while the yield sleeps, which is
+// the thing a signalingState alone cannot express: the connection is
+// back at `stable` afterwards and looks untouched.
function negotiating(opts) {
const o = opts || {};
const sent = [];
@@ -489,7 +504,9 @@ function negotiating(opts) {
const pcs = new Map();
const entry = {
polite: !!o.polite,
- yielded: !!o.yielded,
+ yields: o.yields || 0,
+ negotiation: 0,
+ negotiating: false,
makingOffer: false,
pc: {
signalingState: o.signalingState || 'stable',
@@ -500,19 +517,29 @@ function negotiating(opts) {
},
};
pcs.set('peer1', entry);
- const { negotiate } = negotiator(
+ let box;
+ const built = negotiator(
pcs,
(to, type, payload) => sent.push([to, type, !!(payload && payload.description)]),
(d) => d,
async (ms) => {
waits.push(ms);
- if (o.duringYield) o.duringYield(entry, pcs);
+ if (o.duringYield) o.duringYield(entry, pcs, box);
},
{ stopped: !!o.stopped },
- 250,
+ POLITE_YIELD,
+ POLITE_YIELDS,
quiet,
);
- return { negotiate, entry, pcs, sent, waits };
+ box = {
+ negotiate: built.negotiate,
+ answeredExchange: built.answeredExchange,
+ entry,
+ pcs,
+ sent,
+ waits,
+ };
+ return box;
}
test('the impolite side offers at once, and waits for nothing', async () => {
@@ -525,9 +552,10 @@ test('the impolite side offers at once, and waits for nothing', async () => {
test('the polite side gives way first, then offers', async () => {
const n = negotiating({ polite: true });
await n.negotiate('peer1', n.entry);
- assert.deepEqual(n.waits, [250], 'the polite side did not wait, or waited the wrong amount');
+ assert.deepEqual(n.waits, [POLITE_YIELD],
+ 'the polite side did not wait, or waited the wrong amount');
assert.deepEqual(n.sent, [['peer1', 'offer', true]]);
- assert.equal(n.entry.yielded, false, 'the offer that went out must clear the flag');
+ assert.equal(n.entry.yields, 0, 'the offer that went out must give the allowance back');
});
test('a remote offer arriving during the yield takes the negotiation', async () => {
@@ -540,18 +568,87 @@ test('a remote offer arriving during the yield takes the negotiation', async ()
});
await n.negotiate('peer1', n.entry);
assert.deepEqual(n.sent, [], 'the polite side offered into a remote offer it could see');
- assert.equal(n.entry.yielded, true, 'the yield it spent must be remembered');
+ assert.equal(n.entry.yields, 1, 'the yield it spent must be remembered');
});
-test('giving way happens once per negotiation, never twice', async () => {
+test('giving way is bounded, however many negotiations are lost', async () => {
// A side that gave way and came back to find another offer waiting
// would give way again, and two peers renegotiating in turn would keep
- // one of them permanently a beat behind.
- const n = negotiating({ polite: true, yielded: true });
+ // one of them permanently a beat behind. Nothing here completes an
+ // exchange, so nothing hands the allowance back: after POLITE_YIELDS
+ // in a row it offers and takes the ordinary collision path.
+ const n = negotiating({
+ polite: true,
+ duringYield: (entry) => { entry.pc.signalingState = 'have-remote-offer'; },
+ });
+ for (let i = 0; i < POLITE_YIELDS; i++) await n.negotiate('peer1', n.entry);
+ assert.equal(n.waits.length, POLITE_YIELDS, 'it gave way more times than the bound allows');
+ assert.deepEqual(n.sent, []);
+
+ n.entry.pc.signalingState = 'stable';
+ await n.negotiate('peer1', n.entry);
+ assert.equal(n.waits.length, POLITE_YIELDS, 'it gave way past the bound');
+ assert.deepEqual(n.sent, [['peer1', 'offer', true]]);
+});
+
+// --- the generation, the serializer, and the allowance (Codex S4-4) ---
+
+test('an exchange that completed during the yield is not offered over', async () => {
+ // The case a signalingState check cannot see. A whole remote
+ // exchange — offer in, answer out — arrives, is answered and
+ // completes while the yield sleeps, leaving the connection back at
+ // `stable` looking exactly as it did when the yield began. Offering
+ // now is a second offer for a negotiation somebody else already made,
+ // and an in-memory drive of the shipped functions produced exactly
+ // that.
+ const n = negotiating({
+ polite: true,
+ duringYield: (entry, pcs, box) => box.answeredExchange(entry),
+ });
+ await n.negotiate('peer1', n.entry);
+ assert.deepEqual(n.sent, [], 'an offer went out for a negotiation that was already answered');
+ assert.equal(n.entry.yields, 0, 'the exchange that took the turn must hand the yield back');
+});
+
+test('two negotiations while one sleeps produce one offer', async () => {
+ // `negotiationneeded` fires again while the first attempt is asleep —
+ // a remote exchange completing is exactly what fires it — and the
+ // second attempt used to skip the yield, because the first had
+ // already spent it, and then both of them offered.
+ const n = negotiating({ polite: true });
+ await Promise.all([
+ n.negotiate('peer1', n.entry),
+ n.negotiate('peer1', n.entry),
+ n.negotiate('peer1', n.entry),
+ ]);
+ assert.equal(n.waits.length, 1, 'more than one negotiation was in flight');
+ assert.deepEqual(n.sent, [['peer1', 'offer', true]]);
+});
+
+test('a negotiation after a lost yield is protected again', async () => {
+ // The reset defect the Task 4 re-review carried (item 1): a yield lost
+ // to an exchange that happened to satisfy this side's own change left
+ // the allowance spent, and the next independent negotiation went
+ // straight out with no yield in front of it.
+ //
+ // Enough of them to exhaust the bound, so the bound cannot be what
+ // makes this pass: without the hand-back, `yields` reaches
+ // POLITE_YIELDS and the next negotiation goes straight out.
+ let exchange = true;
+ const n = negotiating({
+ polite: true,
+ duringYield: (entry, pcs, box) => {
+ if (exchange) box.answeredExchange(entry);
+ },
+ });
+ for (let i = 0; i < POLITE_YIELDS; i++) await n.negotiate('peer1', n.entry);
+ assert.deepEqual(n.sent, [], 'an offer went out for a negotiation somebody else made');
+ assert.equal(n.entry.yields, 0, 'every yield an exchange took must be handed back');
+
+ exchange = false;
await n.negotiate('peer1', n.entry);
- assert.deepEqual(n.waits, [], 'it gave way a second time');
+ assert.equal(n.waits.length, POLITE_YIELDS + 1, 'the later negotiation went out unprotected');
assert.deepEqual(n.sent, [['peer1', 'offer', true]]);
- assert.equal(n.entry.yielded, false);
});
test('a connection that went away during the yield is not offered to', async () => {
@@ -734,3 +831,159 @@ test('a screen announced but never arrived is still dropped', () => {
s.dropShare(entry);
assert.equal(entry.sharing, false);
});
+
+test('the numbers these tests assert against are the ones room.js ships', () => {
+ // Everything above reads its constants out of the source, which is
+ // what keeps a test from proving its own harness — but a value read
+ // from the source and then handed straight back in is still no gate on
+ // the value itself: POLITE_YIELD set to zero would leave every test
+ // above green (the whole-branch review's W4-3). Spec §15.16 says 300
+ // ms, so that is what this pins.
+ assert.equal(POLITE_YIELD, 300, 'spec §15.16 names 300 ms');
+ assert.ok(POLITE_YIELDS >= 1, 'the polite side must be allowed to give way at least once');
+ assert.ok(CAMERA_STALL_POLLS >= 2, 'one poll is a sample, not a stall');
+ assert.ok(CAMERA_REPAIRS >= 1, 'a camera must be given at least one repair before a reset');
+});
+
+// ---------------------------------------------------------------
+// The camera watch (Codex S4-5)
+// ---------------------------------------------------------------
+//
+// POLITE_YIELD prevents the rollback that costs a camera its encoder,
+// but it is a duration and not a guarantee: a signalling round trip
+// longer than 300 ms, a 429 retry, or a slow offer can still land the
+// remote offer late. What is left afterwards looks entirely healthy —
+// `connected`, the m-line `sendrecv`, the track `live` — and encodes
+// nothing at all, so no connection-state handler ever fires. The one
+// thing that says so is framesEncoded standing still.
+
+const CAMERA_STALL_POLLS = pin('CAMERA_STALL_POLLS');
+const CAMERA_REPAIRS = pin('CAMERA_REPAIRS');
+
+const cameraBox = new Function(
+ 'CAMERA_STALL_POLLS', 'CAMERA_REPAIRS',
+ [lift(src, 'judgeCamera'), lift(src, 'cameraReport'), lift(src, 'repairCamera')].join('\n') +
+ '\nreturn { judgeCamera, cameraReport, repairCamera };',
+);
+const cam = cameraBox(CAMERA_STALL_POLLS, CAMERA_REPAIRS);
+
+// A camera transceiver as the watch sees one, and the record of what
+// was done to its sender.
+function cameraTx(opts) {
+ const o = opts || {};
+ const calls = [];
+ const track = { kind: 'video', readyState: o.readyState || 'live' };
+ const tx = {
+ mid: o.mid === undefined ? '1' : o.mid,
+ currentDirection: o.direction === undefined ? 'sendrecv' : o.direction,
+ sender: {
+ track: o.noReplace ? track : track,
+ replaceTrack: o.noReplace ? undefined : async (t) => {
+ calls.push(t === null ? 'null' : 'track');
+ },
+ },
+ };
+ return { tx, calls, track };
+}
+
+function watching() {
+ return { cameraFrames: -1, cameraStalls: 0, cameraRepairs: 0 };
+}
+
+// verdicts drives one entry through a sequence of framesEncoded counts
+// and hands back what the watch said each time.
+function verdicts(entry, tx, counts) {
+ return counts.map((n) =>
+ cam.judgeCamera(entry, tx, n === null ? null : { framesEncoded: n, mid: tx.mid }));
+}
+
+test('a camera that keeps encoding is never touched', () => {
+ const c = cameraTx();
+ const entry = watching();
+ assert.deepEqual(verdicts(entry, c.tx, [10, 20, 30, 40, 50, 60]), ['', '', '', '', '', '']);
+ assert.equal(entry.cameraRepairs, 0);
+});
+
+test('a camera that stops encoding is repaired, and then reset', () => {
+ // The signature: the same framesEncoded poll after poll on a sending
+ // m-line with a live track. Two polls of it is a repair; two more is
+ // the pair being rebuilt outright.
+ const c = cameraTx();
+ const entry = watching();
+ const said = verdicts(entry, c.tx, [100, 100, 100, 100, 100, 100, 100]);
+ assert.equal(said.filter((v) => v === 'repair').length, 1, JSON.stringify(said));
+ assert.equal(said.filter((v) => v === 'reset').length, 1, JSON.stringify(said));
+ assert.ok(said.indexOf('repair') < said.indexOf('reset'), 'the reset came before the repair');
+ // And then it stops asking: a reset that did not take is not a reason
+ // to ask for another one every two seconds.
+ assert.deepEqual(verdicts(entry, c.tx, [100, 100, 100, 100]), ['', '', '', '']);
+});
+
+test('a camera that comes back forgets the repair it needed', () => {
+ // A repair that worked must not be counted against the next stall,
+ // hours later: the next one gets a repair of its own before anything
+ // is reset.
+ const c = cameraTx();
+ const entry = watching();
+ let said = verdicts(entry, c.tx, [100, 100, 100]);
+ assert.equal(said[said.length - 1], 'repair');
+ said = verdicts(entry, c.tx, [110, 120, 130]);
+ assert.deepEqual(said, ['', '', ''], 'a camera that is encoding again was still being judged');
+ said = verdicts(entry, c.tx, [130, 130, 130]);
+ assert.ok(said.includes('repair') && !said.includes('reset'),
+ 'the second stall was reset rather than repaired: ' + JSON.stringify(said));
+});
+
+test('nothing is repaired on a connection that is not sending a camera', () => {
+ for (const opts of [{ direction: 'recvonly' }, { direction: 'inactive' }, { readyState: 'ended' }]) {
+ const c = cameraTx(opts);
+ const entry = watching();
+ assert.deepEqual(verdicts(entry, c.tx, [100, 100, 100, 100, 100]), ['', '', '', '', ''],
+ JSON.stringify(opts) + ' was treated as a stalled camera');
+ assert.equal(entry.cameraStalls, 0);
+ }
+});
+
+test('a stall is forgotten when the camera stops being watchable', () => {
+ // Half a stall, then no report at all — a connection that stopped
+ // sending video. The count starts again rather than tipping over into
+ // a repair on the strength of a sample from before.
+ const c = cameraTx();
+ const entry = watching();
+ verdicts(entry, c.tx, [100, 100]);
+ assert.equal(entry.cameraStalls, 1);
+ assert.equal(cam.judgeCamera(entry, c.tx, null), '');
+ assert.equal(entry.cameraStalls, 0);
+ assert.equal(entry.cameraFrames, -1);
+});
+
+test('the outbound report is chosen by mid, and never guessed at', () => {
+ const camera = { mid: '1', framesEncoded: 10 };
+ const screen = { mid: '2', framesEncoded: 20 };
+ assert.equal(cam.cameraReport([camera, screen], '1'), camera);
+ assert.equal(cam.cameraReport([screen, camera], '1'), camera);
+ // A browser that reports no mid on outbound-rtp: one report is
+ // unambiguous, two are the camera and the screen and neither can be
+ // told from the other.
+ const bare = { framesEncoded: 10 };
+ assert.equal(cam.cameraReport([bare], '1'), bare);
+ assert.equal(cam.cameraReport([bare, { framesEncoded: 20 }], '1'), null);
+ assert.equal(cam.cameraReport([], '1'), null);
+ // No mid known at all: the same rule from the other side.
+ assert.equal(cam.cameraReport([bare], ''), bare);
+ assert.equal(cam.cameraReport([bare, { framesEncoded: 20 }], ''), null);
+});
+
+test('the repair takes the track off its sender and puts it back', async () => {
+ const c = cameraTx();
+ await cam.repairCamera(c.tx);
+ assert.deepEqual(c.calls, ['null', 'track'],
+ 'the encoder is rebuilt by removing the track and restoring it, in that order');
+});
+
+test('a sender that cannot replace its track is left alone', async () => {
+ const c = cameraTx({ noReplace: true });
+ await assert.doesNotReject(() => cam.repairCamera(c.tx));
+ await assert.doesNotReject(() => cam.repairCamera(null));
+ await assert.doesNotReject(() => cam.repairCamera({ sender: null }));
+});