From 4508cb03b57d72abe26ce2c9eb56650d07127ccd Mon Sep 17 00:00:00 2001 From: Valentin Date: Mon, 3 Aug 2020 21:08:02 +0200 Subject: [PATCH] libobs: Fix deferred update sometimes using stale data Currently we use a bool flag to signal the video thread that it should call obs_source_deferred_update. This does not work correctly when the update callback is slow and the update is triggered faster than the callback can complete. For example: * the settings are set to state A * defer_update is set * obs_source_deferred_update is called and enters into the callback * the callback starts making use of the settings in state A * the settings are set to state B * defer_update stays set * the callback finishes * defer_update is set to false Now defer_update is false but the callback has only observed settings in state A but not B. This commit fixes this bug by keeping an update counter. If the counter has changed while we were in the callback we know that we need to update again. The counter is atomic. The current version uses a plain bool which is a data race as the value is written and read in parallel. --- libobs/obs-internal.h | 2 +- libobs/obs-source.c | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/libobs/obs-internal.h b/libobs/obs-internal.h index 38b8cba6f..28ba9875c 100644 --- a/libobs/obs-internal.h +++ b/libobs/obs-internal.h @@ -599,7 +599,7 @@ struct obs_source { bool owns_info_id; /* signals to call the source update in the video thread */ - bool defer_update; + long defer_update_count; /* ensures show/hide are only called once */ volatile long show_refs; diff --git a/libobs/obs-source.c b/libobs/obs-source.c index b6408f9a4..0b4f383e4 100644 --- a/libobs/obs-source.c +++ b/libobs/obs-source.c @@ -860,11 +860,13 @@ uint32_t obs_get_source_output_flags(const char *id) static void obs_source_deferred_update(obs_source_t *source) { - if (source->context.data && source->info.update) + if (source->context.data && source->info.update) { + long count = os_atomic_load_long(&source->defer_update_count); source->info.update(source->context.data, source->context.settings); - - source->defer_update = false; + os_atomic_compare_swap_long(&source->defer_update_count, count, + 0); + } } void obs_source_update(obs_source_t *source, obs_data_t *settings) @@ -876,7 +878,7 @@ void obs_source_update(obs_source_t *source, obs_data_t *settings) obs_data_apply(source->context.settings, settings); if (source->info.output_flags & OBS_SOURCE_VIDEO) { - source->defer_update = true; + os_atomic_inc_long(&source->defer_update_count); } else if (source->context.data && source->info.update) { source->info.update(source->context.data, source->context.settings); @@ -1101,7 +1103,7 @@ void obs_source_video_tick(obs_source_t *source, float seconds) if ((source->info.output_flags & OBS_SOURCE_ASYNC) != 0) async_tick(source); - if (source->defer_update) + if (os_atomic_load_long(&source->defer_update_count) > 0) obs_source_deferred_update(source); /* reset the filter render texture information once every frame */