From 2d0327149ec01235d21454c558b7f3d89d6875b3 Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 12 Oct 2026 19:59:51 +0200 Subject: [PATCH 1/2] fix(windows): repeat the owed screen slots so a loaded writer catches up A late writer caught up one tick per frame, and each tick read the screen back whenever it had changed. Under load a readback can cost more than a period, so the count never caught up and the video played ever earlier against the audio (217 ms over a 10 min take). Only the live slot reads back now; the slots already in the past repeat the last picture. Fixes #1148 --- .../native/wgc-capture/src/frame_slot_clock.h | 14 ++++- .../wgc-capture/src/frame_slot_clock_test.cpp | 59 +++++++++++++++++++ electron/native/wgc-capture/src/main.cpp | 14 ++++- 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/electron/native/wgc-capture/src/frame_slot_clock.h b/electron/native/wgc-capture/src/frame_slot_clock.h index ea28d601d..fb5b58f19 100644 --- a/electron/native/wgc-capture/src/frame_slot_clock.h +++ b/electron/native/wgc-capture/src/frame_slot_clock.h @@ -13,8 +13,8 @@ * early against the audio, which follows the wall clock * (getopenscreen/openscreen#945). This clock never resyncs: a late writer * finds the next frame already due and writes it at once, until the count - * has caught up. Repeating an unchanged screen costs no readback (#925), so - * catching up is cheap. + * has caught up. The slots it owes repeat the last picture, which costs no + * readback (#925), so catching up is cheap whatever the screen does. * * Times are recording time in 100 ns units: elapsed since the shared start, * pauses excluded, so a pause owes no frames. @@ -38,6 +38,16 @@ class FrameSlotClock { return originHns_ < 0 ? 0 : originHns_ + frames_ * 10'000'000 / fps_; } + // Whether the next frame's slot is already in the past at nowHns: the slot + // after it is due too. Such a slot is owed, not live, so it takes the + // picture already encoded instead of a new readback. Catching up with + // readbacks only works while a readback costs less than a period; under + // load a changing screen makes every tick a readback that costs more, the + // count never catches up, and the video plays ever earlier against the audio. + bool isBehind(int64_t nowHns) const { + return originHns_ >= 0 && nowHns >= originHns_ + (frames_ + 1) * 10'000'000 / fps_; + } + private: int fps_; int64_t originHns_ = -1; diff --git a/electron/native/wgc-capture/src/frame_slot_clock_test.cpp b/electron/native/wgc-capture/src/frame_slot_clock_test.cpp index 566e1f188..a75a67b4f 100644 --- a/electron/native/wgc-capture/src/frame_slot_clock_test.cpp +++ b/electron/native/wgc-capture/src/frame_slot_clock_test.cpp @@ -41,6 +41,40 @@ int64_t writeFor(int fps, int64_t durationHns, int64_t workHns, int64_t stallAtH return frames; } +struct LoadedTake { + int64_t frames = 0; + // How far the newest picture written ran ahead of the slot it was written + // in: the video shows an event that much before the audio plays it. + int64_t maxPictureLeadHns = 0; +}; + +// The writer under load on a screen that changes every tick: a readback costs +// `readbackHns`, a repeat of the last picture `repeatHns`. With +// `repeatPastSlots`, a slot the clock says is already past repeats instead of +// reading back, as main.cpp does; without it, every tick reads back. +LoadedTake writeLoadedFor(int fps, int64_t durationHns, int64_t readbackHns, int64_t repeatHns, + bool repeatPastSlots) { + FrameSlotClock clock(fps); + const int64_t periodHns = kSecondHns / fps; + LoadedTake take; + int64_t nowHns = 0; + int64_t pictureAtHns = 0; + while (nowHns < durationHns) { + const int64_t tickStartHns = nowHns; + if (repeatPastSlots && clock.isBehind(tickStartHns)) { + nowHns += repeatHns; + } else { + pictureAtHns = tickStartHns; + nowHns += readbackHns; + } + take.maxPictureLeadHns = std::max(take.maxPictureLeadHns, pictureAtHns - clock.nextDueHns()); + ++take.frames; + clock.advance(tickStartHns); + nowHns += std::clamp(clock.nextDueHns() - nowHns, 0, periodHns); + } + return take; +} + // Both streams of one take, the screen at 60 fps and the webcam at 30, on the writer's // rule: each clock is advanced with the instant its tick STARTED, so a stall inside the // tick (the screen readback) cannot give the two streams different origins. The first @@ -90,6 +124,31 @@ int main() { expectEqual("steady writer keeps the count", writeFor(60, 3 * kSecondHns, 50'000, 3 * kSecondHns, 0), 180); expectEqual("stalled writer catches up", writeFor(60, 3 * kSecondHns, 50'000, kSecondHns, 3'000'000), 180); + // Before the first frame nothing is owed, and a writer on time owes only the + // slot it is about to write. + FrameSlotClock owed(60); + expectEqual("nothing owed before the first frame", owed.isBehind(10 * kSecondHns), 0); + owed.advance(0); + expectEqual("live slot is not past", owed.isBehind(166'666), 0); + expectEqual("slot past once the next one is due", owed.isBehind(333'333), 1); + + // The v2.1.0 candidate's take (#945 follow-up): 10 min at 60 fps under a + // CPU/GPU load, screen 217 ms short of the webcam and the audio at the end. + // A readback that costs 20 ms on a changing screen, against a 16.7 ms + // period: catching up with readbacks wrote 30000 frames, 100 s of video + // played early. Repeating the past slots writes all 36000, and no picture + // runs more than one period ahead of its slot. + const int64_t tenMinutesHns = 600 * kSecondHns; + const LoadedTake readbackEveryTick = writeLoadedFor(60, tenMinutesHns, 200'000, 40'000, false); + expectEqual("readback catch-up falls behind under load", readbackEveryTick.frames, 30000); + const LoadedTake repeatPast = writeLoadedFor(60, tenMinutesHns, 200'000, 40'000, true); + expectEqual("repeated past slots keep the count under load", repeatPast.frames, 36000); + expectEqual("no picture leads its slot by more than a period", + repeatPast.maxPictureLeadHns <= kSecondHns / 60, 1); + // Only just over budget, as on that machine: still the full count. + expectEqual("readback just over a period keeps the count", + writeLoadedFor(60, tenMinutesHns, 170'000, 40'000, true).frames, 36000); + // A first tick that stalls 300 ms must not set the screen's origin 300 ms after the // webcam's: both streams of a 3 s take come out within one webcam frame of each other. const int64_t drift = streamsDriftFor(3 * kSecondHns, 3'000'000); diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index 90db83665..d969c869f 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -1114,6 +1114,9 @@ int wmain(int argc, wchar_t* argv[]) { // screen's would trail the webcam's by any stall in it, and the first tick // would give the two streams different origins. const int64_t tickStartHns = recordingElapsedHns(); + // A writer behind by more than this slot fills it with the last + // picture: only the latest owed slot gets a readback (#945). + const bool videoSlotIsPast = videoClock.isBehind(tickStartHns); Microsoft::WRL::ComPtr videoSample; Microsoft::WRL::ComPtr webcamSample; bool hasVideoSample = false; @@ -1288,10 +1291,16 @@ int wmain(int argc, wchar_t* argv[]) { // this struct's request: it falls back to the CPU path on // its own when the GPU path does not fit the machine. bool captured = false; - if (!usesDxgiInput && !pictureChanged && + if (!usesDxgiInput && (!pictureChanged || videoSlotIsPast) && encoder.repeatLastVideoSample(frameTimestampHns, videoSample)) { // Nothing new to read back (#925): the static screen, // most of a demo, costs no GPU copy and no conversion. + // A slot already in the past repeats too, changed + // screen or not: under load a readback can cost more + // than a period, and a writer that catches up with + // readbacks then never catches up at all (#945). The + // new picture waits in latestFrameTexture for the + // slot that is live. captured = true; } else if (usesDxgiInput) { captured = encoder.captureDxgiSample( @@ -1363,7 +1372,8 @@ int wmain(int argc, wchar_t* argv[]) { // at the nominal rate, so a skipped slot would put every later // frame early against the audio (#945). The next frame is simply // already due, and ticks run back to back until the count has - // caught up; on a screen that did not change they are repeats. + // caught up. Every slot but the live one is a repeat, which costs + // an encode and no readback, so the count does catch up. // Capped at one period so a stop is never waited on longer than // that, whatever the count says. const int64_t nowHns = recordingElapsedHns(); From 7da42e7a3752087c1f59be312b05bf2d2da1ff8a Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 12 Oct 2026 21:08:07 +0200 Subject: [PATCH 2/2] fix(windows): keep a change skipped on a past slot pending for the live one WGC delivers a frame only when the screen changes, so a change that landed on a past slot and was followed by a still screen never reached the video. PictureReadback keeps it pending until a live slot reads it back. --- .../native/wgc-capture/src/frame_slot_clock.h | 26 +++++++++++++++++++ .../wgc-capture/src/frame_slot_clock_test.cpp | 18 +++++++++++-- electron/native/wgc-capture/src/main.cpp | 8 +++--- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/electron/native/wgc-capture/src/frame_slot_clock.h b/electron/native/wgc-capture/src/frame_slot_clock.h index fb5b58f19..f13778fef 100644 --- a/electron/native/wgc-capture/src/frame_slot_clock.h +++ b/electron/native/wgc-capture/src/frame_slot_clock.h @@ -53,3 +53,29 @@ class FrameSlotClock { int64_t originHns_ = -1; int64_t frames_ = 0; }; + +/** + * Whether a tick reads the screen back or repeats the last picture. + * + * A changed screen is read back, an unchanged one repeated (#925). A slot + * already in the past (FrameSlotClock::isBehind) repeats either way, and the + * change it skipped stays pending until a live slot reads it: WGC delivers a + * frame only when the screen changes, so a change seen on a past slot and + * followed by a still screen would otherwise never reach the video. + */ +class PictureReadback { +public: + // `changed`: a new picture arrived this tick. True when this tick must + // read back, which also takes the pending change off the books. + bool readBackNow(bool changed, bool slotIsPast) { + pending_ = pending_ || changed; + if (!pending_ || slotIsPast) { + return false; + } + pending_ = false; + return true; + } + +private: + bool pending_ = false; +}; diff --git a/electron/native/wgc-capture/src/frame_slot_clock_test.cpp b/electron/native/wgc-capture/src/frame_slot_clock_test.cpp index a75a67b4f..89727bc44 100644 --- a/electron/native/wgc-capture/src/frame_slot_clock_test.cpp +++ b/electron/native/wgc-capture/src/frame_slot_clock_test.cpp @@ -51,17 +51,19 @@ struct LoadedTake { // The writer under load on a screen that changes every tick: a readback costs // `readbackHns`, a repeat of the last picture `repeatHns`. With // `repeatPastSlots`, a slot the clock says is already past repeats instead of -// reading back, as main.cpp does; without it, every tick reads back. +// reading back, through the same PictureReadback main.cpp asks; without it, +// every tick reads back. LoadedTake writeLoadedFor(int fps, int64_t durationHns, int64_t readbackHns, int64_t repeatHns, bool repeatPastSlots) { FrameSlotClock clock(fps); + PictureReadback readback; const int64_t periodHns = kSecondHns / fps; LoadedTake take; int64_t nowHns = 0; int64_t pictureAtHns = 0; while (nowHns < durationHns) { const int64_t tickStartHns = nowHns; - if (repeatPastSlots && clock.isBehind(tickStartHns)) { + if (!readback.readBackNow(true, repeatPastSlots && clock.isBehind(tickStartHns))) { nowHns += repeatHns; } else { pictureAtHns = tickStartHns; @@ -132,6 +134,18 @@ int main() { expectEqual("live slot is not past", owed.isBehind(166'666), 0); expectEqual("slot past once the next one is due", owed.isBehind(333'333), 1); + // A still screen repeats, a changed one reads back once. + PictureReadback readback; + expectEqual("still screen repeats", readback.readBackNow(false, false), 0); + expectEqual("changed screen reads back", readback.readBackNow(true, false), 1); + expectEqual("read change is not read again", readback.readBackNow(false, false), 0); + // A change that lands on a past slot, then a screen that stays still: WGC + // delivers no frame for the live slot, yet the change must reach it. + expectEqual("past slot repeats a changed screen", readback.readBackNow(true, true), 0); + expectEqual("change skipped on a past slot is read on the live one", + readback.readBackNow(false, false), 1); + expectEqual("and only once", readback.readBackNow(false, false), 0); + // The v2.1.0 candidate's take (#945 follow-up): 10 min at 60 fps under a // CPU/GPU load, screen 217 ms short of the webcam and the audio at the end. // A readback that costs 20 ms on a changing screen, against a 16.7 ms diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index d969c869f..545a1f9e4 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -1108,6 +1108,7 @@ int wmain(int argc, wchar_t* argv[]) { }; int64_t firstFrameTimestampHns = -1; int64_t latestFrameTimestampHns = 0; + PictureReadback pictureReadback; while (!control.stopRequested && !encodeFailed) { // One instant per tick for both clocks: read after the screen readback, the @@ -1291,7 +1292,8 @@ int wmain(int argc, wchar_t* argv[]) { // this struct's request: it falls back to the CPU path on // its own when the GPU path does not fit the machine. bool captured = false; - if (!usesDxgiInput && (!pictureChanged || videoSlotIsPast) && + if (!usesDxgiInput && + !pictureReadback.readBackNow(pictureChanged, videoSlotIsPast) && encoder.repeatLastVideoSample(frameTimestampHns, videoSample)) { // Nothing new to read back (#925): the static screen, // most of a demo, costs no GPU copy and no conversion. @@ -1299,8 +1301,8 @@ int wmain(int argc, wchar_t* argv[]) { // screen or not: under load a readback can cost more // than a period, and a writer that catches up with // readbacks then never catches up at all (#945). The - // new picture waits in latestFrameTexture for the - // slot that is live. + // new picture waits in latestFrameTexture, pending, + // for the first slot that is live. captured = true; } else if (usesDxgiInput) { captured = encoder.captureDxgiSample(