mirror of
https://gitlab.freedesktop.org/gstreamer/gstreamer.git
synced 2025-02-17 03:35:21 +00:00
vulkan/image: don't rely on weak-ref notifies for views
Weak refs don't quite work here correctly as there is always a race with taking the lock between find_view() and remove_view(). If find_view() returns a view that is going to removed by remove_view() then we have an interesting situation. In theory, the number and type of views for an image are relatively constant and should not change one they've been set up which means that it is actually practical to perform pool-like reference counting here where the image holds a pool of different views that it can give out as necessary.
This commit is contained in:
Matthew Waters
GStreamer Merge Bot
parent
4f3051fd2d
commit
a7c2aa473f
5 changed files with 354 additions and 25 deletions
|
|
@ -196,6 +196,7 @@ _vk_image_mem_init (GstVulkanImageMemory * mem, GstAllocator * allocator,
|
|||
g_mutex_init (&mem->lock);
|
||||
|
||||
mem->views = g_ptr_array_new ();
|
||||
mem->outstanding_views = g_ptr_array_new ();
|
||||
|
||||
GST_CAT_DEBUG (GST_CAT_VULKAN_IMAGE_MEMORY,
|
||||
"new Vulkan Image memory:%p size:%" G_GSIZE_FORMAT, mem, maxsize);
|
||||
|
|
@ -397,6 +398,12 @@ _vk_image_mem_alloc (GstAllocator * allocator, gsize size,
|
|||
return NULL;
|
||||
}
|
||||
|
||||
static void
|
||||
_free_view (GstVulkanImageView * view, gpointer unused)
|
||||
{
|
||||
gst_vulkan_image_view_unref (view);
|
||||
}
|
||||
|
||||
static void
|
||||
_vk_image_mem_free (GstAllocator * allocator, GstMemory * memory)
|
||||
{
|
||||
|
|
@ -405,11 +412,11 @@ _vk_image_mem_free (GstAllocator * allocator, GstMemory * memory)
|
|||
GST_CAT_TRACE (GST_CAT_VULKAN_IMAGE_MEMORY, "freeing image memory:%p "
|
||||
"id:%" G_GUINT64_FORMAT, mem, (guint64) mem->image);
|
||||
|
||||
if (mem->views->len > 0) {
|
||||
g_warning ("GstVulkanImageMemory <%p> is being freed with outstanding "
|
||||
"GstVulkanImageView's. This usually means there is a reference "
|
||||
"counting issue.", mem);
|
||||
}
|
||||
g_warn_if_fail (mem->outstanding_views > 0);
|
||||
g_ptr_array_unref (mem->outstanding_views);
|
||||
|
||||
g_ptr_array_foreach (mem->views, (GFunc) _free_view, NULL);
|
||||
g_ptr_array_unref (mem->views);
|
||||
|
||||
if (mem->image && !mem->wrapped)
|
||||
vkDestroyImage (mem->device->device, mem->image, NULL);
|
||||
|
|
@ -529,8 +536,12 @@ find_view_index_unlocked (GstVulkanImageMemory * image,
|
|||
return (gint) index;
|
||||
}
|
||||
|
||||
static void
|
||||
gst_vulkan_image_memory_remove_view (GstVulkanImageMemory * image,
|
||||
extern void
|
||||
gst_vulkan_image_memory_release_view (GstVulkanImageMemory * image,
|
||||
GstVulkanImageView * view);
|
||||
|
||||
void
|
||||
gst_vulkan_image_memory_release_view (GstVulkanImageMemory * image,
|
||||
GstVulkanImageView * view)
|
||||
{
|
||||
guint index;
|
||||
|
|
@ -538,21 +549,20 @@ gst_vulkan_image_memory_remove_view (GstVulkanImageMemory * image,
|
|||
g_return_if_fail (gst_is_vulkan_image_memory (GST_MEMORY_CAST (image)));
|
||||
|
||||
g_mutex_lock (&image->lock);
|
||||
if (!g_ptr_array_find (image->views, view, &index)) {
|
||||
g_warning ("GstVulkanImageMemory:%p Attempting to remove a view %p"
|
||||
GST_CAT_TRACE (GST_CAT_VULKAN_IMAGE_MEMORY, "image %p removing view %p",
|
||||
image, view);
|
||||
if (g_ptr_array_find (image->outstanding_views, view, &index)) {
|
||||
/* really g_ptr_array_steal_index_fast() but that requires glib 2.58 */
|
||||
g_ptr_array_remove_index_fast (image->outstanding_views, index);
|
||||
g_ptr_array_add (image->views, view);
|
||||
} else {
|
||||
g_warning ("GstVulkanImageMemory:%p attempt to remove a view %p "
|
||||
"that we do not own", image, view);
|
||||
g_assert_not_reached ();
|
||||
}
|
||||
g_ptr_array_remove_index_fast (image->views, index);
|
||||
gst_clear_mini_object ((GstMiniObject **) & view->image);
|
||||
g_mutex_unlock (&image->lock);
|
||||
}
|
||||
|
||||
static void
|
||||
view_free_notify (GstVulkanImageMemory * image, GstVulkanImageView * view)
|
||||
{
|
||||
gst_vulkan_image_memory_remove_view (image, view);
|
||||
}
|
||||
|
||||
/**
|
||||
* gst_vulkan_image_memory_add_view:
|
||||
* @image: a #GstVulkanImageMemory
|
||||
|
|
@ -576,13 +586,37 @@ gst_vulkan_image_memory_add_view (GstVulkanImageMemory * image,
|
|||
g_mutex_unlock (&image->lock);
|
||||
return;
|
||||
}
|
||||
g_ptr_array_add (image->views, view);
|
||||
gst_mini_object_weak_ref (GST_MINI_OBJECT_CAST (view),
|
||||
(GstMiniObjectNotify) view_free_notify, image);
|
||||
g_ptr_array_add (image->outstanding_views, view);
|
||||
GST_CAT_TRACE (GST_CAT_VULKAN_IMAGE_MEMORY, "Image %p adding view %p",
|
||||
image, view);
|
||||
|
||||
g_mutex_unlock (&image->lock);
|
||||
}
|
||||
|
||||
struct view_data
|
||||
{
|
||||
GstVulkanImageMemory *img;
|
||||
GstVulkanImageMemoryFindViewFunc find_func;
|
||||
gpointer find_data;
|
||||
};
|
||||
|
||||
static gboolean
|
||||
find_view_func (GstVulkanImageView * view, gpointer user_data)
|
||||
{
|
||||
struct view_data *data = user_data;
|
||||
GstVulkanImageMemory *previous;
|
||||
gboolean ret;
|
||||
|
||||
previous = view->image;
|
||||
view->image = data->img;
|
||||
|
||||
ret = data->find_func (view, data->find_data);
|
||||
|
||||
view->image = previous;
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
/**
|
||||
* gst_vulkan_image_memory_find_view:
|
||||
* @image: a #GstVulkanImageMemory
|
||||
|
|
@ -599,6 +633,7 @@ gst_vulkan_image_memory_find_view (GstVulkanImageMemory * image,
|
|||
GstVulkanImageMemoryFindViewFunc find_func, gpointer data)
|
||||
{
|
||||
GstVulkanImageView *ret = NULL;
|
||||
struct view_data view;
|
||||
guint index;
|
||||
|
||||
g_return_val_if_fail (gst_is_vulkan_image_memory (GST_MEMORY_CAST (image)),
|
||||
|
|
@ -606,9 +641,25 @@ gst_vulkan_image_memory_find_view (GstVulkanImageMemory * image,
|
|||
g_return_val_if_fail (find_func != NULL, NULL);
|
||||
|
||||
g_mutex_lock (&image->lock);
|
||||
if (g_ptr_array_find_with_equal_func (image->views, data,
|
||||
(GEqualFunc) find_func, &index))
|
||||
ret = gst_vulkan_image_view_ref (g_ptr_array_index (image->views, index));
|
||||
view.img = image;
|
||||
view.find_func = find_func;
|
||||
view.find_data = data;
|
||||
|
||||
if (g_ptr_array_find_with_equal_func (image->outstanding_views, &view,
|
||||
(GEqualFunc) find_view_func, &index)) {
|
||||
ret =
|
||||
gst_vulkan_image_view_ref (g_ptr_array_index (image->outstanding_views,
|
||||
index));
|
||||
} else if (g_ptr_array_find_with_equal_func (image->views, &view,
|
||||
(GEqualFunc) find_view_func, &index)) {
|
||||
/* really g_ptr_array_steal_index_fast() but that requires glib 2.58 */
|
||||
ret = g_ptr_array_remove_index_fast (image->views, index);
|
||||
g_ptr_array_add (image->outstanding_views, ret);
|
||||
ret->image = (GstVulkanImageMemory *) gst_memory_ref ((GstMemory *) image);
|
||||
}
|
||||
|
||||
GST_CAT_TRACE (GST_CAT_VULKAN_IMAGE_MEMORY, "Image %p found view %p",
|
||||
image, ret);
|
||||
g_mutex_unlock (&image->lock);
|
||||
|
||||
return ret;
|
||||
|
|
|
|||
|
|
@ -73,6 +73,9 @@ struct _GstVulkanImageMemory
|
|||
gpointer user_data;
|
||||
|
||||
GPtrArray *views;
|
||||
GPtrArray *outstanding_views;
|
||||
|
||||
gpointer _padding[GST_PADDING];
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -49,6 +49,24 @@ init_debug (void)
|
|||
}
|
||||
}
|
||||
|
||||
extern void
|
||||
gst_vulkan_image_memory_release_view (GstVulkanImageMemory * image,
|
||||
GstVulkanImageView * view);
|
||||
|
||||
static gboolean
|
||||
gst_vulkan_image_view_dispose (GstVulkanImageView * view)
|
||||
{
|
||||
GstVulkanImageMemory *image;
|
||||
|
||||
if ((image = view->image) == NULL)
|
||||
return TRUE;
|
||||
|
||||
gst_vulkan_image_view_ref (view);
|
||||
gst_vulkan_image_memory_release_view (image, view);
|
||||
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
static void
|
||||
gst_vulkan_image_view_free (GstVulkanImageView * view)
|
||||
{
|
||||
|
|
@ -57,8 +75,9 @@ gst_vulkan_image_view_free (GstVulkanImageView * view)
|
|||
if (view->view)
|
||||
vkDestroyImageView (view->device->device, view->view, NULL);
|
||||
|
||||
if (view->image)
|
||||
if (view->image) {
|
||||
gst_memory_unref (GST_MEMORY_CAST (view->image));
|
||||
}
|
||||
view->image = NULL;
|
||||
gst_clear_object (&view->device);
|
||||
|
||||
|
|
@ -90,7 +109,8 @@ gst_vulkan_image_view_new (GstVulkanImageMemory * image,
|
|||
view = g_new0 (GstVulkanImageView, 1);
|
||||
|
||||
gst_mini_object_init ((GstMiniObject *) view, 0,
|
||||
gst_vulkan_image_view_get_type (), NULL, NULL,
|
||||
gst_vulkan_image_view_get_type (), NULL,
|
||||
(GstMiniObjectDisposeFunction) gst_vulkan_image_view_dispose,
|
||||
(GstMiniObjectFreeFunction) gst_vulkan_image_view_free);
|
||||
|
||||
err =
|
||||
|
|
@ -105,6 +125,9 @@ gst_vulkan_image_view_new (GstVulkanImageMemory * image,
|
|||
/* we cannot keep this as it may point to stack allocated memory */
|
||||
view->create_info.pNext = NULL;
|
||||
|
||||
GST_CAT_TRACE (GST_CAT_VULKAN_IMAGE_VIEW, "new image view for image: %p",
|
||||
image);
|
||||
|
||||
return view;
|
||||
|
||||
vk_error:
|
||||
|
|
|
|||
251
tests/check/libs/vkimage.c
Normal file
251
tests/check/libs/vkimage.c
Normal file
|
|
@ -0,0 +1,251 @@
|
|||
/* GStreamer
|
||||
*
|
||||
* Copyright (C) 2019 Matthew Waters <matthew@centricular.com>
|
||||
*
|
||||
* This library is free software; you can redistribute it and/or
|
||||
* modify it under the terms of the GNU Library General Public
|
||||
* License as published by the Free Software Foundation; either
|
||||
* version 2 of the License, or (at your option) any later version.
|
||||
*
|
||||
* This library is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
||||
* Library General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU Library General Public
|
||||
* License along with this library; if not, write to the
|
||||
* Free Software Foundation, Inc., 59 Temple Place - Suite 330,
|
||||
* Boston, MA 02111-1307, USA.
|
||||
*/
|
||||
|
||||
#ifdef HAVE_CONFIG_H
|
||||
#include "config.h"
|
||||
#endif
|
||||
|
||||
#include <gst/gst.h>
|
||||
#include <gst/check/gstcheck.h>
|
||||
#include <gst/check/gstharness.h>
|
||||
#include <gst/vulkan/vulkan.h>
|
||||
#include "../../ext/vulkan/vkelementutils.h"
|
||||
#include "../../ext/vulkan/vkelementutils.c"
|
||||
|
||||
static GstVulkanInstance *instance;
|
||||
static GstVulkanDevice *device;
|
||||
|
||||
static void
|
||||
setup (void)
|
||||
{
|
||||
instance = gst_vulkan_instance_new ();
|
||||
fail_unless (gst_vulkan_instance_open (instance, NULL));
|
||||
device = gst_vulkan_device_new_with_index (instance, 0);
|
||||
fail_unless (gst_vulkan_device_open (device, NULL));
|
||||
}
|
||||
|
||||
static void
|
||||
teardown (void)
|
||||
{
|
||||
gst_object_unref (instance);
|
||||
gst_object_unref (device);
|
||||
}
|
||||
|
||||
static void
|
||||
check_size (GstMemory * mem, gsize at_least)
|
||||
{
|
||||
gsize size, maxsize, offset;
|
||||
|
||||
size = gst_memory_get_sizes (mem, &offset, &maxsize);
|
||||
fail_unless (size <= maxsize);
|
||||
fail_unless (size >= at_least);
|
||||
}
|
||||
|
||||
static GstVulkanImageMemory *
|
||||
create_image_mem (GstVideoInfo * v_info)
|
||||
{
|
||||
GstVulkanImageMemory *vk_mem;
|
||||
VkImageUsageFlags usage;
|
||||
VkFormat vk_format;
|
||||
GstMemory *mem;
|
||||
|
||||
vk_format = gst_vulkan_format_from_video_info (v_info, 0);
|
||||
|
||||
usage = VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT;
|
||||
mem =
|
||||
gst_vulkan_image_memory_alloc (device, vk_format,
|
||||
GST_VIDEO_INFO_COMP_WIDTH (v_info, 0),
|
||||
GST_VIDEO_INFO_COMP_HEIGHT (v_info, 0), VK_IMAGE_TILING_LINEAR,
|
||||
usage, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT);
|
||||
fail_unless (gst_is_vulkan_image_memory (mem));
|
||||
vk_mem = (GstVulkanImageMemory *) mem;
|
||||
fail_unless (vk_mem->usage == usage);
|
||||
return vk_mem;
|
||||
}
|
||||
|
||||
GST_START_TEST (test_image_new)
|
||||
{
|
||||
GstVulkanImageMemory *vk_mem;
|
||||
GstVideoInfo v_info;
|
||||
gsize offset, size;
|
||||
|
||||
gst_video_info_set_format (&v_info, GST_VIDEO_FORMAT_RGBA, 16, 16);
|
||||
vk_mem = create_image_mem (&v_info);
|
||||
|
||||
fail_unless (vk_mem->device == device);
|
||||
fail_unless (vk_mem->vk_mem != NULL);
|
||||
|
||||
size = gst_memory_get_sizes ((GstMemory *) vk_mem, &offset, NULL);
|
||||
fail_unless (offset == 0);
|
||||
check_size ((GstMemory *) vk_mem, v_info.size);
|
||||
fail_unless (vk_mem->requirements.size >= size);
|
||||
|
||||
size = gst_memory_get_sizes ((GstMemory *) vk_mem->vk_mem, &offset, NULL);
|
||||
fail_unless (offset == 0);
|
||||
check_size ((GstMemory *) vk_mem->vk_mem, v_info.size);
|
||||
|
||||
gst_memory_unref ((GstMemory *) vk_mem);
|
||||
}
|
||||
|
||||
GST_END_TEST;
|
||||
|
||||
GST_START_TEST (test_image_view_new)
|
||||
{
|
||||
GstVulkanImageMemory *vk_mem;
|
||||
GstVulkanImageView *view;
|
||||
GstVideoInfo v_info;
|
||||
|
||||
gst_video_info_set_format (&v_info, GST_VIDEO_FORMAT_RGBA, 16, 16);
|
||||
vk_mem = create_image_mem (&v_info);
|
||||
|
||||
view = get_or_create_image_view (vk_mem);
|
||||
|
||||
gst_vulkan_image_view_unref (view);
|
||||
gst_memory_unref ((GstMemory *) vk_mem);
|
||||
}
|
||||
|
||||
GST_END_TEST;
|
||||
|
||||
GST_START_TEST (test_image_view_get)
|
||||
{
|
||||
GstVulkanImageMemory *vk_mem;
|
||||
GstVulkanImageView *view;
|
||||
GstVideoInfo v_info;
|
||||
|
||||
gst_video_info_set_format (&v_info, GST_VIDEO_FORMAT_RGBA, 16, 16);
|
||||
vk_mem = create_image_mem (&v_info);
|
||||
|
||||
view = get_or_create_image_view (vk_mem);
|
||||
gst_vulkan_image_view_unref (view);
|
||||
view = get_or_create_image_view (vk_mem);
|
||||
gst_vulkan_image_view_unref (view);
|
||||
|
||||
gst_memory_unref ((GstMemory *) vk_mem);
|
||||
}
|
||||
|
||||
GST_END_TEST;
|
||||
|
||||
#define N_THREADS 2
|
||||
#define N_MEMORY 4
|
||||
#define N_OPS 512
|
||||
|
||||
struct view_stress
|
||||
{
|
||||
GMutex lock;
|
||||
GCond cond;
|
||||
gboolean ready;
|
||||
volatile int n_ops;
|
||||
GQueue *memories;
|
||||
GstHarnessThread *threads[N_THREADS];
|
||||
};
|
||||
|
||||
static void
|
||||
wait_for_ready (GstHarnessThread * thread, struct view_stress *stress)
|
||||
{
|
||||
g_mutex_lock (&stress->lock);
|
||||
while (!stress->ready)
|
||||
g_cond_wait (&stress->cond, &stress->lock);
|
||||
g_mutex_unlock (&stress->lock);
|
||||
}
|
||||
|
||||
static void
|
||||
get_unref_image_view (GstHarnessThread * thread, struct view_stress *stress)
|
||||
{
|
||||
int rand = g_random_int_range (0, N_MEMORY);
|
||||
GstVulkanImageMemory *mem;
|
||||
GstVulkanImageView *view;
|
||||
|
||||
mem = g_queue_peek_nth (stress->memories, rand);
|
||||
view = get_or_create_image_view (mem);
|
||||
gst_vulkan_image_view_unref (view);
|
||||
|
||||
g_atomic_int_inc (&stress->n_ops);
|
||||
if (g_atomic_int_get (&stress->n_ops) > N_OPS)
|
||||
g_usleep (100);
|
||||
}
|
||||
|
||||
GST_START_TEST (test_image_view_stress)
|
||||
{
|
||||
GstHarness *h = gst_harness_new_empty ();
|
||||
struct view_stress stress;
|
||||
GstVideoInfo v_info;
|
||||
int i;
|
||||
|
||||
g_mutex_init (&stress.lock);
|
||||
g_cond_init (&stress.cond);
|
||||
stress.ready = FALSE;
|
||||
stress.n_ops = 0;
|
||||
stress.memories = g_queue_new ();
|
||||
|
||||
gst_video_info_set_format (&v_info, GST_VIDEO_FORMAT_RGBA, 16, 16);
|
||||
for (i = 0; i < N_MEMORY; i++) {
|
||||
g_queue_push_head (stress.memories, create_image_mem (&v_info));
|
||||
}
|
||||
|
||||
g_mutex_lock (&stress.lock);
|
||||
for (i = 0; i < N_THREADS; i++) {
|
||||
stress.threads[i] = gst_harness_stress_custom_start (h,
|
||||
(GFunc) wait_for_ready, (GFunc) get_unref_image_view, &stress, 10);
|
||||
}
|
||||
stress.ready = TRUE;
|
||||
g_cond_broadcast (&stress.cond);
|
||||
g_mutex_unlock (&stress.lock);
|
||||
|
||||
while (g_atomic_int_get (&stress.n_ops) < N_OPS)
|
||||
g_usleep (10000);
|
||||
|
||||
for (i = 0; i < N_THREADS; i++) {
|
||||
gst_harness_stress_thread_stop (stress.threads[i]);
|
||||
}
|
||||
|
||||
g_mutex_clear (&stress.lock);
|
||||
g_cond_clear (&stress.cond);
|
||||
g_queue_free_full (stress.memories, (GDestroyNotify) gst_memory_unref);
|
||||
gst_harness_teardown (h);
|
||||
}
|
||||
|
||||
GST_END_TEST;
|
||||
|
||||
static Suite *
|
||||
vkimage_suite (void)
|
||||
{
|
||||
Suite *s = suite_create ("vkimage");
|
||||
TCase *tc_basic = tcase_create ("general");
|
||||
gboolean have_instance;
|
||||
|
||||
suite_add_tcase (s, tc_basic);
|
||||
tcase_add_checked_fixture (tc_basic, setup, teardown);
|
||||
|
||||
/* FIXME: CI doesn't have a software vulkan renderer (and none exists currently) */
|
||||
instance = gst_vulkan_instance_new ();
|
||||
have_instance = gst_vulkan_instance_open (instance, NULL);
|
||||
gst_object_unref (instance);
|
||||
if (have_instance) {
|
||||
tcase_add_test (tc_basic, test_image_new);
|
||||
tcase_add_test (tc_basic, test_image_view_new);
|
||||
tcase_add_test (tc_basic, test_image_view_get);
|
||||
tcase_add_test (tc_basic, test_image_view_stress);
|
||||
}
|
||||
|
||||
return s;
|
||||
}
|
||||
|
||||
|
||||
GST_CHECK_MAIN (vkimage);
|
||||
|
|
@ -62,6 +62,7 @@ base_tests = [
|
|||
[['libs/vkdevice.c'], not gstvulkan_dep.found(), [gstvulkan_dep]],
|
||||
[['elements/vkdeviceprovider.c'], not gstvulkan_dep.found(), [gstvulkan_dep]],
|
||||
[['libs/vkcommandpool.c'], not gstvulkan_dep.found(), [gstvulkan_dep]],
|
||||
[['libs/vkimage.c'], not gstvulkan_dep.found(), [gstvulkan_dep]],
|
||||
]
|
||||
|
||||
# FIXME: unistd dependency, unstable or not tested yet on windows
|
||||
|
|
|
|||
Loading…
Reference in a new issue