diff --git a/src/spice2x/api/capture_pump.cpp b/src/spice2x/api/capture_pump.cpp index fe75e56..ecedcab 100644 --- a/src/spice2x/api/capture_pump.cpp +++ b/src/spice2x/api/capture_pump.cpp @@ -1,6 +1,7 @@ #include "capture_pump.h" #include +#include #include #include "hooks/graphics/graphics.h" @@ -11,8 +12,8 @@ namespace api::capture_pump { std::array CONSUMER_M; - std::mutex CLAIMED_M; - std::array CLAIMED {}; + // read once per capture from the present thread, so it stays lock free + std::atomic CLAIMED[GRAPHICS_CAPTURE_SCREEN_NO] {}; bool valid_screen(int screen) { return 0 <= screen && screen < static_cast(GRAPHICS_CAPTURE_SCREEN_NO); @@ -37,14 +38,7 @@ namespace api::capture_pump { return false; } - std::lock_guard 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 lock(CLAIMED_M); CLAIMED[screen] = false; } + + bool screen_claimed(int screen) { + if (!valid_screen(screen)) { + return false; + } + + return CLAIMED[screen]; + } } diff --git a/src/spice2x/api/capture_pump.h b/src/spice2x/api/capture_pump.h index b8ae9d0..650fb38 100644 --- a/src/spice2x/api/capture_pump.h +++ b/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); } diff --git a/src/spice2x/hooks/graphics/backends/d3d9/d3d9_screenshot.cpp b/src/spice2x/hooks/graphics/backends/d3d9/d3d9_screenshot.cpp index e23f09c..ed8ddfb 100644 --- a/src/spice2x/hooks/graphics/backends/d3d9/d3d9_screenshot.cpp +++ b/src/spice2x/hooks/graphics/backends/d3d9/d3d9_screenshot.cpp @@ -15,6 +15,7 @@ #include #include +#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 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)) { - graphics_capture_skip(request.screen); + 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; } diff --git a/src/spice2x/launcher/options.cpp b/src/spice2x/launcher/options.cpp index de7beab..3bd1e1e 100644 --- a/src/spice2x/launcher/options.cpp +++ b/src/spice2x/launcher/options.cpp @@ -1745,7 +1745,7 @@ static const std::vector 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 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", },