From patchwork Sat Nov 25 19:48:13 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rostislav Pehlivanov X-Patchwork-Id: 6358 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.161.94 with SMTP id m30csp3800825jah; Sat, 25 Nov 2017 11:56:46 -0800 (PST) X-Google-Smtp-Source: AGs4zMbgsk1L5kXUyzSbT73t2aX7wCl54DdNIthhOHzZl7mBV1qQbDa4SePmrAofyrWxBWU0tsBy X-Received: by 10.28.156.67 with SMTP id f64mr12488930wme.42.1511639805960; Sat, 25 Nov 2017 11:56:45 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1511639805; cv=none; d=google.com; s=arc-20160816; b=T59gRC0shQe4WHsZdY6N1Z2Hrhak/jNfi6X+vMeGM1Jv+W9ebw6isAapJvwCtzmXxA Q3QLaKDOmzcEh+Fs5tVC7LBOUyLDQrOVpAjmFdXKBhH1K7fvPEPfIMzBvvzsj1zwY9jg vAPxmN+OQEYYZtpUdjmmPGK3sNnd/xCfLLI2AZmqhhY1xAP8dsF1BVYyc00N7dAZ/BeZ o6nYG/qAFTT66rHwQxL0PiishFUuWIJHQrpLZqjYxxr+i3w5QIBhAQcOVlFriHOrNA12 j7t1EHT5uejNlIL08GVukK/AXSdBYZqlCngq8H+h0+cItw4cSGBZOnLfqJ2XsmQv3aQy QgKQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:to :message-id:date:from:references:in-reply-to:mime-version :dkim-signature:delivered-to:arc-authentication-results; bh=8osK/FOrhSQo594iD6Wk6YybVPrM5YymEA2Fs9S3tDE=; b=RYrjjPWtYmSIx/ehZ46e+2ACp8//AQ5TIw34ineEOrZ2pqdPxF/8Xj9b5n4BCfkEPL nZ07Pjp5o9+Nro9f4xbtlj5yFZoB9/C2WWl4UR2A3qGlaxAKdb+0naBO09jrfKOLoXRT Cg6aWfxW83nOgAP9Et37GOcbNh3qD6u0xFvd83hWJ49Rf0Ve8QRCKVoL+CtHNbvdIEY0 7PhWGIuEl4ouCNq7gaGUUAgfm5up+qaXupj3c7Kofn7fHA9YHz1zVtpi8YyIvdNXPXjG opnyy2cvAyaxuu3/l0I6q0Q+/cKRDWhmQ/2oTNdSKS2FdhG+OyJ3CITV732i/D/qDOX3 C5yg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=UWrbS+jT; 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=NONE sp=NONE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id e80si9289092wmi.26.2017.11.25.11.56.44; Sat, 25 Nov 2017 11:56:45 -0800 (PST) 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=@gmail.com header.s=20161025 header.b=UWrbS+jT; 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=NONE sp=NONE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EEEE168A348; Sat, 25 Nov 2017 21:56:41 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qt0-f170.google.com (mail-qt0-f170.google.com [209.85.216.170]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C49C668A2A5 for ; Sat, 25 Nov 2017 21:56:35 +0200 (EET) Received: by mail-qt0-f170.google.com with SMTP id p44so34836786qtj.6 for ; Sat, 25 Nov 2017 11:56:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=z95hFP7Tz2WH5gtF0U1t54fOKnGtCF3YBkmmNVqP3Ss=; b=UWrbS+jTY5IABymfST5LCkaJY8l3VivusLzSXCZSidoqp07DlFi74hApIRbcFpPatm VO3+CBylOgOYA5zZk35itUQj86DfX9Bx42UDP4F1khzX9cQvtlt84R10FfbjlT0E+MlQ I+ktdDir1IKUPYMW5h26RcrfvSa+B9dRAO75EKq2tdeh+DJd9P3+gRXo3TVLPzKh5rt5 RkkBPctvck563kSDoI5UEhCB0NQmXfx0puqtFH+pHlAR86wOAO62xHywO9IgtAXWhyT6 pIJTT28+PwXVw0F1N0OVvan5L7nIiWqFQb6k2rESBoPOfbNktzKPv+Z4klLeJujPO779 QQ0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=z95hFP7Tz2WH5gtF0U1t54fOKnGtCF3YBkmmNVqP3Ss=; b=Axp6Q1WdCevgVorH3TTIXLkfOcRgcBuSdSHjeYzs8eD0dVSxMoy8hKTrcf38EeCT5+ dpkTgkujEWW1lwfrUoE6CvW783E/mKtCIY3crA+AB5wXIyz2iypMQ3XXwKAy7LC1v6aO 0zIlnI8724aveGG3tCLgPqP2zOdf68cRyIJCR6OlgQKB6P+9/qHfVlKXCsGBm7ppy+LE i+/Wirix4RsbhuM7qySu35zt+7QgUNdegbyZYtKznLAJMO/VwvOYsZMKHsz3D6GVTeGa spD+2XUBhPDUs4qoBp1ZDBkwiGmNnCt2uhKUCLE2TWDdxZrDeHbbm28BrBuS+gwuLwqh Uj9w== X-Gm-Message-State: AJaThX6vd4zZPvL5KM2+l32gtvBqXja2dVnR6URoSrYOpejzdl5Wh7ht d8f8ssmeBN9mxsKr1uW5uyYlE8pwseU0S8hFjsTnPw== X-Received: by 10.200.36.134 with SMTP id s6mr29730258qts.276.1511639294764; Sat, 25 Nov 2017 11:48:14 -0800 (PST) MIME-Version: 1.0 Received: by 10.140.28.117 with HTTP; Sat, 25 Nov 2017 11:48:13 -0800 (PST) In-Reply-To: References: <20171125170157.32154-1-atomnuker@gmail.com> <20171125170157.32154-2-atomnuker@gmail.com> From: Rostislav Pehlivanov Date: Sat, 25 Nov 2017 19:48:13 +0000 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [PATCH v3 2/3] libavcodec/utils.c: simplify avcodec locking with atomics 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On 25 November 2017 at 18:40, James Almer wrote: > On 11/25/2017 2:01 PM, Rostislav Pehlivanov wrote: > > Also makes it more robust than using volatiles. > > > > Signed-off-by: Rostislav Pehlivanov > > --- > > libavcodec/internal.h | 1 - > > libavcodec/utils.c | 12 ++++++------ > > 2 files changed, 6 insertions(+), 7 deletions(-) > > > > diff --git a/libavcodec/internal.h b/libavcodec/internal.h > > index d3310b6afe..1c54966f37 100644 > > --- a/libavcodec/internal.h > > +++ b/libavcodec/internal.h > > @@ -246,7 +246,6 @@ int ff_init_buffer_info(AVCodecContext *s, AVFrame > *frame); > > > > void ff_color_frame(AVFrame *frame, const int color[4]); > > > > -extern volatile int ff_avcodec_locked; > > int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec); > > int ff_unlock_avcodec(const AVCodec *codec); > > > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > > index 3a0f3c11f5..96bc9ff4a4 100644 > > --- a/libavcodec/utils.c > > +++ b/libavcodec/utils.c > > @@ -114,7 +114,7 @@ static int (*lockmgr_cb)(void **mutex, enum AVLockOp > op) = NULL; > > #endif > > > > > > -volatile int ff_avcodec_locked; > > +static atomic_bool ff_avcodec_locked; > > static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0); > > static void *codec_mutex; > > static void *avformat_mutex; > > @@ -1937,6 +1937,7 @@ int av_lockmgr_register(int (*cb)(void **mutex, > enum AVLockOp op)) > > > > int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec) > > { > > + _Bool exp = 1; > > if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || > !codec->init) > > return 0; > > > > @@ -1952,22 +1953,21 @@ int ff_lock_avcodec(AVCodecContext *log_ctx, > const AVCodec *codec) > > atomic_load(&entangled_thread_counter)); > > if (!lockmgr_cb) > > av_log(log_ctx, AV_LOG_ERROR, "No lock manager is set, > please see av_lockmgr_register()\n"); > > - ff_avcodec_locked = 1; > > + atomic_store(&ff_avcodec_locked, 1); > > ff_unlock_avcodec(codec); > > return AVERROR(EINVAL); > > } > > - av_assert0(!ff_avcodec_locked); > > - ff_avcodec_locked = 1; > > + av_assert0(atomic_compare_exchange_strong(&ff_avcodec_locked, > &exp, 1)); > > _Bool atomic_compare_exchange_strong( volatile A* obj, > C* expected, C desired ); > > "Atomically compares the value pointed to by obj with the value pointed > to by expected, and if those are equal, replaces the former with desired > (performs read-modify-write operation). > Return value: The result of the comparison: true if *obj was equal to > *exp, false otherwise." > > exp should be 0. You need to assert that ff_avcodec_locked == 0, then > set it to 1. > The whole experssion seems fine to me as-is and works as expected. > > > return 0; > > } > > > > int ff_unlock_avcodec(const AVCodec *codec) > > { > > + _Bool exp = 0; > > if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || > !codec->init) > > return 0; > > > > - av_assert0(ff_avcodec_locked); > > - ff_avcodec_locked = 0; > > + av_assert0(atomic_compare_exchange_strong(&ff_avcodec_locked, > &exp, 0)); > > And here exp should be 1. > > > atomic_fetch_add(&entangled_thread_counter, -1); > > if (lockmgr_cb) { > > if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE)) > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Seems like I sent an older version of the patch (had to rebase to fix older commit), attached patch fixed both. From e85ab6b0733e97e501dd24a02c921ed1b6394e3c Mon Sep 17 00:00:00 2001 From: Rostislav Pehlivanov Date: Sat, 25 Nov 2017 16:55:44 +0000 Subject: [PATCH] libavcodec/utils.c: simplify avcodec locking with atomics Also makes it more robust than using volatiles. Signed-off-by: Rostislav Pehlivanov --- libavcodec/internal.h | 1 - libavcodec/utils.c | 12 ++++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/libavcodec/internal.h b/libavcodec/internal.h index d3310b6afe..1c54966f37 100644 --- a/libavcodec/internal.h +++ b/libavcodec/internal.h @@ -246,7 +246,6 @@ int ff_init_buffer_info(AVCodecContext *s, AVFrame *frame); void ff_color_frame(AVFrame *frame, const int color[4]); -extern volatile int ff_avcodec_locked; int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec); int ff_unlock_avcodec(const AVCodec *codec); diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 3a0f3c11f5..c733709171 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -114,7 +114,7 @@ static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = NULL; #endif -volatile int ff_avcodec_locked; +static atomic_bool ff_avcodec_locked; static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0); static void *codec_mutex; static void *avformat_mutex; @@ -1937,6 +1937,7 @@ int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec) { + _Bool exp = 0; if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init) return 0; @@ -1952,22 +1953,21 @@ int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec) atomic_load(&entangled_thread_counter)); if (!lockmgr_cb) av_log(log_ctx, AV_LOG_ERROR, "No lock manager is set, please see av_lockmgr_register()\n"); - ff_avcodec_locked = 1; + atomic_store(&ff_avcodec_locked, 1); ff_unlock_avcodec(codec); return AVERROR(EINVAL); } - av_assert0(!ff_avcodec_locked); - ff_avcodec_locked = 1; + av_assert0(atomic_compare_exchange_strong(&ff_avcodec_locked, &exp, 1)); return 0; } int ff_unlock_avcodec(const AVCodec *codec) { + _Bool exp = 1; if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init) return 0; - av_assert0(ff_avcodec_locked); - ff_avcodec_locked = 0; + av_assert0(atomic_compare_exchange_strong(&ff_avcodec_locked, &exp, 0)); atomic_fetch_add(&entangled_thread_counter, -1); if (lockmgr_cb) { if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE)) -- 2.15.0.417.g466bffb3ac