From f7d33c299ae0d0c4f406779b8f5324d80637adc8 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Mon, 21 Sep 2026 00:21:50 +0200 Subject: perf(hub): bound video read-ahead by bytes, not by seconds A browser limits bytes, so a bound in seconds had to be sized for the highest-bitrate file and every ordinary one then held a fraction of what the same buffer would have taken. Floored at the old 90 s so nothing pulls less than before, and walked up rather than declared. A refused append now waits for an eviction instead of retrying on every tick. Co-Authored-By: Claude Opus 5 --- packages/meshbay-hub/tests/harness/mse_harness.mjs | 56 ++++- .../meshbay-hub/tests/test_video_buffer_ceiling.py | 231 ++++++++++++++++++++- 2 files changed, 269 insertions(+), 18 deletions(-) (limited to 'packages/meshbay-hub/tests') diff --git a/packages/meshbay-hub/tests/harness/mse_harness.mjs b/packages/meshbay-hub/tests/harness/mse_harness.mjs index 91018ef..f0fd04c 100644 --- a/packages/meshbay-hub/tests/harness/mse_harness.mjs +++ b/packages/meshbay-hub/tests/harness/mse_harness.mjs @@ -35,7 +35,8 @@ const useCallback = (fn) => fn; // be written in. Extracting into an order of our own would quietly repair a // hook declared before its own dependency — a real fault, which reached // production once, and which the harness is otherwise well placed to catch. -const src = ['currentRange', 'bufferedAhead', 'evictBehind', 'flushQueue', 'pump'] +const src = ['currentRange', 'bufferedAhead', 'aheadLimit', 'evictBehind', + 'flushQueue', 'pump'] .sort((a, b) => app.indexOf(`const ${a} = useCallback(`) - app.indexOf(`const ${b} = useCallback(`)) .map(grab).join('\n'); @@ -45,14 +46,21 @@ const CAP = capMB * 1048576; const BITRATE = fileMB * 1048576 / durationS; // Read from app.js too, so a change to the constants is a change to the test. +// Arithmetic is allowed because some of them are written as one (`48 * 1024 * +// 1024`), and refusing anything that is not arithmetic keeps this a reader +// rather than an evaluator of whatever happens to be on the line. const constOf = (name) => { - const m = app.match(new RegExp(`const ${name} = (\\d+)`)); + const m = app.match(new RegExp(`const ${name} = ([^;]+);`)); if (!m) throw new Error(`${name} not found`); - return Number(m[1]); + if (!/^[\d\s.*/+-]+$/.test(m[1])) throw new Error(`${name} is not a number`); + return Number(new Function(`return (${m[1]});`)()); }; const BUFFER_BEHIND_S = constOf('BUFFER_BEHIND_S'); const STREAM_WINDOW = constOf('STREAM_WINDOW'); const BUFFER_AHEAD_S = constOf('BUFFER_AHEAD_S'); +const BUFFER_AHEAD_STEP_BYTES = constOf('BUFFER_AHEAD_STEP_BYTES'); +const BUFFER_AHEAD_MAX_BYTES = constOf('BUFFER_AHEAD_MAX_BYTES'); +const BUFFER_AHEAD_MAX_S = constOf('BUFFER_AHEAD_MAX_S'); const QUEUE_HIGH_WATER = constOf('QUEUE_HIGH_WATER'); const CREDIT_KEEPALIVE_MS = constOf('CREDIT_KEEPALIVE_MS'); @@ -110,7 +118,11 @@ const sb = { }; let pendingRemoveEvents = 0; -const video = { currentTime: 0 }; +// `paused` is part of the element the player reads, not decoration: the +// read-ahead budget only grows while the film is running, and a fake video +// that never reports being paused would exercise the growing branch in a run +// whose whole point is that nobody pressed play. +const video = { currentTime: 0, paused: !playing }; const sbRef = { current: sb }, videoRef = { current: video }; const msRef = { current: { readyState: 'open', endOfStream() {} } }; const queueRef = { current: [] }; @@ -118,6 +130,13 @@ const appendingRef = { current: false }, endedRef = { current: false }; const outstandingRef = { current: 0 }, lastPokeRef = { current: 0 }; const quotaRef = { current: 0 }, stalledRef = { current: false }; const awaitingInitRef = { current: false }; +// The film's own average bitrate, as `stream_init` gives the player, and the +// budget state the read-ahead walks up from. Both start where the component +// starts them. +const bitrateRef = { current: BITRATE }; +const aheadBytesRef = { current: 0 }; +const aheadCapRef = { current: BUFFER_AHEAD_MAX_BYTES }; +const quotaHoldRef = { current: false }; const transportRef = { current: { connected: true, @@ -132,12 +151,16 @@ const fns = new Function( 'sbRef,videoRef,msRef,queueRef,appendingRef,endedRef,outstandingRef,' + 'lastPokeRef,transportRef,BUFFER_BEHIND_S,BUFFER_AHEAD_S,QUEUE_HIGH_WATER,' + 'CREDIT_KEEPALIVE_MS,STREAM_WINDOW,quotaRef,stalledRef,awaitingInitRef,' + - 'console,useCallback', - src + '\n return {currentRange, bufferedAhead, evictBehind, flushQueue, pump};' + 'bitrateRef,aheadBytesRef,aheadCapRef,quotaHoldRef,' + + 'BUFFER_AHEAD_STEP_BYTES,' + + 'BUFFER_AHEAD_MAX_BYTES,BUFFER_AHEAD_MAX_S,console,useCallback', + src + '\n return {currentRange, bufferedAhead, aheadLimit, evictBehind,' + + ' flushQueue, pump};' )(sbRef, videoRef, msRef, queueRef, appendingRef, endedRef, outstandingRef, lastPokeRef, transportRef, BUFFER_BEHIND_S, BUFFER_AHEAD_S, QUEUE_HIGH_WATER, CREDIT_KEEPALIVE_MS, STREAM_WINDOW, quotaRef, stalledRef, awaitingInitRef, - console, useCallback); + bitrateRef, aheadBytesRef, aheadCapRef, quotaHoldRef, BUFFER_AHEAD_STEP_BYTES, + BUFFER_AHEAD_MAX_BYTES, BUFFER_AHEAD_MAX_S, console, useCallback); // The player's own `updateend` listener, transcribed — the one part of the // component that is a listener rather than a callback, and the place the @@ -151,6 +174,15 @@ credit = STREAM_WINDOW; outstandingRef.current = STREAM_WINDOW; let wall = 0; const TICK = 0.05; +// Bytes the link has carried but not yet spent on a whole segment. This used +// to be re-created inside the loop and thrown away at the end of every tick, +// so a link slower than one segment per tick — 5 MB/s at this resolution, +// which is every mobile network there is — delivered *nothing at all* and the +// run reported a player that had simply never been fed. Anything below about +// forty megabits was unmeasurable here, which is most of the cases worth +// measuring. Carried over instead, and capped at one window's worth, because +// a link cannot bank a burst larger than what the node may have in flight. +let netBudget = 0; while (wall < wallS) { wall += TICK; if (playing) { @@ -160,9 +192,9 @@ while (wall < wallS) { fns.pump(); // the 1 s timer and timeupdate while (pendingRemoveEvents > 0) { pendingRemoveEvents--; updateend(); } - let budget = netMBs * 1048576 * TICK; - while (credit > 0 && budget >= SEG) { - credit--; budget -= SEG; sent++; + netBudget = Math.min(netBudget + netMBs * 1048576 * TICK, SEG * STREAM_WINDOW); + while (credit > 0 && netBudget >= SEG) { + credit--; netBudget -= SEG; sent++; outstandingRef.current = Math.max(0, outstandingRef.current - 1); queueRef.current.push({ byteLength: SEG }); fns.flushQueue(); @@ -176,6 +208,10 @@ console.log(JSON.stringify({ heldInBufferMB: +(bytes / 1048576).toFixed(1), queueDepth: queueRef.current.length, bufferedAheadS: +fns.bufferedAhead().toFixed(1), + aheadLimitS: +fns.aheadLimit().toFixed(1), + budgetMB: +(aheadBytesRef.current / 1048576).toFixed(1), + budgetCapMB: +(aheadCapRef.current / 1048576).toFixed(1), + bitrateMbits: +(BITRATE * 8 / 1e6).toFixed(2), watchedS: +video.currentTime.toFixed(1), grants: granted, keepalives, diff --git a/packages/meshbay-hub/tests/test_video_buffer_ceiling.py b/packages/meshbay-hub/tests/test_video_buffer_ceiling.py index 59f97de..7dd9c81 100644 --- a/packages/meshbay-hub/tests/test_video_buffer_ceiling.py +++ b/packages/meshbay-hub/tests/test_video_buffer_ceiling.py @@ -20,6 +20,18 @@ it. Memory was bounded by the browser's ceiling rather than by anything we chose. Buffering by *time* past the playhead instead makes a two-hour film cost the same as a two-minute clip. +**And then a bound in seconds turned out to be the wrong unit.** The browser +limits BYTES, so ninety seconds had to be sized for the highest-bitrate file in +a library and every ordinary file held a fraction of what the same browser +would have taken: measured here against a 100 MB ceiling, a 1.0 Mbit/s film +reached its ninety seconds on 20.7 MB and a 2.3 Mbit/s one on 45.6 MB. On a +mobile network that unused ceiling is the whole of the margin there is for a +dead spot. The read-ahead is now a byte budget converted to seconds at the +file's own bitrate, floored at the fixed bound so nothing pulls less than it +used to, and *walked up* rather than declared — because the real ceiling is +per-engine and per-device, and the alternative to walking up to it is +overshooting it, which costs a refused append every time it is tried. + **The pipeline could not restart itself.** An append refused for quota fires no `updateend`. `updateend` was where credit was granted, so no credit went out; the node then sent nothing, so no segment arrived to call `flushQueue` again. @@ -65,6 +77,17 @@ def _player(app: str) -> str: return app[i:nxt if nxt > 0 else len(app)] +def _const(app: str, name: str) -> float: + """One of the player's numeric constants, arithmetic and all. + + Read out of the source rather than restated here, so a change to a constant + is a change to what these tests assert — the same rule the harness follows. + """ + m = re.search(rf"^const {name} = ([\d\s.*/+-]+);$", app, re.M) + assert m, f"{name} is not a numeric constant in video-player.js" + return float(eval(m.group(1), {"__builtins__": {}}, {})) # noqa: S307 + + # ── The shipped functions, run against a browser that has a ceiling ─────────── HARNESS = Path(__file__).parent / "harness" / "mse_harness.mjs" @@ -108,12 +131,23 @@ def test_the_ceiling_is_never_reached_while_watching(watched): assert watched["quotaRefusals"] == 0 -def test_the_read_ahead_is_bounded_by_the_playhead(idle, watched): - """What replaced "as fast as the network allows".""" - ahead = int(re.search(r"const BUFFER_AHEAD_S = (\d+)", APP.read_text()).group(1)) +def test_the_read_ahead_is_bounded_by_the_playhead(app, idle, watched): + """What replaced "as fast as the network allows". + + The bound is the one the player computed for this film, not the constant: + the constant is only its floor. Asserting against the constant would say + nothing about a film whose bitrate raises the bound above it, which is now + most of them. + """ for name, run in (("idle", idle), ("watching", watched)): - assert run["bufferedAheadS"] < ahead * 2, ( - f"{name}: {run['bufferedAheadS']}s buffered against a {ahead}s " + limit = run["aheadLimitS"] + assert limit >= _const(app, "BUFFER_AHEAD_S"), ( + f"{name}: the bound came out at {limit}s, below the fixed floor — " + "some film now buffers less than it did before the budget existed") + # One window of segments may land past the gate before it shuts, which + # is the flow control working, not the gate leaking. + assert run["bufferedAheadS"] < limit * 1.5, ( + f"{name}: {run['bufferedAheadS']}s buffered against a {limit}s " "bound — the gate is not holding") @@ -131,9 +165,190 @@ def test_a_watched_film_keeps_being_fed(watched): assert watched["removes"] > 0, "nothing was ever evicted behind the playhead" -def test_memory_stays_bounded_over_a_long_watch(watched): - assert watched["heldInBufferMB"] < 40, ( - f"holding {watched['heldInBufferMB']} MB — eviction is not keeping up") +def test_memory_stays_bounded_over_a_long_watch(app, watched): + """The budget is a bound on memory, so it is the one to assert against. + + What the player may hold is the read-ahead budget plus the minute it keeps + behind the playhead for a small seek back. Anything past that is eviction + failing to keep up, which is the fault this guards. + """ + mb = 1024 * 1024 + per_s = watched["bitrateMbits"] * 1e6 / 8 / mb + allowed = _const(app, "BUFFER_AHEAD_MAX_BYTES") / mb \ + + _const(app, "BUFFER_BEHIND_S") * per_s + # One window of segments may be in flight past the budget when the run ends. + allowed += 8 * 256 * 1024 / mb + assert watched["heldInBufferMB"] < allowed, ( + f"holding {watched['heldInBufferMB']} MB against {allowed:.1f} MB of " + "budget and eviction window — eviction is not keeping up") + + +# ── The byte budget, and what it must not cost ──────────────────────────────── + +# A library's worth of bitrates against one ceiling, and the same low-bitrate +# film against a ceiling small enough that the budget cannot have it. Sizes are +# a two-hour film at each rate; nothing here names a real title. +_BITRATE_SWEEP = [ + ("0.6 Mbit/s", {"fileMB": 493.5, "durationS": 7200, "capMB": 100}), + ("2.3 Mbit/s", {"fileMB": 2000, "durationS": 7200, "capMB": 100}), + ("4.7 Mbit/s", {"fileMB": 4000, "durationS": 7200, "capMB": 100}), + ("9.3 Mbit/s", {"fileMB": 8000, "durationS": 7200, "capMB": 100}), + ("0.6 Mbit/s, small ceiling", {"fileMB": 493.5, "durationS": 7200, "capMB": 40}), + ("2.3 Mbit/s, small ceiling", {"fileMB": 2000, "durationS": 7200, "capMB": 40}), +] + + +@pytest.fixture(scope="module") +def sweep(): + return {name: _harness(playing=True, netMBs=35, wallS=900, **cfg) + for name, cfg in _BITRATE_SWEEP} + + +def _fits_under_the_ceiling(app: str, name: str, run: dict) -> bool: + """Whether even the *floor* bound fits in this browser's buffer. + + Where it does not, the film was already unservable before the budget + existed — the fixed ninety seconds plus the minute kept behind is more than + the ceiling, so there is nowhere to put it. See MESHBAY_DESIGN.md §15.3. + """ + cap_mb = dict(_BITRATE_SWEEP)[name]["capMB"] + per_s = run["bitrateMbits"] * 1e6 / 8 / (1024 * 1024) + need = (_const(app, "BUFFER_AHEAD_S") + _const(app, "BUFFER_BEHIND_S")) * per_s + return need <= cap_mb + + +def test_no_film_buffers_less_than_the_fixed_bound_used_to(app, sweep): + """The floor, which is the whole no-regression claim. + + The budget may only ever raise the read-ahead. A film whose bitrate spends + it in under the fixed bound keeps that bound exactly, and nothing that + played before pulls less than it did. + """ + floor = _const(app, "BUFFER_AHEAD_S") + for name, run in sweep.items(): + assert run["aheadLimitS"] >= floor, ( + f"{name}: bound came out at {run['aheadLimitS']}s, under the {floor}s " + "floor — this film now buffers less than it did before") + + +def test_a_film_nobody_started_does_not_spend_the_budget(app): + """A film on the loading screen cannot be interrupted, so it needs no lead. + + Autoplay is blocked on a phone more often than not, so this is the ordinary + first state of every playback and not a corner. Left to grow there, the + budget filled a small browser's entire buffer — 40 MB against the 8 MB the + fixed bound took — with film on a mobile connection that is paying for it, + and reached the ceiling that the fixed bound never came near. + """ + idle_small = _harness(playing=False, fileMB=493.5, durationS=7200, + capMB=40, netMBs=35, wallS=900) + assert idle_small["aheadLimitS"] == _const(app, "BUFFER_AHEAD_S"), ( + f"an unstarted film is allowed {idle_small['aheadLimitS']}s of " + "read-ahead — the budget grows before anyone has pressed play") + assert not idle_small["hitCeiling"], ( + f"a film nobody started filled the buffer to the ceiling " + f"({idle_small['heldInBufferMB']} MB)") + assert idle_small["quotaRefusals"] == 0 + + +def test_an_ordinary_film_buffers_far_past_the_fixed_bound(app, sweep): + """And the point of the exercise. + + Most of a library is well under five megabits, and that is where the + unused ceiling was. A bound that never actually rises is a budget that + bought nothing. + """ + floor = _const(app, "BUFFER_AHEAD_S") + low = sweep["0.6 Mbit/s"] + assert low["aheadLimitS"] > floor * 3, ( + f"a 0.6 Mbit/s film is allowed {low['aheadLimitS']}s of read-ahead " + f"against a {floor}s floor — the byte budget is not being spent") + assert low["bufferedAheadS"] > floor * 3, ( + f"allowed {low['aheadLimitS']}s but only reached " + f"{low['bufferedAheadS']}s — something other than the bound is holding") + + +def test_finding_the_ceiling_costs_a_handful_of_refusals_not_thousands(sweep): + """The budget is walked up to, never overshot wholesale. + + A flat budget discovers a browser's ceiling by being refused at it, and + that refusal repeats: a 48 MB budget against a 40 MB ceiling was measured + at 1386 refused appends on a film that had none before it. Each one is a + thrown exception, a warning on a phone's console and an append's work + thrown away, so the count is the thing to bound. + """ + for name, run in sweep.items(): + assert run["quotaRefusals"] < 50, ( + f"{name}: {run['quotaRefusals']} appends refused for quota — the " + "budget is overshooting the ceiling rather than walking up to it") + + +def test_the_walk_settles_and_playback_survives_it(app, sweep): + """Discovering the ceiling must not cost the film. + + Every rate that was servable at all has to still be watchable while the + budget is being found, and the run has to end below the ceiling rather than + pinned against it. A rate whose *floor* bound does not fit the ceiling is + excluded, not because the budget is allowed to break it, but because it was + already broken: the 9.3 Mbit/s run plays the same 100.8 s with the budget + as it did without one, to the tenth of a second. That wedge is §15.3, not + this commit. + """ + for name, run in sweep.items(): + assert not run["hitCeiling"], f"{name}: finished pinned at the ceiling" + if not _fits_under_the_ceiling(app, name, run): + continue + assert run["watchedS"] > 600, ( + f"{name}: only {run['watchedS']}s played in 900s of wall clock — " + "playback did not survive the walk") + + +def test_a_refused_append_is_not_retried_until_something_frees_room(app): + """Why the walk is cheap. + + An append refused for quota with nothing to evict will be refused again + until room appears, and pump() calls flushQueue on a clock — so the retry + alone cost a refusal several times a second for as long as it took the film + to play far enough for eviction to have anything to drop. `evictBehind` is + the only thing that gives a SourceBuffer room back, so it is the only thing + that may lift the hold; a timer or the playhead would move long before the + answer changes. + """ + player = _player(app) + flush = player[player.index("const flushQueue = useCallback("):] + flush = flush[:flush.index("\n }, [")] + assert "if (quotaHoldRef.current) return;" in flush, ( + "flushQueue retries a refused append on every pump tick") + evict = player[player.index("const evictBehind = useCallback("):] + evict = evict[:evict.index("\n }, [")] + assert "quotaHoldRef.current = false" in evict, ( + "nothing lifts the hold when room is freed, so the first refusal stops " + "every append for the rest of the film") + # A seek backwards leaves the playhead behind everything buffered, and + # reinitAt empties the buffer outright — so the hold must not survive it. + reinit = player[player.index("const reinitAt = async ("):] + reinit = reinit[:reinit.index("\n };")] + assert "quotaHoldRef.current = false" in reinit, ( + "a hold set before a seek survives the buffer it was held against") + + +def test_the_budget_starts_from_nothing_on_every_new_stream(app): + """A new stream starts from a known state, and this is two more refs. + + What one browser refused for a nine-megabit film says nothing about the + next one, and a budget left at the floor by the film before would silently + cap every film after it for the life of the page. + """ + player = _player(app) + i = player.index("appendingRef.current = false;\n endedRef.current = false;") + reset = player[i:i + 2000] + for ref, value in (("bitrateRef", "0"), + ("aheadBytesRef", "0"), + ("aheadCapRef", "BUFFER_AHEAD_MAX_BYTES"), + ("quotaHoldRef", "false")): + assert f"{ref}.current = {value};" in reset, ( + f"{ref} is not reset at the start of a stream, so the next film " + "inherits what this one learnt") # ── The shape the fix depends on ────────────────────────────────────────────── -- cgit v1.2.3