Browse Source

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.
jp9000 10 years ago
parent
commit
dde1a73132
2 changed files with 16 additions and 17 deletions
  1. 3 1
      libobs/obs-internal.h
  2. 13 16
      libobs/obs-source.c

+ 3 - 1
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;
 

+ 13 - 16
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++) {