From patchwork Fri Mar 24 12:47:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: wm4 X-Patchwork-Id: 3083 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.50.79 with SMTP id y76csp78366vsy; Fri, 24 Mar 2017 05:47:16 -0700 (PDT) X-Received: by 10.28.13.207 with SMTP id 198mr2985539wmn.76.1490359636103; Fri, 24 Mar 2017 05:47:16 -0700 (PDT) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id h17si3168353wrb.98.2017.03.24.05.47.15; Fri, 24 Mar 2017 05:47:15 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@googlemail.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=googlemail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E2AC7688281; Fri, 24 Mar 2017 14:46:51 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9D432687EC7 for ; Fri, 24 Mar 2017 14:46:45 +0200 (EET) Received: by mail-wm0-f44.google.com with SMTP id n11so12133861wma.1 for ; Fri, 24 Mar 2017 05:47:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=pOZaIV50y2RwBwFVABBmHYgml7Vuk2DmK8QKrVwMDP8=; b=HoKLptJZxq0d/Ec1YQbDhN8C7/Z6hpo+9BaNebMhWHmnzvKmk4dqj1TOLRY4dKQE5Z nxEuNie/8c4T/VllHFlO1SNJUuJwJnqQyDEsv/gltyNoIrsdnqq2G0VBWgSDVlVaTZIi MLH0BA6A7zGvVpmGsZEJvvqOyPTqMGtTXunF+4HJI6ErWr1O5/iWxlr8gfTOVp6+U+ps mYAzVzFhh1nnBjQIJMqv/4TY8CVDlzoCVjpvRTsImMh67r3iR9Y1MFOSgn3nQMVxAqT/ mj3UCZylDxLQEzQwMz0a/FVMLDNJd4kZyPd2Bg6NC78W8A4ShqW4nhXyvR506ecS8kz0 Bkig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=pOZaIV50y2RwBwFVABBmHYgml7Vuk2DmK8QKrVwMDP8=; b=p9EAWdv4tNjzDnBhGAeCo/ogco9THvf26ZcrAwBco2NUnPTLujVgPtxdGqHAzvzs3X ck4TRYbdAX45iYGQqgge0lRD1hHn2ZrIFJkcKeo44KZR2bdExnJcCSYHISer6IBT/cQU QHH9i1y6vCu2+y/4+00fbf1PAjRUOz7AtxBgG9SM8wv6dn0M8zTP4QJL5Sk61jLAhF3l 4j3kvZHDEJJUMoM8wrFMr8ByQSEVkVQk1iyOGw8u2SAR0VpGHOWSdYVrt5TeDAds4UPb Wo6wGe0NTwnuaTcw+Lxvrb/CEbD7YxtROZpKhVEa6ZxXIcUAhXRTwEWrl4/L3OolJlG8 F80A== X-Gm-Message-State: AFeK/H0D7glrJXAXby8krGe/f8N1zut1V3DH6MRAfuYL5wnQ82dl/c2NlkIV2KbUwYl4zw== X-Received: by 10.28.169.130 with SMTP id s124mr2753336wme.137.1490359625684; Fri, 24 Mar 2017 05:47:05 -0700 (PDT) Received: from localhost.localdomain (p4FF02EE1.dip0.t-ipconnect.de. [79.240.46.225]) by smtp.googlemail.com with ESMTPSA id k128sm2325760wmf.16.2017.03.24.05.47.04 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 Mar 2017 05:47:04 -0700 (PDT) From: wm4 To: ffmpeg-devel@ffmpeg.org Date: Fri, 24 Mar 2017 13:47:10 +0100 Message-Id: <20170324124710.10576-1-nfxjfg@googlemail.com> X-Mailer: git-send-email 2.11.0 Subject: [FFmpeg-devel] [PATCH] pthread_frame: do not attempt to unlock a mutex on the wrong thread X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: wm4 MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 6768402ed8..6620a8d324 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -123,6 +123,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. @@ -136,6 +138,24 @@ typedef struct FrameThreadContext { #define THREAD_SAFE_CALLBACKS(avctx) \ ((avctx)->thread_safe_callbacks || (avctx)->get_buffer2 == avcodec_default_get_buffer2) +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); +} + +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); + pthread_mutex_unlock(&fctx->async_mutex); +} + /** * Codec worker thread. * @@ -195,7 +215,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); } pthread_mutex_lock(&p->progress_mutex); @@ -451,7 +472,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. @@ -532,7 +553,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; @@ -594,7 +615,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); @@ -613,7 +635,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]; @@ -627,7 +649,7 @@ static void park_frame_worker_threads(FrameThreadContext *fctx, int thread_count p->got_frame = 0; } - pthread_mutex_lock(&fctx->async_mutex); + async_lock(fctx); } void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) @@ -691,9 +713,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); @@ -742,10 +763,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++) {