From patchwork Thu Dec 21 22:22:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: wm4 X-Patchwork-Id: 6899 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.79.195 with SMTP id r64csp1509569jad; Thu, 21 Dec 2017 14:23:22 -0800 (PST) X-Google-Smtp-Source: ACJfBou5gY7VObYckUynMM/7MtX/Jnq7ibLrPX+sMnLbkPGa2NFV0omjm8wd5dXfgPxkym7cc7Gs X-Received: by 10.223.182.7 with SMTP id f7mr12633525wre.71.1513895002021; Thu, 21 Dec 2017 14:23:22 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1513895001; cv=none; d=google.com; s=arc-20160816; b=RgixOD/2RNkvDN6Yeiz0q8K7wm34WfpR/caANnZ4/uEFTCGkRVYMUoAhwNFxsK7vAE 6STzvoWRTLccL1CM7hW4PMzxmiC5Wi1MaWB43Ht9pGgA6ngUUrX4+nINlDsnfgTwXG4L UA8KKkNjIZWtL3DDTPNERmxd2P10yO3tHD5ZQxPPAHmHiA7dlp40KhO4bq3pJ92BwrLU ng1ECbKeaJARXZC2/tgTaV4pB5ZVvx6g4nKuJOjjHJaKf8jR84id1ToP1a51W6dOxX0k n+QBOMqXdn+QYnjcugT7Naita1BlZ+akBkjLquG8UQWVMDZ7HmuzHYwnAMZY/V4aJZsl Adyg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:cc:reply-to :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:references:in-reply-to:message-id:date :to:from:dkim-signature:delivered-to:arc-authentication-results; bh=jOCBRtrPRPe9JbwifyWa5X3z2BjmIur1Z0iokywNlog=; b=02EuvM5RYHT1323e+cFK+uWRtialVXu01SJBHojBr0lIRWAXsCuaQmbojicAtxHppW zynoGZQ439Dq9RRQE4ErKpMBy2ASiXE4MWns36pedOrL5Ybq5eYvI8sb8VNK0XNsWOSV XwfLrCqWu1CdJ6rNNB8mXzmnFI3GZc3IU5QMNLyBhwhk1V9kiaqsHpGN4U2tk8SkBlTU tQiMdmcuZLorjiHC0u4F/x/vVf23Icfq7qznO9qlwkRqyeVJRB+EY19m6AX94Pi5inkg Tf8OdeTpSuSvwG5zzYpS2VL2I6OReRzVaFzbzEack9e13GbKt8Y5GMyDA46vxAC6begt 0N0A== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@googlemail.com header.s=20161025 header.b=AbbV3Rn7; 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 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id g1si15845832wrc.509.2017.12.21.14.23.21; Thu, 21 Dec 2017 14:23:21 -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=@googlemail.com header.s=20161025 header.b=AbbV3Rn7; 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 DEEA4680917; Fri, 22 Dec 2017 00:22:33 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f67.google.com (mail-wm0-f67.google.com [74.125.82.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 21B39680758 for ; Fri, 22 Dec 2017 00:22:26 +0200 (EET) Received: by mail-wm0-f67.google.com with SMTP id 9so18697990wme.4 for ; Thu, 21 Dec 2017 14:22:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=IBmukRa5EUN87l4+aOyyfHp29v8s1XcM7dwGI6xuN5A=; b=AbbV3Rn7q8EFdpxFFXdcYZSAIjnSOh4Oe9SoU1Vlu1A4U0Abe1egA/ZKrWPTXXk4ch R7Qc3zLTkadRL7OMI/XTm8rkCB1goV/2C+z0rDfVZwdtuqjP0FjuML22JAY5w7ZI4QOr yag/M54S6SbyuZbmhHBY+jNfwXLdhjWffDZJi+Ft34aCaPJmUMZhjxL+QY94cxwUmlCP 5DqUSSOgyuKWMwWPYm97TGxN1fXTBHY5xZQpR14Ggt9Hn9RaNnomtx37EJ838UBPgHoY EMZEgNUXIKlwFxj1C+E1GMry4nGt2wVuhmAeN8ruKRVIMowfER5e6nVh9J60wKAbh2SB nYfA== 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:in-reply-to :references; bh=IBmukRa5EUN87l4+aOyyfHp29v8s1XcM7dwGI6xuN5A=; b=MN1fdDr+MlZ4aA/2Bj5Cj8XBAitgkXGoK2ICRmmy1BFS1TdthshLWsHfYBUwAuIXwi RVrWVoAIjWpkiZocQGJ7GLGdGaqu/kQAhUd0yFu317kGHb6dWH92ZLg8E7ur2TQLj7hp OARaArWqwxVUcdFLos6E3dOAdgmtg+Jge/iwTiFZIuDQWtvIXCrI7brRDT/NFhb7J6bL QjZpBwSJSsE6mTt8BbKf7Kf6/hm6OgJwZXxu4iU2I6XBrSTZwH2SW1D25ES4jbP8h7HW LvCsLQ3S1w8RNiorSQoQtgGAu5T6yYJ5oheLo66lQseXzwaMI6H3WZAHDDwrdndFo3Ye uO/w== X-Gm-Message-State: AKGB3mJCJ5ZWdvFFH7vLTlekliqc1jlMIlTJn5shZgZxjhVKTXW8FcUA ssFy6qMA12oXP2vjX/4VF3fapA== X-Received: by 10.80.183.172 with SMTP id h41mr12746451ede.149.1513894957078; Thu, 21 Dec 2017 14:22:37 -0800 (PST) Received: from debian.speedport.ip (p2003006CCD4EDC713C06725800124D49.dip0.t-ipconnect.de. [2003:6c:cd4e:dc71:3c06:7258:12:4d49]) by smtp.googlemail.com with ESMTPSA id p47sm19834538edc.30.2017.12.21.14.22.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 21 Dec 2017 14:22:36 -0800 (PST) From: wm4 To: ffmpeg-devel@ffmpeg.org Date: Thu, 21 Dec 2017 23:22:24 +0100 Message-Id: <20171221222224.18577-5-nfxjfg@googlemail.com> X-Mailer: git-send-email 2.15.1 In-Reply-To: <20171221222224.18577-1-nfxjfg@googlemail.com> References: <20171221222224.18577-1-nfxjfg@googlemail.com> Subject: [FFmpeg-devel] [PATCH 5/5] lavc: remove weird debug code around avcodec init locking 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" This is just a lot of complicated and confusing code that does nothing. Also, the functions return values were checked only sometimes. Locking shouldn't fail anyway, so remove the return values. Barely any other pthread lock calls check the return value (including more important code that is more likely to fail horribly if locking fails). It could be argued that it might be helpful in some debugging situations, or when the user built FFmpeg without thread support against all good advice. But there are dummy atomics too, so the atomic check won't help with ensuring correctness absolutely. You gain very little. Also, for debugging, you can just raise the ASSERT_LEVEL, and then libavutil/thread.h will redefine the locking functions to explicitly check the return values. --- libavcodec/internal.h | 4 ---- libavcodec/utils.c | 55 +++++++++++++-------------------------------------- 2 files changed, 14 insertions(+), 45 deletions(-) diff --git a/libavcodec/internal.h b/libavcodec/internal.h index bf58f36ad3..1c901a7cbc 100644 --- a/libavcodec/internal.h +++ b/libavcodec/internal.h @@ -241,10 +241,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); - /** * Maximum size in bytes of extradata. * This value was chosen such that every bit of the buffer is diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 718063f1a1..7432bb78a6 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -67,8 +67,6 @@ #include "libavutil/ffversion.h" const char av_codec_ffversion[] = "FFmpeg version " FFMPEG_VERSION; -volatile int ff_avcodec_locked; -static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0); static pthread_mutex_t codec_mutex = PTHREAD_MUTEX_INITIALIZER; void av_fast_padded_malloc(void *ptr, unsigned int *size, size_t min_size) @@ -550,6 +548,19 @@ static int64_t get_bit_rate(AVCodecContext *ctx) return bit_rate; } + +static void ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec) +{ + if (!(codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE) && codec->init) + pthread_mutex_lock(&codec_mutex); +} + +static void ff_unlock_avcodec(const AVCodec *codec) +{ + if (!(codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE) && codec->init) + pthread_mutex_unlock(&codec_mutex); +} + int attribute_align_arg ff_codec_open2_recursive(AVCodecContext *avctx, const AVCodec *codec, AVDictionary **options) { int ret = 0; @@ -589,9 +600,7 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code if (options) av_dict_copy(&tmp, *options, 0); - ret = ff_lock_avcodec(avctx, codec); - if (ret < 0) - return ret; + ff_lock_avcodec(avctx, codec); avctx->internal = av_mallocz(sizeof(AVCodecInternal)); if (!avctx->internal) { @@ -1867,42 +1876,6 @@ int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) } #endif -int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec) -{ - if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init) - return 0; - - if (pthread_mutex_lock(&codec_mutex)) - return -1; - - if (atomic_fetch_add(&entangled_thread_counter, 1)) { - av_log(log_ctx, AV_LOG_ERROR, - "Insufficient thread locking. At least %d threads are " - "calling avcodec_open2() at the same time right now.\n", - atomic_load(&entangled_thread_counter)); - ff_avcodec_locked = 1; - ff_unlock_avcodec(codec); - return AVERROR(EINVAL); - } - av_assert0(!ff_avcodec_locked); - ff_avcodec_locked = 1; - return 0; -} - -int ff_unlock_avcodec(const AVCodec *codec) -{ - if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init) - return 0; - - av_assert0(ff_avcodec_locked); - ff_avcodec_locked = 0; - atomic_fetch_add(&entangled_thread_counter, -1); - if (pthread_mutex_unlock(&codec_mutex)) - return -1; - - return 0; -} - unsigned int avpriv_toupper4(unsigned int x) { return av_toupper(x & 0xFF) +