From dde1a73132ec5cbd7a5704be361ef405332dbb0a Mon Sep 17 00:00:00 2001 From: jp9000 Date: Fri, 10 Apr 2015 17:56:21 -0700 Subject: [PATCH] libobs: Fix async texture data race The obs_source::async_reset_texture variable can cause a data race between threads to occur because it could be set to true in one thread then changed back to false in another thread. This could cause the async texture to not update its size when it's supposed to, which can cause a crash or corruption when copying data from a frame of a differing size. The solution to this is to: - Delete the async_reset_texture variable, and make the set_async_texture_size function change the texture size if the async_width, async_height, or async_format variables differ from the frame's width/height/format. Those variables are then only ever set in the libobs graphics thread. - Make the cache_video function use separate variables from other functions to detect a change in size (due to the fact that the texture size should only be resized in the libobs graphics thread). These variables are async_cache_width, async_cache_height, and async_cache_format, which are only be set in the thread that calls obs_source_output_video. How to replicate the data race: - On OSX, use window capture on a textedit window, then continually resize the textedit window. --- libobs/obs-internal.h | 4 +++- libobs/obs-source.c | 29 +++++++++++++---------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/libobs/obs-internal.h b/libobs/obs-internal.h index 852d89aec..1a9350fa7 100644 --- a/libobs/obs-internal.h +++ b/libobs/obs-internal.h @@ -344,6 +344,7 @@ struct obs_source { struct obs_source_frame *cur_async_frame; bool async_gpu_conversion; enum video_format async_format; + enum video_format async_cache_format; enum gs_color_format async_texture_format; float async_color_matrix[16]; bool async_full_range; @@ -352,12 +353,13 @@ struct obs_source { int async_plane_offset[2]; bool async_flip; bool async_active; - bool async_reset_texture; DARRAY(struct async_frame) async_cache; DARRAY(struct obs_source_frame*)async_frames; pthread_mutex_t async_mutex; uint32_t async_width; uint32_t async_height; + uint32_t async_cache_width; + uint32_t async_cache_height; uint32_t async_convert_width; uint32_t async_convert_height; diff --git a/libobs/obs-source.c b/libobs/obs-source.c index ecc177d10..a6d0d3ee2 100644 --- a/libobs/obs-source.c +++ b/libobs/obs-source.c @@ -206,9 +206,6 @@ obs_source_t *obs_source_create(enum obs_source_type type, const char *id, source->flags = source->default_flags; source->enabled = true; - /* prevents the source from clearing the cache on the first frame */ - source->async_reset_texture = true; - if (info && info->type == OBS_SOURCE_TYPE_TRANSITION) os_atomic_inc_long(&obs->data.active_transitions); return source; @@ -879,10 +876,14 @@ static inline bool set_async_texture_size(struct obs_source *source, { enum convert_type cur = get_convert_type(frame->format); - if (!source->async_reset_texture) + if (source->async_width == frame->width && + source->async_height == frame->height && + source->async_format == frame->format) return true; - source->async_reset_texture = false; + source->async_width = frame->width; + source->async_height = frame->height; + source->async_format = frame->format; gs_texture_destroy(source->async_texture); gs_texrender_destroy(source->async_convert_texrender); @@ -1612,11 +1613,11 @@ static inline bool async_texture_changed(struct obs_source *source, const struct obs_source_frame *frame) { enum convert_type prev, cur; - prev = get_convert_type(source->async_format); + prev = get_convert_type(source->async_cache_format); cur = get_convert_type(frame->format); - return source->async_width != frame->width || - source->async_height != frame->height || + return source->async_cache_width != frame->width || + source->async_cache_height != frame->height || prev != cur; } @@ -1655,14 +1656,10 @@ static inline struct obs_source_frame *cache_video(struct obs_source *source, pthread_mutex_lock(&source->async_mutex); if (async_texture_changed(source, frame)) { - /* prevents the cache from being freed on the first frame */ - if (!source->async_reset_texture) - free_async_cache(source); - - source->async_width = frame->width; - source->async_height = frame->height; - source->async_format = frame->format; - source->async_reset_texture = true; + free_async_cache(source); + source->async_cache_width = frame->width; + source->async_cache_height = frame->height; + source->async_cache_format = frame->format; } for (size_t i = 0; i < source->async_cache.num; i++) {