From c1f085d2209b63f483014abde4afaf8df540d3f7 Mon Sep 17 00:00:00 2001 From: Aleksandr Voitenko Date: Fri, 25 Sep 2026 13:42:29 +1200 Subject: [PATCH 1/3] Fix media cache deadlocks when switching scenes Old memory manager has been rewritten from the ground. Replace per-update threads with a single worker that coalesces requests and manages cache budgets. Run media queries and cache-setting updates on the OBS graphics thread, and release source references outside locks. Cancel pending work safely during removal and shutdown. Add native tests for budgeting, readiness retries, source lifetime, and video resets. --- obs-studio-server/CMakeLists.txt | 2 + obs-studio-server/source/memory-manager.cpp | 616 ++++++++++-------- obs-studio-server/source/memory-manager.h | 169 +++-- obs-studio-server/source/nodeobs_api.cpp | 14 +- obs-studio-server/source/nodeobs_settings.cpp | 2 +- obs-studio-server/source/osn-source.cpp | 13 +- .../tests/test-memory-manager.cpp | 309 +++++++++ 7 files changed, 778 insertions(+), 347 deletions(-) create mode 100644 obs-studio-server/tests/test-memory-manager.cpp diff --git a/obs-studio-server/CMakeLists.txt b/obs-studio-server/CMakeLists.txt index 557f49177..021ec0eb7 100644 --- a/obs-studio-server/CMakeLists.txt +++ b/obs-studio-server/CMakeLists.txt @@ -624,6 +624,7 @@ if(BUILD_TESTING) add_executable( obs_studio_server_unit_tests + "tests/test-memory-manager.cpp" "tests/test-osn-source.cpp" "tests/test-osn-encoder-settings.cpp" "tests/test-osn-file-output.cpp" @@ -679,5 +680,6 @@ if(BUILD_TESTING) TEST_PREFIX "obs_studio_server_unit_tests::" DL_PATHS ${_osn_server_test_dl_paths} + PROPERTIES TIMEOUT 60 ) endif() diff --git a/obs-studio-server/source/memory-manager.cpp b/obs-studio-server/source/memory-manager.cpp index 1c3f6258f..6d92510b8 100644 --- a/obs-studio-server/source/memory-manager.cpp +++ b/obs-studio-server/source/memory-manager.cpp @@ -18,63 +18,173 @@ #include "memory-manager.h" #include "nodeobs_api.h" +#include +#include +#include +#include +#include +#include -struct MemoryManager::source_info { - bool cached = false; - uint64_t size = 0; - obs_source_t *source = nullptr; - std::vector workers; - std::mutex mtx; - bool have_video = false; +#ifdef WIN32 +#include +#elif defined(__APPLE__) +#include +#endif + +namespace { +constexpr uint64_t CACHE_LIMIT = 2004800000; +constexpr unsigned MAX_POLLS = 10; +constexpr size_t JOBS_PER_TICK = 8; + +uint64_t cacheBudget() +{ +#ifdef WIN32 + MEMORYSTATUSEX status{}; + status.dwLength = sizeof(status); + if (GlobalMemoryStatusEx(&status)) + return std::min(CACHE_LIMIT, status.ullTotalPhys / 2); +#elif defined(__APPLE__) + if (g_util_osx) + return std::min(CACHE_LIMIT, g_util_osx->getTotalPhysicalMemory() / 2); +#endif + return CACHE_LIMIT; +} +} + +struct MediaCacheManager::SourceEntry { + explicit SourceEntry(obs_source_t *source) : source(obs_source_get_ref(source)) {} + ~SourceEntry() { obs_source_release(source); } + // Keeps the OBS source alive; its current media player can still be replaced. + obs_source_t *source; + std::atomic revision{1}; + std::atomic removed{false}; + + // Only the worker changes these fields, under the queue mutex. + uint64_t scheduledRevision = 0; + uint64_t reservedBytes = 0; // Includes an enable operation awaiting completion. + std::string file; + unsigned retries = 0; + bool initialized = false; + bool hasPendingGraphicsJob = false; + bool waitingForBudget = false; + Clock::time_point retryAt = Clock::time_point::max(); }; -MemoryManager &MemoryManager::GetInstance() +MediaCacheManager &MediaCacheManager::GetInstance() { - static MemoryManager instance; + static MediaCacheManager instance; return instance; } -MemoryManager::MemoryManager() +MediaCacheManager::MediaCacheManager(uint64_t budget) : m_cacheBudgetBytes(budget ? budget : cacheBudget()) {} + +MediaCacheManager::~MediaCacheManager() { - blog(LOG_INFO, "MemoryManager: constructor called"); -#ifdef WIN32 - MEMORYSTATUSEX statex; - statex.dwLength = sizeof(statex); - current_cached_size = 0; - - if (::GlobalMemoryStatusEx(&statex)) { - available_memory = statex.ullTotalPhys; - allowed_cached_size = std::min(LIMIT, available_memory / 2); - } else { - available_memory = 0; - allowed_cached_size = LIMIT; + shutdown(); +} + +void MediaCacheManager::initialize() +{ + // Initialization and shutdown are serialized by the OBS API lifecycle. + if (m_worker.joinable()) + return; + m_stopping = false; + m_accepting = true; + m_notified = false; + m_rebalance = false; + m_reservedCacheBytes = 0; + obs_add_tick_callback(graphicsTick, this); + m_worker = std::thread(&MediaCacheManager::run, this); +} + +void MediaCacheManager::registerSource(obs_source_t *source) +{ + if (!source) + return; + const char *id = obs_source_get_unversioned_id(source); + if (!id || strcmp(id, "ffmpeg_source") != 0) + return; + + // Keep every possible last release outside the queue lock. + auto entry = std::make_shared(source); + { + std::lock_guard lock(m_mutex); + if (!m_accepting) + return; + m_sources.emplace(source, entry); + m_notified = true; } -#elif __APPLE__ - available_memory = g_util_osx->getTotalPhysicalMemory(); - allowed_cached_size = std::min((uint64_t)LIMIT, (uint64_t)available_memory / 2); -#endif + m_changed.notify_all(); } -MemoryManager::~MemoryManager() +void MediaCacheManager::unregisterSource(obs_source_t *source) { - blog(LOG_INFO, "MemoryManager: destructor called"); + { + std::lock_guard lock(m_mutex); + auto it = m_sources.find(source); + if (it == m_sources.end()) + return; + it->second->removed = true; + ++it->second->revision; + m_retiredEntries.push_back(std::move(it->second)); + m_sources.erase(it); + m_notified = true; + } + m_changed.notify_all(); +} - for (auto &source : sources) { - unregisterSource(source.second->source); +void MediaCacheManager::requestCacheUpdate(obs_source_t *source) +{ + { + std::lock_guard lock(m_mutex); + auto it = m_sources.find(source); + if (!m_accepting || it == m_sources.end()) + return; + ++it->second->revision; + m_notified = true; } + m_changed.notify_all(); } -// Not thread safe. 'si.mtx' should be locked -void MemoryManager::calculateRawSize(source_info &si) +void MediaCacheManager::requestAllCacheUpdates() { - calldata_t cd = {0}; - proc_handler_t *ph = obs_source_get_proc_handler(si.source); - proc_handler_call(ph, "get_file_info", &cd); - si.have_video = calldata_bool(&cd, "have_video"); + { + std::lock_guard lock(m_mutex); + if (!m_accepting) + return; + for (auto &item : m_sources) + ++item.second->revision; + m_notified = true; + } + m_changed.notify_all(); +} - const uint64_t pix_fmt = calldata_int(&cd, "pix_format"); +MediaCacheManager::Snapshot MediaCacheManager::readSettings(obs_source_t *source) +{ + OBSDataAutoRelease settings = obs_source_get_settings(source); + Snapshot snapshot; + snapshot.file = obs_data_get_string(settings, "local_file"); + snapshot.caching = obs_data_get_bool(settings, "caching"); + snapshot.eligible = !obs_source_removed(source) && OBS_API::getMediaFileCaching() && obs_data_get_bool(settings, "looping") && + obs_data_get_bool(settings, "is_local_file") && (obs_source_showing(source) || !obs_data_get_bool(settings, "close_when_inactive")); + return snapshot; +} + +void MediaCacheManager::queryMediaOnGraphicsThread(obs_source_t *source, Snapshot &snapshot) +{ + // These handlers dereference the plugin's current media player. Run them on + // graphics, alongside the plugin's deferred update and video_tick, and never + // carry a player pointer across ticks. Cached players do not expose this info. + if (!snapshot.eligible || snapshot.caching) + return; + calldata_t cd{}; + proc_handler_t *handler = obs_source_get_proc_handler(source); + proc_handler_call(handler, "get_file_info", &cd); + const auto width = calldata_int(&cd, "width"); + const auto height = calldata_int(&cd, "height"); + const auto frames = calldata_int(&cd, "num_frames"); double bpp = 0; - switch (pix_fmt) { + switch (calldata_int(&cd, "pix_format")) { case VIDEO_FORMAT_I420: case VIDEO_FORMAT_NV12: case VIDEO_FORMAT_I40A: @@ -98,298 +208,226 @@ void MemoryManager::calculateRawSize(source_info &si) bpp = 4; break; } - - const uint64_t width = calldata_int(&cd, "width"); - const uint64_t height = calldata_int(&cd, "height"); - const uint64_t nb_frames = calldata_int(&cd, "num_frames"); - si.size = static_cast(width * height * bpp * nb_frames); -} - -// Not thread safe. 'si.mtx' AND 'manager_mutex' should be locked -bool MemoryManager::shouldCacheSource(source_info &si) -{ - obs_data_t *settings = obs_source_get_settings(si.source); - - const bool looping = obs_data_get_bool(settings, "looping"); - const bool local_file = obs_data_get_bool(settings, "is_local_file"); - const bool enable_caching = OBS_API::getMediaFileCaching(); - bool showing = obs_source_showing(si.source); - - const bool is_small = obs_data_get_bool(settings, "caching") ? current_cached_size < allowed_cached_size - : current_cached_size + si.size < allowed_cached_size; - - if (!showing && !obs_data_get_bool(settings, "close_when_inactive")) - showing = true; - - obs_data_release(settings); - - return looping && local_file && enable_caching && is_small && showing; + const long double bytes = static_cast(width) * height * frames * bpp; + if (calldata_bool(&cd, "have_video") && width > 0 && height > 0 && frames > 0 && bytes > 0 && bytes < std::numeric_limits::max()) + snapshot.estimatedCacheBytes = static_cast(bytes); + calldata_free(&cd); + calldata_init(&cd); + proc_handler_call(handler, "get_playing", &cd); + snapshot.playing = calldata_bool(&cd, "playing"); + calldata_free(&cd); } -void updateSource(obs_source_t *source, bool caching) +void MediaCacheManager::setCaching(obs_source_t *source, bool caching) { - obs_data_t *settings = obs_source_get_settings(source); - if (!settings) - return; - - if (obs_data_get_bool(settings, "caching") != caching) { - obs_data_set_bool(settings, "caching", caching); - obs_source_update(source, settings); - } - obs_data_release(settings); + // 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 + obs_data_set_bool(patch, "caching", caching); + obs_source_update(source, patch); } -// Not thread safe. 'manager_mutex' and 'si.mtx' should be locked -void MemoryManager::addCachedMemory(source_info &si) +void MediaCacheManager::graphicsTick(void *param, float) { - if (!si.size || si.cached || current_cached_size + si.size > allowed_cached_size) - return; - - int32_t retry = MAX_POLLS; - - proc_handler_t *ph = obs_source_get_proc_handler(si.source); - bool playing = false; - while (retry > 0) { - calldata_t cd = {0}; - proc_handler_call(ph, "get_playing", &cd); - playing = calldata_bool(&cd, "playing"); - if (playing) - break; - - retry--; - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } - - if (!playing) - return; - - blog(LOG_INFO, "adding %dMB, source: %s", si.size / 1000000, obs_source_get_name(si.source)); - current_cached_size += si.size; - si.cached = true; - - updateSource(si.source, true); -} - -// Not thread safe. 'manager_mutex' and 'si.mtx' should be locked -void MemoryManager::removeCachedMemory(source_info &si, bool cacheNewFiles, const std::string &sourceName) -{ - if (!si.cached) - return; - - blog(LOG_INFO, "removing %dMB, source: %s", si.size / 1000000, sourceName.c_str()); - current_cached_size -= si.size; - si.cached = false; - - updateSource(si.source, false); - - if (!cacheNewFiles || current_cached_size >= allowed_cached_size) - return; - - for (const auto &data : sources) { - if (data.second.get() == &si) { - // Do not check self - continue; - } - - std::unique_lock ulock(data.second->mtx); - if (shouldCacheSource(*data.second)) - addCachedMemory(*data.second); - } -} - -void MemoryManager::sourceManager(const std::string &sourceName) -{ - std::shared_ptr si; + auto &manager = *static_cast(param); + std::vector results; { - std::unique_lock manager_lock(manager_mutex); - auto it = sources.find(sourceName); - if (it == sources.end()) { + std::lock_guard lock(manager.m_mutex); + if (!manager.m_accepting) return; - } - si = it->second; - si->mtx.lock(); - } - - bool need_get_size = false; - { - std::unique_lock si_mtx_lock(si->mtx, std::adopt_lock); - obs_data_t *settings = obs_source_get_settings(si->source); - const bool looping = obs_data_get_bool(settings, "looping"); - const bool local_file = obs_data_get_bool(settings, "is_local_file"); - obs_data_release(settings); - - if (!looping || !local_file) { - 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_graphicsJobs.pop_front(); } - - need_get_size = si->size == 0; } + if (results.empty()) + return; - if (need_get_size) { - uint32_t retry = MAX_POLLS; - while (retry > 0) { - { - std::unique_lock si_mtx_lock(si->mtx); - // Double-check if source still registered in the manager before proceeding - { - std::unique_lock manager_lock(manager_mutex); - if (sources.find(sourceName) == sources.end()) { - return; - } - } - calculateRawSize(*si); // This also sets 'si.have_video' - if (si->size || !si->have_video) { - break; - } - } - retry--; - std::this_thread::sleep_for(std::chrono::milliseconds(500)); + // Process only the batch taken above. After a SetCaching job, any follow-up + // QuerySource job must wait for a later tick so ffmpeg_source can process + // its deferred settings update first. + for (auto &result : results) { + auto &job = result.job; + if (job.source->removed || job.source->revision != job.revision) + continue; + result.valid = true; + result.snapshot = readSettings(job.source->source); + if (job.type == JobType::QuerySource) { + 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); + result.applied = true; } } - { - std::unique_lock si_mtx_lock(si->mtx); - if (!si->size) { - return; - } - - // Double-check if source still registered in the manager before proceeding - { - std::unique_lock manager_lock(manager_mutex); - if (sources.find(sourceName) == sources.end()) { - return; - } - } - - const bool should_cache = shouldCacheSource(*si); - if (should_cache) { - addCachedMemory(*si); - } else { - removeCachedMemory(*si, true, sourceName); - } + std::lock_guard lock(manager.m_mutex); + for (auto &result : results) + manager.m_completions.push_back(std::move(result)); + manager.m_notified = true; } + manager.m_changed.notify_all(); } -// Not thread safe, should be called with locked 'manager_mutex' -void MemoryManager::updateSettings(obs_source_t *source) +void MediaCacheManager::releaseBudget(SourceEntry &source) { - std::string sourceName = obs_source_get_name(source); - auto it = sources.find(sourceName); - if (it == sources.end()) { - return; + if (source.reservedBytes) { + m_reservedCacheBytes -= source.reservedBytes; + source.reservedBytes = 0; + m_rebalance = true; } - - std::unique_lock si_lock(it->second->mtx); - it->second->workers.push_back(std::thread(&MemoryManager::sourceManager, this, sourceName)); -} - -void MemoryManager::updateSourceCache(obs_source_t *source) -{ - std::unique_lock ulock(manager_mutex); - - updateSettings(source); } -void MemoryManager::updateSourcesCache() +void MediaCacheManager::queueCacheSettingUpdate(const std::shared_ptr &source, uint64_t revision, bool targetCachingEnabled) { - std::unique_lock ulock(manager_mutex); - - for (const auto &data : sources) - updateSettings(data.second->source); + source->hasPendingGraphicsJob = true; + m_graphicsJobs.push_back({source, revision, JobType::SetCaching, targetCachingEnabled, source->file}); } -bool MemoryManager::isSourceValid(obs_source_t *source) const +void MediaCacheManager::complete(Completion &result) { - if (!source) - return false; - - const char *source_id = obs_source_get_unversioned_id(source); - if (!source_id) - return false; - - if (strcmp(source_id, "ffmpeg_source") != 0) - return false; - - return true; -} + auto &entry = *result.job.source; + entry.hasPendingGraphicsJob = false; + if (entry.removed) + return; -void MemoryManager::registerSource(obs_source_t *source) -{ - if (!isSourceValid(source)) { + if (result.job.type == JobType::SetCaching) { + // Graphics completion confirms only that the caching setting was applied + // or already matched. Decoder cache creation is deferred. + // Process this even if a newer notification arrived during the operation. + if ((result.job.targetCachingEnabled && !result.applied) || (!result.job.targetCachingEnabled && result.applied)) + releaseBudget(entry); + if (!result.applied || !result.job.targetCachingEnabled) + entry.scheduledRevision = 0; + if (result.applied) + entry.initialized = true; return; } - - std::unique_lock ulock(manager_mutex); - - auto si = std::make_shared(); - si->source = obs_source_get_ref(source); - sources.emplace(obs_source_get_name(source), si); - updateSource(source, false); -} - -void MemoryManager::unregisterSource(obs_source_t *source) -{ - if (!isSourceValid(source)) { + if (!result.valid || entry.revision != result.job.revision) { + entry.scheduledRevision = 0; return; } - const std::string source_name = obs_source_get_name(source); - - std::shared_ptr moved_ptr; - std::vector workers_to_join; - { - std::unique_lock ulock(manager_mutex); - auto it = sources.find(source_name); - if (it == sources.end()) { - return; - } - std::unique_lock si_lock(it->second->mtx); - - // Moving pointer to have a valid object when proceeding with further deinit. - moved_ptr = std::move(it->second); - // Removing object from the collection early to be sure that it is unavailable anymore for outer clients - // and someone can not spawn a new worker while we are waiting for worker threads to join. - // Also this prevents data race if someone called 'unregisterSource' from other thread - // while removal is in progress. - workers_to_join = std::move(moved_ptr->workers); - sources.erase(source_name); + const auto &snapshot = result.snapshot; + const bool fileChanged = entry.file != snapshot.file; + entry.file = snapshot.file; + if (snapshot.caching && (!entry.initialized || fileChanged || !snapshot.eligible || !entry.reservedBytes)) { + queueCacheSettingUpdate(result.job.source, result.job.revision, false); + return; } - - for (auto &worker : workers_to_join) { - if (worker.joinable()) { - worker.join(); - } + entry.initialized = true; + if (snapshot.caching) + return; + releaseBudget(entry); + if (!snapshot.eligible) + return; + if (!snapshot.estimatedCacheBytes || !snapshot.playing) { + if (++entry.retries < MAX_POLLS) + entry.retryAt = m_now() + (snapshot.estimatedCacheBytes ? std::chrono::milliseconds(100) : std::chrono::milliseconds(500)); + return; } - - { - std::lock_guard lock(moved_ptr->mtx); - removeCachedMemory(*moved_ptr, true, source_name); - obs_source_release(moved_ptr->source); + if (snapshot.estimatedCacheBytes > m_cacheBudgetBytes - m_reservedCacheBytes) { + entry.waitingForBudget = true; + return; } + entry.reservedBytes = snapshot.estimatedCacheBytes; + m_reservedCacheBytes += snapshot.estimatedCacheBytes; + queueCacheSettingUpdate(result.job.source, result.job.revision, true); } -void MemoryManager::shutdownAllSources() +void MediaCacheManager::run() { - std::vector>> sources_to_shutdown; - { - std::unique_lock ulock(manager_mutex); - for (auto &pair : sources) { - sources_to_shutdown.emplace_back(pair.first, std::move(pair.second)); + std::unique_lock lock(m_mutex); + while (!m_stopping) { + m_notified = false; + std::vector> released; + released.swap(m_retiredEntries); + for (auto &entry : released) + releaseBudget(*entry); + std::vector cancelled; + for (auto it = m_graphicsJobs.begin(); it != m_graphicsJobs.end();) { + if (it->source->removed) { + cancelled.push_back(std::move(*it)); + it = m_graphicsJobs.erase(it); + } else { + ++it; + } + } + std::vector finished; + finished.swap(m_completions); + for (auto &result : finished) + complete(result); + + const auto currentTime = m_now(); + auto nextRetry = Clock::time_point::max(); + for (auto &item : m_sources) { + auto &entry = *item.second; + if (entry.removed || entry.hasPendingGraphicsJob) + continue; + const auto revision = entry.revision.load(); + const bool updated = entry.scheduledRevision != revision; + if (updated || (m_rebalance && entry.waitingForBudget) || entry.retryAt <= currentTime) { + if (updated) + entry.retries = 0; + entry.scheduledRevision = revision; + entry.retryAt = Clock::time_point::max(); + entry.waitingForBudget = false; + entry.hasPendingGraphicsJob = true; + m_graphicsJobs.push_back({item.second, revision, JobType::QuerySource}); + } else { + nextRetry = std::min(nextRetry, entry.retryAt); + } } - sources.clear(); + m_rebalance = false; + m_changed.notify_all(); + + // Final source release performs synchronous OBS cleanup and can invoke + // callbacks, so release references without holding m_mutex. + // Destruction also queues deferred callbacks; these clears do not wait + // for them to finish. + lock.unlock(); + released.clear(); + cancelled.clear(); + finished.clear(); + lock.lock(); + if (nextRetry == Clock::time_point::max()) + m_changed.wait(lock, [this] { return m_stopping || m_notified; }); + else + m_changed.wait_for(lock, nextRetry - m_now(), [this] { return m_stopping || m_notified; }); } - for (const auto &[key, value] : sources_to_shutdown) { - blog(LOG_INFO, "MemoryManager: shutdownAllSources: source %s", key.c_str()); - - for (auto &worker : value->workers) { - if (worker.joinable()) - worker.join(); - } + // These locals keep source references alive until after the mutex is unlocked. + // Final release can acquire OBS locks or invoke callbacks that reenter the + // manager; doing that while holding the mutex could deadlock. + auto old_sources = std::exchange(m_sources, {}); + auto old_graphicsJobs = std::exchange(m_graphicsJobs, {}); + auto old_completions = std::exchange(m_completions, {}); + auto old_retiredEntries = std::exchange(m_retiredEntries, {}); + m_reservedCacheBytes = 0; + lock.unlock(); +} - std::scoped_lock lock(manager_mutex, value->mtx); - removeCachedMemory(*value, false, key); - obs_source_release(value->source); +void MediaCacheManager::shutdown() +{ + if (!m_worker.joinable()) + return; + { + std::lock_guard lock(m_mutex); + m_accepting = false; + for (auto &item : m_sources) + item.second->removed = true; + } + // OBS synchronizes removal with any tick currently executing. Our queue is + // owned here, not by obs_queue_task: reset/stopped video cannot discard a + // task payload or strand the worker waiting for a graphics completion. + obs_remove_tick_callback(graphicsTick, this); + { + std::lock_guard lock(m_mutex); + m_stopping = true; } + m_changed.notify_all(); + m_worker.join(); } diff --git a/obs-studio-server/source/memory-manager.h b/obs-studio-server/source/memory-manager.h index 398a3de54..9438e80c3 100644 --- a/obs-studio-server/source/memory-manager.h +++ b/obs-studio-server/source/memory-manager.h @@ -17,62 +17,137 @@ ******************************************************************************/ #pragma once -#include "obs.h" -#include "nodeobs_configManager.hpp" + +#include +#include +#include +#include +#include #include +#include #include -#include -#include +#include #include -#include +#include -#ifdef WIN32 -#include -#include "psapi.h" -#endif +// Manages in-memory caching of looping local media to reduce repeated decoding. +// Enables or disables source caches as activity and settings change, keeping +// 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. +// +// The ffmpeg_source metadata handlers execute synchronously and access its +// current media player. OBS source updates and video ticks can destroy or +// replace that player on the graphics thread. Retaining the OBS source does +// 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. +class MediaCacheManager { +public: + // 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(); -#define LIMIT 2004800000ul -#define MAX_POLLS 10 + // Calls shutdown(). If still running, destruction has the same thread and + // OBS-lifetime requirements; explicitly shut down before obs_shutdown(). + ~MediaCacheManager(); -// Implements 'Singleton' design pattern -class MemoryManager { -public: - static MemoryManager &GetInstance(); - virtual ~MemoryManager(); + // Starts the worker and registers its graphics tick callback after OBS startup. + // Call from the serialized OBS lifecycle, before connecting source callbacks; + // do not overlap initialization with source operations or shutdown(). + // Does nothing if already running; can be called again after shutdown(). + // Video may be absent: queued media work waits for graphics ticks. + void initialize(); + // Registers a borrowed, live ffmpeg_source and retains its own OBS reference. + // Safe from OBS callbacks; schedules evaluation without waiting for completion. + // Null/unsupported sources and calls while stopped or stopping are ignored. + // Duplicate source identities keep their existing entry. + // Evaluation requires graphics ticks. Registration does not guarantee caching: + // settings, activity, media readiness and the memory budget govern the decision. void registerSource(obs_source_t *source); + + // Stops tracking this source identity and invalidates its pending jobs. + // Safe from OBS callbacks; null or unregistered sources are ignored. + // Does not wait for pending work to release its references; an operation already + // executing may finish. Caller-owned references are unchanged. Does not call + // obs_source_remove() or explicitly disable the source's cache setting. void unregisterSource(obs_source_t *source); - void shutdownAllSources(); - void updateSourceCache(obs_source_t *source); - void updateSourcesCache(); + + // Requests reevaluation after a registered source's settings or activity change. + // 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. + void requestCacheUpdate(obs_source_t *source); + + // Requests reevaluation of every registered source, for example after a global + // caching preference change. Safe from OBS callbacks; coalesces requests and + // does not wait for completion. Ignored while stopped or stopping. + void requestAllCacheUpdates(); + + // Stops accepting requests, removes the tick callback, cancels queued work, + // joins the worker and drops all retained source references. + // 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, + // but can cancel queued work with stopped or uninitialized video. + // Does nothing if already stopped. Does not drain OBS's deferred destroy queue. + void shutdown(); private: - // Types - struct source_info; - - // Constructors - MemoryManager(); - - // Copiers/movers - MemoryManager(MemoryManager const &) = delete; - MemoryManager &operator=(MemoryManager const &) = delete; - MemoryManager(MemoryManager const &&) = delete; - MemoryManager &operator=(MemoryManager const &&) = delete; - - // Methods - bool isSourceValid(obs_source_t *source) const; - void updateSettings(obs_source_t *source); - - void calculateRawSize(source_info &si); - bool shouldCacheSource(source_info &si); - void addCachedMemory(source_info &si); - void removeCachedMemory(source_info &si, bool cacheNewFiles, const std::string &sourceName); - void sourceManager(const std::string &sourceName); - - // Data - std::mutex manager_mutex; - std::map> sources; - uint64_t available_memory; - uint64_t current_cached_size; - uint64_t allowed_cached_size; + friend class MediaCacheManagerTestAccess; + using Clock = std::chrono::steady_clock; + struct SourceEntry; + struct Snapshot { + std::string file; + bool eligible = false; + bool caching = false; + bool playing = false; + uint64_t estimatedCacheBytes = 0; + }; + enum class JobType { QuerySource, SetCaching }; + struct GraphicsJob { + std::shared_ptr source; + uint64_t revision; + JobType type = JobType::QuerySource; + bool targetCachingEnabled = false; + std::string file; + }; + struct Completion { + GraphicsJob job; + Snapshot snapshot; + bool valid = false; + bool applied = false; + }; + + explicit MediaCacheManager(uint64_t budget = 0); + MediaCacheManager(const MediaCacheManager &) = delete; + MediaCacheManager &operator=(const MediaCacheManager &) = delete; + static void graphicsTick(void *param, float seconds); + static Snapshot readSettings(obs_source_t *source); + static void queryMediaOnGraphicsThread(obs_source_t *source, Snapshot &snapshot); + static void setCaching(obs_source_t *source, bool caching); + void run(); + void complete(Completion &result); + void queueCacheSettingUpdate(const std::shared_ptr &source, uint64_t revision, bool targetCachingEnabled); + void releaseBudget(SourceEntry &source); + + // Protects only queues and bookkeeping. Never held during an OBS call, + // source release, wait for graphics, or thread join. + std::mutex m_mutex; + std::condition_variable m_changed; + std::map> m_sources; + std::vector> m_retiredEntries; + std::deque m_graphicsJobs; + std::vector m_completions; + std::thread m_worker; + bool m_accepting = false; + bool m_stopping = false; + bool m_notified = false; + bool m_rebalance = false; + uint64_t m_reservedCacheBytes = 0; + uint64_t m_cacheBudgetBytes; + // Injectable clock for deterministic retry tests. + std::function m_now = Clock::now; }; diff --git a/obs-studio-server/source/nodeobs_api.cpp b/obs-studio-server/source/nodeobs_api.cpp index 0b4a60c8f..c204fa79c 100644 --- a/obs-studio-server/source/nodeobs_api.cpp +++ b/obs-studio-server/source/nodeobs_api.cpp @@ -17,6 +17,7 @@ ******************************************************************************/ #include "nodeobs_api.h" +#include #include "osn-source.hpp" #include "osn-scene.hpp" #include "osn-sceneitem.hpp" @@ -144,7 +145,7 @@ struct ModuleLoadFailure { static std::vector moduleLoadFailures; static bool browserAccel = true; -static bool mediaFileCaching = true; +static std::atomic mediaFileCaching{true}; static uint32_t sdrWhiteLevel = 300; static uint32_t hdrNominalPeakLevel = 1000; static bool lowLatencyAudioBuffering = false; @@ -1862,9 +1863,9 @@ void OBS_API::destroyOBS_API(void) blog(LOG_WARNING, "OBS_API::destroyOBS_API - obs_wait_for_destroy_queue has finished, osn::Source::Manager::GetInstance() size is %d", osn::Source::Manager::GetInstance().size()); osn::Source::Manager::GetInstance().for_each([&sources](obs_source_t *source) { osn::Source::detach_source_signals(source); }); - MemoryManager::GetInstance().shutdownAllSources(); + MediaCacheManager::GetInstance().shutdown(); - // Releasing the memory manager refs can enqueue/complete more deferred + // Releasing the media cache manager refs can enqueue/complete more deferred // libobs destruction. Wait for that before obs_shutdown(). obs_wait_for_destroy_queue(); @@ -1905,6 +1906,9 @@ void OBS_API::destroyOBS_API(void) } } else { + // The cache worker exists even when there are no remaining media sources. + // Remove its graphics callback before the OBS core goes away. + MediaCacheManager::GetInstance().shutdown(); blog(LOG_DEBUG, "OBS_API::destroyOBS_API calling obs_shutdown, objects allocated %d", bnum_allocs()); obs_shutdown(); } @@ -2147,7 +2151,7 @@ void OBS_API::SetMediaFileCaching(void *data, const int64_t id, const std::vecto mediaFileCaching = args[0].value_union.ui32; config_set_bool(ConfigManager::getInstance().getGlobal(), "General", "fileCaching", mediaFileCaching); config_save_safe(ConfigManager::getInstance().getGlobal(), "tmp", nullptr); - MemoryManager::GetInstance().updateSourcesCache(); + MediaCacheManager::GetInstance().requestAllCacheUpdates(); rval.push_back(ipc::value((uint64_t)ErrorCode::Ok)); AUTO_DEBUG; } @@ -2195,7 +2199,7 @@ void OBS_API::GetBrowserAcceleration(void *data, const int64_t id, const std::ve void OBS_API::GetMediaFileCaching(void *data, const int64_t id, const std::vector &args, std::vector &rval) { rval.push_back(ipc::value((uint64_t)ErrorCode::Ok)); - rval.push_back(ipc::value((uint32_t)mediaFileCaching)); + rval.push_back(ipc::value((uint32_t)mediaFileCaching.load())); AUTO_DEBUG; } diff --git a/obs-studio-server/source/nodeobs_settings.cpp b/obs-studio-server/source/nodeobs_settings.cpp index 76793f399..87b3c34f9 100644 --- a/obs-studio-server/source/nodeobs_settings.cpp +++ b/obs-studio-server/source/nodeobs_settings.cpp @@ -3712,7 +3712,7 @@ void OBS_settings::saveAdvancedSettings(std::vector advancedSetting mediaFilesSettings.push_back(advancedSettings.at(index++)); saveGenericSettings(mediaFilesSettings, "General", ConfigManager::getInstance().getGlobal()); - MemoryManager::GetInstance().updateSourcesCache(); + MediaCacheManager::GetInstance().requestAllCacheUpdates(); } std::vector OBS_settings::getSettings(std::string nameCategory, CategoryTypes &type) diff --git a/obs-studio-server/source/osn-source.cpp b/obs-studio-server/source/osn-source.cpp index ef24b4477..d52544423 100644 --- a/obs-studio-server/source/osn-source.cpp +++ b/obs-studio-server/source/osn-source.cpp @@ -34,6 +34,7 @@ void osn::Source::initialize_global_signals() { + MediaCacheManager::GetInstance().initialize(); signal_handler_t *sh = obs_get_signal_handler(); signal_handler_connect(sh, "source_create", osn::Source::global_source_create_cb, nullptr); signal_handler_connect(sh, "source_activate", osn::Source::global_source_activate_cb, nullptr); @@ -44,6 +45,8 @@ void osn::Source::finalize_global_signals() { signal_handler_t *sh = obs_get_signal_handler(); signal_handler_disconnect(sh, "source_create", osn::Source::global_source_create_cb, nullptr); + signal_handler_disconnect(sh, "source_activate", osn::Source::global_source_activate_cb, nullptr); + signal_handler_disconnect(sh, "source_deactivate", osn::Source::global_source_deactivate_cb, nullptr); } void osn::Source::attach_source_signals(obs_source_t *src) @@ -74,7 +77,7 @@ void osn::Source::global_source_create_cb(void *ptr, calldata_t *cd) osn::Source::Manager::GetInstance().allocate(source); osn::Source::attach_source_signals(source); CallbackManager::addSource(source); - MemoryManager::GetInstance().registerSource(source); + MediaCacheManager::GetInstance().registerSource(source); } void osn::Source::global_source_activate_cb(void *ptr, calldata_t *cd) @@ -83,7 +86,7 @@ void osn::Source::global_source_activate_cb(void *ptr, calldata_t *cd) if (!calldata_get_ptr(cd, "source", &source)) { throw std::runtime_error("calldata did not contain source pointer"); } - MemoryManager::GetInstance().updateSourceCache(source); + MediaCacheManager::GetInstance().requestCacheUpdate(source); } void osn::Source::global_source_deactivate_cb(void *ptr, calldata_t *cd) @@ -92,7 +95,7 @@ void osn::Source::global_source_deactivate_cb(void *ptr, calldata_t *cd) if (!calldata_get_ptr(cd, "source", &source)) { throw std::runtime_error("calldata did not contain source pointer"); } - MemoryManager::GetInstance().updateSourceCache(source); + MediaCacheManager::GetInstance().requestCacheUpdate(source); } void osn::Source::global_source_destroy_cb(void *ptr, calldata_t *cd) @@ -114,7 +117,7 @@ void osn::Source::global_source_remove_cb(void *ptr, calldata_t *cd) throw std::runtime_error("calldata did not contain source pointer"); } - MemoryManager::GetInstance().unregisterSource(source); + MediaCacheManager::GetInstance().unregisterSource(source); obs_source_release(source); } @@ -352,7 +355,7 @@ void osn::Source::Update(void *data, const int64_t id, const std::vector +#include "memory-manager.h" +#include +#include +#include +#include +#include +#include +#include + +using namespace std::chrono_literals; + +// Control time and the graphics executor, but run the production worker, +// accounting and OBS adapter. No cache policy is duplicated in the tests. +class MediaCacheManagerTestAccess { +public: + static std::unique_ptr create(uint64_t budget, std::atomic *milliseconds = nullptr) + { + auto manager = std::unique_ptr(new MediaCacheManager(budget)); + if (milliseconds) + manager->m_now = [milliseconds] { return MediaCacheManager::Clock::time_point(std::chrono::milliseconds(milliseconds->load())); }; + manager->initialize(); + return manager; + } + static void tick(MediaCacheManager &manager) { MediaCacheManager::graphicsTick(&manager, 0); } + static bool waitForQueuedGraphicsJob(MediaCacheManager &manager, std::chrono::milliseconds timeout = 2s) + { + std::unique_lock lock(manager.m_mutex); + return manager.m_changed.wait_for(lock, timeout, [&] { return !manager.m_graphicsJobs.empty(); }); + } + // Checks queues and notifications, excluding future retries and executing graphics jobs. + static bool waitForQueuesToDrain(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() && manager.m_graphicsJobs.empty() && manager.m_retiredEntries.empty(); + }); + } + static void wake(MediaCacheManager &manager) + { + { + std::lock_guard lock(manager.m_mutex); + manager.m_notified = true; + } + manager.m_changed.notify_all(); + } + static size_t queuedGraphicsJobCount(MediaCacheManager &manager) + { + std::lock_guard lock(manager.m_mutex); + return manager.m_graphicsJobs.size(); + } + static uint64_t reservedBytes(MediaCacheManager &manager) + { + std::lock_guard lock(manager.m_mutex); + return manager.m_reservedCacheBytes; + } +}; + +namespace { +struct MediaState { + std::atomic ready{true}; + std::atomic queries{0}; + std::atomic destroys{0}; + std::atomic graphicsOnly{true}; + std::function onQuery; +}; + +void getFileInfo(void *data, calldata_t *cd) +{ + auto &state = *static_cast(data); + ++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, "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. +} + +void getPlaying(void *data, calldata_t *cd) +{ + calldata_set_bool(cd, "playing", static_cast(data)->ready); +} + +class ObsCore { +public: + ObsCore() + { + REQUIRE(obs_startup("en-US", nullptr, nullptr)); + // It's a fake ffmpeg_source used for testing. + // As we didn't load any real plugins above, the source ID is available. + // No external files or decoder are needed for the scheduling tests. + obs_source_info info{}; + info.id = "ffmpeg_source"; + info.type = OBS_SOURCE_TYPE_INPUT; + info.output_flags = OBS_SOURCE_VIDEO; + 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")); + 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.destroy = [](void *data) { ++static_cast(data)->destroys; }; + info.get_width = [](void *) -> uint32_t { return 10; }; + info.get_height = [](void *) -> uint32_t { return 10; }; + obs_register_source(&info); + } + ~ObsCore() + { + obs_wait_for_destroy_queue(); + obs_shutdown(); + } +}; + +OBSSourceAutoRelease makeCacheTestSource(const char *name, MediaState &state) +{ + OBSDataAutoRelease settings = obs_data_create(); + obs_data_set_int(settings, "test_state", reinterpret_cast(&state)); + obs_data_set_string(settings, "local_file", name); + obs_data_set_bool(settings, "is_local_file", true); + obs_data_set_bool(settings, "looping", true); + obs_data_set_bool(settings, "close_when_inactive", false); + return obs_source_create("ffmpeg_source", name, settings, nullptr); +} + +bool isCachingEnabled(obs_source_t *source) +{ + OBSDataAutoRelease settings = obs_source_get_settings(source); + return obs_data_get_bool(settings, "caching"); +} + +void setLooping(MediaCacheManager &manager, obs_source_t *source, bool looping) +{ + OBSDataAutoRelease settings = obs_data_create(); + obs_data_set_bool(settings, "looping", looping); + obs_source_update(source, settings); + manager.requestCacheUpdate(source); +} + +bool processGraphicsJobsUntil(MediaCacheManager &manager, const std::function &predicate) +{ + const auto deadline = std::chrono::steady_clock::now() + 3s; + while (!predicate() && std::chrono::steady_clock::now() < deadline) { + if (MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(manager, 20ms)) + MediaCacheManagerTestAccess::tick(manager); + } + return predicate(); +} +} + +TEST_CASE("Media cache serializes rebalance and coalesces notifications", "[media-cache]") +{ + std::array states; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300); + std::array sources{makeCacheTestSource("first", states[0]), makeCacheTestSource("second", states[1]), + makeCacheTestSource("third", states[2])}; + for (auto &source : sources) + manager->registerSource(source); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(*manager)); + for (int i = 0; i < 1000; ++i) + manager->requestAllCacheUpdates(); + CHECK(MediaCacheManagerTestAccess::queuedGraphicsJobCount(*manager) <= sources.size()); + auto cachedCount = [&] { return std::count_if(sources.begin(), sources.end(), [](const auto &source) { return isCachingEnabled(source); }); }; + REQUIRE(processGraphicsJobsUntil(*manager, [&] { return cachedCount() == 2; })); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 300); + + // Both cached entries become ineligible before either completion is handled. + for (auto &source : sources) { + if (isCachingEnabled(source)) + setLooping(*manager, source, false); + } + REQUIRE(processGraphicsJobsUntil(*manager, [&] { return cachedCount() == 1 && MediaCacheManagerTestAccess::reservedBytes(*manager) == 150; })); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); +} + +TEST_CASE("Media cache readiness retries do not block another source", "[media-cache]") +{ + MediaState waiting, ready; + waiting.ready = false; + std::atomic milliseconds{0}; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300, &milliseconds); + auto first = makeCacheTestSource("waiting", waiting); + auto second = makeCacheTestSource("ready", ready); + manager->registerSource(first); + manager->registerSource(second); + REQUIRE(processGraphicsJobsUntil(*manager, [&] { return isCachingEnabled(second); })); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK_FALSE(isCachingEnabled(first)); + CHECK(waiting.queries == 1); + for (unsigned attempt = 2; attempt <= 10; ++attempt) { + milliseconds += 500; + MediaCacheManagerTestAccess::wake(*manager); + REQUIRE(processGraphicsJobsUntil(*manager, [&] { return waiting.queries >= attempt; })); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + } + milliseconds += 60000; + MediaCacheManagerTestAccess::wake(*manager); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + CHECK_FALSE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(*manager, 20ms)); + CHECK(waiting.queries == 10); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); +} + +TEST_CASE("Media cache removal cancels stale work after a rename", "[media-cache]") +{ + MediaState oldState, newState; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300); + auto oldSource = makeCacheTestSource("reused name", oldState); + manager->registerSource(oldSource); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(*manager)); + obs_source_set_name(oldSource, "renamed"); + manager->unregisterSource(oldSource); + oldSource = nullptr; + auto replacement = makeCacheTestSource("reused name", newState); + manager->registerSource(replacement); + REQUIRE(processGraphicsJobsUntil(*manager, [&] { return isCachingEnabled(replacement); })); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuesToDrain(*manager)); + obs_wait_for_destroy_queue(); + CHECK(oldState.queries == 0); + CHECK(oldState.destroys == 1); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 150); +} + +TEST_CASE("Media cache shutdown cancels work without graphics and can restart", "[media-cache]") +{ + std::array states; + ObsCore core; + auto manager = MediaCacheManagerTestAccess::create(300); + for (auto &state : states) { + auto source = makeCacheTestSource("pending at shutdown", state); + manager->registerSource(source); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(*manager)); + source = nullptr; + manager->shutdown(); + obs_wait_for_destroy_queue(); + CHECK(state.queries == 0); + CHECK(state.destroys == 1); + CHECK(MediaCacheManagerTestAccess::reservedBytes(*manager) == 0); + manager->initialize(); + } +} + +TEST_CASE("Media cache uses graphics across video reset and permits callback reentry", "[media-cache][graphics]") +{ + MediaState state; + ObsCore core; + obs_add_data_path((std::string(OSN_TEST_WD) + "/data/libobs/").c_str()); + auto manager = MediaCacheManagerTestAccess::create(300); + auto source = makeCacheTestSource("graphics media", state); + std::atomic reentered{false}; + state.onQuery = [&] { + if (!reentered.exchange(true)) + manager->requestCacheUpdate(source); + }; + manager->registerSource(source); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(*manager)); + + obs_video_info info{}; +#ifdef _WIN32 + info.graphics_module = "libobs-d3d11.dll"; +#elif defined(__APPLE__) + info.graphics_module = "libobs-opengl.dylib"; +#else + info.graphics_module = "libobs-opengl.so"; +#endif + info.fps_num = 60; + info.fps_den = 1; + info.base_width = info.output_width = 320; + info.base_height = info.output_height = 180; + info.output_format = VIDEO_FORMAT_NV12; + info.colorspace = VIDEO_CS_709; + info.range = VIDEO_RANGE_PARTIAL; + info.scale_type = OBS_SCALE_BILINEAR; + REQUIRE(obs_reset_video(&info) == OBS_VIDEO_SUCCESS); + auto waitCached = [&] { + const auto deadline = std::chrono::steady_clock::now() + 3s; + while (!isCachingEnabled(source) && std::chrono::steady_clock::now() < deadline) + std::this_thread::sleep_for(5ms); + return isCachingEnabled(source); + }; + REQUIRE(waitCached()); + CHECK(reentered); + CHECK(state.graphicsOnly); + + REQUIRE(obs_remove_video_info(obs_get_video_info_by_index2(0)) == OBS_VIDEO_SUCCESS); + setLooping(*manager, source, false); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(*manager)); + // The cache manager owns this pending task, so resetting video cannot lose it. + REQUIRE(obs_reset_video(&info) == OBS_VIDEO_SUCCESS); + const auto deadline = std::chrono::steady_clock::now() + 3s; + while (isCachingEnabled(source) && std::chrono::steady_clock::now() < deadline) + std::this_thread::sleep_for(5ms); + CHECK_FALSE(isCachingEnabled(source)); + 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 f8fb046ac94e1e36849fa6bb39fb0d0ece234b93 Mon Sep 17 00:00:00 2001 From: Aleksandr Voitenko Date: Sat, 26 Sep 2026 04:50:21 +1200 Subject: [PATCH 2/3] Fix shutdown deadlock by stopping media cache before source teardown --- obs-studio-server/source/memory-manager.h | 2 + obs-studio-server/source/nodeobs_api.cpp | 21 ++--- .../tests/test-memory-manager.cpp | 90 +++++++++++++++++++ 3 files changed, 100 insertions(+), 13 deletions(-) diff --git a/obs-studio-server/source/memory-manager.h b/obs-studio-server/source/memory-manager.h index 9438e80c3..60866945a 100644 --- a/obs-studio-server/source/memory-manager.h +++ b/obs-studio-server/source/memory-manager.h @@ -93,6 +93,8 @@ class MediaCacheManager { // shutdown() or initialize(). Waits for any executing tick callback and worker, // but can cancel queued work with stopped or uninitialized video. // Does nothing if already stopped. Does not drain OBS's deferred destroy queue. + // During OSN teardown, call this and then obs_wait_for_destroy_queue() before + // traversing the source registry to connect/disconnect source signals. void shutdown(); private: diff --git a/obs-studio-server/source/nodeobs_api.cpp b/obs-studio-server/source/nodeobs_api.cpp index c204fa79c..dbec2cc16 100644 --- a/obs-studio-server/source/nodeobs_api.cpp +++ b/obs-studio-server/source/nodeobs_api.cpp @@ -1774,10 +1774,13 @@ void OBS_API::destroyOBS_API(void) delete fileOutput; }); + // Stop the cache worker before draining destruction: it can still own the + // last reference to a removed source and enqueue its destruction later. + // Both source-registry walks below connect/disconnect signals while holding + // the registry mutex; a concurrent destroy callback takes those locks in the + // opposite order. Join the worker, then drain its callbacks before either walk. + MediaCacheManager::GetInstance().shutdown(); obs_wait_for_destroy_queue(); - // obs_set_output_source might cause destruction of some sources. - // Wait for the destruction thread to destroy the sources to be sure - // |for_each| below will only return actual remaining sources. // Check if the frontend was able to shutdown correctly: // If there are some sources here it's because it ended unexpectedly, this represents a @@ -1858,16 +1861,11 @@ void OBS_API::destroyOBS_API(void) // manager only contains sources that are still hanging around unexpectedly. obs_wait_for_destroy_queue(); - // Detach node-side destroy/remove callbacks from the remaining leaked - // sources, then drop the memory manager refs they still hold. + // The cache worker is stopped and deferred destruction has drained, so + // detach node-side destroy/remove callbacks from the remaining leaked sources. blog(LOG_WARNING, "OBS_API::destroyOBS_API - obs_wait_for_destroy_queue has finished, osn::Source::Manager::GetInstance() size is %d", osn::Source::Manager::GetInstance().size()); osn::Source::Manager::GetInstance().for_each([&sources](obs_source_t *source) { osn::Source::detach_source_signals(source); }); - MediaCacheManager::GetInstance().shutdown(); - - // Releasing the media cache manager refs can enqueue/complete more deferred - // libobs destruction. Wait for that before obs_shutdown(). - obs_wait_for_destroy_queue(); #ifdef WIN32 // Directly blame the frontend since it didn't release all objects and that could cause @@ -1906,9 +1904,6 @@ void OBS_API::destroyOBS_API(void) } } else { - // The cache worker exists even when there are no remaining media sources. - // Remove its graphics callback before the OBS core goes away. - MediaCacheManager::GetInstance().shutdown(); blog(LOG_DEBUG, "OBS_API::destroyOBS_API calling obs_shutdown, objects allocated %d", bnum_allocs()); obs_shutdown(); } diff --git a/obs-studio-server/tests/test-memory-manager.cpp b/obs-studio-server/tests/test-memory-manager.cpp index 578e39233..01180ef68 100644 --- a/obs-studio-server/tests/test-memory-manager.cpp +++ b/obs-studio-server/tests/test-memory-manager.cpp @@ -1,5 +1,7 @@ #include #include "memory-manager.h" +#include "obs-setup.hpp" +#include "osn-source.hpp" #include #include #include @@ -54,6 +56,11 @@ class MediaCacheManagerTestAccess { std::lock_guard lock(manager.m_mutex); return manager.m_reservedCacheBytes; } + static bool isAcceptingRequests(MediaCacheManager &manager) + { + std::lock_guard lock(manager.m_mutex); + return manager.m_accepting; + } }; namespace { @@ -250,6 +257,89 @@ TEST_CASE("Media cache shutdown cancels work without graphics and can restart", } } +namespace { +void checkApiShutdown(bool removeMedia) +{ + std::atomic mediaDestroys{0}; + std::atomic observerDestroyed{false}; + std::atomic cacheAcceptingAtDestroy{true}; + struct Observation { + std::atomic &destroyed; + std::atomic &cacheAccepting; + } observation{observerDestroyed, cacheAcceptingAtDestroy}; + { + // Use the real OSN source registry, callbacks and shutdown entry point, + // including the crash-reporting state required by leftover-source cleanup. + osn::tests::ObsSetup setup; + auto &manager = MediaCacheManager::GetInstance(); + // Initialization removes its temporary video mix. Leave graphics stopped + // so the cache query remains pending until shutdown cancels it. + REQUIRE(obs_get_video_info_by_index2(0) == nullptr); + OBSDataAutoRelease settings = obs_data_create(); + obs_data_set_bool(settings, "is_local_file", true); + obs_data_set_bool(settings, "looping", true); + OBSSourceAutoRelease media = obs_source_create("ffmpeg_source", "pending at API shutdown", settings, nullptr); + REQUIRE(media != nullptr); + REQUIRE(MediaCacheManagerTestAccess::waitForQueuedGraphicsJob(manager)); + signal_handler_connect( + obs_source_get_signal_handler(media), "destroy", [](void *data, calldata_t *) { ++*static_cast *>(data); }, + &mediaDestroys); + + // Leave one frontend-owned reference for the API's leftover-source cleanup. + // The cache manager holds its own reference until shutdown joins the worker. + obs_source_get_ref(media); + obs_source_info info{}; + info.id = "cache_shutdown_observer"; + info.type = OBS_SOURCE_TYPE_INPUT; + info.get_name = [](void *) { return "Cache shutdown observer"; }; + info.create = [](obs_data_t *, obs_source_t *source) -> void * { return source; }; + info.destroy = [](void *) {}; + obs_register_source(&info); + obs_source_t *observer = obs_source_create(info.id, "shutdown observer", nullptr, nullptr); + REQUIRE(observer != nullptr); + // This input is not retained by the media cache. Its destroy callback therefore + // runs during the API's explicit source-release/drain phase. Stopping the + // cache worker after that phase is too late to protect the registry walks. + signal_handler_connect( + obs_source_get_signal_handler(observer), "destroy", + [](void *data, calldata_t *) { + auto &result = *static_cast(data); + result.cacheAccepting = MediaCacheManagerTestAccess::isAcceptingRequests(MediaCacheManager::GetInstance()); + result.destroyed = true; + }, + &observation); + + if (removeMedia) + obs_source_remove(media); + } + CHECK(observerDestroyed); + CHECK_FALSE(cacheAcceptingAtDestroy); + CHECK(mediaDestroys == 1); + CHECK(osn::Source::Manager::GetInstance().size() == 0); + CHECK_FALSE(MediaCacheManagerTestAccess::isAcceptingRequests(MediaCacheManager::GetInstance())); +} +} + +TEST_CASE("OBS API stops the media cache before source teardown", "[media-cache][shutdown]") +{ + checkApiShutdown(false); +} + +TEST_CASE("OBS API stops the media cache after removing a source", "[media-cache][shutdown]") +{ + checkApiShutdown(true); +} + +TEST_CASE("OBS API stops the media cache with an empty source registry", "[media-cache][shutdown]") +{ + { + osn::tests::ObsSetup setup; + CHECK(MediaCacheManagerTestAccess::isAcceptingRequests(MediaCacheManager::GetInstance())); + CHECK(osn::Source::Manager::GetInstance().size() == 0); + } + CHECK_FALSE(MediaCacheManagerTestAccess::isAcceptingRequests(MediaCacheManager::GetInstance())); +} + TEST_CASE("Media cache uses graphics across video reset and permits callback reentry", "[media-cache][graphics]") { MediaState state; From 840c9509569e619d1134411181576b1d02c577b2 Mon Sep 17 00:00:00 2001 From: Aleksandr Voitenko Date: Tue, 29 Sep 2026 12:47:34 +1300 Subject: [PATCH 3/3] Fix media caching preference synchronization Update the runtime preference when saving Advanced Settings and loading configuration at startup. Share the update path with SetMediaFileCaching. Add regression tests for preference changes and startup persistence. --- js/module.ts | 2 + obs-studio-server/CMakeLists.txt | 1 + obs-studio-server/source/nodeobs_api.cpp | 13 ++- obs-studio-server/source/nodeobs_api.h | 4 + obs-studio-server/source/nodeobs_settings.cpp | 3 +- obs-studio-server/source/nodeobs_settings.h | 2 + .../tests/test-media-cache-settings.cpp | 107 ++++++++++++++++++ 7 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 obs-studio-server/tests/test-media-cache-settings.cpp diff --git a/js/module.ts b/js/module.ts index 293c70210..7ea7d1828 100644 --- a/js/module.ts +++ b/js/module.ts @@ -2459,6 +2459,8 @@ interface INodeObs { * Converting a common RTMP service to custom restores the exact native URL * when the submitted server matches the previous form value. Inherited automatic * selections are resolved to a URL; explicitly edited server values are preserved. + * Saving the Advanced `fileCaching` option also updates the runtime preference; + * source caches are reevaluated asynchronously on subsequent graphics ticks. * @param category - Category name, such as `Stream` or `StreamSecond` * @param settings - The `data` array from `OBS_settings_getSettings(category)`, * with edited `currentValue` fields; keep the stream type field first diff --git a/obs-studio-server/CMakeLists.txt b/obs-studio-server/CMakeLists.txt index 021ec0eb7..b0ab17365 100644 --- a/obs-studio-server/CMakeLists.txt +++ b/obs-studio-server/CMakeLists.txt @@ -624,6 +624,7 @@ if(BUILD_TESTING) add_executable( obs_studio_server_unit_tests + "tests/test-media-cache-settings.cpp" "tests/test-memory-manager.cpp" "tests/test-osn-source.cpp" "tests/test-osn-encoder-settings.cpp" diff --git a/obs-studio-server/source/nodeobs_api.cpp b/obs-studio-server/source/nodeobs_api.cpp index dbec2cc16..1785bb03f 100644 --- a/obs-studio-server/source/nodeobs_api.cpp +++ b/obs-studio-server/source/nodeobs_api.cpp @@ -1010,6 +1010,7 @@ void OBS_API::OBS_API_initAPI(void *data, const int64_t id, const std::vector &args, std::vector &rval) { - mediaFileCaching = args[0].value_union.ui32; - config_set_bool(ConfigManager::getInstance().getGlobal(), "General", "fileCaching", mediaFileCaching); + config_set_bool(ConfigManager::getInstance().getGlobal(), "General", "fileCaching", args[0].value_union.ui32 != 0); config_save_safe(ConfigManager::getInstance().getGlobal(), "tmp", nullptr); - MediaCacheManager::GetInstance().requestAllCacheUpdates(); + applyMediaFileCachingSetting(); rval.push_back(ipc::value((uint64_t)ErrorCode::Ok)); AUTO_DEBUG; } @@ -2184,6 +2184,13 @@ bool OBS_API::getMediaFileCaching() return mediaFileCaching; } +void OBS_API::applyMediaFileCachingSetting() +{ + // Cache evaluation reads the atomic preference rather than the mutable configuration. + mediaFileCaching = config_get_bool(ConfigManager::getInstance().getGlobal(), "General", "fileCaching"); + MediaCacheManager::GetInstance().requestAllCacheUpdates(); +} + void OBS_API::GetBrowserAcceleration(void *data, const int64_t id, const std::vector &args, std::vector &rval) { rval.push_back(ipc::value((uint64_t)ErrorCode::Ok)); diff --git a/obs-studio-server/source/nodeobs_api.h b/obs-studio-server/source/nodeobs_api.h index 8904db07e..c7bf9cc03 100644 --- a/obs-studio-server/source/nodeobs_api.h +++ b/obs-studio-server/source/nodeobs_api.h @@ -106,6 +106,10 @@ class OBS_API { static bool getBrowserAcceleration(); static bool getMediaFileCaching(); + // Publishes General/fileCaching to the runtime flag before reevaluating sources. + // Call on the settings thread after loading or saving the global configuration. + // Cache changes are asynchronous and require graphics ticks. + static void applyMediaFileCachingSetting(); public: static void initAPI(void); diff --git a/obs-studio-server/source/nodeobs_settings.cpp b/obs-studio-server/source/nodeobs_settings.cpp index 87b3c34f9..237c1c321 100644 --- a/obs-studio-server/source/nodeobs_settings.cpp +++ b/obs-studio-server/source/nodeobs_settings.cpp @@ -22,7 +22,6 @@ #include "nodeobs_api.h" #include "shared.hpp" #include -#include "memory-manager.h" #include "osn-video.hpp" #include "osn-encoders.hpp" @@ -3712,7 +3711,7 @@ void OBS_settings::saveAdvancedSettings(std::vector advancedSetting mediaFilesSettings.push_back(advancedSettings.at(index++)); saveGenericSettings(mediaFilesSettings, "General", ConfigManager::getInstance().getGlobal()); - MediaCacheManager::GetInstance().requestAllCacheUpdates(); + OBS_API::applyMediaFileCachingSetting(); } std::vector OBS_settings::getSettings(std::string nameCategory, CategoryTypes &type) diff --git a/obs-studio-server/source/nodeobs_settings.h b/obs-studio-server/source/nodeobs_settings.h index 16db6bcbc..e7d74ab4c 100644 --- a/obs-studio-server/source/nodeobs_settings.h +++ b/obs-studio-server/source/nodeobs_settings.h @@ -162,6 +162,8 @@ class OBS_settings { static void OBS_settings_setEnhancedBroadcasting(void *data, const int64_t id, const std::vector &args, std::vector &rval); private: + friend class OBSSettingsTestAccess; + // Exposed methods to the frontend static std::vector getSettings(std::string nameCategory, CategoryTypes &); static bool saveSettings(std::string nameCategory, std::vector settings); diff --git a/obs-studio-server/tests/test-media-cache-settings.cpp b/obs-studio-server/tests/test-media-cache-settings.cpp new file mode 100644 index 000000000..47254c107 --- /dev/null +++ b/obs-studio-server/tests/test-media-cache-settings.cpp @@ -0,0 +1,107 @@ +#include + +#include "nodeobs_api.h" +#include "nodeobs_configManager.hpp" +#include "nodeobs_settings.h" +#include "obs-setup.hpp" +#include "osn-error.hpp" + +#include +#include + +// Exercise Advanced Settings with its native metadata, without resetting video +// or duplicating the IPC serialization in this preference regression. +class OBSSettingsTestAccess { +public: + static void saveAdvancedMediaCaching(bool enabled) + { + auto settings = OBS_settings::getAdvancedSettings(); + for (auto &category : settings) { + for (auto ¶meter : category.params) { + if (parameter.name != "fileCaching") + continue; + REQUIRE(parameter.currentValue.size() == sizeof(enabled)); + std::memcpy(parameter.currentValue.data(), &enabled, sizeof(enabled)); + OBS_settings::saveAdvancedSettings(settings); + return; + } + } + FAIL("Advanced Settings did not expose fileCaching"); + } +}; + +namespace { + +// Restore the saved preference, including the absence of an explicit override. +// Resolve the configuration again because the startup test reloads it from disk. +class ScopedMediaCachingPreference { +public: + ScopedMediaCachingPreference() + { + auto *config = ConfigManager::getInstance().getGlobal(); + hadUserValue = config_has_user_value(config, "General", "fileCaching"); + value = config_get_bool(config, "General", "fileCaching"); + } + ~ScopedMediaCachingPreference() + { + std::vector response; + OBS_API::SetMediaFileCaching(nullptr, 0, {ipc::value(static_cast(value))}, response); + if (!hadUserValue) { + auto *config = ConfigManager::getInstance().getGlobal(); + config_remove_value(config, "General", "fileCaching"); + config_save_safe(config, "tmp", nullptr); + } + } + +private: + bool hadUserValue; + bool value; +}; + +void checkMediaCachingPreference(bool expected) +{ + CHECK(config_get_bool(ConfigManager::getInstance().getGlobal(), "General", "fileCaching") == expected); + CHECK(OBS_API::getMediaFileCaching() == expected); + std::vector response; + OBS_API::GetMediaFileCaching(nullptr, 0, {}, response); + REQUIRE(response.size() == 2); + CHECK(static_cast(response[0].value_union.ui64) == ErrorCode::Ok); + CHECK(static_cast(response[1].value_union.ui32) == expected); +} + +} // namespace + +TEST_CASE("Media caching preferences stay synchronized across settings APIs", "[media-cache][settings]") +{ + osn::tests::ObsSetup setup; + ScopedMediaCachingPreference restorePreference; + for (bool enabled : {false, true, false}) { + INFO("Advanced fileCaching = " << enabled); + // Seed the opposite value through the dedicated API. Then exercise both + // directions through the Advanced Settings save path that missed the flag. + std::vector response; + OBS_API::SetMediaFileCaching(nullptr, 0, {ipc::value(static_cast(!enabled))}, response); + REQUIRE(response.size() == 1); + CHECK(static_cast(response[0].value_union.ui64) == ErrorCode::Ok); + checkMediaCachingPreference(!enabled); + OBSSettingsTestAccess::saveAdvancedMediaCaching(enabled); + checkMediaCachingPreference(enabled); + } +} + +TEST_CASE("Media caching honors the saved preference on startup", "[media-cache][settings]") +{ + const std::string configPath = std::string(OSN_SOURCE_DIR) + "/tests/osn-tests/osnData/slobs-client"; + std::filesystem::create_directories(configPath); + auto &configs = ConfigManager::getInstance(); + configs.setAppdataPath(configPath); + ScopedMediaCachingPreference restorePreference; + config_set_bool(configs.getGlobal(), "General", "fileCaching", false); + REQUIRE(config_save_safe(configs.getGlobal(), "tmp", nullptr) == CONFIG_SUCCESS); + configs.reloadConfig(); + + // The default runtime flag is true. Startup must read the saved false value + // without requiring the frontend to call SetMediaFileCaching afterward. + osn::tests::ObsSetup setup; + checkMediaCachingPreference(false); +}