From patchwork Tue Mar 23 12:35:51 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 26557 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 70CC6449E98 for ; Tue, 23 Mar 2021 14:36:17 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4677A68AAEB; Tue, 23 Mar 2021 14:36:17 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D8CF268AAD5 for ; Tue, 23 Mar 2021 14:36:10 +0200 (EET) Received: by mail-ed1-f46.google.com with SMTP id y6so23277470eds.1 for ; Tue, 23 Mar 2021 05:36:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:reply-to:mime-version :content-transfer-encoding; bh=Qwi0dfU71vtXrRS7uMxj4Ku4iFOYjGFs/naWmdKFjMs=; b=tlTCBQwDDPHMyJZ1EAwwrgt0xMS6z87JK6c/rjMlHPpzYOCNQE5jSvSEJlw1+cWLPk i910jNel0iR9h7hQrsiiwprY4cD9P0aZOjZNo98zZ1MJ3jX3IYYpI+4ISSxvCcw6UQKK 1BjFUGKc6QUGKxR0fqJhBWzW/VdDNgNAUEhp5rMpjmyPhQCJVPjYikElJdbMh0PlZLhO FGKfp0Hc3Zws/0Y/GQrkN765e7tWgT4kfnUyy7GdW1C0XXCvWMhGSE2gJUT8DRlAkHdV uipKFwcQVUrF5DzmGXkL7h85vA+iscNadyyTNF03hJhxpitq9gdrAn6vzbW/iRhzt1Ek Vu2Q== 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:reply-to :mime-version:content-transfer-encoding; bh=Qwi0dfU71vtXrRS7uMxj4Ku4iFOYjGFs/naWmdKFjMs=; b=DQByPmGbq7AwaF8+2TkcRq7Vhtu9/5nDOtdQsk49zur3aDk1036cvuHjDrB4Avcz/Z /mHm2VCSXx8YqNuFbxwWfdHN65BoywUN6duAbnbi/JD/TDBwE3CcW2F8GUxYKLQdrEfN NfPKaz5B9IveFCasY21+5fyG6nI+t941hRw/WE0qA7rgyKgYjlRDzM86MZtRsxPUl0kX als2JdE5o6MvbJZs6g/iDNmfvxjf5kPSkuR0bwWux0RkUBgvuNmhsumebk61/uyM2TTX ONdkDasK6XaU3kv6VCwhr5+QvVxRbIbtEYEIy5sgDTCQhGH0Jqm2zHJJSEshg6Z/PeTQ Cwfw== X-Gm-Message-State: AOAM531wExDXeFn7GqOub/jYNsWXeMLEJ0LA4aSPfwaMdsNZiul/3cwf OgggEXVAuqtVbsKPzJcWX3e6w/VKxcwhqA== X-Google-Smtp-Source: ABdhPJxYrUFW/ec5rbd7tjAI/Owom64oHcznd+/G36F+KcIWZGiREGnsm5UWzw2dvGdbKLwPGJh/Hg== X-Received: by 2002:a50:fe06:: with SMTP id f6mr4385012edt.349.1616502969864; Tue, 23 Mar 2021 05:36:09 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08960.dynamic.kabel-deutschland.de. [188.192.137.96]) by smtp.gmail.com with ESMTPSA id mp36sm10728532ejc.48.2021.03.23.05.36.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Mar 2021 05:36:07 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 23 Mar 2021 13:35:51 +0100 Message-Id: <20210323123554.1370260-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 1/4] avcodec/pthread_frame: Factor initializing single thread out 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" Signed-off-by: Andreas Rheinhardt --- libavcodec/pthread_frame.c | 127 ++++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 59 deletions(-) diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index 7bcb9a7bcc..311d6ed771 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -763,52 +763,12 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) avctx->codec = NULL; } -int ff_frame_thread_init(AVCodecContext *avctx) +static av_cold int init_thread(PerThreadContext *p, + FrameThreadContext *fctx, AVCodecContext *avctx, + AVCodecContext *src, const AVCodec *codec, int first) { - int thread_count = avctx->thread_count; - const AVCodec *codec = avctx->codec; - AVCodecContext *src = avctx; - FrameThreadContext *fctx; - int i, err = 0; - - if (!thread_count) { - int nb_cpus = av_cpu_count(); - // use number of cores + 1 as thread count if there is more than one - if (nb_cpus > 1) - thread_count = avctx->thread_count = FFMIN(nb_cpus + 1, MAX_AUTO_THREADS); - else - thread_count = avctx->thread_count = 1; - } - - if (thread_count <= 1) { - avctx->active_thread_type = 0; - return 0; - } - - avctx->internal->thread_ctx = fctx = av_mallocz(sizeof(FrameThreadContext)); - if (!fctx) - return AVERROR(ENOMEM); - - fctx->threads = av_mallocz_array(thread_count, sizeof(PerThreadContext)); - if (!fctx->threads) { - av_freep(&avctx->internal->thread_ctx); - return AVERROR(ENOMEM); - } - - pthread_mutex_init(&fctx->buffer_mutex, NULL); - pthread_mutex_init(&fctx->hwaccel_mutex, NULL); - pthread_mutex_init(&fctx->async_mutex, NULL); - pthread_cond_init(&fctx->async_cond, NULL); - - fctx->async_lock = 1; - fctx->delaying = 1; - - if (codec->type == AVMEDIA_TYPE_VIDEO) - avctx->delay = src->thread_count - 1; - - for (i = 0; i < thread_count; i++) { AVCodecContext *copy = av_malloc(sizeof(AVCodecContext)); - PerThreadContext *p = &fctx->threads[i]; + int err; pthread_mutex_init(&p->mutex, NULL); pthread_mutex_init(&p->progress_mutex, NULL); @@ -819,22 +779,19 @@ int ff_frame_thread_init(AVCodecContext *avctx) p->frame = av_frame_alloc(); if (!p->frame) { av_freep(©); - err = AVERROR(ENOMEM); - goto error; + return AVERROR(ENOMEM); } p->avpkt = av_packet_alloc(); if (!p->avpkt) { av_freep(©); - err = AVERROR(ENOMEM); - goto error; + return AVERROR(ENOMEM); } p->parent = fctx; p->avctx = copy; if (!copy) { - err = AVERROR(ENOMEM); - goto error; + return AVERROR(ENOMEM); } *copy = *src; @@ -842,8 +799,7 @@ int ff_frame_thread_init(AVCodecContext *avctx) copy->internal = av_malloc(sizeof(AVCodecInternal)); if (!copy->internal) { copy->priv_data = NULL; - err = AVERROR(ENOMEM); - goto error; + return AVERROR(ENOMEM); } *copy->internal = *src->internal; copy->internal->thread_ctx = p; @@ -854,27 +810,27 @@ int ff_frame_thread_init(AVCodecContext *avctx) if (codec->priv_data_size) { copy->priv_data = av_mallocz(codec->priv_data_size); if (!copy->priv_data) { - err = AVERROR(ENOMEM); - goto error; + return AVERROR(ENOMEM); } if (codec->priv_class) { *(const AVClass **)copy->priv_data = codec->priv_class; err = av_opt_copy(copy->priv_data, src->priv_data); if (err < 0) - goto error; + return err; } } - if (i) + if (!first) copy->internal->is_copy = 1; if (codec->init) err = codec->init(copy); + if (err < 0) { + return err; + } - if (err) goto error; - - if (!i) + if (first) update_context_from_thread(avctx, copy, 1); atomic_init(&p->debug_threads, (copy->debug & FF_DEBUG_THREADS) != 0); @@ -882,6 +838,59 @@ int ff_frame_thread_init(AVCodecContext *avctx) err = AVERROR(pthread_create(&p->thread, NULL, frame_worker_thread, p)); p->thread_init= !err; if(!p->thread_init) + return err; + return 0; +} + +int ff_frame_thread_init(AVCodecContext *avctx) +{ + int thread_count = avctx->thread_count; + const AVCodec *codec = avctx->codec; + AVCodecContext *src = avctx; + FrameThreadContext *fctx; + int i, err = 0; + + if (!thread_count) { + int nb_cpus = av_cpu_count(); + // use number of cores + 1 as thread count if there is more than one + if (nb_cpus > 1) + thread_count = avctx->thread_count = FFMIN(nb_cpus + 1, MAX_AUTO_THREADS); + else + thread_count = avctx->thread_count = 1; + } + + if (thread_count <= 1) { + avctx->active_thread_type = 0; + return 0; + } + + avctx->internal->thread_ctx = fctx = av_mallocz(sizeof(FrameThreadContext)); + if (!fctx) + return AVERROR(ENOMEM); + + fctx->threads = av_mallocz_array(thread_count, sizeof(PerThreadContext)); + if (!fctx->threads) { + av_freep(&avctx->internal->thread_ctx); + return AVERROR(ENOMEM); + } + + pthread_mutex_init(&fctx->buffer_mutex, NULL); + pthread_mutex_init(&fctx->hwaccel_mutex, NULL); + pthread_mutex_init(&fctx->async_mutex, NULL); + pthread_cond_init(&fctx->async_cond, NULL); + + fctx->async_lock = 1; + fctx->delaying = 1; + + if (codec->type == AVMEDIA_TYPE_VIDEO) + avctx->delay = src->thread_count - 1; + + for (i = 0; i < thread_count; i++) { + PerThreadContext *p = &fctx->threads[i]; + int first = !i; + + err = init_thread(p, fctx, avctx, src, codec, first); + if (err < 0) goto error; } From patchwork Tue Mar 23 12:35:52 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 26561 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 AB453449413 for ; Tue, 23 Mar 2021 15:05:24 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7CBB068AB11; Tue, 23 Mar 2021 15:05:24 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5F68268A77A for ; Tue, 23 Mar 2021 15:05:17 +0200 (EET) Received: by mail-lf1-f44.google.com with SMTP id m12so26497510lfq.10 for ; Tue, 23 Mar 2021 06:05:17 -0700 (PDT) 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=tF86qt0ZcqswGeBHt7JuCpKarqfgrTAovW9UKbDapGY=; b=Fdb3GtmRLmR1gU9nfTTHuJ1Cu2ymG3dRWrL2LRtfeBXvyFjRoPl5NgOCrD8GQqXEeU nOMvqDWYmrtjscgyxVJ5GRFLuBXfyDu6OytZUnKKHSMRmCIJka8kQ9JDaCj5P0m10y8r ue7WJvP2QGaI5G7mhfmWqzMOk6+a89BzhxPBp4VhUxHV4irpurxguLgQ8F829os74Le7 ehUC1oo10FLgU0krDJqv5ZdmjehbSOzEGdqUQhKQQwKjRweE/FxJHA0sEeUXKaC1fjkX /TSkLKMs8uz5uRUyOuqeq/x14Fm9rFDOIQ//ZeLWzJxWMhzlQ9cFzvUVFTW4+pobhInA nGzg== 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=tF86qt0ZcqswGeBHt7JuCpKarqfgrTAovW9UKbDapGY=; b=tJpef8ujJZnj/Oq4YDF7IWTJ/+p3fF5CHf55dd5RmBBe/io+l2Qjj16QD6M7jG1Zvw n5omkXn7bDQux0neX93s0PyqICU1E5iiixJlk5S9rzeft/8INHHUNknTGQsKfPEMiEEg LQcuMW289L5BW/DAi+9jL+HQqxZ6IYXxDDWd5RyrapES5VzhzamKF98UnpIZyulzddYQ djQ4133cDukQc/we9jLT4foYd5zzowgDn4ajcoIeKVyB6wrSVMDhfnxU8zvstiFnEuwm iyTLeKEkjLwBwhpsZy3cqVf3LfTMslEBfsYgfjK1AgAdfsilgwoXAwykz28HCd+lNkKZ t2og== X-Gm-Message-State: AOAM532+1H14aL1tiQfVI781iL6OkzB+keRB0IQwFOdYZ/wjZOQuISML j0mjaCh3RxAV+0aAlLpfoj+ZhZjVC01jhQ== X-Google-Smtp-Source: ABdhPJxacNm0kNVzGttBgUElWpyol5/ERbtROchmEBrvmECTdVdGoTcF3hjwRZpM1TfOxRDewf0oKA== X-Received: by 2002:a17:906:3159:: with SMTP id e25mr4716102eje.303.1616503012073; Tue, 23 Mar 2021 05:36:52 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08960.dynamic.kabel-deutschland.de. [188.192.137.96]) by smtp.gmail.com with ESMTPSA id mp36sm10728532ejc.48.2021.03.23.05.36.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Mar 2021 05:36:49 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 23 Mar 2021 13:35:52 +0100 Message-Id: <20210323123554.1370260-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20210323123554.1370260-1-andreas.rheinhardt@gmail.com> References: <20210323123554.1370260-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 2/4] avcodec/pthread_frame: Fix cleanup during init 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" In case an error happened when setting up the child threads, ff_frame_thread_init() would up until now call ff_frame_thread_free() to clean up all threads set up so far, including the current, not properly initialized one. But a half-allocated context needs special handling which ff_frame_thread_frame_free() doesn't provide. Notably, if allocating the AVCodecInternal, the codec's private data or setting the options fails, the codec's close function will be called (if there is one); it will also be called if the codec's init function fails, regardless of whether the FF_CODEC_CAP_INIT_CLEANUP is set. This is not supported by all codecs; in ticket #9099 it led to a crash. Signed-off-by: Andreas Rheinhardt --- libavcodec/pthread_frame.c | 137 ++++++++++++++++++------------------- 1 file changed, 67 insertions(+), 70 deletions(-) diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index 311d6ed771..db2f0cb3d2 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -64,6 +64,12 @@ enum { STATE_SETUP_FINISHED, }; +enum { + UNINITIALIZED, ///< Thread has not been created, AVCodec->close mustn't be called + NEEDS_CLOSE, ///< AVCodec->close needs to be called + INITIALIZED, ///< Thread has been properly set up +}; + /** * Context used by codec threads and stored in their AVCodecInternal thread_ctx. */ @@ -698,27 +704,40 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) for (i = 0; i < thread_count; i++) { PerThreadContext *p = &fctx->threads[i]; + AVCodecContext *ctx = p->avctx; - pthread_mutex_lock(&p->mutex); - p->die = 1; - pthread_cond_signal(&p->input_cond); - pthread_mutex_unlock(&p->mutex); - - if (p->thread_init) - pthread_join(p->thread, NULL); - p->thread_init=0; + if (ctx->internal) { + if (p->thread_init == INITIALIZED) { + pthread_mutex_lock(&p->mutex); + p->die = 1; + pthread_cond_signal(&p->input_cond); + pthread_mutex_unlock(&p->mutex); - if (codec->close && p->avctx) - codec->close(p->avctx); + pthread_join(p->thread, NULL); + } + if (codec->close && p->thread_init != UNINITIALIZED) + codec->close(ctx); #if FF_API_THREAD_SAFE_CALLBACKS - release_delayed_buffers(p); + release_delayed_buffers(p); + for (int j = 0; j < p->released_buffers_allocated; j++) + av_frame_free(&p->released_buffers[j]); + av_freep(&p->released_buffers); #endif - av_frame_free(&p->frame); - } + if (ctx->priv_data) { + if (codec->priv_class) + av_opt_free(ctx->priv_data); + av_freep(&ctx->priv_data); + } - for (i = 0; i < thread_count; i++) { - PerThreadContext *p = &fctx->threads[i]; + av_freep(&ctx->slice_offset); + + av_buffer_unref(&ctx->internal->pool); + av_freep(&ctx->internal); + av_buffer_unref(&ctx->hw_frames_ctx); + } + + av_frame_free(&p->frame); pthread_mutex_destroy(&p->mutex); pthread_mutex_destroy(&p->progress_mutex); @@ -727,26 +746,6 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) pthread_cond_destroy(&p->output_cond); av_packet_free(&p->avpkt); -#if FF_API_THREAD_SAFE_CALLBACKS - for (int j = 0; j < p->released_buffers_allocated; j++) - av_frame_free(&p->released_buffers[j]); - av_freep(&p->released_buffers); -#endif - - if (p->avctx) { - if (codec->priv_class) - av_opt_free(p->avctx->priv_data); - av_freep(&p->avctx->priv_data); - - av_freep(&p->avctx->slice_offset); - } - - if (p->avctx) { - av_buffer_unref(&p->avctx->internal->pool); - av_freep(&p->avctx->internal); - av_buffer_unref(&p->avctx->hw_frames_ctx); - } - av_freep(&p->avctx); } @@ -763,47 +762,37 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) avctx->codec = NULL; } -static av_cold int init_thread(PerThreadContext *p, +static av_cold int init_thread(PerThreadContext *p, int *threads_to_free, FrameThreadContext *fctx, AVCodecContext *avctx, AVCodecContext *src, const AVCodec *codec, int first) { - AVCodecContext *copy = av_malloc(sizeof(AVCodecContext)); + AVCodecContext *copy; int err; + atomic_init(&p->state, STATE_INPUT_READY); + + copy = av_memdup(src, sizeof(*src)); + if (!copy) + return AVERROR(ENOMEM); + copy->priv_data = NULL; + + /* From now on, this PerThreadContext will be cleaned up by + * ff_frame_thread_free in case of errors. */ + (*threads_to_free)++; + pthread_mutex_init(&p->mutex, NULL); pthread_mutex_init(&p->progress_mutex, NULL); pthread_cond_init(&p->input_cond, NULL); pthread_cond_init(&p->progress_cond, NULL); pthread_cond_init(&p->output_cond, NULL); - p->frame = av_frame_alloc(); - if (!p->frame) { - av_freep(©); - return AVERROR(ENOMEM); - } - p->avpkt = av_packet_alloc(); - if (!p->avpkt) { - av_freep(©); - return AVERROR(ENOMEM); - } - - p->parent = fctx; - p->avctx = copy; + p->parent = fctx; + p->avctx = copy; - if (!copy) { - return AVERROR(ENOMEM); - } - - *copy = *src; - - copy->internal = av_malloc(sizeof(AVCodecInternal)); - if (!copy->internal) { - copy->priv_data = NULL; - return AVERROR(ENOMEM); - } - *copy->internal = *src->internal; + copy->internal = av_memdup(src->internal, sizeof(*src->internal)); + if (!copy->internal) + return AVERROR(ENOMEM); copy->internal->thread_ctx = p; - copy->internal->last_pkt_props = p->avpkt; copy->delay = avctx->delay; @@ -821,14 +810,22 @@ static av_cold int init_thread(PerThreadContext *p, } } + if (!(p->frame = av_frame_alloc()) || + !(p->avpkt = av_packet_alloc())) + return AVERROR(ENOMEM); + copy->internal->last_pkt_props = p->avpkt; + if (!first) copy->internal->is_copy = 1; if (codec->init) err = codec->init(copy); if (err < 0) { + if (codec->caps_internal & FF_CODEC_CAP_INIT_CLEANUP) + p->thread_init = NEEDS_CLOSE; return err; } + p->thread_init = NEEDS_CLOSE; if (first) update_context_from_thread(avctx, copy, 1); @@ -836,9 +833,10 @@ static av_cold int init_thread(PerThreadContext *p, atomic_init(&p->debug_threads, (copy->debug & FF_DEBUG_THREADS) != 0); err = AVERROR(pthread_create(&p->thread, NULL, frame_worker_thread, p)); - p->thread_init= !err; - if(!p->thread_init) - return err; + if (err < 0) + return err; + p->thread_init = INITIALIZED; + return 0; } @@ -885,11 +883,11 @@ int ff_frame_thread_init(AVCodecContext *avctx) if (codec->type == AVMEDIA_TYPE_VIDEO) avctx->delay = src->thread_count - 1; - for (i = 0; i < thread_count; i++) { + for (i = 0; i < thread_count; ) { PerThreadContext *p = &fctx->threads[i]; int first = !i; - err = init_thread(p, fctx, avctx, src, codec, first); + err = init_thread(p, &i, fctx, avctx, src, codec, first); if (err < 0) goto error; } @@ -897,8 +895,7 @@ int ff_frame_thread_init(AVCodecContext *avctx) return 0; error: - ff_frame_thread_free(avctx, i+1); - + ff_frame_thread_free(avctx, i); return err; } From patchwork Tue Mar 23 12:35:53 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 26558 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 972E1449E98 for ; Tue, 23 Mar 2021 14:37:01 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7A51A68AB1E; Tue, 23 Mar 2021 14:37:01 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B690F68A64D for ; Tue, 23 Mar 2021 14:36:55 +0200 (EET) Received: by mail-ed1-f46.google.com with SMTP id dm8so23277045edb.2 for ; Tue, 23 Mar 2021 05:36:55 -0700 (PDT) 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=LCoWQpwXBos5LI3/5dkYNlAUemsiiFKUSi5zoDjBkxo=; b=B6R40fvAO4123G5nYp2yLpf8G9CguXVh2O61jWFk++DsBtSjPCLFw5Yzaff7KKLkAk pTBccYne/wnyEB/fE9cUTX0j3JRdTf80PQrvHBWsyWCkfrLOB4IJC6A/YfWoQ5+UPmh9 PJeMEf2OW+6+MZZ+PenYtXDkg+CouNwoD3xPiNvKkx20IvRgIb/i3Wv6m2huo4jBO/tE GX5M0w2R/Bm5kwg4r8KKytzWvwAIRxYRnkhQxFc4BZu0i1mEfotPtPkkOfzbGBPiK9a5 kCn86Azvj7wLvkEOUOgR6EH23m9xeLOb23zaDXrpFb/alSFbjdvmoCvox3HgA1M1f3dD S90A== 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=LCoWQpwXBos5LI3/5dkYNlAUemsiiFKUSi5zoDjBkxo=; b=QoMKyQe2chR9RXyzJ6PP7BhqixNg8c4JsIHUj8LCqtgGX3tZ1UknZFtc10p+ENWjEk Xv/t+gqPJTiATtd4JWhkW7YsRODSyc4nfr2yzCdV4uLdYQv0Lfmdwipp9TfNwKNnxIfV GxZzN8GlKMcHOzbNOcJeZ9+poE6b3ETgKNvCbCswBS5A7EBWQGX5lALll7/V2nW9CFXx BK2PSY9Ul7DlcSAm6/fm9ZdHAdWdGV1fxC+3+3+HuPBeQYAMa6qES8BQocuk7AAnY8wJ nbPryOfAMG9cBz8A5OEgnXapneTy8vzTmNIBSG+gc2U7nAkd288Untf+ElWwQ4bv5EEj zmww== X-Gm-Message-State: AOAM530a4qG+vr1gUXVOH+x3Krr5Uz0QFdGEytzn5o05HPR/u8wvuZS4 WlFo3ajr1mIgXKM1c/izzausAGJbXTkmrQ== X-Google-Smtp-Source: ABdhPJzRbxOXSMsTOWPnECNeZSUn002BZR88cslQsdmIwL14lkIZZNjZyyIj8v2NrIbldzXiRstl1Q== X-Received: by 2002:a50:fd83:: with SMTP id o3mr4550675edt.90.1616503015014; Tue, 23 Mar 2021 05:36:55 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08960.dynamic.kabel-deutschland.de. [188.192.137.96]) by smtp.gmail.com with ESMTPSA id mp36sm10728532ejc.48.2021.03.23.05.36.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Mar 2021 05:36:52 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 23 Mar 2021 13:35:53 +0100 Message-Id: <20210323123554.1370260-3-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20210323123554.1370260-1-andreas.rheinhardt@gmail.com> References: <20210323123554.1370260-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 3/4] avcodec/pthread_frame: Check initializing mutexes/condition variables 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" Up until now, initializing the mutexes/condition variables wasn't checked by ff_frame_thread_init(). This commit changes this. Given that it is not documented to be save to destroy a zeroed but otherwise uninitialized mutex/condition variable, one has to choose between two approaches: Either one duplicates the code to free them in ff_frame_thread_init() in case of errors or one records which have been successfully initialized. This commit takes the latter approach: For each of the two structures with mutexes/condition variables an array containing the offsets of the members to initialize is added. Said array is used both for initializing and freeing and the only thing that needs to be recorded is how many of these have been successfully initialized. Signed-off-by: Andreas Rheinhardt --- libavcodec/pthread_frame.c | 98 ++++++++++++++++++++++++++++---------- 1 file changed, 73 insertions(+), 25 deletions(-) diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index db2f0cb3d2..1c17d8c3b6 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -78,6 +78,7 @@ typedef struct PerThreadContext { pthread_t thread; int thread_init; + unsigned pthread_init_cnt;///< Number of successfully initialized mutexes/conditions pthread_cond_t input_cond; ///< Used to wait for a new packet from the main thread. pthread_cond_t progress_cond; ///< Used by child threads to wait for progress to change. pthread_cond_t output_cond; ///< Used by the main thread to wait for frames to finish. @@ -126,6 +127,7 @@ typedef struct FrameThreadContext { PerThreadContext *threads; ///< The contexts for each thread. PerThreadContext *prev_thread; ///< The last thread submit_packet() was called on. + unsigned pthread_init_cnt; ///< Number of successfully initialized mutexes/conditions pthread_mutex_t buffer_mutex; ///< Mutex used to protect get/release_buffer(). /** * This lock is used for ensuring threads run in serial when hwaccel @@ -680,6 +682,59 @@ static void park_frame_worker_threads(FrameThreadContext *fctx, int thread_count async_lock(fctx); } +#define SENTINEL 0 // This forbids putting a mutex/condition variable at the front. +#define OFFSET_ARRAY(...) __VA_ARGS__, SENTINEL +#define DEFINE_OFFSET_ARRAY(type, name, mutexes, conds) \ +static const unsigned name ## _offsets[] = { offsetof(type, pthread_init_cnt),\ + OFFSET_ARRAY mutexes, \ + OFFSET_ARRAY conds } + +#define OFF(member) offsetof(FrameThreadContext, member) +DEFINE_OFFSET_ARRAY(FrameThreadContext, thread_ctx, + (OFF(buffer_mutex), OFF(hwaccel_mutex), OFF(async_mutex)), + (OFF(async_cond))); +#undef OFF + +#define OFF(member) offsetof(PerThreadContext, member) +DEFINE_OFFSET_ARRAY(PerThreadContext, per_thread, + (OFF(progress_mutex), OFF(mutex)), + (OFF(input_cond), OFF(progress_cond), OFF(output_cond))); +#undef OFF + +static av_cold void free_pthread(void *obj, const unsigned offsets[]) +{ + unsigned cnt = *(unsigned*)((char*)obj + offsets[0]); + const unsigned *cur_offset = offsets; + + for (; *(++cur_offset) != SENTINEL && cnt; cnt--) + pthread_mutex_destroy((pthread_mutex_t*)((char*)obj + *cur_offset)); + for (; *(++cur_offset) != SENTINEL && cnt; cnt--) + pthread_cond_destroy ((pthread_cond_t *)((char*)obj + *cur_offset)); +} + +static av_cold int init_pthread(void *obj, const unsigned offsets[]) +{ + const unsigned *cur_offset = offsets; + unsigned cnt = 0; + int err; + +#define PTHREAD_INIT_LOOP(type, max_idx) \ + for (; *(++cur_offset) != SENTINEL; cnt++) { \ + pthread_ ## type ## _t *dst = (void*)((char*)obj + *cur_offset); \ + err = pthread_ ## type ## _init(dst, NULL); \ + if (err) { \ + err = AVERROR(err); \ + goto fail; \ + } \ + } + PTHREAD_INIT_LOOP(mutex, MAX_MUTEX_IDX(offsets)) + PTHREAD_INIT_LOOP(cond, MAX_COND_IDX(offsets)) + +fail: + *(unsigned*)((char*)obj + offsets[0]) = cnt; + return err; +} + void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) { FrameThreadContext *fctx = avctx->internal->thread_ctx; @@ -739,21 +794,14 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) av_frame_free(&p->frame); - pthread_mutex_destroy(&p->mutex); - pthread_mutex_destroy(&p->progress_mutex); - pthread_cond_destroy(&p->input_cond); - pthread_cond_destroy(&p->progress_cond); - pthread_cond_destroy(&p->output_cond); + free_pthread(p, per_thread_offsets); av_packet_free(&p->avpkt); av_freep(&p->avctx); } av_freep(&fctx->threads); - pthread_mutex_destroy(&fctx->buffer_mutex); - pthread_mutex_destroy(&fctx->hwaccel_mutex); - pthread_mutex_destroy(&fctx->async_mutex); - pthread_cond_destroy(&fctx->async_cond); + free_pthread(fctx, thread_ctx_offsets); av_freep(&avctx->internal->thread_ctx); @@ -780,12 +828,6 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free, * ff_frame_thread_free in case of errors. */ (*threads_to_free)++; - pthread_mutex_init(&p->mutex, NULL); - pthread_mutex_init(&p->progress_mutex, NULL); - pthread_cond_init(&p->input_cond, NULL); - pthread_cond_init(&p->progress_cond, NULL); - pthread_cond_init(&p->output_cond, NULL); - p->parent = fctx; p->avctx = copy; @@ -810,6 +852,10 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free, } } + err = init_pthread(p, per_thread_offsets); + if (err < 0) + return err; + if (!(p->frame = av_frame_alloc()) || !(p->avpkt = av_packet_alloc())) return AVERROR(ENOMEM); @@ -846,7 +892,7 @@ int ff_frame_thread_init(AVCodecContext *avctx) const AVCodec *codec = avctx->codec; AVCodecContext *src = avctx; FrameThreadContext *fctx; - int i, err = 0; + int err, i = 0; if (!thread_count) { int nb_cpus = av_cpu_count(); @@ -866,24 +912,26 @@ int ff_frame_thread_init(AVCodecContext *avctx) if (!fctx) return AVERROR(ENOMEM); - fctx->threads = av_mallocz_array(thread_count, sizeof(PerThreadContext)); - if (!fctx->threads) { + err = init_pthread(fctx, thread_ctx_offsets); + if (err < 0) { + free_pthread(fctx, thread_ctx_offsets); av_freep(&avctx->internal->thread_ctx); - return AVERROR(ENOMEM); + return err; } - pthread_mutex_init(&fctx->buffer_mutex, NULL); - pthread_mutex_init(&fctx->hwaccel_mutex, NULL); - pthread_mutex_init(&fctx->async_mutex, NULL); - pthread_cond_init(&fctx->async_cond, NULL); - fctx->async_lock = 1; fctx->delaying = 1; if (codec->type == AVMEDIA_TYPE_VIDEO) avctx->delay = src->thread_count - 1; - for (i = 0; i < thread_count; ) { + fctx->threads = av_mallocz_array(thread_count, sizeof(PerThreadContext)); + if (!fctx->threads) { + err = AVERROR(ENOMEM); + goto error; + } + + for (; i < thread_count; ) { PerThreadContext *p = &fctx->threads[i]; int first = !i; From patchwork Tue Mar 23 12:35:54 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 26560 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 B01DC44A00F for ; Tue, 23 Mar 2021 14:37:05 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9276068AB1D; Tue, 23 Mar 2021 14:37:05 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ed1-f53.google.com (mail-ed1-f53.google.com [209.85.208.53]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 97FC068AB5E for ; Tue, 23 Mar 2021 14:36:56 +0200 (EET) Received: by mail-ed1-f53.google.com with SMTP id bf3so23281861edb.6 for ; Tue, 23 Mar 2021 05:36:56 -0700 (PDT) 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=wSbJ8C2J6x7ES34W/4xYtPFj0o0kk1rmHMNNTrRzRGQ=; b=a1+78X1SLgCGuOWM7qWiLNRiBpCK/qkUGsAjg+L/RdF/LDZ3JR6MfaGVuQ72AKntuN fg2+oeqSlarjdsh0QXrnfyIL/Te7XD9ZcleZx26+AXpcuDeI6zSaNuwsIMlNFVvWTnXw 6tpn2FHNlfXfoqoBUrPwvbyPcYEzoFKzGXUJrskqtYGE19RaOsPaTffsqEdaQYNttx9M DaVhMflzgwNBMbIcfSFC86GG35uGiPHsbdkkPFJMl89LcoLwuH67O06+Mh8B/VeN97mo Qo0yzrREkrB3EPMUFaFBLNPVKx9w9F7Z6Lr6ny2hAADCki4cltcqD9U5Sy83NSqJagry i0XA== 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=wSbJ8C2J6x7ES34W/4xYtPFj0o0kk1rmHMNNTrRzRGQ=; b=duF7s/2FIL3ahSEviR05hgpJW6pxRSN8+FaUJXmZi3b2z+vMYCpbD5db00ohgtVPUN AXJnDojgT4ODyssh0O80+/UpIhZEP9/m3RZRJk2u69IJ0C81tNhWXzb5/G0W6Z1bvxYE wcBqzrcaiTKLkbUMmeXl3EBluzPqe5weKeIFvBFcKH1McrOw3+dCDCmcJ/FbeY5fDLTT VVkp95jCk3NalFF00T0zX68hVwtOfJmkhgSyVOtQqk5I/F+JLEWtCQr3oUCNh/xYrGqq RVGlpaebZ3JQ39zUvlEesTvA4rfUye+aQhgugndgRg0stA/XqFpsK+gaskvGzAR3C/R1 ++0g== X-Gm-Message-State: AOAM530On+otv0/xx+bucJq1O5hpFpjmHOz2StLHjRT4raBjFgPwVicT 1WTJsNbENdx1HRPyZlSRMwXgrKtQMEVkuw== X-Google-Smtp-Source: ABdhPJwum6Yiw3xaHip84u+tI+VEEOyHTN4DMAR5xvAyXOXebkTMmVR/M+H3wv1vZ69fjtjLpQzb6g== X-Received: by 2002:a50:f113:: with SMTP id w19mr4312842edl.226.1616503015818; Tue, 23 Mar 2021 05:36:55 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08960.dynamic.kabel-deutschland.de. [188.192.137.96]) by smtp.gmail.com with ESMTPSA id mp36sm10728532ejc.48.2021.03.23.05.36.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Mar 2021 05:36:55 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 23 Mar 2021 13:35:54 +0100 Message-Id: <20210323123554.1370260-4-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20210323123554.1370260-1-andreas.rheinhardt@gmail.com> References: <20210323123554.1370260-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 4/4] avcodec/pthread_frame: Reindentation 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" Signed-off-by: Andreas Rheinhardt --- libavcodec/pthread_frame.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index 1c17d8c3b6..ad3eba9016 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -834,23 +834,22 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free, copy->internal = av_memdup(src->internal, sizeof(*src->internal)); if (!copy->internal) return AVERROR(ENOMEM); - copy->internal->thread_ctx = p; + copy->internal->thread_ctx = p; - copy->delay = avctx->delay; + copy->delay = avctx->delay; - if (codec->priv_data_size) { - copy->priv_data = av_mallocz(codec->priv_data_size); - if (!copy->priv_data) { - return AVERROR(ENOMEM); - } + if (codec->priv_data_size) { + copy->priv_data = av_mallocz(codec->priv_data_size); + if (!copy->priv_data) + return AVERROR(ENOMEM); - if (codec->priv_class) { - *(const AVClass **)copy->priv_data = codec->priv_class; - err = av_opt_copy(copy->priv_data, src->priv_data); - if (err < 0) - return err; - } + if (codec->priv_class) { + *(const AVClass **)copy->priv_data = codec->priv_class; + err = av_opt_copy(copy->priv_data, src->priv_data); + if (err < 0) + return err; } + } err = init_pthread(p, per_thread_offsets); if (err < 0) @@ -862,23 +861,24 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free, copy->internal->last_pkt_props = p->avpkt; if (!first) - copy->internal->is_copy = 1; + copy->internal->is_copy = 1; - if (codec->init) - err = codec->init(copy); + if (codec->init) { + err = codec->init(copy); if (err < 0) { if (codec->caps_internal & FF_CODEC_CAP_INIT_CLEANUP) p->thread_init = NEEDS_CLOSE; return err; } + } p->thread_init = NEEDS_CLOSE; if (first) - update_context_from_thread(avctx, copy, 1); + update_context_from_thread(avctx, copy, 1); - atomic_init(&p->debug_threads, (copy->debug & FF_DEBUG_THREADS) != 0); + atomic_init(&p->debug_threads, (copy->debug & FF_DEBUG_THREADS) != 0); - err = AVERROR(pthread_create(&p->thread, NULL, frame_worker_thread, p)); + err = AVERROR(pthread_create(&p->thread, NULL, frame_worker_thread, p)); if (err < 0) return err; p->thread_init = INITIALIZED;