summaryrefslogtreecommitdiffstats
path: root/video
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2013-07-28 00:10:58 +0200
committerwm4 <wm4@nowhere>2013-07-28 19:25:07 +0200
commitc28bafcfb6a6b564bbce4105c4ea83fe81865585 (patch)
tree598313191916cf1af9a209294fe9e8c4ebfda22b /video
parenta9a6cf3b6c26eb21bd628c61e53c62fa7e565dbd (diff)
downloadmpv-c28bafcfb6a6b564bbce4105c4ea83fe81865585.tar.bz2
mpv-c28bafcfb6a6b564bbce4105c4ea83fe81865585.tar.xz
mp_image_pool: make reference counting thread-safe
See previous commits. Also simplify this thing: 2 flags per pool image are enough to avoid a weird central refcount and an associated shared object keeping the refcount. We could even just store these two flags in the mp_image itself (like in mp_image.flags or mp_image.priv), but let's not for the sake of readability.
Diffstat (limited to 'video')
-rw-r--r--video/mp_image_pool.c102
-rw-r--r--video/mp_image_pool.h8
2 files changed, 70 insertions, 40 deletions
diff --git a/video/mp_image_pool.c b/video/mp_image_pool.c
index dd424ad2a4..a81a008aa1 100644
--- a/video/mp_image_pool.c
+++ b/video/mp_image_pool.c
@@ -16,6 +16,10 @@
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
+#include "config.h"
+
+#include <stddef.h>
+#include <stdbool.h>
#include <assert.h>
#include "talloc.h"
@@ -25,18 +29,39 @@
#include "mp_image_pool.h"
-struct pool_trampoline {
- struct mp_image_pool *pool;
- int refcount;
+#if HAVE_PTHREADS
+#include <pthread.h>
+static pthread_mutex_t pool_mutex = PTHREAD_MUTEX_INITIALIZER;
+#define pool_lock() pthread_mutex_lock(&pool_mutex)
+#define pool_unlock() pthread_mutex_unlock(&pool_mutex)
+#else
+#define pool_lock() 0
+#define pool_unlock() 0
+#endif
+
+// Thread-safety: the pool itself is not thread-safe, but pool-allocated images
+// can be referenced and unreferenced from other threads. (As long as compiled
+// with pthreads, and the image destructors are thread-safe.)
+
+struct mp_image_pool {
+ int max_count;
+
+ struct mp_image **images;
+ int num_images;
+};
+
+// Used to gracefully handle the case when the pool is freed while image
+// references allocated from the image pool are still held by someone.
+struct image_flags {
+ // If both of these are false, the image must be freed.
+ bool referenced; // outside mp_image reference exists
+ bool pool_alive; // the mp_image_pool references this
};
static int image_pool_destructor(void *ptr)
{
struct mp_image_pool *pool = ptr;
mp_image_pool_clear(pool);
- pool->trampoline->pool = NULL;
- if (!pool->trampoline->refcount)
- talloc_free(pool->trampoline);
return 0;
}
@@ -46,9 +71,6 @@ struct mp_image_pool *mp_image_pool_new(int max_count)
talloc_set_destructor(pool, image_pool_destructor);
*pool = (struct mp_image_pool) {
.max_count = max_count,
- .trampoline = talloc_struct(NULL, struct pool_trampoline, {
- .pool = pool,
- }),
};
return pool;
}
@@ -57,32 +79,33 @@ void mp_image_pool_clear(struct mp_image_pool *pool)
{
for (int n = 0; n < pool->num_images; n++) {
struct mp_image *img = pool->images[n];
- if (img->priv) {
- // Image data is being used - detach from the pool, and make it
- // free itself when pool_free_image() is called
- img->priv = talloc_struct(NULL, struct pool_trampoline, {
- .refcount = 1,
- });
- } else {
+ struct image_flags *it = img->priv;
+ bool referenced;
+ pool_lock();
+ assert(it->pool_alive);
+ it->pool_alive = false;
+ referenced = it->referenced;
+ pool_unlock();
+ if (!referenced)
talloc_free(img);
- }
}
pool->num_images = 0;
- pool->trampoline->refcount = 0;
}
-static void pool_free_image(void *ptr)
+// This is the only function that is allowed to run in a different thread.
+// (Consider passing an image to another thread, which frees it.)
+static void unref_image(void *ptr)
{
struct mp_image *img = ptr;
- struct pool_trampoline *trampoline = img->priv;
- trampoline->refcount--;
- img->priv = NULL;
- if (!trampoline->pool) {
- // pool was free'd while image reference was still held
- if (trampoline->refcount == 0)
- talloc_free(trampoline);
+ struct image_flags *it = img->priv;
+ bool alive;
+ pool_lock();
+ assert(it->referenced);
+ it->referenced = false;
+ alive = it->pool_alive;
+ pool_unlock();
+ if (!alive)
talloc_free(img);
- }
}
// Return a new image of given format/size. The only difference to
@@ -93,22 +116,35 @@ struct mp_image *mp_image_pool_get(struct mp_image_pool *pool, unsigned int fmt,
int w, int h)
{
struct mp_image *new = NULL;
+
+ pool_lock();
for (int n = 0; n < pool->num_images; n++) {
struct mp_image *img = pool->images[n];
- if (!img->priv && img->imgfmt == fmt && img->w == w && img->h == h) {
- new = img;
- break;
+ struct image_flags *it = img->priv;
+ assert(it->pool_alive);
+ if (!it->referenced) {
+ if (img->imgfmt == fmt && img->w == w && img->h == h) {
+ new = img;
+ break;
+ }
}
}
+ pool_unlock();
+
if (!new) {
if (pool->num_images >= pool->max_count)
mp_image_pool_clear(pool);
new = mp_image_alloc(fmt, w, h);
+ struct image_flags *it = talloc_ptrtype(new, it);
+ *it = (struct image_flags) { .pool_alive = true };
+ new->priv = it;
MP_TARRAY_APPEND(pool, pool->images, pool->num_images, new);
}
- pool->trampoline->refcount++;
- new->priv = pool->trampoline;
- return mp_image_new_custom_ref(new, new, pool_free_image);
+
+ struct image_flags *it = new->priv;
+ assert(!it->referenced && it->pool_alive);
+ it->referenced = true;
+ return mp_image_new_custom_ref(new, new, unref_image);
}
// Like mp_image_new_copy(), but allocate the image out of the pool.
diff --git a/video/mp_image_pool.h b/video/mp_image_pool.h
index fdb0e76c39..0982fd94f8 100644
--- a/video/mp_image_pool.h
+++ b/video/mp_image_pool.h
@@ -1,13 +1,7 @@
#ifndef MPV_MP_IMAGE_POOL_H
#define MPV_MP_IMAGE_POOL_H
-struct mp_image_pool {
- int max_count;
-
- struct mp_image **images;
- int num_images;
- struct pool_trampoline *trampoline;
-};
+struct mp_image_pool;
struct mp_image_pool *mp_image_pool_new(int max_count);
struct mp_image *mp_image_pool_get(struct mp_image_pool *pool, unsigned int fmt,