diff --git a/include/vlc_picture_pool.h b/include/vlc_picture_pool.h index 37ef7b34f8..1ce92ce412 100644 --- a/include/vlc_picture_pool.h +++ b/include/vlc_picture_pool.h @@ -137,9 +137,10 @@ VLC_API void picture_pool_Enum( picture_pool_t *, /** * Forcefully return all pictures in the pool to free/unallocated state. * - * @warning This can only be called when it is known that all pending - * references to the picture pool are stale, e.g. a decoder failed to - * release pictures properly when it terminated. + * @warning If any picture in the pool is not free, this function will leak + * and may eventually cause invalid memory accesses. + * + * @note This function has no effects if all pictures in the pool are free. * * @return the number of picture references that were freed */ diff --git a/src/misc/picture_pool.c b/src/misc/picture_pool.c index c544253ed9..77d613fada 100644 --- a/src/misc/picture_pool.c +++ b/src/misc/picture_pool.c @@ -69,14 +69,8 @@ void picture_pool_Release(picture_pool_t *pool) if (likely(!destroy)) return; - for (unsigned i = 0; i < pool->picture_count; i++) { - picture_t *picture = pool->picture[i]; - picture_priv_t *priv = (picture_priv_t *)picture; - - picture_Release(priv->gc.opaque->picture); - free(priv->gc.opaque); - free(picture); - } + for (unsigned i = 0; i < pool->picture_count; i++) + picture_Release(pool->picture[i]); vlc_mutex_destroy(&pool->lock); free(pool); @@ -89,7 +83,7 @@ static void picture_pool_ReleasePicture(picture_t *picture) picture_pool_t *pool = sys->pool; if (pool->pic_unlock != NULL) - pool->pic_unlock(picture); + pool->pic_unlock(sys->picture); vlc_mutex_lock(&pool->lock); assert(!(pool->available & (1ULL << sys->offset))); @@ -97,6 +91,9 @@ static void picture_pool_ReleasePicture(picture_t *picture) vlc_mutex_unlock(&pool->lock); picture_pool_Release(pool); + + free(sys); + free(picture); } static picture_t *picture_pool_ClonePicture(picture_pool_t *pool, @@ -147,16 +144,8 @@ picture_pool_t *picture_pool_NewExtended(const picture_pool_configuration_t *cfg pool->available = (1ULL << cfg->picture_count) - 1; pool->refs = 1; pool->picture_count = cfg->picture_count; - - for (unsigned i = 0; i < cfg->picture_count; i++) { - picture_t *picture = picture_pool_ClonePicture(pool, cfg->picture[i], i); - if (unlikely(picture == NULL)) - abort(); - - atomic_init(&((picture_priv_t *)picture)->gc.refs, 0); - - pool->picture[i] = picture; - } + memcpy(pool->picture, cfg->picture, + cfg->picture_count * sizeof (picture_t *)); return pool; } @@ -224,8 +213,6 @@ picture_t *picture_pool_Get(picture_pool_t *pool) assert(pool->refs > 0); for (unsigned i = 0; i < pool->picture_count; i++) { - picture_t *picture = pool->picture[i]; - if (!(pool->available & (1ULL << i))) continue; @@ -233,6 +220,8 @@ picture_t *picture_pool_Get(picture_pool_t *pool) pool->refs++; vlc_mutex_unlock(&pool->lock); + picture_t *picture = pool->picture[i]; + if (pool->pic_lock != NULL && pool->pic_lock(picture) != 0) { vlc_mutex_lock(&pool->lock); pool->available |= 1ULL << i; @@ -240,10 +229,9 @@ picture_t *picture_pool_Get(picture_pool_t *pool) continue; } - assert(atomic_load(&((picture_priv_t *)picture)->gc.refs) == 0); - atomic_init(&((picture_priv_t *)picture)->gc.refs, 1); - picture->p_next = NULL; - return picture; + picture_t *clone = picture_pool_ClonePicture(pool, picture, i); + assert(unlikely(clone == NULL) || clone->p_next == NULL); + return clone; } vlc_mutex_unlock(&pool->lock); @@ -252,21 +240,12 @@ picture_t *picture_pool_Get(picture_pool_t *pool) unsigned picture_pool_Reset(picture_pool_t *pool) { - unsigned ret = 0; -retry: + unsigned ret; + vlc_mutex_lock(&pool->lock); assert(pool->refs > 0); - - for (unsigned i = 0; i < pool->picture_count; i++) { - picture_t *picture = pool->picture[i]; - - if (!(pool->available & (1ULL << i))) { - vlc_mutex_unlock(&pool->lock); - picture_Release(picture); - ret++; - goto retry; - } - } + ret = pool->picture_count - popcountll(pool->available); + pool->available = (1ULL << pool->picture_count) - 1; vlc_mutex_unlock(&pool->lock); return ret; diff --git a/src/test/picture_pool.c b/src/test/picture_pool.c index 93d70c0256..2ff3d51563 100644 --- a/src/test/picture_pool.c +++ b/src/test/picture_pool.c @@ -60,8 +60,13 @@ static void test(bool zombie) picture_Release(pics[i]); for (unsigned i = 0; i < PICTURES; i++) { + void *plane = pics[i]->p[0].p_pixels; + assert(plane != NULL); picture_Release(pics[i]); - assert(picture_pool_Get(pool) == pics[i]); + + pics[i] = picture_pool_Get(pool); + assert(pics[i] != NULL); + assert(pics[i]->p[0].p_pixels == plane); } for (unsigned i = 0; i < PICTURES; i++)