From 61bda66b80b52dfccda87de52f28e5b69340788a Mon Sep 17 00:00:00 2001 From: Aleksandr Voitenko Date: Wed, 30 Sep 2026 09:36:41 +1300 Subject: [PATCH 1/5] Fix media cache accounting during deferred source updates --- js/module.ts | 12 +- obs-studio-server/source/memory-manager.cpp | 71 +++- obs-studio-server/source/memory-manager.h | 48 ++- obs-studio-server/source/osn-source.cpp | 8 +- .../tests/test-memory-manager.cpp | 326 +++++++++++++++++- 5 files changed, 450 insertions(+), 15 deletions(-) diff --git a/js/module.ts b/js/module.ts index 44e60e698..250c23e9a 100644 --- a/js/module.ts +++ b/js/module.ts @@ -1302,9 +1302,15 @@ export interface ITransition extends ISource { export interface IConfigurable { /** - * Update the settings of the source instance - * correlating to the values held within the - * object passed. + * Merge settings into this live instance. Existing references remain valid. + * For video sources, the plugin applies the update on a later graphics tick; + * media playback and cache reevaluation may still be pending when this returns. + * + * @param settings JSON-serializable settings to merge; omitted keys are preserved. + * @returns Nothing. Success does not guarantee that the plugin has finished applying the settings. + * @throws {TypeError} If settings cannot be converted to an object or JSON serialized. + * @throws {Error} If the native reference is invalid or the IPC request fails. + * An IPC failure does not guarantee that a dispatched update was rolled back. */ update(settings: ISettings): void; diff --git a/obs-studio-server/source/memory-manager.cpp b/obs-studio-server/source/memory-manager.cpp index 6d92510b8..356da250d 100644 --- a/obs-studio-server/source/memory-manager.cpp +++ b/obs-studio-server/source/memory-manager.cpp @@ -59,6 +59,10 @@ struct MediaCacheManager::SourceEntry { std::atomic revision{1}; std::atomic removed{false}; + // Settings guards and the graphics callback access these under the queue mutex. + unsigned settingsUpdatesInProgress = 0; + uint64_t queryAllowedFromTick = 0; + // Only the worker changes these fields, under the queue mutex. uint64_t scheduledRevision = 0; uint64_t reservedBytes = 0; // Includes an enable operation awaiting completion. @@ -83,6 +87,23 @@ MediaCacheManager::~MediaCacheManager() shutdown(); } +MediaCacheManager::SourceSettingsUpdate::SourceSettingsUpdate(MediaCacheManager *manager, std::shared_ptr source) noexcept + : m_manager(manager), m_sourceEntry(std::move(source)) +{ +} + +MediaCacheManager::SourceSettingsUpdate::SourceSettingsUpdate(SourceSettingsUpdate &&other) noexcept + : m_manager(std::exchange(other.m_manager, nullptr)), m_sourceEntry(std::move(other.m_sourceEntry)) +{ +} + +MediaCacheManager::SourceSettingsUpdate::~SourceSettingsUpdate() +{ + if (m_manager) + m_manager->finishSourceSettingsUpdate(m_sourceEntry); + // m_sourceEntry releases its OBS reference after the queue mutex is unlocked. +} + void MediaCacheManager::initialize() { // Initialization and shutdown are serialized by the OBS API lifecycle. @@ -146,6 +167,40 @@ void MediaCacheManager::requestCacheUpdate(obs_source_t *source) m_changed.notify_all(); } +MediaCacheManager::SourceSettingsUpdate MediaCacheManager::trackSourceSettingsUpdate(obs_source_t *source) +{ + std::shared_ptr entry; + { + std::lock_guard lock(m_mutex); + auto it = m_sources.find(source); + if (!m_accepting || it == m_sources.end()) + return {nullptr, {}}; + entry = it->second; + ++entry->revision; + ++entry->settingsUpdatesInProgress; + m_notified = true; + } + m_changed.notify_all(); + return {this, std::move(entry)}; +} + +void MediaCacheManager::finishSourceSettingsUpdate(const std::shared_ptr &source) +{ + { + std::lock_guard lock(m_mutex); + if (!m_accepting || source->removed) + return; + --source->settingsUpdatesInProgress; + ++source->revision; + // Tick callbacks run before deferred source updates. The first callback + // after this write must skip querying; the following callback is after + // OBS has had a source-update phase, even if the write finished mid-frame. + source->queryAllowedFromTick = m_graphicsTick + 2; + m_notified = true; + } + m_changed.notify_all(); +} + void MediaCacheManager::requestAllCacheUpdates() { { @@ -237,10 +292,20 @@ void MediaCacheManager::graphicsTick(void *param, float) if (!manager.m_accepting) return; - // Take one batch per tick. - while (!manager.m_graphicsJobs.empty() && results.size() < JOBS_PER_TICK) { - results.push_back({std::move(manager.m_graphicsJobs.front())}); + ++manager.m_graphicsTick; + // Scan one bounded batch. Keep delayed queries queued without consuming + // readiness retries or blocking work for another source behind them. + const auto count = std::min(manager.m_graphicsJobs.size(), JOBS_PER_TICK); + for (size_t i = 0; i < count; ++i) { + auto job = std::move(manager.m_graphicsJobs.front()); manager.m_graphicsJobs.pop_front(); + const auto &entry = *job.source; + if (job.type == JobType::QuerySource && !entry.removed && entry.revision == job.revision && + (entry.settingsUpdatesInProgress || manager.m_graphicsTick < entry.queryAllowedFromTick)) { + manager.m_graphicsJobs.push_back(std::move(job)); + continue; + } + results.push_back({std::move(job)}); } } if (results.empty()) diff --git a/obs-studio-server/source/memory-manager.h b/obs-studio-server/source/memory-manager.h index 60866945a..967cae933 100644 --- a/obs-studio-server/source/memory-manager.h +++ b/obs-studio-server/source/memory-manager.h @@ -43,8 +43,34 @@ // not keep that particular player alive, and our mutex does not protect it. // Run those queries in the graphics tick to serialize them with replacement, // then return copied metadata to the worker. +// Settings become visible before the plugin applies them. External writers must +// use trackSourceSettingsUpdate() so queries wait for a source-update phase and +// cannot associate a new filename with the previous player's metadata. class MediaCacheManager { + struct SourceEntry; + public: + // Keeps cache queries pending while the caller changes a source's settings. + class SourceSettingsUpdate { + public: + // Transfers responsibility for finishing the update; the moved-from guard + // is inactive. The manager must outlive the guard. + SourceSettingsUpdate(SourceSettingsUpdate &&other) noexcept; + // Finishes tracking and schedules evaluation after OBS can apply the update. + // Safe after source unregistration or manager shutdown, but must run before + // obs_shutdown(): the guard still retains its source reference. + ~SourceSettingsUpdate(); + SourceSettingsUpdate(const SourceSettingsUpdate &) = delete; + SourceSettingsUpdate &operator=(const SourceSettingsUpdate &) = delete; + SourceSettingsUpdate &operator=(SourceSettingsUpdate &&) = delete; + + private: + friend class MediaCacheManager; + SourceSettingsUpdate(MediaCacheManager *manager, std::shared_ptr source) noexcept; + MediaCacheManager *m_manager; + std::shared_ptr m_sourceEntry; + }; + // Returns a non-owning reference to the process-wide singleton. Access is // thread-safe, but does not initialize OBS or start the cache worker. static MediaCacheManager &GetInstance(); @@ -75,7 +101,19 @@ class MediaCacheManager { // obs_source_remove() or explicitly disable the source's cache setting. void unregisterSource(obs_source_t *source); - // Requests reevaluation after a registered source's settings or activity change. + // Invalidates older work before external code changes a source's live settings. + // Keep the returned guard alive through obs_source_update(), including any + // property callbacks that mutate those settings. Retains the exact source entry. + // Nested updates are supported; queries wait until all guards finish and OBS + // has had a source-update phase. Does not wait for executing queries; their + // results are invalidated. Does not serialize the external settings writers. + // No queue lock is held across the caller's OBS operations. Null/unregistered + // sources and calls while stopped or stopping return an inactive guard. + // The manager and OBS runtime must outlive the guard. + [[nodiscard]] SourceSettingsUpdate trackSourceSettingsUpdate(obs_source_t *source); + + // Requests reevaluation after a registered source's activity change. + // Use trackSourceSettingsUpdate() around settings writes instead. // Safe from OBS callbacks; repeated requests are coalesced, and this call does // not wait for media queries or cache-setting changes to complete. // Null/unregistered sources and calls while stopped or stopping are ignored. @@ -87,7 +125,9 @@ class MediaCacheManager { void requestAllCacheUpdates(); // Stops accepting requests, removes the tick callback, cancels queued work, - // joins the worker and drops all retained source references. + // joins the worker and drops its queued and tracked source references. + // Outstanding SourceSettingsUpdate guards retain their own references and must + // finish before obs_shutdown(), while this manager is still alive. // Call before obs_shutdown(), from the serialized OBS lifecycle, on neither // the graphics thread nor this manager's worker. Do not overlap another // shutdown() or initialize(). Waits for any executing tick callback and worker, @@ -100,7 +140,6 @@ class MediaCacheManager { private: friend class MediaCacheManagerTestAccess; using Clock = std::chrono::steady_clock; - struct SourceEntry; struct Snapshot { std::string file; bool eligible = false; @@ -134,6 +173,7 @@ class MediaCacheManager { void complete(Completion &result); void queueCacheSettingUpdate(const std::shared_ptr &source, uint64_t revision, bool targetCachingEnabled); void releaseBudget(SourceEntry &source); + void finishSourceSettingsUpdate(const std::shared_ptr &source); // Protects only queues and bookkeeping. Never held during an OBS call, // source release, wait for graphics, or thread join. @@ -148,6 +188,8 @@ class MediaCacheManager { bool m_stopping = false; bool m_notified = false; bool m_rebalance = false; + // Counts every graphics callback, including empty ticks; survives video reset. + uint64_t m_graphicsTick = 0; uint64_t m_reservedCacheBytes = 0; uint64_t m_cacheBudgetBytes; // Injectable clock for deterministic retry tests. diff --git a/obs-studio-server/source/osn-source.cpp b/obs-studio-server/source/osn-source.cpp index d52544423..0da768b0e 100644 --- a/obs-studio-server/source/osn-source.cpp +++ b/obs-studio-server/source/osn-source.cpp @@ -262,6 +262,8 @@ void osn::Source::GetProperties(void *data, const int64_t id, const std::vector< rval.push_back(ipc::value((uint64_t)ErrorCode::Ok)); + // Property callbacks may change live settings before obs_source_update(). + auto settingsUpdate = MediaCacheManager::GetInstance().trackSourceSettingsUpdate(src); obs_properties_t *prp = obs_source_properties(src); obs_data *settings = obs_source_get_settings(src); @@ -354,8 +356,10 @@ void osn::Source::Update(void *data, const int64_t id, const std::vector #include "memory-manager.h" #include "obs-setup.hpp" +#include "osn-error.hpp" #include "osn-source.hpp" #include #include #include #include #include +#include #include +#include +#include #include +#include using namespace std::chrono_literals; @@ -38,6 +43,12 @@ class MediaCacheManagerTestAccess { return !manager.m_notified && manager.m_completions.empty() && manager.m_graphicsJobs.empty() && manager.m_retiredEntries.empty(); }); } + // A settings update can deliberately leave a graphics query queued. + static bool waitForWorkerPass(MediaCacheManager &manager) + { + std::unique_lock lock(manager.m_mutex); + return manager.m_changed.wait_for(lock, 2s, [&] { return !manager.m_notified && manager.m_completions.empty(); }); + } static void wake(MediaCacheManager &manager) { { @@ -66,25 +77,36 @@ class MediaCacheManagerTestAccess { namespace { struct MediaState { std::atomic ready{true}; + std::atomic appliedWidth{10}; std::atomic queries{0}; std::atomic destroys{0}; std::atomic graphicsOnly{true}; std::function onQuery; + std::function onProperties; }; +void applyMediaSettings(void *data, obs_data_t *settings) +{ + // Model player replacement only when OBS invokes the source's update callback. + // One I420 frame is 150 bytes normally, 300 for medium.webm, or 1500 for large.webm. + const char *file = obs_data_get_string(settings, "local_file"); + static_cast(data)->appliedWidth = strcmp(file, "large.webm") == 0 ? 100 : strcmp(file, "medium.webm") == 0 ? 20 : 10; +} + void getFileInfo(void *data, calldata_t *cd) { auto &state = *static_cast(data); + const auto width = state.appliedWidth.load(); ++state.queries; if (!obs_in_task_thread(OBS_TASK_GRAPHICS)) state.graphicsOnly = false; if (state.onQuery) state.onQuery(); calldata_set_bool(cd, "have_video", true); - calldata_set_int(cd, "width", 10); + calldata_set_int(cd, "width", width); calldata_set_int(cd, "height", 10); calldata_set_int(cd, "num_frames", state.ready ? 1 : 0); - calldata_set_int(cd, "pix_format", VIDEO_FORMAT_I420); // 10 × 10 × 1 × 1.5 = 150 bytes per source. + calldata_set_int(cd, "pix_format", VIDEO_FORMAT_I420); } void getPlaying(void *data, calldata_t *cd) @@ -107,14 +129,30 @@ class ObsCore { info.get_name = [](void *) { return "Cache test media"; }; info.create = [](obs_data_t *settings, obs_source_t *source) -> void * { auto *state = reinterpret_cast(obs_data_get_int(settings, "test_state")); + applyMediaSettings(state, settings); proc_handler_t *handler = obs_source_get_proc_handler(source); proc_handler_add(handler, "void get_file_info(out int num_frames)", getFileInfo, state); proc_handler_add(handler, "void get_playing(out bool playing)", getPlaying, state); return state; }; + info.update = applyMediaSettings; info.destroy = [](void *data) { ++static_cast(data)->destroys; }; - info.get_width = [](void *) -> uint32_t { return 10; }; + info.get_width = [](void *data) -> uint32_t { return static_cast(data)->appliedWidth; }; info.get_height = [](void *) -> uint32_t { return 10; }; + info.get_properties = [](void *data) { + auto *properties = obs_properties_create(); + auto *file = obs_properties_add_text(properties, "local_file", "File", OBS_TEXT_DEFAULT); + obs_property_set_modified_callback2( + file, + [](void *data, obs_properties_t *, obs_property_t *, obs_data_t *settings) { + auto &state = *static_cast(data); + if (state.onProperties) + state.onProperties(settings); + return false; + }, + data); + return properties; + }; obs_register_source(&info); } ~ObsCore() @@ -145,10 +183,47 @@ void setLooping(MediaCacheManager &manager, obs_source_t *source, bool looping) { OBSDataAutoRelease settings = obs_data_create(); obs_data_set_bool(settings, "looping", looping); + auto update = manager.trackSourceSettingsUpdate(source); obs_source_update(source, settings); - manager.requestCacheUpdate(source); } +void setFile(MediaCacheManager &manager, obs_source_t *source, const char *file) +{ + OBSDataAutoRelease settings = obs_data_create(); + obs_data_set_string(settings, "local_file", file); + auto update = manager.trackSourceSettingsUpdate(source); + obs_source_update(source, settings); +} + +void runFrame(MediaCacheManager &manager, std::initializer_list sources) +{ + REQUIRE(MediaCacheManagerTestAccess::waitForWorkerPass(manager)); + // Preserve libobs's order: tick callbacks precede deferred source updates. + MediaCacheManagerTestAccess::tick(manager); + for (auto *source : sources) + obs_source_video_tick(source, 0); + REQUIRE(MediaCacheManagerTestAccess::waitForWorkerPass(manager)); +} + +// Exercise OSN's native source entry points with the controlled plugin above. +// The real OSN bootstrap loads the FFmpeg plugin and cannot use this source ID. +class RegisteredApiSource { +public: + explicit RegisteredApiSource(obs_source_t *source) : id(osn::Source::Manager::GetInstance().allocate(source)) + { + MediaCacheManager::GetInstance().initialize(); + MediaCacheManager::GetInstance().registerSource(source); + } + ~RegisteredApiSource() + { + MediaCacheManager::GetInstance().shutdown(); + osn::Source::Manager::GetInstance().free(id); + } + RegisteredApiSource(const RegisteredApiSource &) = delete; + RegisteredApiSource &operator=(const RegisteredApiSource &) = delete; + const uint64_t id; +}; + bool processGraphicsJobsUntil(MediaCacheManager &manager, const std::function &predicate) { const auto deadline = std::chrono::steady_clock::now() + 3s; @@ -160,6 +235,226 @@ bool processGraphicsJobsUntil(MediaCacheManager &manager, const std::function milliseconds{0}; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300, &milliseconds); + auto source = makeCacheTestSource("small.webm", state); + manager->registerSource(source); + runFrame(*manager, {source}); + REQUIRE(state.queries == 1); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + + state.ready = true; + setFile(*manager, source, editAgain ? "medium.webm" : "large.webm"); + REQUIRE(state.appliedWidth == 10); // Live settings changed, but the player has not. + runFrame(*manager, {source}); + CHECK(state.queries == 1); + CHECK(state.appliedWidth == (editAgain ? 20 : 100)); + if (editAgain) { + setFile(*manager, source, "large.webm"); + runFrame(*manager, {source}); + CHECK(state.queries == 1); + } + for (int i = 0; i < 4; ++i) + runFrame(*manager, {source}); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(state.queries == 2); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); +} + +TEST_CASE("Media cache settings guards cover paused nested and moved updates", "[media-cache][settings]") +{ + MediaState changing, ready; + std::atomic milliseconds{0}; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300, &milliseconds); + auto source = makeCacheTestSource("small.webm", changing); + auto other = makeCacheTestSource("other", ready); + manager->registerSource(source); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(*manager)); + + std::optional update(manager->trackSourceSettingsUpdate(source)); + OBSDataAutoRelease settings = obs_source_get_settings(source); + obs_data_set_string(settings, "local_file", "large.webm"); + // Pause after the live settings mutation, before scheduling the plugin update. + // More than MAX_POLLS frames must neither query old metadata nor consume retries. + manager->registerSource(other); + for (int i = 0; i < 12; ++i) + runFrame(*manager, {source, other}); + CHECK(changing.queries == 0); + CHECK(changing.appliedWidth == 10); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(isCachingEnabled(other)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); + + std::optional moved(std::move(*update)); + update.reset(); // The moved-from guard must not finish the update. + { + auto nested = manager->trackSourceSettingsUpdate(source); + obs_source_update(source, nullptr); + } + runFrame(*manager, {source, other}); + CHECK(changing.queries == 0); // The outer guard still holds the query. + CHECK(changing.appliedWidth == 100); + moved.reset(); + runFrame(*manager, {source, other}); + CHECK(changing.queries == 0); + for (int i = 0; i < 4; ++i) + runFrame(*manager, {source, other}); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(changing.queries == 1); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); +} + +TEST_CASE("Media cache discards metadata when settings change during a query", "[media-cache][settings]") +{ + MediaState state; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300); + auto source = makeCacheTestSource("small.webm", state); + bool edited = false; + state.onQuery = [&] { + if (!std::exchange(edited, true)) + setFile(*manager, source, "large.webm"); + }; + manager->registerSource(source); + runFrame(*manager, {source}); + REQUIRE(edited); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); + for (int i = 0; i < 4; ++i) + runFrame(*manager, {source}); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(state.queries == 2); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); +} + +TEST_CASE("Media cache settings guards retain removed sources through shutdown", "[media-cache][settings][shutdown]") +{ + bool stop = false; + SECTION("Unregister with a pending guard") {} + SECTION("Stop with a pending guard") + { + stop = true; + } + + MediaState state; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300); + auto source = makeCacheTestSource("small.webm", state); + manager->registerSource(source); + std::optional update(manager->trackSourceSettingsUpdate(source)); + setFile(*manager, source, "large.webm"); + runFrame(*manager, {source}); + CHECK(state.queries == 0); + if (stop) + manager->shutdown(); + else { + manager->unregisterSource(source); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + } + source = nullptr; + obs_wait_for_destroy_queue(); + CHECK(state.destroys == 0); + update.reset(); + manager->shutdown(); // Join before checking the worker's final reference release. + obs_wait_for_destroy_queue(); + CHECK(state.destroys == 1); + CHECK(MediaCacheManagerTestAccess::queuedGraphicsJobCount(*manager) == 0); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); +} + +TEST_CASE("Media cache settings guards keep the original registration identity", "[media-cache][settings]") +{ + MediaState state; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300); + auto source = makeCacheTestSource("small.webm", state); + manager->registerSource(source); + std::optional update(manager->trackSourceSettingsUpdate(source)); + manager->unregisterSource(source); + manager->registerSource(source); // Same OBS pointer, different cache entry. + update.reset(); + for (int i = 0; i < 4; ++i) + runFrame(*manager, {source}); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(state.queries == 1); + CHECK(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); +} + +TEST_CASE("OSN source settings entry points defer media cache queries", "[media-cache][settings]") +{ + bool useProperties = false; + SECTION("Source Update") {} + SECTION("Source GetProperties") + { + useProperties = true; + } + + MediaState state; + state.ready = false; + ObsCore core; + auto source = makeCacheTestSource("small.webm", state); + RegisteredApiSource registration(source); + auto &manager = MediaCacheManager::GetInstance(); + runFrame(manager, {source}); + REQUIRE(state.queries == 1); + state.ready = true; + std::vector response; + if (useProperties) { + // Ensure old work can run inside the property callback if invalidation + // is moved after obs_source_properties(). + manager.requestCacheUpdate(source); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(manager)); + bool called = false; + state.onProperties = [&](obs_data_t *settings) { + called = true; + obs_data_set_string(settings, "local_file", "large.webm"); + // A property callback can expose new settings before GetProperties + // calls obs_source_update(). The guard must already be active here. + for (int i = 0; i < 3; ++i) + runFrame(manager, {source}); + CHECK(state.queries == 1); + CHECK(state.appliedWidth == 10); + }; + osn::Source::GetProperties(nullptr, 0, {ipc::value(registration.id)}, response); + CHECK(called); + state.onProperties = {}; + } else { + osn::Source::Update(nullptr, 0, {ipc::value(registration.id), ipc::value(R"({"local_file":"large.webm"})")}, response); + } + REQUIRE(!response.empty()); + REQUIRE(static_cast(response[0].value_union.ui64) == ErrorCode::Ok); + CHECK(state.appliedWidth == 10); + runFrame(manager, {source}); + CHECK(state.queries == 1); + CHECK(state.appliedWidth == 100); + for (int i = 0; i < 4; ++i) + runFrame(manager, {source}); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(manager)); + CHECK(state.queries == 2); + CHECK(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(manager) == 1500); + // The manager's own caching write must settle, without invalidating itself. + for (int i = 0; i < 3; ++i) + runFrame(manager, {source}); + CHECK(state.queries == 2); +} + TEST_CASE("Media cache serializes rebalance and coalesces notifications", "[media-cache]") { std::array states; @@ -393,6 +688,29 @@ TEST_CASE("Media cache uses graphics across video reset and permits callback ree CHECK_FALSE(isCachingEnabled(source)); CHECK(state.graphicsOnly); REQUIRE(obs_remove_video_info(obs_get_video_info_by_index2(0)) == OBS_VIDEO_SUCCESS); + + // Replace an uncached player's file with no graphics thread running. The + // pending query must survive reset and wait for the new player's metadata. + const auto queriesBeforeReset = state.queries.load(); + { + auto update = manager->trackSourceSettingsUpdate(source); + OBSDataAutoRelease settings = obs_data_create(); + obs_data_set_string(settings, "local_file", "large.webm"); + obs_data_set_bool(settings, "looping", true); + obs_source_update(source, settings); + } + REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(*manager)); + REQUIRE(obs_reset_video(&info) == OBS_VIDEO_SUCCESS); + const auto resetDeadline = std::chrono::steady_clock::now() + 3s; + while (state.queries == queriesBeforeReset && std::chrono::steady_clock::now() < resetDeadline) + std::this_thread::sleep_for(5ms); + REQUIRE(state.queries > queriesBeforeReset); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(state.appliedWidth == 100); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); + CHECK(state.graphicsOnly); + REQUIRE(obs_remove_video_info(obs_get_video_info_by_index2(0)) == OBS_VIDEO_SUCCESS); manager->requestCacheUpdate(source); REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(*manager)); manager->shutdown(); From 4b45f7ff420afad124e7abd5d6c757d380b6131a Mon Sep 17 00:00:00 2001 From: Aleksandr Voitenko Date: Wed, 30 Sep 2026 10:07:18 +1300 Subject: [PATCH 2/5] Comments update --- js/module.ts | 13 +++++++------ obs-studio-server/source/memory-manager.cpp | 5 +++-- obs-studio-server/source/memory-manager.h | 11 +++++++---- obs-studio-server/tests/test-memory-manager.cpp | 5 +++-- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/js/module.ts b/js/module.ts index 250c23e9a..d00870333 100644 --- a/js/module.ts +++ b/js/module.ts @@ -1302,15 +1302,16 @@ export interface ITransition extends ISource { export interface IConfigurable { /** - * Merge settings into this live instance. Existing references remain valid. - * For video sources, the plugin applies the update on a later graphics tick; - * media playback and cache reevaluation may still be pending when this returns. + * Merge the supplied values into this instance's settings. + * Supported settings depend on the source or encoder type. + * For video sources, OBS applies changes during video-frame processing, + * so their effects may still be pending when this method returns. * * @param settings JSON-serializable settings to merge; omitted keys are preserved. - * @returns Nothing. Success does not guarantee that the plugin has finished applying the settings. + * @returns Nothing. Success does not guarantee that the changes have taken effect. * @throws {TypeError} If settings cannot be converted to an object or JSON serialized. - * @throws {Error} If the native reference is invalid or the IPC request fails. - * An IPC failure does not guarantee that a dispatched update was rolled back. + * @throws {Error} If the source or encoder no longer exists, or the request to OBS fails. + * If communication fails after the update is sent, the settings may already have changed. */ update(settings: ISettings): void; diff --git a/obs-studio-server/source/memory-manager.cpp b/obs-studio-server/source/memory-manager.cpp index 356da250d..48c1e28ac 100644 --- a/obs-studio-server/source/memory-manager.cpp +++ b/obs-studio-server/source/memory-manager.cpp @@ -101,7 +101,8 @@ MediaCacheManager::SourceSettingsUpdate::~SourceSettingsUpdate() { if (m_manager) m_manager->finishSourceSettingsUpdate(m_sourceEntry); - // m_sourceEntry releases its OBS reference after the queue mutex is unlocked. + // Dropping m_sourceEntry may release the last OBS source reference, so it must + // happen after the queue mutex is unlocked. } void MediaCacheManager::initialize() @@ -278,7 +279,7 @@ void MediaCacheManager::setCaching(obs_source_t *source, bool caching) // Apply only our setting; never write back an old copy of the source's // unrelated settings after the user has edited them. OBSDataAutoRelease patch = obs_data_create(); - // "caching" is a custom Streamlabs setting, OBS does not use it + // Streamlabs setting consumed by ffmpeg_source to enable media caching. obs_data_set_bool(patch, "caching", caching); obs_source_update(source, patch); } diff --git a/obs-studio-server/source/memory-manager.h b/obs-studio-server/source/memory-manager.h index 967cae933..cfd1cbc34 100644 --- a/obs-studio-server/source/memory-manager.h +++ b/obs-studio-server/source/memory-manager.h @@ -103,10 +103,13 @@ class MediaCacheManager { // Invalidates older work before external code changes a source's live settings. // Keep the returned guard alive through obs_source_update(), including any - // property callbacks that mutate those settings. Retains the exact source entry. - // Nested updates are supported; queries wait until all guards finish and OBS - // has had a source-update phase. Does not wait for executing queries; their - // results are invalidated. Does not serialize the external settings writers. + // property callbacks that mutate those settings. Retains the original + // registration so finishing this guard cannot affect a later registration + // of the same source. + // Nested updates are supported; queries wait until all guards for this source + // finish and OBS has had a source-update phase. Does not wait for executing + // queries; their results are invalidated. Does not serialize external settings + // writers. // No queue lock is held across the caller's OBS operations. Null/unregistered // sources and calls while stopped or stopping return an inactive guard. // The manager and OBS runtime must outlive the guard. diff --git a/obs-studio-server/tests/test-memory-manager.cpp b/obs-studio-server/tests/test-memory-manager.cpp index 4ae0ab0c7..7ba9a8928 100644 --- a/obs-studio-server/tests/test-memory-manager.cpp +++ b/obs-studio-server/tests/test-memory-manager.cpp @@ -205,8 +205,9 @@ void runFrame(MediaCacheManager &manager, std::initializer_list REQUIRE(MediaCacheManagerTestAccess::waitForWorkerPass(manager)); } -// Exercise OSN's native source entry points with the controlled plugin above. -// The real OSN bootstrap loads the FFmpeg plugin and cannot use this source ID. +// Exercise OSN source entry points with the fake ffmpeg_source registered by +// ObsCore. Full OSN initialization would load the real implementation under +// the same source ID. class RegisteredApiSource { public: explicit RegisteredApiSource(obs_source_t *source) : id(osn::Source::Manager::GetInstance().allocate(source)) From ab95021e7ba8e6222ad43c841e391509ec2aaa5a Mon Sep 17 00:00:00 2001 From: Aleksandr Voitenko Date: Wed, 30 Sep 2026 11:56:11 +1300 Subject: [PATCH 3/5] Fix race between media cache writes and source settings updates --- obs-studio-server/source/memory-manager.cpp | 23 +++- obs-studio-server/source/memory-manager.h | 11 +- .../tests/test-memory-manager.cpp | 112 +++++++++++++++++- 3 files changed, 141 insertions(+), 5 deletions(-) diff --git a/obs-studio-server/source/memory-manager.cpp b/obs-studio-server/source/memory-manager.cpp index 48c1e28ac..e81159adf 100644 --- a/obs-studio-server/source/memory-manager.cpp +++ b/obs-studio-server/source/memory-manager.cpp @@ -59,6 +59,11 @@ struct MediaCacheManager::SourceEntry { std::atomic revision{1}; std::atomic removed{false}; + // Serializes guard entry with cache-write validation and the OBS settings + // patch. Acquire only without m_mutex; graphics uses try_lock to avoid waiting. + // Guards release this before returning to the external settings writer. + std::mutex cacheWriteMutex; + // Settings guards and the graphics callback access these under the queue mutex. unsigned settingsUpdatesInProgress = 0; uint64_t queryAllowedFromTick = 0; @@ -177,6 +182,14 @@ MediaCacheManager::SourceSettingsUpdate MediaCacheManager::trackSourceSettingsUp if (!m_accepting || it == m_sources.end()) return {nullptr, {}}; entry = it->second; + } + // A cache write already past validation must finish before the caller can + // change live settings. Never wait for that write while holding m_mutex. + std::lock_guard cacheWriteLock(entry->cacheWriteMutex); + { + std::lock_guard lock(m_mutex); + if (!m_accepting || entry->removed) + return {nullptr, {}}; ++entry->revision; ++entry->settingsUpdatesInProgress; m_notified = true; @@ -317,6 +330,14 @@ void MediaCacheManager::graphicsTick(void *param, float) // its deferred settings update first. for (auto &result : results) { auto &job = result.job; + std::unique_lock cacheWriteLock; + if (job.type == JobType::SetCaching) { + // Hold through revision/settings validation and the write. Otherwise a + // guard could change local_file after we validate the old reservation. + cacheWriteLock = std::unique_lock(job.source->cacheWriteMutex, std::try_to_lock); + if (!cacheWriteLock.owns_lock()) + continue; // An unapplied completion schedules fresh evaluation. + } if (job.source->removed || job.source->revision != job.revision) continue; result.valid = true; @@ -325,7 +346,7 @@ void MediaCacheManager::graphicsTick(void *param, float) queryMediaOnGraphicsThread(job.source->source, result.snapshot); } else if (!job.targetCachingEnabled || (result.snapshot.eligible && result.snapshot.file == job.file)) { if (result.snapshot.caching != job.targetCachingEnabled) - setCaching(job.source->source, job.targetCachingEnabled); + manager.m_setCaching(job.source->source, job.targetCachingEnabled); result.applied = true; } } diff --git a/obs-studio-server/source/memory-manager.h b/obs-studio-server/source/memory-manager.h index cfd1cbc34..0f7a60190 100644 --- a/obs-studio-server/source/memory-manager.h +++ b/obs-studio-server/source/memory-manager.h @@ -46,6 +46,8 @@ // Settings become visible before the plugin applies them. External writers must // use trackSourceSettingsUpdate() so queries wait for a source-update phase and // cannot associate a new filename with the previous player's metadata. +// Guard entry also serializes with cache-setting writes: validation and the +// caching patch must finish before an external writer can change the file. class MediaCacheManager { struct SourceEntry; @@ -107,9 +109,10 @@ class MediaCacheManager { // registration so finishing this guard cannot affect a later registration // of the same source. // Nested updates are supported; queries wait until all guards for this source - // finish and OBS has had a source-update phase. Does not wait for executing - // queries; their results are invalidated. Does not serialize external settings - // writers. + // finish and OBS has had a source-update phase. May wait for an executing cache + // setting write before returning; no queue lock is held while waiting. Does not + // wait for queries; their results are invalidated. Does not serialize external + // settings writers or hold a lock for the returned guard's lifetime. // No queue lock is held across the caller's OBS operations. Null/unregistered // sources and calls while stopped or stopping return an inactive guard. // The manager and OBS runtime must outlive the guard. @@ -197,4 +200,6 @@ class MediaCacheManager { uint64_t m_cacheBudgetBytes; // Injectable clock for deterministic retry tests. std::function m_now = Clock::now; + // Injectable OBS settings write for deterministic cache-write race tests. + std::function m_setCaching = setCaching; }; diff --git a/obs-studio-server/tests/test-memory-manager.cpp b/obs-studio-server/tests/test-memory-manager.cpp index 7ba9a8928..5d77fa463 100644 --- a/obs-studio-server/tests/test-memory-manager.cpp +++ b/obs-studio-server/tests/test-memory-manager.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -30,6 +31,12 @@ class MediaCacheManagerTestAccess { return manager; } static void tick(MediaCacheManager &manager) { MediaCacheManager::graphicsTick(&manager, 0); } + // Install only while the graphics executor is stopped. + static void interceptCacheWrites(MediaCacheManager &manager, std::function write) + { + manager.m_setCaching = std::move(write); + } + static void setCaching(obs_source_t *source, bool caching) { MediaCacheManager::setCaching(source, caching); } static bool waitForQueuedGraphicsJob(MediaCacheManager &manager, std::chrono::milliseconds timeout = 2s) { std::unique_lock lock(manager.m_mutex); @@ -80,6 +87,7 @@ struct MediaState { std::atomic appliedWidth{10}; std::atomic queries{0}; std::atomic destroys{0}; + std::atomic largeFileCachedUpdates{0}; std::atomic graphicsOnly{true}; std::function onQuery; std::function onProperties; @@ -90,7 +98,10 @@ void applyMediaSettings(void *data, obs_data_t *settings) // Model player replacement only when OBS invokes the source's update callback. // One I420 frame is 150 bytes normally, 300 for medium.webm, or 1500 for large.webm. const char *file = obs_data_get_string(settings, "local_file"); - static_cast(data)->appliedWidth = strcmp(file, "large.webm") == 0 ? 100 : strcmp(file, "medium.webm") == 0 ? 20 : 10; + auto &state = *static_cast(data); + state.appliedWidth = strcmp(file, "large.webm") == 0 ? 100 : strcmp(file, "medium.webm") == 0 ? 20 : 10; + if (state.appliedWidth == 100 && obs_data_get_bool(settings, "caching")) + ++state.largeFileCachedUpdates; } void getFileInfo(void *data, calldata_t *cd) @@ -343,6 +354,105 @@ TEST_CASE("Media cache discards metadata when settings change during a query", " CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); } +TEST_CASE("Media cache finishes a validated cache write before a settings guard returns", "[media-cache][settings]") +{ + MediaState state; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300); + auto source = makeCacheTestSource("small.webm", state); + manager->registerSource(source); + runFrame(*manager, {source}); // Reserve 150 bytes and queue the enable operation. + REQUIRE(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); + REQUIRE(MediaCacheManagerTestAccess::queuedGraphicsJobCount(*manager) == 1); + REQUIRE_FALSE(isCachingEnabled(source)); + + std::promise writeReached, guardAttempted, guardEntered; + auto writeReachedFuture = writeReached.get_future(); + auto guardAttemptedFuture = guardAttempted.get_future(); + auto guardEnteredFuture = guardEntered.get_future(); + std::atomic writeInProgress{false}; + bool attempted = false; + bool overlapped = false; + std::string fileAtWrite; + MediaCacheManagerTestAccess::interceptCacheWrites(*manager, [&](obs_source_t *target, bool caching) { + writeInProgress = true; + writeReached.set_value(); + attempted = guardAttemptedFuture.wait_for(2s) == std::future_status::ready; + // Give the other thread a chance to enter the guard while this selected + // enable is paused after validation, immediately before its OBS write. + guardEnteredFuture.wait_for(100ms); + OBSDataAutoRelease settings = obs_source_get_settings(target); + fileAtWrite = obs_data_get_string(settings, "local_file"); + // Reentry must remain safe: the cache-write mutex is not the queue mutex. + manager->requestCacheUpdate(target); + MediaCacheManagerTestAccess::setCaching(target, caching); + writeInProgress = false; + }); + auto editor = std::async(std::launch::async, [&] { + if (writeReachedFuture.wait_for(2s) != std::future_status::ready) + return false; + guardAttempted.set_value(); + auto update = manager->trackSourceSettingsUpdate(source); + overlapped = writeInProgress; + // Use the uncached settings captured by an external editor. A stale + // enable must not overwrite its false flag after it switches files. + OBSDataAutoRelease settings = obs_data_create(); + obs_data_set_string(settings, "local_file", "large.webm"); + obs_data_set_bool(settings, "caching", false); + obs_source_update(source, settings); + guardEntered.set_value(); + return true; + }); + // Only run the manager callback here; the source update follows after the + // editor finishes, making the stale-enable consequence deterministic. + MediaCacheManagerTestAccess::tick(*manager); + REQUIRE(editor.get()); + MediaCacheManagerTestAccess::interceptCacheWrites(*manager, MediaCacheManagerTestAccess::setCaching); + obs_source_video_tick(source, 0); + CHECK(attempted); + CHECK_FALSE(overlapped); + CHECK(fileAtWrite == "small.webm"); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(state.largeFileCachedUpdates == 0); + for (int i = 0; i < 4; ++i) + runFrame(*manager, {source}); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); + CHECK_FALSE(isCachingEnabled(source)); +} + +TEST_CASE("Media cache cancels a queued enable when a settings guard starts first", "[media-cache][settings]") +{ + MediaState state; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300); + auto source = makeCacheTestSource("small.webm", state); + manager->registerSource(source); + runFrame(*manager, {source}); + REQUIRE(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); + REQUIRE_FALSE(isCachingEnabled(source)); + { + auto update = manager->trackSourceSettingsUpdate(source); + OBSDataAutoRelease settings = obs_data_create(); + obs_data_set_string(settings, "local_file", "large.webm"); + obs_source_update(source, settings); + // Graphics must finish while the guard is still alive; the old enable + // is invalidated, and the new query waits without blocking this tick. + for (int i = 0; i < 3; ++i) + runFrame(*manager, {source}); + CHECK(state.queries == 1); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); + } + for (int i = 0; i < 4; ++i) + runFrame(*manager, {source}); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(state.queries == 2); + CHECK(state.largeFileCachedUpdates == 0); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); +} + TEST_CASE("Media cache settings guards retain removed sources through shutdown", "[media-cache][settings][shutdown]") { bool stop = false; From 4f24c1f0a04b38f869c802b4f45dfd1d172d3bf4 Mon Sep 17 00:00:00 2001 From: Aleksandr Voitenko Date: Wed, 30 Sep 2026 12:26:12 +1300 Subject: [PATCH 4/5] Reset media caching before source settings updates --- js/module.ts | 6 +- obs-studio-server/source/memory-manager.cpp | 13 +- obs-studio-server/source/memory-manager.h | 12 +- obs-studio-server/source/osn-source.cpp | 2 +- .../tests/test-memory-manager.cpp | 124 +++++++++++++++++- 5 files changed, 147 insertions(+), 10 deletions(-) diff --git a/js/module.ts b/js/module.ts index d00870333..0917bef79 100644 --- a/js/module.ts +++ b/js/module.ts @@ -1306,8 +1306,12 @@ export interface IConfigurable { * Supported settings depend on the source or encoder type. * For video sources, OBS applies changes during video-frame processing, * so their effects may still be pending when this method returns. + * For media-file sources (`ffmpeg_source`), OSN owns the internal `caching` + * setting. Updates reset it to false while OSN reevaluates cache eligibility + * and memory use; supplying `caching: true` does not force caching. * - * @param settings JSON-serializable settings to merge; omitted keys are preserved. + * @param settings JSON-serializable settings to merge; omitted keys are preserved + * except for the OSN-managed media caching flag described above. * @returns Nothing. Success does not guarantee that the changes have taken effect. * @throws {TypeError} If settings cannot be converted to an object or JSON serialized. * @throws {Error} If the source or encoder no longer exists, or the request to OBS fails. diff --git a/obs-studio-server/source/memory-manager.cpp b/obs-studio-server/source/memory-manager.cpp index e81159adf..77cc55a92 100644 --- a/obs-studio-server/source/memory-manager.cpp +++ b/obs-studio-server/source/memory-manager.cpp @@ -173,7 +173,7 @@ void MediaCacheManager::requestCacheUpdate(obs_source_t *source) m_changed.notify_all(); } -MediaCacheManager::SourceSettingsUpdate MediaCacheManager::trackSourceSettingsUpdate(obs_source_t *source) +MediaCacheManager::SourceSettingsUpdate MediaCacheManager::trackSourceSettingsUpdate(obs_source_t *source, obs_data_t *pendingSettings) { std::shared_ptr entry; { @@ -194,6 +194,17 @@ MediaCacheManager::SourceSettingsUpdate MediaCacheManager::trackSourceSettingsUp ++entry->settingsUpdatesInProgress; m_notified = true; } + // A partial update would otherwise inherit the old player's cache enable. + // Clear it before the caller can mutate local_file, and also sanitize any + // supplied settings copy. Do not call obs_source_update here: that would + // schedule the plugin before the caller has finished its settings changes. + OBSDataAutoRelease settings = obs_source_get_settings(source); + obs_data_set_bool(settings, "caching", false); + if (pendingSettings) + obs_data_set_bool(pendingSettings, "caching", false); + // Keep the old reservation until a fresh query after the guarded update. + // An older SetCaching completion may still be waiting for the worker, and + // the old player can still be alive until OBS applies the external update. m_changed.notify_all(); return {this, std::move(entry)}; } diff --git a/obs-studio-server/source/memory-manager.h b/obs-studio-server/source/memory-manager.h index 0f7a60190..003f154fb 100644 --- a/obs-studio-server/source/memory-manager.h +++ b/obs-studio-server/source/memory-manager.h @@ -35,7 +35,7 @@ // their estimated total size within a shared memory budget. // // One worker owns cache decisions and accounting. OBS callbacks only invalidate -// entries; media queries and settings changes run in the graphics tick callback. +// entries; media queries and cache-enable writes run in the graphics tick callback. // // The ffmpeg_source metadata handlers execute synchronously and access its // current media player. OBS source updates and video ticks can destroy or @@ -48,6 +48,8 @@ // cannot associate a new filename with the previous player's metadata. // Guard entry also serializes with cache-setting writes: validation and the // caching patch must finish before an external writer can change the file. +// The guard then clears the cache flag in live and incoming settings so the +// changed player starts uncached until its metadata has been reevaluated. class MediaCacheManager { struct SourceEntry; @@ -108,6 +110,12 @@ class MediaCacheManager { // property callbacks that mutate those settings. Retains the original // registration so finishing this guard cannot affect a later registration // of the same source. + // Clears the manager-owned "caching" flag in live settings before returning. + // Pass any separate settings object that will be applied as pendingSettings; + // its caching flag is also cleared so a copied enable cannot be written back. + // Both pointers are borrowed. Other settings are preserved. The caller must + // not set caching again; the worker reevaluates it after the guarded update. + // The existing reservation is reconciled by that evaluation, not guard entry. // Nested updates are supported; queries wait until all guards for this source // finish and OBS has had a source-update phase. May wait for an executing cache // setting write before returning; no queue lock is held while waiting. Does not @@ -116,7 +124,7 @@ class MediaCacheManager { // No queue lock is held across the caller's OBS operations. Null/unregistered // sources and calls while stopped or stopping return an inactive guard. // The manager and OBS runtime must outlive the guard. - [[nodiscard]] SourceSettingsUpdate trackSourceSettingsUpdate(obs_source_t *source); + [[nodiscard]] SourceSettingsUpdate trackSourceSettingsUpdate(obs_source_t *source, obs_data_t *pendingSettings = nullptr); // Requests reevaluation after a registered source's activity change. // Use trackSourceSettingsUpdate() around settings writes instead. diff --git a/obs-studio-server/source/osn-source.cpp b/obs-studio-server/source/osn-source.cpp index 0da768b0e..0e1377bdf 100644 --- a/obs-studio-server/source/osn-source.cpp +++ b/obs-studio-server/source/osn-source.cpp @@ -357,7 +357,7 @@ void osn::Source::Update(void *data, const int64_t id, const std::vectortrackSourceSettingsUpdate(source); overlapped = writeInProgress; - // Use the uncached settings captured by an external editor. A stale - // enable must not overwrite its false flag after it switches files. + // Omit caching, as callers of the public partial-update API can do. + // The guard must clear the old enable before these settings are merged. OBSDataAutoRelease settings = obs_data_create(); obs_data_set_string(settings, "local_file", "large.webm"); - obs_data_set_bool(settings, "caching", false); obs_source_update(source, settings); guardEntered.set_value(); return true; @@ -421,6 +420,49 @@ TEST_CASE("Media cache finishes a validated cache write before a settings guard CHECK_FALSE(isCachingEnabled(source)); } +TEST_CASE("Media cache reconciles reservations after partial file updates", "[media-cache][settings]") +{ + MediaState changing, waiting; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300); + auto source = makeCacheTestSource("small.webm", changing); + auto other = makeCacheTestSource("medium.webm", waiting); + manager->registerSource(source); + for (int i = 0; i < 2; ++i) + runFrame(*manager, {source}); + REQUIRE(isCachingEnabled(source)); + REQUIRE(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); + manager->registerSource(other); + runFrame(*manager, {source, other}); + REQUIRE_FALSE(isCachingEnabled(other)); // Its 300 bytes do not fit yet. + { + auto update = manager->trackSourceSettingsUpdate(source); + CHECK_FALSE(isCachingEnabled(source)); + OBSDataAutoRelease settings = obs_data_create(); + obs_data_set_string(settings, "local_file", "large.webm"); + obs_source_update(source, settings); + for (int i = 0; i < 3; ++i) + runFrame(*manager, {source, other}); + CHECK(changing.appliedWidth == 100); + CHECK(changing.queries == 1); + CHECK(changing.largeFileCachedUpdates == 0); + CHECK_FALSE(isCachingEnabled(source)); + CHECK_FALSE(isCachingEnabled(other)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); + } + runFrame(*manager, {source, other}); // First callback still precedes the fence. + CHECK(changing.queries == 1); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); + for (int i = 0; i < 5; ++i) + runFrame(*manager, {source, other}); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(changing.queries >= 2); // Budget rebalancing can request another query. + CHECK(changing.largeFileCachedUpdates == 0); + CHECK_FALSE(isCachingEnabled(source)); // 1500 bytes exceed the budget. + CHECK(isCachingEnabled(other)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 300); +} + TEST_CASE("Media cache cancels a queued enable when a settings guard starts first", "[media-cache][settings]") { MediaState state; @@ -566,6 +608,78 @@ TEST_CASE("OSN source settings entry points defer media cache queries", "[media- CHECK(state.queries == 2); } +TEST_CASE("OSN source edits discard the previous media cache enable", "[media-cache][settings]") +{ + bool useProperties = false; + bool useSettingsCopy = false; + SECTION("Partial Source Update") {} + SECTION("Source Update with a copied cache enable") + { + useSettingsCopy = true; + } + SECTION("Source GetProperties") + { + useProperties = true; + } + + MediaState state; + ObsCore core; + auto source = makeCacheTestSource("small.webm", state); + RegisteredApiSource registration(source); + auto &manager = MediaCacheManager::GetInstance(); + for (int i = 0; i < 2; ++i) + runFrame(manager, {source}); + REQUIRE(isCachingEnabled(source)); + REQUIRE(state.queries == 1); + REQUIRE(MediaCacheManagerTestAccess::reservedBytes(manager) == 150); + + std::vector response; + if (useProperties) { + bool called = false; + state.onProperties = [&](obs_data_t *settings) { + called = true; + CHECK_FALSE(obs_data_get_bool(settings, "caching")); + obs_data_set_string(settings, "local_file", "large.webm"); + runFrame(manager, {source}); + // Guard entry must not schedule a plugin update of its own while + // property callbacks are still changing the live settings. + CHECK(state.appliedWidth == 10); + CHECK(state.queries == 1); + }; + osn::Source::GetProperties(nullptr, 0, {ipc::value(registration.id)}, response); + CHECK(called); + state.onProperties = {}; + } else { + OBSDataAutoRelease current = obs_source_get_settings(source); + OBSDataAutoRelease settings = useSettingsCopy ? obs_data_create_from_json(obs_data_get_json(current)) : obs_data_create(); + obs_data_set_string(settings, "local_file", "large.webm"); + if (useSettingsCopy) + REQUIRE(obs_data_get_bool(settings, "caching")); + else + REQUIRE_FALSE(obs_data_has_user_value(settings, "caching")); + osn::Source::Update(nullptr, 0, {ipc::value(registration.id), ipc::value(obs_data_get_json(settings))}, response); + } + REQUIRE(!response.empty()); + REQUIRE(static_cast(response[0].value_union.ui64) == ErrorCode::Ok); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(state.appliedWidth == 10); + CHECK(MediaCacheManagerTestAccess::reservedBytes(manager) == 150); + runFrame(manager, {source}); + CHECK(state.appliedWidth == 100); + CHECK(state.queries == 1); + CHECK(state.largeFileCachedUpdates == 0); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(manager) == 150); + runFrame(manager, {source}); + CHECK(state.queries == 2); + CHECK_FALSE(isCachingEnabled(source)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(manager) == 1500); + runFrame(manager, {source}); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(manager)); + CHECK(isCachingEnabled(source)); + CHECK(state.largeFileCachedUpdates == 1); // Only after the new size is admitted. +} + TEST_CASE("Media cache serializes rebalance and coalesces notifications", "[media-cache]") { std::array states; From 849b3fcb46d7495f181409d87b04d6e8535d6944 Mon Sep 17 00:00:00 2001 From: Aleksandr Voitenko Date: Wed, 30 Sep 2026 12:47:56 +1300 Subject: [PATCH 5/5] Align source integration test with managed media caching --- tests/osn-tests/src/test_osn_source.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/osn-tests/src/test_osn_source.ts b/tests/osn-tests/src/test_osn_source.ts index fbbac72c0..aadfb81a2 100644 --- a/tests/osn-tests/src/test_osn_source.ts +++ b/tests/osn-tests/src/test_osn_source.ts @@ -366,8 +366,15 @@ describe(testName, () => { input.save(); }).to.not.throw(); - // Checking if setting was added to source - expect(input.settings).to.eql(settings, GetErrorMessage(ETestErrorMsg.SaveSettings, inputType)); + const expectedSettings = { ...settings }; + if (inputType === EOBSInputTypes.FFMPEGSource) { + // OSN owns caching. With no media file to cache, the caller's + // enable request is reset while all other settings are preserved. + expectedSettings['caching'] = false; + } + + // Checking if settings were saved, including the managed cache flag. + expect(input.settings).to.eql(expectedSettings, GetErrorMessage(ETestErrorMsg.SaveSettings, inputType)); settings = {}; input.release();