[PATCH 0/3] MR3101: winegstreamer: Remove callback from wg_allocator and lazily allocate unix memory.
Allocating large amount of unix memory for not using them when we write directly to the client sample is costly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3101
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/unix_private.h | 4 ++-- dlls/winegstreamer/wg_allocator.c | 37 ++++++++++++++++++++++--------- dlls/winegstreamer/wg_transform.c | 21 ++++-------------- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 808b59dba57..f1847eb27ef 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -60,9 +60,9 @@ extern NTSTATUS wg_transform_flush(void *args) DECLSPEC_HIDDEN; /* wg_allocator_release_sample can be used to release any sample that was requested. */ typedef struct wg_sample *(*wg_allocator_request_sample_cb)(gsize size, void *context); -extern GstAllocator *wg_allocator_create(wg_allocator_request_sample_cb request_sample, - void *request_sample_context) DECLSPEC_HIDDEN; +extern GstAllocator *wg_allocator_create(void) DECLSPEC_HIDDEN; extern void wg_allocator_destroy(GstAllocator *allocator) DECLSPEC_HIDDEN; +extern void wg_allocator_provide_sample(GstAllocator *allocator, struct wg_sample *sample) DECLSPEC_HIDDEN; extern void wg_allocator_release_sample(GstAllocator *allocator, struct wg_sample *sample, bool discard_data) DECLSPEC_HIDDEN; diff --git a/dlls/winegstreamer/wg_allocator.c b/dlls/winegstreamer/wg_allocator.c index 14550ad8bcc..fd940d5961e 100644 --- a/dlls/winegstreamer/wg_allocator.c +++ b/dlls/winegstreamer/wg_allocator.c @@ -51,12 +51,11 @@ typedef struct { GstAllocator parent; - wg_allocator_request_sample_cb request_sample; - void *request_sample_context; - pthread_mutex_t mutex; pthread_cond_t release_cond; struct list memory_list; + + struct wg_sample *next_sample; } WgAllocator; typedef struct @@ -147,7 +146,6 @@ static GstMemory *wg_allocator_alloc(GstAllocator *gst_allocator, gsize size, GstAllocationParams *params) { WgAllocator *allocator = (WgAllocator *)gst_allocator; - struct wg_sample *sample; WgMemory *memory; GST_LOG("allocator %p, size %#zx, params %p", allocator, size, params); @@ -160,11 +158,11 @@ static GstMemory *wg_allocator_alloc(GstAllocator *gst_allocator, gsize size, pthread_mutex_lock(&allocator->mutex); - sample = allocator->request_sample(size, allocator->request_sample_context); - if (sample && sample->max_size < size) - InterlockedDecrement(&sample->refcount); + memory->sample = allocator->next_sample; + if (memory->sample && memory->sample->max_size >= size) + allocator->next_sample = NULL; else - memory->sample = sample; + memory->sample = NULL; list_add_tail(&allocator->memory_list, &memory->entry); @@ -209,15 +207,13 @@ static void wg_allocator_class_init(WgAllocatorClass *klass) root_class->finalize = wg_allocator_finalize; } -GstAllocator *wg_allocator_create(wg_allocator_request_sample_cb request_sample, void *request_sample_context) +GstAllocator *wg_allocator_create(void) { WgAllocator *allocator; if (!(allocator = g_object_new(wg_allocator_get_type(), NULL))) return NULL; - allocator->request_sample = request_sample; - allocator->request_sample_context = request_sample_context; return GST_ALLOCATOR(allocator); } @@ -273,6 +269,25 @@ static WgMemory *find_sample_memory(WgAllocator *allocator, struct wg_sample *sa return NULL; } +void wg_allocator_provide_sample(GstAllocator *gst_allocator, struct wg_sample *sample) +{ + WgAllocator *allocator = (WgAllocator *)gst_allocator; + struct wg_sample *previous; + + GST_LOG("allocator %p, sample %p", allocator, sample); + + if (sample) + InterlockedIncrement(&sample->refcount); + + pthread_mutex_lock(&allocator->mutex); + previous = allocator->next_sample; + allocator->next_sample = sample; + pthread_mutex_unlock(&allocator->mutex); + + if (previous) + InterlockedDecrement(&previous->refcount); +} + void wg_allocator_release_sample(GstAllocator *gst_allocator, struct wg_sample *sample, bool discard_data) { diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index d6218af04b9..01c7aad1268 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -57,7 +57,6 @@ struct wg_transform GstElement *video_flip; struct wg_format output_format; - struct wg_sample *output_wg_sample; GstAtomicQueue *output_queue; GstSample *output_sample; bool output_caps_changed; @@ -281,15 +280,6 @@ NTSTATUS wg_transform_destroy(void *args) return STATUS_SUCCESS; } -static struct wg_sample *transform_request_sample(gsize size, void *context) -{ - struct wg_transform *transform = context; - - GST_LOG("size %#zx, context %p", size, transform); - - return InterlockedExchangePointer((void **)&transform->output_wg_sample, NULL); -} - static bool wg_format_video_is_flipped(const struct wg_format *format) { return format->major_type == WG_MAJOR_TYPE_VIDEO && (format->u.video.height < 0); @@ -318,7 +308,7 @@ NTSTATUS wg_transform_create(void *args) goto out; if (!(transform->drain_query = gst_query_new_drain())) goto out; - if (!(transform->allocator = wg_allocator_create(transform_request_sample, transform))) + if (!(transform->allocator = wg_allocator_create())) goto out; transform->attrs = *params->attrs; transform->output_format = output_format; @@ -762,9 +752,7 @@ static bool get_transform_output(struct wg_transform *transform, struct wg_sampl GstBuffer *input_buffer; GstFlowReturn ret; - /* Provide the sample for transform_request_sample to pick it up */ - InterlockedIncrement(&sample->refcount); - InterlockedExchangePointer((void **)&transform->output_wg_sample, sample); + wg_allocator_provide_sample(transform->allocator, sample); while (!(transform->output_sample = gst_atomic_queue_pop(transform->output_queue)) && (input_buffer = gst_atomic_queue_pop(transform->input_queue))) @@ -773,9 +761,8 @@ static bool get_transform_output(struct wg_transform *transform, struct wg_sampl GST_WARNING("Failed to push transform input, error %d", ret); } - /* Remove the sample so transform_request_sample cannot use it */ - if (InterlockedExchangePointer((void **)&transform->output_wg_sample, NULL)) - InterlockedDecrement(&sample->refcount); + /* Remove the sample so the allocator cannot use it */ + wg_allocator_provide_sample(transform->allocator, NULL); return !!transform->output_sample; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3101
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_allocator.c | 48 +++++++++++++++---------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/dlls/winegstreamer/wg_allocator.c b/dlls/winegstreamer/wg_allocator.c index fd940d5961e..1edd27a0fd4 100644 --- a/dlls/winegstreamer/wg_allocator.c +++ b/dlls/winegstreamer/wg_allocator.c @@ -65,6 +65,30 @@ typedef struct G_DEFINE_TYPE(WgAllocator, wg_allocator, GST_TYPE_ALLOCATOR); +static void release_memory_sample(WgAllocator *allocator, WgMemory *memory, bool discard_data) +{ + struct wg_sample *sample; + + if (!(sample = memory->sample)) + return; + + while (sample->refcount > 1) + { + GST_WARNING("Waiting for sample %p to be unmapped", sample); + pthread_cond_wait(&allocator->release_cond, &allocator->mutex); + } + InterlockedDecrement(&sample->refcount); + + if (memory->written && !discard_data) + { + GST_WARNING("Copying %#zx bytes from sample %p, back to memory %p", memory->written, sample, memory); + memcpy(memory->unix_map_info.data, memory->sample->data, memory->written); + } + + memory->sample = NULL; + GST_INFO("Released sample %p from memory %p", sample, memory); +} + static gpointer wg_allocator_map(GstMemory *gst_memory, GstMapInfo *info, gsize maxsize) { WgAllocator *allocator = (WgAllocator *)gst_memory->allocator; @@ -217,30 +241,6 @@ GstAllocator *wg_allocator_create(void) return GST_ALLOCATOR(allocator); } -static void release_memory_sample(WgAllocator *allocator, WgMemory *memory, bool discard_data) -{ - struct wg_sample *sample; - - if (!(sample = memory->sample)) - return; - - while (sample->refcount > 1) - { - GST_WARNING("Waiting for sample %p to be unmapped", sample); - pthread_cond_wait(&allocator->release_cond, &allocator->mutex); - } - InterlockedDecrement(&sample->refcount); - - if (memory->written && !discard_data) - { - GST_WARNING("Copying %#zx bytes from sample %p, back to memory %p", memory->written, sample, memory); - memcpy(memory->unix_map_info.data, memory->sample->data, memory->written); - } - - memory->sample = NULL; - GST_INFO("Released sample %p from memory %p", sample, memory); -} - void wg_allocator_destroy(GstAllocator *gst_allocator) { WgAllocator *allocator = (WgAllocator *)gst_allocator; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3101
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_allocator.c | 33 ++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/dlls/winegstreamer/wg_allocator.c b/dlls/winegstreamer/wg_allocator.c index 1edd27a0fd4..629546b237c 100644 --- a/dlls/winegstreamer/wg_allocator.c +++ b/dlls/winegstreamer/wg_allocator.c @@ -40,6 +40,7 @@ typedef struct GstMemory parent; struct list entry; + GstAllocationParams alloc_params; GstMemory *unix_memory; GstMapInfo unix_map_info; @@ -65,6 +66,19 @@ typedef struct G_DEFINE_TYPE(WgAllocator, wg_allocator, GST_TYPE_ALLOCATOR); +static void *get_unix_memory_data(WgMemory *memory) +{ + if (!memory->unix_memory) + { + memory->unix_memory = gst_allocator_alloc(NULL, memory->parent.maxsize, &memory->alloc_params); + gst_memory_map(memory->unix_memory, &memory->unix_map_info, GST_MAP_WRITE); + GST_INFO("Allocated unix memory %p, data %p for memory %p, sample %p", memory->unix_memory, + memory->unix_map_info.data, memory, memory->sample); + } + + return memory->unix_map_info.data; +} + static void release_memory_sample(WgAllocator *allocator, WgMemory *memory, bool discard_data) { struct wg_sample *sample; @@ -82,7 +96,7 @@ static void release_memory_sample(WgAllocator *allocator, WgMemory *memory, bool if (memory->written && !discard_data) { GST_WARNING("Copying %#zx bytes from sample %p, back to memory %p", memory->written, sample, memory); - memcpy(memory->unix_map_info.data, memory->sample->data, memory->written); + memcpy(get_unix_memory_data(memory), memory->sample->data, memory->written); } memory->sample = NULL; @@ -102,7 +116,7 @@ static gpointer wg_allocator_map(GstMemory *gst_memory, GstMapInfo *info, gsize pthread_mutex_lock(&allocator->mutex); if (!memory->sample) - info->data = memory->unix_map_info.data; + info->data = get_unix_memory_data(memory); else { InterlockedIncrement(&memory->sample->refcount); @@ -177,8 +191,7 @@ static GstMemory *wg_allocator_alloc(GstAllocator *gst_allocator, gsize size, memory = g_slice_new0(WgMemory); gst_memory_init(GST_MEMORY_CAST(memory), 0, GST_ALLOCATOR_CAST(allocator), NULL, size, 0, 0, size); - memory->unix_memory = gst_allocator_alloc(NULL, size, params); - gst_memory_map(memory->unix_memory, &memory->unix_map_info, GST_MAP_WRITE); + memory->alloc_params = *params; pthread_mutex_lock(&allocator->mutex); @@ -186,14 +199,13 @@ static GstMemory *wg_allocator_alloc(GstAllocator *gst_allocator, gsize size, if (memory->sample && memory->sample->max_size >= size) allocator->next_sample = NULL; else - memory->sample = NULL; + release_memory_sample(allocator, memory, true); list_add_tail(&allocator->memory_list, &memory->entry); pthread_mutex_unlock(&allocator->mutex); - GST_INFO("Allocated memory %p, sample %p, unix_memory %p, data %p", memory, - memory->sample, memory->unix_memory, memory->unix_map_info.data); + GST_INFO("Allocated memory %p, sample %p", memory, memory->sample); return (GstMemory *)memory; } @@ -214,8 +226,11 @@ static void wg_allocator_free(GstAllocator *gst_allocator, GstMemory *gst_memory pthread_mutex_unlock(&allocator->mutex); - gst_memory_unmap(memory->unix_memory, &memory->unix_map_info); - gst_memory_unref(memory->unix_memory); + if (memory->unix_memory) + { + gst_memory_unmap(memory->unix_memory, &memory->unix_map_info); + gst_memory_unref(memory->unix_memory); + } g_slice_free(WgMemory, memory); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3101
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3101
participants (2)
-
Rémi Bernon -
Zebediah Figura (＠zfigura)