From ffcff368aea36df251b4e2478c7b61e1b7556976 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 25 Jul 2026 16:22:37 -0500 Subject: [PATCH 1/3] Bumping version to 1.0.0, SO 31, requiring OpenShotAudio 1.0.0 --- CMakeLists.txt | 4 ++-- src/CMakeLists.txt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 634784791..9a812bb7b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,8 +24,8 @@ For more information, please visit . set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/Modules") ################ PROJECT VERSION #################### -set(PROJECT_VERSION_FULL "0.7.0") -set(PROJECT_SO_VERSION 30) +set(PROJECT_VERSION_FULL "1.0.0") +set(PROJECT_SO_VERSION 31) # Remove the dash and anything following, to get the #.#.# version for project() STRING(REGEX REPLACE "\-.*$" "" VERSION_NUM "${PROJECT_VERSION_FULL}") diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 331ae405c..338d98946 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -225,7 +225,7 @@ add_feature_info("Wayland screen capture" OPENSHOT_WAYLAND_CAPTURE "Use xdg-desk # Find JUCE-based openshot Audio libraries if(NOT TARGET OpenShot::Audio) # Only load if necessary (not for integrated builds) - find_package(OpenShotAudio 0.6.0 REQUIRED) + find_package(OpenShotAudio 1.0.0 REQUIRED) endif() target_link_libraries(openshot PUBLIC OpenShot::Audio) From d4d87d8dcfffe52e853c4222e5e999511b32b695 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 25 Jul 2026 18:09:54 -0500 Subject: [PATCH 2/3] Synchronize screen capture shutdown --- src/ScreenCaptureReader.cpp | 17 ++++++++++++----- tests/ScreenCaptureReader.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/ScreenCaptureReader.cpp b/src/ScreenCaptureReader.cpp index 10359421b..8a545a517 100644 --- a/src/ScreenCaptureReader.cpp +++ b/src/ScreenCaptureReader.cpp @@ -611,6 +611,7 @@ ScreenCaptureReader::~ScreenCaptureReader() bool ScreenCaptureReader::IsOpen() { + const std::lock_guard lock(getFrameMutex); return backend_reader ? backend_reader->IsOpen() : is_open; } @@ -947,11 +948,11 @@ void ScreenCaptureReader::OpenDecoder() std::shared_ptr ScreenCaptureReader::GetFrame(int64_t number) { + const std::lock_guard lock(getFrameMutex); if (backend_reader) { if (!backend_reader->IsOpen()) { throw ReaderClosed("The ScreenCaptureReader is closed. Call Open() before GetFrame()."); } - const std::lock_guard lock(getFrameMutex); auto frame = backend_reader->GetFrame(number); if (system_audio && !manual_system_audio) system_audio->AddFrameAudio(frame, number, info.fps); return frame; @@ -960,7 +961,6 @@ std::shared_ptr ScreenCaptureReader::GetFrame(int64_t number) throw ReaderClosed("The ScreenCaptureReader is closed. Call Open() before GetFrame()."); } - const std::lock_guard lock(getFrameMutex); auto frame = DecodeNextFrame(number); if (system_audio && !manual_system_audio) system_audio->AddFrameAudio(frame, number, info.fps); return frame; @@ -1101,13 +1101,20 @@ std::shared_ptr ScreenCaptureReader::DecodeNextFrame(int64_t number) void ScreenCaptureReader::Close() { + // Signal blocking native reads before waiting for GetFrame(). The FFmpeg + // interrupt callback observes close_requested, while backend readers use + // Close() to wake their own blocking waits. close_requested = true; - if (system_audio) { - system_audio->Close(); - } if (backend_reader) { backend_reader->Close(); } + + // GetFrame() owns all decoder and system-audio use under this mutex. Do not + // release those resources until an interrupted read has completely exited. + const std::lock_guard lock(getFrameMutex); + if (system_audio) { + system_audio->Close(); + } if (packet) { av_packet_free(&packet); } diff --git a/tests/ScreenCaptureReader.cpp b/tests/ScreenCaptureReader.cpp index bb5a024b5..01b1c7e58 100644 --- a/tests/ScreenCaptureReader.cpp +++ b/tests/ScreenCaptureReader.cpp @@ -104,6 +104,33 @@ TEST_CASE("Screen capture reader reports configured video info", "[libopenshot][ #endif } +TEST_CASE("Closed screen capture reader consistently rejects frames", "[libopenshot][screencapturereader][lifecycle]") +{ + ScreenCaptureSettings settings; +#if defined(__linux__) + settings.backend = SCREEN_CAPTURE_X11; + settings.display = ":99.0"; +#elif defined(_WIN32) + settings.backend = SCREEN_CAPTURE_WINDOWS_GDI; + settings.display = "desktop"; +#elif defined(__APPLE__) + settings.backend = SCREEN_CAPTURE_MAC_AVFOUNDATION; + settings.display = "Capture screen 0:none"; +#else + return; +#endif + settings.width = 640; + settings.height = 360; + settings.fps = Fraction(30, 1); + + ScreenCaptureReader reader(settings); + CHECK_FALSE(reader.IsOpen()); + CHECK_NOTHROW(reader.Close()); + CHECK_NOTHROW(reader.Close()); + CHECK_FALSE(reader.IsOpen()); + CHECK_THROWS_AS(reader.GetFrame(1), ReaderClosed); +} + TEST_CASE("Screen capture system audio settings follow backend capability", "[libopenshot][screencapturereader][audio]") { ScreenCaptureSettings settings; From 7bf0aeb4bc5c706291127f787bf7eecf5d7b4b4f Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sun, 26 Jul 2026 00:10:10 -0500 Subject: [PATCH 3/3] Harden Wayland PipeWire window capture --- src/WaylandBufferUtilities.h | 133 ++++++++++++++++++++++++++++ src/WaylandScreenCaptureReader.cpp | 134 ++++++++++++++++++++--------- tests/ScreenCaptureReader.cpp | 83 ++++++++++++++++++ 3 files changed, 311 insertions(+), 39 deletions(-) create mode 100644 src/WaylandBufferUtilities.h diff --git a/src/WaylandBufferUtilities.h b/src/WaylandBufferUtilities.h new file mode 100644 index 000000000..caf989577 --- /dev/null +++ b/src/WaylandBufferUtilities.h @@ -0,0 +1,133 @@ +/** + * @file + * @brief Bounds-safe helpers for mapped PipeWire video buffers + */ + +// Copyright (c) 2008-2026 OpenShot Studios, LLC +// +// SPDX-License-Identifier: LGPL-3.0-or-later + +#ifndef OPENSHOT_WAYLAND_BUFFER_UTILITIES_H +#define OPENSHOT_WAYLAND_BUFFER_UTILITIES_H + +#include +#include +#include +#include +#include + +namespace openshot::wayland +{ + struct PackedVideoLayout + { + size_t offset = 0; + size_t valid_size = 0; + int stride = 0; + int crop_x = 0; + int crop_y = 0; + int width = 0; + int height = 0; + bool valid = false; + }; + + inline PackedVideoLayout ResolvePackedVideoLayout( + size_t max_size, + size_t chunk_offset, + size_t chunk_size, + int chunk_stride, + int stream_width, + int stream_height, + int crop_x, + int crop_y, + int crop_width, + int crop_height) + { + PackedVideoLayout layout; + if (max_size == 0 || stream_width <= 0 || stream_height <= 0 + || stream_width > std::numeric_limits::max() / 4 + || chunk_stride < 0) { + return layout; + } + + layout.offset = chunk_offset % max_size; + layout.valid_size = std::min(chunk_size, max_size); + layout.stride = chunk_stride > 0 ? chunk_stride : stream_width * 4; + if (layout.stride < 4 || layout.valid_size < static_cast(layout.stride)) { + return layout; + } + + const int readable_width = std::min( + stream_width, + layout.stride / 4); + const int readable_height = std::min( + stream_height, + static_cast(layout.valid_size / static_cast(layout.stride))); + + layout.crop_x = std::max(0, crop_x); + layout.crop_y = std::max(0, crop_y); + if (layout.crop_x >= readable_width || layout.crop_y >= readable_height) { + return layout; + } + + const int requested_width = crop_width > 0 + ? crop_width + : stream_width - layout.crop_x; + const int requested_height = crop_height > 0 + ? crop_height + : stream_height - layout.crop_y; + layout.width = std::min(requested_width, readable_width - layout.crop_x); + layout.height = std::min(requested_height, readable_height - layout.crop_y); + + // H.264 requires even dimensions, and capture callers already expect them. + layout.width -= layout.width % 2; + layout.height -= layout.height % 2; + if (layout.width <= 0 || layout.height <= 0) { + return layout; + } + + const size_t final_row_end = + static_cast(layout.crop_y + layout.height - 1) * layout.stride + + static_cast(layout.crop_x + layout.width) * 4; + if (final_row_end > layout.valid_size) { + return layout; + } + + layout.valid = true; + return layout; + } + + inline int DamageFrameWaitMilliseconds(int fps_num, int fps_den, bool have_last_frame) + { + if (!have_last_frame) { + return 5000; + } + const double fps = fps_num > 0 && fps_den > 0 + ? static_cast(fps_num) / fps_den + : 30.0; + return std::max(1, static_cast(1000.0 / std::max(1.0, fps))); + } + + inline bool CopyWrappedBytes( + const uint8_t* source, + size_t max_size, + size_t chunk_offset, + size_t logical_offset, + uint8_t* destination, + size_t byte_count) + { + if (!source || !destination || max_size == 0 || byte_count > max_size) { + return false; + } + const size_t physical_offset = ( + (chunk_offset % max_size) + (logical_offset % max_size) + ) % max_size; + const size_t first_size = std::min(byte_count, max_size - physical_offset); + std::memcpy(destination, source + physical_offset, first_size); + if (first_size < byte_count) { + std::memcpy(destination + first_size, source, byte_count - first_size); + } + return true; + } +} + +#endif diff --git a/src/WaylandScreenCaptureReader.cpp b/src/WaylandScreenCaptureReader.cpp index 8147877f3..b67de7176 100644 --- a/src/WaylandScreenCaptureReader.cpp +++ b/src/WaylandScreenCaptureReader.cpp @@ -35,6 +35,7 @@ #include "Exceptions.h" #include "Frame.h" +#include "WaylandBufferUtilities.h" #include "ZmqLogger.h" using namespace openshot; @@ -397,7 +398,12 @@ class WaylandScreenCaptureReader final : public ScreenCaptureReader::CaptureBack CapturedFrame captured; { std::unique_lock lock(queue_mutex); - const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + const auto wait_duration = std::chrono::milliseconds( + wayland::DamageFrameWaitMilliseconds( + settings.fps.num, + settings.fps.den, + have_last_frame)); + const auto deadline = std::chrono::steady_clock::now() + wait_duration; while (frame_queue.empty() && !stream_error && open && std::chrono::steady_clock::now() < deadline) { lock.unlock(); while (g_main_context_iteration(nullptr, FALSE)) { @@ -406,16 +412,28 @@ class WaylandScreenCaptureReader final : public ScreenCaptureReader::CaptureBack queue_condition.wait_for(lock, std::chrono::milliseconds(100)); } if (frame_queue.empty() && !stream_error && open) { - throw InvalidFile("Timed out waiting for a Wayland capture frame.", "wayland"); + if (have_last_frame) { + captured = last_frame; + } else { + throw InvalidFile("Timed out waiting for the first Wayland capture frame.", "wayland"); + } } if (stream_error) { throw InvalidFile("Wayland capture stream failed.", "wayland"); } if (frame_queue.empty()) { - throw ReaderClosed("The Wayland screen capture stream is closed."); + if (!open) { + throw ReaderClosed("The Wayland screen capture stream is closed."); + } + if (!have_last_frame) { + throw InvalidFile("Timed out waiting for the first Wayland capture frame.", "wayland"); + } + } else { + captured = std::move(frame_queue.front()); + frame_queue.pop_front(); + last_frame = captured; + have_last_frame = true; } - captured = std::move(frame_queue.front()); - frame_queue.pop_front(); } const int bytes_per_pixel = 4; @@ -680,10 +698,27 @@ class WaylandScreenCaptureReader final : public ScreenCaptureReader::CaptureBack self->info.display_ratio.Reduce(); } - uint8_t params_buffer[256]; + uint8_t params_buffer[1024]; spa_pod_builder builder = SPA_POD_BUILDER_INIT(params_buffer, sizeof(params_buffer)); - const spa_pod* params[2]; + const int stride = self->stream_width * 4; + const int buffer_size = stride * self->stream_height; + const spa_pod* params[3]; params[0] = static_cast(spa_pod_builder_add_object( + &builder, + SPA_TYPE_OBJECT_ParamBuffers, + SPA_PARAM_Buffers, + SPA_PARAM_BUFFERS_buffers, + SPA_POD_CHOICE_RANGE_Int(8, 2, 32), + SPA_PARAM_BUFFERS_blocks, + SPA_POD_Int(1), + SPA_PARAM_BUFFERS_size, + SPA_POD_Int(buffer_size), + SPA_PARAM_BUFFERS_stride, + SPA_POD_Int(stride), + SPA_PARAM_BUFFERS_dataType, + SPA_POD_CHOICE_FLAGS_Int( + (1 << SPA_DATA_MemPtr) | (1 << SPA_DATA_MemFd)))); + params[1] = static_cast(spa_pod_builder_add_object( &builder, SPA_TYPE_OBJECT_ParamMeta, SPA_PARAM_Meta, @@ -691,7 +726,7 @@ class WaylandScreenCaptureReader final : public ScreenCaptureReader::CaptureBack SPA_POD_Id(SPA_META_Header), SPA_PARAM_META_size, SPA_POD_Int(sizeof(spa_meta_header)))); - params[1] = static_cast(spa_pod_builder_add_object( + params[2] = static_cast(spa_pod_builder_add_object( &builder, SPA_TYPE_OBJECT_ParamMeta, SPA_PARAM_Meta, @@ -699,13 +734,25 @@ class WaylandScreenCaptureReader final : public ScreenCaptureReader::CaptureBack SPA_POD_Id(SPA_META_VideoCrop), SPA_PARAM_META_size, SPA_POD_Int(sizeof(spa_meta_region)))); - pw_stream_update_params(self->stream, params, 2); + pw_stream_update_params(self->stream, params, 3); } static void OnStreamProcess(void* data) { auto* self = static_cast(data); - pw_buffer* buffer = pw_stream_dequeue_buffer(self->stream); + // Drain the stream and process only the newest buffer. Returning stale + // buffers immediately prevents a damage-driven window stream from + // accumulating latency after it becomes visible again. + pw_buffer* buffer = nullptr; + pw_buffer* next_buffer = pw_stream_dequeue_buffer(self->stream); + while (next_buffer) { + if (buffer) { + pw_stream_queue_buffer(self->stream, buffer); + self->dropped_packets++; + } + buffer = next_buffer; + next_buffer = pw_stream_dequeue_buffer(self->stream); + } if (!buffer) { return; } @@ -731,9 +778,6 @@ class WaylandScreenCaptureReader final : public ScreenCaptureReader::CaptureBack spa_data& data = spa_buffer->datas[0]; const spa_chunk* chunk = data.chunk; - const uint8_t* src = static_cast(data.data) + (chunk ? chunk->offset : 0); - const int stride = chunk && chunk->stride > 0 ? chunk->stride : stream_width * 4; - const int readable_rows = chunk && chunk->size > 0 ? static_cast(chunk->size) / stride : stream_height; int crop_x = 0; int crop_y = 0; int crop_width = stream_width; @@ -754,35 +798,45 @@ class WaylandScreenCaptureReader final : public ScreenCaptureReader::CaptureBack crop_logged = true; } } - if (crop_width <= 0 || crop_height <= 0) { - dropped_packets++; - return; - } - if (crop_width % 2 != 0) { - crop_width--; - } - if (crop_height % 2 != 0) { - crop_height--; - } - if (crop_width <= 0 || crop_height <= 0) { - dropped_packets++; - return; - } - const int rows = std::min(crop_height, readable_rows - crop_y); - if (rows <= 0) { + + const auto layout = wayland::ResolvePackedVideoLayout( + static_cast(data.maxsize), + chunk ? static_cast(chunk->offset) : 0, + chunk ? static_cast(chunk->size) : static_cast(data.maxsize), + chunk ? chunk->stride : 0, + stream_width, + stream_height, + crop_x, + crop_y, + crop_width, + crop_height); + if (!layout.valid) { dropped_packets++; return; } CapturedFrame frame; - frame.width = crop_width; - frame.height = crop_height; + frame.width = layout.width; + frame.height = layout.height; frame.rgba.assign(static_cast(frame.width) * frame.height * 4, 0); - for (int y = 0; y < rows; ++y) { - const uint8_t* row = src + static_cast(y + crop_y) * stride; - for (int x = 0; x < crop_width; ++x) { - const uint8_t* pixel = row + static_cast(x + crop_x) * 4; + std::vector source_row(static_cast(frame.width) * 4); + for (int y = 0; y < frame.height; ++y) { + const size_t logical_offset = + static_cast(y + layout.crop_y) * layout.stride + + static_cast(layout.crop_x) * 4; + if (!wayland::CopyWrappedBytes( + static_cast(data.data), + static_cast(data.maxsize), + layout.offset, + logical_offset, + source_row.data(), + source_row.size())) { + dropped_packets++; + return; + } + for (int x = 0; x < frame.width; ++x) { + const uint8_t* pixel = source_row.data() + static_cast(x) * 4; uint8_t r = 0; uint8_t g = 0; uint8_t b = 0; @@ -818,10 +872,10 @@ class WaylandScreenCaptureReader final : public ScreenCaptureReader::CaptureBack } frame_queue.push_back(std::move(frame)); } - if (info.width != crop_width || info.height != crop_height) { - info.width = crop_width; - info.height = crop_height; - info.display_ratio = Fraction(crop_width, crop_height); + if (info.width != frame.width || info.height != frame.height) { + info.width = frame.width; + info.height = frame.height; + info.display_ratio = Fraction(frame.width, frame.height); info.display_ratio.Reduce(); } queue_condition.notify_one(); @@ -901,6 +955,8 @@ class WaylandScreenCaptureReader final : public ScreenCaptureReader::CaptureBack std::mutex queue_mutex; std::condition_variable queue_condition; std::deque frame_queue; + CapturedFrame last_frame; + bool have_last_frame = false; static const pw_stream_events stream_events; }; diff --git a/tests/ScreenCaptureReader.cpp b/tests/ScreenCaptureReader.cpp index 01b1c7e58..b0a50316e 100644 --- a/tests/ScreenCaptureReader.cpp +++ b/tests/ScreenCaptureReader.cpp @@ -14,12 +14,95 @@ #include "Exceptions.h" #include "ScreenCaptureReader.h" +#include "WaylandBufferUtilities.h" #include +#include #include +#include using namespace openshot; +TEST_CASE("Wayland packed video layout clamps unsafe PipeWire metadata", + "[libopenshot][screencapturereader][wayland]") +{ + SECTION("chunk offsets follow PipeWire modulo semantics") + { + const auto layout = wayland::ResolvePackedVideoLayout( + 32, 37, 32, 16, 4, 2, 0, 0, 4, 2); + REQUIRE(layout.valid); + CHECK(layout.offset == 5); + CHECK(layout.valid_size == 32); + CHECK(layout.width == 4); + CHECK(layout.height == 2); + } + + SECTION("chunk size limits readable rows") + { + const auto layout = wayland::ResolvePackedVideoLayout( + 64, 0, 32, 16, 4, 4, 0, 0, 4, 4); + REQUIRE(layout.valid); + CHECK(layout.width == 4); + CHECK(layout.height == 2); + } + + SECTION("empty chunks are rejected instead of reading stale allocation data") + { + const auto layout = wayland::ResolvePackedVideoLayout( + 64, 0, 0, 16, 4, 4, 0, 0, 4, 4); + CHECK_FALSE(layout.valid); + } + + SECTION("crop width cannot exceed row stride") + { + const auto layout = wayland::ResolvePackedVideoLayout( + 64, 0, 64, 16, 8, 4, 2, 0, 6, 4); + REQUIRE(layout.valid); + CHECK(layout.crop_x == 2); + CHECK(layout.width == 2); + CHECK(layout.height == 4); + } + + SECTION("crop outside readable memory is rejected") + { + const auto layout = wayland::ResolvePackedVideoLayout( + 64, 0, 64, 16, 8, 4, 4, 0, 4, 4); + CHECK_FALSE(layout.valid); + } + + SECTION("negative producer stride is rejected safely") + { + const auto layout = wayland::ResolvePackedVideoLayout( + 64, 0, 64, -16, 4, 4, 0, 0, 4, 4); + CHECK_FALSE(layout.valid); + } +} + +TEST_CASE("Wayland packed video rows copy safely across ring-buffer wrap", + "[libopenshot][screencapturereader][wayland]") +{ + const std::vector source {0, 1, 2, 3, 4, 5, 6, 7}; + std::vector destination(6, 0); + + REQUIRE(wayland::CopyWrappedBytes( + source.data(), source.size(), 6, 0, + destination.data(), destination.size())); + CHECK(destination == std::vector {6, 7, 0, 1, 2, 3}); + + CHECK_FALSE(wayland::CopyWrappedBytes( + source.data(), source.size(), 0, 0, + destination.data(), source.size() + 1)); +} + +TEST_CASE("Wayland damage-driven streams repeat at the requested cadence", + "[libopenshot][screencapturereader][wayland]") +{ + CHECK(wayland::DamageFrameWaitMilliseconds(30, 1, false) == 5000); + CHECK(wayland::DamageFrameWaitMilliseconds(30, 1, true) == 33); + CHECK(wayland::DamageFrameWaitMilliseconds(60, 1, true) == 16); + CHECK(wayland::DamageFrameWaitMilliseconds(0, 0, true) == 33); +} + TEST_CASE("Screen capture settings validation", "[libopenshot][screencapturereader]") { ScreenCaptureSettings settings;