GNOME Bugzilla – Bug 779247
Fix race in videotexturecache
Last modified: 2017-03-17 10:35:54 UTC
It is possible for the videotexturecache to be finalized before all of its textures. But when you then try to finalize a texture, the program will crash. This happens during a race where avfvideosrc is stopped and there is a still a buffer that it produced somewhere in the pipeline.
Created attachment 346740 [details] [review] Refactor duplicate code; ensure objects freed in right order
Review of attachment 346740 [details] [review]: ::: sys/applemedia/videotexturecache.m @@ +166,3 @@ goto error; + textype = GST_VIDEO_GL_TEXTURE_TYPE_RGBA; You're not setting the target here anymore, or are you elsewhere? @@ +194,3 @@ +success: { + TextureWrapper *texture_data = malloc(sizeof(TextureWrapper)); g_new0(TextureWrapper, 1)
Created attachment 346745 [details] [review] Fixed very dumb bug
Review of attachment 346745 [details] [review]: ::: sys/applemedia/videotexturecache.m @@ +194,3 @@ +success: { + TextureWrapper *texture_data = malloc(sizeof(TextureWrapper)); g_new0() here, or at least g_malloc()
Created attachment 347194 [details] [review] g_malloc
commit e557e93f3b4e922f9dd6fc5dcc04284522a693e8 Author: Nick Kallen <nickkallen@me.com> Date: Sun Feb 26 10:24:46 2017 +0100 applemedia: ensure all textures are released before texturecache is released It was previously possible for videotexturecache to be finalized before all of its textures. Finalizing outstanding textures in this circumstance leads to a crash. This patch ensure resources are freed in the proper order. https://bugzilla.gnome.org/show_bug.cgi?id=779247
Review of attachment 347194 [details] [review]: ::: sys/applemedia/videotexturecache.m @@ +140,3 @@ + CFRelease(data->texture); + CFRelease(data->cache); + free(data); Must be g_free() (changed before merging) GLib allows to replace the malloc/free/calloc functions with something else, e.g. for debugging purposes. Not that anybody is ever going to do that on iOS... @@ +194,3 @@ +success: { + TextureWrapper *texture_data = g_malloc(sizeof(TextureWrapper)); Changed to g_new(TextureWrapper, 1) which ensures that you don't accidentally allocate with the size of type A and assign that to a pointer to type B.
OK THanks for making these changes on my behalf.... too much back and forth takes forever
Created attachment 347198 [details] [review] Compiles on MacOS
Created attachment 347199 [details] [review] Compiles on MacOS
commit 6e4354600d833deb1ac31a1303e80301658701d4 Author: Nick Kallen <nickkallen@me.com> Date: Sat Mar 4 12:12:52 2017 +0100 applemedia: Fix video texture cache build issue on MacOS https://bugzilla.gnome.org/show_bug.cgi?id=779247