On Sat, 25 Mar 2017 09:03:19 +0200
Rémi Denis-Courmont <[email protected]> wrote:
> Le torstaina 23. maaliskuuta 2017, 13.25.29 EET wm4 a écrit :
> > async_mutex has is used in a very strange but intentional way: it is
> > locked by default, and unlocked only in regions that can be run
> > concurrently.
> >
> > If the user was calling API functions to the same context from different
> > threads (in a safe way), this could unintentionally unlock the mutex on
> > a different thread than the previous lock operation. It's not allowed by
> > the pthread API.
> >
> > Fix this by emulating a binary semaphore using a mutex and condition
> > variable. (Posix semaphores are not available on all platforms.)
> > ---
> > libavcodec/pthread_frame.c | 41 +++++++++++++++++++++++++++++++----------
> > 1 file changed, 31 insertions(+), 10 deletions(-)
> >
> > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> > index 65a04d8e90..9ca7a766df 100644
> > --- a/libavcodec/pthread_frame.c
> > +++ b/libavcodec/pthread_frame.c
> > @@ -119,6 +119,8 @@ typedef struct FrameThreadContext {
> > */
> > pthread_mutex_t hwaccel_mutex;
> > pthread_mutex_t async_mutex;
> > + pthread_cond_t async_cond;
> > + int async_lock;
> >
> > int next_decoding; ///< The next context to submit a packet
> > to. int next_finished; ///< The next context to return output
> > from. @@ -129,6 +131,24 @@ typedef struct FrameThreadContext {
> > */
> > } FrameThreadContext;
> >
> > +static void async_lock(FrameThreadContext *fctx)
> > +{
> > + pthread_mutex_lock(&fctx->async_mutex);
> > + while (fctx->async_lock)
> > + pthread_cond_wait(&fctx->async_cond, &fctx->async_mutex);
> > + fctx->async_lock = 1;
> > + pthread_mutex_unlock(&fctx->async_mutex);
> > +}
>
> That´s a text book but rather inefficient (both in terms of memory and
> computation) reimplementation of sem_wait().
>
> I would use sem_wait() directly.
Not available on all platforms, the next patch would not work with
semaphores, and I don't really it matters all that much.
> > +
> > +static void async_unlock(FrameThreadContext *fctx)
> > +{
> > + pthread_mutex_lock(&fctx->async_mutex);
> > + av_assert0(fctx->async_lock);
> > + fctx->async_lock = 0;
> > + pthread_cond_broadcast(&fctx->async_cond);
>
> signal.
Sure.
>
> > + pthread_mutex_unlock(&fctx->async_mutex);
> > +}
>
> This is sem_post() but much more inefficient.
>
> > +
> > /**
> > * Codec worker thread.
> > *
> > @@ -196,7 +216,8 @@ static attribute_align_arg void
> > *frame_worker_thread(void *arg)
> >
> > if (p->async_serializing) {
> > p->async_serializing = 0;
> > - pthread_mutex_unlock(&p->parent->async_mutex);
> > +
> > + async_unlock(p->parent);
> > }
> >
> > atomic_store(&p->state, STATE_INPUT_READY);
> > @@ -428,7 +449,7 @@ int ff_thread_decode_frame(AVCodecContext *avctx,
> >
> > /* release the async lock, permitting blocked hwaccel threads to
> > * go forward while we are in this function */
> > - pthread_mutex_unlock(&fctx->async_mutex);
> > + async_unlock(fctx);
> >
> > /*
> > * Submit a packet to the next decoding thread.
> > @@ -497,7 +518,7 @@ int ff_thread_decode_frame(AVCodecContext *avctx,
> > /* return the size of the consumed packet if no error occurred */
> > ret = (p->result >= 0) ? avpkt->size : p->result;
> > finish:
> > - pthread_mutex_lock(&fctx->async_mutex);
> > + async_lock(fctx);
> > if (err < 0)
> > return err;
> > return ret;
> > @@ -559,7 +580,8 @@ void ff_thread_finish_setup(AVCodecContext *avctx) {
> > if (avctx->hwaccel &&
> > !(avctx->hwaccel->caps_internal & HWACCEL_CAP_ASYNC_SAFE)) {
> > p->async_serializing = 1;
> > - pthread_mutex_lock(&p->parent->async_mutex);
> > +
> > + async_lock(p->parent);
> > }
> >
> > pthread_mutex_lock(&p->progress_mutex);
> > @@ -575,7 +597,7 @@ static void park_frame_worker_threads(FrameThreadContext
> > *fctx, int thread_count {
> > int i;
> >
> > - pthread_mutex_unlock(&fctx->async_mutex);
> > + async_unlock(fctx);
> >
> > for (i = 0; i < thread_count; i++) {
> > PerThreadContext *p = &fctx->threads[i];
> > @@ -588,7 +610,7 @@ static void park_frame_worker_threads(FrameThreadContext
> > *fctx, int thread_count }
> > }
> >
> > - pthread_mutex_lock(&fctx->async_mutex);
> > + async_lock(fctx);
> > }
> >
> > void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
> > @@ -647,9 +669,8 @@ void ff_frame_thread_free(AVCodecContext *avctx, int
> > thread_count) av_freep(&fctx->threads);
> > pthread_mutex_destroy(&fctx->buffer_mutex);
> > pthread_mutex_destroy(&fctx->hwaccel_mutex);
> > -
> > - pthread_mutex_unlock(&fctx->async_mutex);
> > pthread_mutex_destroy(&fctx->async_mutex);
> > + pthread_cond_destroy(&fctx->async_cond);
> >
> > av_freep(&avctx->internal->thread_ctx);
> > }
> > @@ -693,10 +714,10 @@ int ff_frame_thread_init(AVCodecContext *avctx)
> >
> > pthread_mutex_init(&fctx->buffer_mutex, NULL);
> > pthread_mutex_init(&fctx->hwaccel_mutex, NULL);
> > -
> > pthread_mutex_init(&fctx->async_mutex, NULL);
> > - pthread_mutex_lock(&fctx->async_mutex);
> > + pthread_cond_init(&fctx->async_cond, NULL);
> >
> > + fctx->async_lock = 1;
> > fctx->delaying = 1;
> >
> > for (i = 0; i < thread_count; i++) {
>
>
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
