graphics: move captures off-thread when streaming (#885)

## Link to GitHub Issue or related Pull Request, if one exists
#0

## Description of change

Moves the API capture readback off the game's Present thread while a
video stream client is connected.

The readback is a `LockRect` plus a memcpy of the whole back buffer,
roughly 635us at 720p and 1270us at 1080p. On the Present thread that
comes out of the game's frame budget: TDJ (at 120Hz) dropped to 117fps
with a 60fps stream running, and reading on a pool thread instead gave
the full 120 back.

Only streaming takes the off-thread path, gated on a new
`capture_pump::screen_claimed()`.

Screenshots, one-off API captures, and `THREAD_BAN` games all keep the
existing inline read for compat reasons. A pool thread in `LockRect`
while the Present thread sat inside `GetRenderTargetData` deadlocks DDR
X2 for example.

`CLAIMED[]` becomes `std::atomic<bool>` so the capture path does not
take a lock on the Present thread. The read pool has a single worker so
frames cannot be enqueued out of order, and both capture pools are never
destroyed so a late read cannot queue onto a torn-down pool.

The capture pipeline itself is unchanged: `GetRenderTargetData` is still
synchronous on the Present thread.

## Testing
DDR X2
World
IIDX TDJ
SDVX VM

src/spice2x/api/capture_pump.cpp

@@ -1,6 +1,7 @@
 #include "capture_pump.h"
 
 #include <array>
+#include <atomic>
 #include <mutex>
 
 #include "hooks/graphics/graphics.h"
@@ -11,8 +12,8 @@ namespace api::capture_pump {
 
         std::array<std::mutex, GRAPHICS_CAPTURE_SCREEN_NO> CONSUMER_M;
 
-        std::mutex CLAIMED_M;
-        std::array<bool, GRAPHICS_CAPTURE_SCREEN_NO> CLAIMED {};
+        // read once per capture from the present thread, so it stays lock free
+        std::atomic<bool> CLAIMED[GRAPHICS_CAPTURE_SCREEN_NO] {};
 
         bool valid_screen(int screen) {
             return 0 <= screen && screen < static_cast<int>(GRAPHICS_CAPTURE_SCREEN_NO);
@@ -37,14 +38,7 @@ namespace api::capture_pump {
             return false;
         }
 
-        std::lock_guard<std::mutex> lock(CLAIMED_M);
-
-        if (CLAIMED[screen]) {
-            return false;
-        }
-
-        CLAIMED[screen] = true;
-        return true;
+        return !CLAIMED[screen].exchange(true);
     }
 
     void release_screen(int screen) {
@@ -52,7 +46,14 @@ namespace api::capture_pump {
             return;
         }
 
-        std::lock_guard<std::mutex> lock(CLAIMED_M);
         CLAIMED[screen] = false;
     }
+
+    bool screen_claimed(int screen) {
+        if (!valid_screen(screen)) {
+            return false;
+        }
+
+        return CLAIMED[screen];
+    }
 }

src/spice2x/api/capture_pump.h

@@ -21,4 +21,7 @@ namespace api::capture_pump {
     // a screen carries one stream at a time; false when another connection already holds it
     bool claim_screen(int screen);
     void release_screen(int screen);
+
+    // true while a video stream client holds this screen
+    bool screen_claimed(int screen);
 }

src/spice2x/hooks/graphics/backends/d3d9/d3d9_screenshot.cpp

@@ -15,6 +15,7 @@
 #include <external/robin_hood.h>
 #include <external/fpng/fpng.h>
 
+#include "api/capture_pump.h"
 #include "avs/game.h"
 #include "hooks/graphics/graphics.h"
 #include "misc/clipboard.h"
@@ -169,6 +170,13 @@ ThreadPool &encode_pool() {
     return *instance;
 }
 
+// where a capture's pixels are converted and handed to the api. never destroyed: the read
+// pool below can still be working at process exit, and it queues onto this one
+ThreadPool &capture_save_pool() {
+    static auto *instance = new ThreadPool(2);
+    return *instance;
+}
+
 // normalize the supported D3D formats to packed 24bpp RGB. callers screen the
 // format through surface_pixel_size first, so the black fill below is a fallback
 void surface_to_rgb(
@@ -437,11 +445,49 @@ static void dispatch_capture_save(PendingCapture capture) {
     if (image_processing_must_be_inline()) {
         capture_process();
     } else {
-        static auto pool = ThreadPool(2);
-        pool.add(std::move(capture_process));
+        capture_save_pool().add(std::move(capture_process));
     }
 }
 
+// destroying the BackbufferCopy returns its surface to the pool, which is a device call, so
+// it has to happen on whichever thread was cleared to do the read
+static void read_and_dispatch_capture(int screen, BackbufferCopy copy) {
+    PendingCapture capture;
+    if (!read_capture_surface(copy, capture)) {
+        graphics_capture_skip(screen);
+        return;
+    }
+
+    dispatch_capture_save(std::move(capture));
+}
+
+// Whether the readback runs on the present thread or a pool thread trades the game's frame
+// time against the risk of two threads being inside the device at once.
+//
+// The read is a LockRect plus a row by row memcpy of the whole back buffer: roughly 635us at
+// 720p and 1270us at 1080p. On the present thread that comes straight out of the game's frame
+// budget, and at 120Hz with a 60fps stream running it measured as a drop to 117fps. Moving it
+// to a pool thread gave the full 120 back.
+//
+// Only streaming is worth that trade. It is the only path that pays the cost on every frame,
+// and it is the only one the user has opted into by connecting a client. Screenshots and the
+// one off api captures stay inline: they are rare enough that a single slow frame does not
+// matter, and the hazard being avoided is reproduced rather than theoretical, since a pool
+// thread in LockRect while the present thread sat inside GetRenderTargetData deadlocked
+// DDR X2, whose device has no internal locking. Games already known to dislike threaded image
+// processing are excluded as well, on the assumption that whatever breaks them applies here.
+static bool capture_read_off_thread(int screen) {
+    return api::capture_pump::screen_claimed(screen) && !image_processing_must_be_inline();
+}
+
+ThreadPool &capture_read_pool() {
+    // one worker, so reads finish in the order they were submitted: a second worker could
+    // overtake a descheduled one and enqueue a stale frame over a newer one. never destroyed,
+    // so a read still running at process exit cannot touch a dead pool
+    static auto *instance = new ThreadPool(1);
+    return *instance;
+}
+
 // by this point the pixels are plain memory, so none of this needs the device
 static void dispatch_screenshot_save(std::vector<PendingWrite> writes, size_t screen_count) {
     auto screenshot_process = [writes = std::move(writes), screen_count]() mutable {
@@ -623,14 +669,34 @@ static void process_image_request(
     }
 
     if (!screenshot) {
-        PendingCapture capture;
-        if (!read_capture_surface(copies.front(), capture)) {
+        auto copy = std::move(copies.front());
+        copies.clear();
+
+        if (capture_read_off_thread(request.screen)) {
+            try {
+                capture_read_pool().add(
+                        [screen = request.screen, copy = std::move(copy)]() mutable {
+                    // an escape from here would cross a thread boundary and terminate
+                    try {
+                        read_and_dispatch_capture(screen, std::move(copy));
+                    } catch (const std::exception &error) {
+                        log_warning("graphics::d3d9", "capture read failed: {}", error.what());
+                        graphics_capture_skip(screen);
+                    } catch (...) {
+                        log_warning("graphics::d3d9", "capture read failed");
+                        graphics_capture_skip(screen);
+                    }
+                });
+            } catch (const std::exception &) {
+                // the copy went into the lambda before the queue could fail, so there is
+                // nothing left to read here and the client misses this frame
                 graphics_capture_skip(request.screen);
+            }
+
             return;
         }
 
-        copies.clear();
-        dispatch_capture_save(std::move(capture));
+        read_and_dispatch_capture(request.screen, std::move(copy));
         return;
     }
 

src/spice2x/launcher/options.cpp

@@ -1745,7 +1745,7 @@ static const std::vector<OptionDefinition> OPTION_DEFINITIONS = {
     },
     {
         // APIStreamEnable
-        .title = "API Video Stream Server Enable",
+        .title = "API Video Stream Server Enable (EXPERIMENTAL)",
         .name = "apistream",
         .desc = "Serves the mirrored screen as a video stream, on the API port plus two; "
             "alternative to API screen capture. Requires -api.\n\n"
@@ -1754,7 +1754,8 @@ static const std::vector<OptionDefinition> OPTION_DEFINITIONS = {
             "Parameters: screen (0-3), fps (1-60, default 30), q (1-100, default 70).\n\n"
             "Example with -api 1337: http://host:1339/stream.h264?fps=30&q=70\n\n"
             "VIEW ONLY - touch input still requires -api. "
-            "No password protection or encryption of any kind; video sent in the clear!",
+            "No password protection or encryption of any kind; video sent in the clear!\n\n"
+            "Streaming is known to cause older games to hang and crash.",
         .type = OptionType::Bool,
         .category = "Companion & API",
     },