From patchwork Fri Nov 20 07:19:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 23810 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 3DA2D44AD73 for ; Fri, 20 Nov 2020 09:34:19 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2231468BB69; Fri, 20 Nov 2020 09:25:19 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ed1-f68.google.com (mail-ed1-f68.google.com [209.85.208.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E9E7168BAA5 for ; Fri, 20 Nov 2020 09:25:06 +0200 (EET) Received: by mail-ed1-f68.google.com with SMTP id d18so8469839edt.7 for ; Thu, 19 Nov 2020 23:25:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references:reply-to :mime-version:content-transfer-encoding; bh=M4OzrxtMv0gXEQLnauRa4dv7SjxvQK9EdktXNOtl0+w=; b=D3p2pOqYiRAnt0CeHvoJHORn5Rw5qdTnbFwbmmMWbLL+MDkBgRH6qoCLbnXI7Injkc 3HFj1wi0xwtro3je1G08S0BRVPQ5iB8532wauoWZSGqMBsA8vPlC+M9h3DkDt4wkfcQI p6ire+WNOV6Fv1U28eLgeFETN0RVD2Iq4KU4yUckD4h6b2oPc9g07Fv0hLG+cNj8XEwv ZhagUjLEnvisFibKuQxuCTfenBkwfdKlEwj5PlucddrT9mzk9YKg9idgRd8U7TPgY4wV 6YMPpaRVnDGMmFpbx6tDJog8NuVnEV5Up3nYvul4tEqB+ViPuKxLKaKpN3viLyDJY9xc gcEA== 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:reply-to:mime-version:content-transfer-encoding; bh=M4OzrxtMv0gXEQLnauRa4dv7SjxvQK9EdktXNOtl0+w=; b=jG/8H67jsYC26LPwx7KG4RHkqc2w7hQaBpjpErjdXApGIjU4AYf6y76IENy3nR2kEx 5XXYE1447ZI/dbplLAS/3BHMyV8f7GwbeWnbcOmqCnPogaINDldzvEiyVFgPlXddGamB UwdKIPeyMOXyVAvVwpO6PDjqysM1mL8ey24X+Xl7PCx8M8Nn9jumFSgn4LrbjuEr89RK DEm79vMdMBA2ReJdVyVf48pHvOtnB4aCAtSHT55Fi1E00eqQ8lFz/zrabLkn+pduCiaQ rVWHwzuV7tqpaknd8SjiRNdHiNdahjzvFYXVDDubcjTX2bfTm7WLxjSk4AZTCgO91wob 8dvQ== X-Gm-Message-State: AOAM530E0YxfNBm+Ovca3yffeqfHpmA4Q1t/oQQmkkLxM4rKUkqjUvaa QzgwS+pjab2MIrPReu6Oq7Tk0M+h3lM2mQ== X-Google-Smtp-Source: ABdhPJxMsYXWKx4U3ssaR0nJ8pvnLd5JMB/bOrrzR6QozkHe86olhXTGo4I78Hh4WVHVE6c5/OMtKw== X-Received: by 2002:a50:c30d:: with SMTP id a13mr11069307edb.89.1605857106122; Thu, 19 Nov 2020 23:25:06 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc1aa4b.dynamic.kabel-deutschland.de. [188.193.170.75]) by smtp.gmail.com with ESMTPSA id lz27sm779419ejb.39.2020.11.19.23.25.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Nov 2020 23:25:05 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 20 Nov 2020 08:19:28 +0100 Message-Id: <20201120072116.818090-56-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201120072116.818090-1-andreas.rheinhardt@gmail.com> References: <20201120072116.818090-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 055/162] avcodec/fft_template, fft_init_table: Make ff_fft_init() thread-safe 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: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Commit 1af615683e4a1a858407afbaa2fd686842da7e49 put initializing the ff_fft_offsets_lut (which is typically used if FFT_FIXED_32) behind an ff_thread_once() to make ff_fft_init() thread-safe; yet there is a second place where said table may be initialized which is not guarded by this AVOnce: ff_fft_init_mips(). MIPS uses this LUT even for ordinary floating point FFTs, so that ff_fft_init() is not thread-safe (on MIPS) for both 32bit fixed-point as well as floating-point FFTs; e.g. ff_mdct_init() inherits this flaw and therefore initializing e.g. the AAC decoders is not thread-safe (on MIPS) despite them having FF_CODEC_CAP_INIT_CLEANUP set. This commit fixes this by moving the AVOnce to fft_init_table.c and using it to guard all initializations of ff_fft_offsets_lut. (It is not that bad in practice, because every entry of ff_fft_offsets_lut is never read during initialization and is only once ever written to (namely to its final value); but even these are conflicting actions which are (by definition) data races and lead to undefined behaviour.) Signed-off-by: Andreas Rheinhardt --- I don't have a MIPS, so the MIPS part is untested. libavcodec/fft_init_table.c | 24 ++++++++++++++++++++---- libavcodec/fft_table.h | 2 +- libavcodec/fft_template.c | 12 +----------- libavcodec/mips/fft_mips.c | 4 +--- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/libavcodec/fft_init_table.c b/libavcodec/fft_init_table.c index c488018f62..83e35ffb7c 100644 --- a/libavcodec/fft_init_table.c +++ b/libavcodec/fft_init_table.c @@ -51,6 +51,8 @@ * @file * definitions and initialization of LUT table for FFT */ +#include "libavutil/thread.h" + #include "libavcodec/fft_table.h" const int32_t ff_w_tab_sr[MAX_FFT_SIZE/(4*16)] = { @@ -314,15 +316,29 @@ const int32_t ff_w_tab_sr[MAX_FFT_SIZE/(4*16)] = { uint16_t ff_fft_offsets_lut[21845]; -void ff_fft_lut_init(uint16_t *table, int off, int size, int *index) +static void fft_lut_init(uint16_t *table, int off, int size, int *index) { if (size < 16) { table[*index] = off >> 2; (*index)++; } else { - ff_fft_lut_init(table, off, size>>1, index); - ff_fft_lut_init(table, off+(size>>1), size>>2, index); - ff_fft_lut_init(table, off+3*(size>>2), size>>2, index); + fft_lut_init(table, off, size >> 1, index); + fft_lut_init(table, off + (size >> 1), size >> 2, index); + fft_lut_init(table, off + 3 * (size >> 2), size >> 2, index); } } + +static void fft_lut_init_start(void) +{ + int n = 0; + + fft_lut_init(ff_fft_offsets_lut, 0, 1 << 17, &n); +} + +void ff_fft_lut_init(void) +{ + static AVOnce init_once = AV_ONCE_INIT; + + ff_thread_once(&init_once, fft_lut_init_start); +} diff --git a/libavcodec/fft_table.h b/libavcodec/fft_table.h index ed0a6588b4..09df49f2b8 100644 --- a/libavcodec/fft_table.h +++ b/libavcodec/fft_table.h @@ -61,6 +61,6 @@ extern const int32_t ff_w_tab_sr[]; extern uint16_t ff_fft_offsets_lut[]; -void ff_fft_lut_init(uint16_t *table, int off, int size, int *index); +void ff_fft_lut_init(void); #endif /* AVCODEC_FFT_TABLE_H */ diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c index 20a62e4290..8825e39f79 100644 --- a/libavcodec/fft_template.c +++ b/libavcodec/fft_template.c @@ -35,13 +35,6 @@ #if FFT_FIXED_32 #include "fft_table.h" - -static void av_cold fft_lut_init(void) -{ - int n = 0; - ff_fft_lut_init(ff_fft_offsets_lut, 0, 1 << 17, &n); -} - #else /* FFT_FIXED_32 */ /* cos(2*pi*x/n) for 0<=x<=n/4, followed by its reverse */ @@ -236,10 +229,7 @@ av_cold int ff_fft_init(FFTContext *s, int nbits, int inverse) #endif #if FFT_FIXED_32 - { - static AVOnce control = AV_ONCE_INIT; - ff_thread_once(&control, fft_lut_init); - } + ff_fft_lut_init(); #else /* FFT_FIXED_32 */ #if FFT_FLOAT if (ARCH_AARCH64) ff_fft_init_aarch64(s); diff --git a/libavcodec/mips/fft_mips.c b/libavcodec/mips/fft_mips.c index 69abdc8a08..a6656d9650 100644 --- a/libavcodec/mips/fft_mips.c +++ b/libavcodec/mips/fft_mips.c @@ -500,9 +500,7 @@ static void ff_imdct_calc_mips(FFTContext *s, FFTSample *output, const FFTSample av_cold void ff_fft_init_mips(FFTContext *s) { - int n=0; - - ff_fft_lut_init(ff_fft_offsets_lut, 0, 1 << 17, &n); + ff_fft_lut_init(); ff_init_ff_cos_tabs(17); #if HAVE_INLINE_ASM