From patchwork Tue Mar 15 10:49:32 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Khirnov X-Patchwork-Id: 34752 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6838:3486:0:0:0:0 with SMTP id ek6csp2896064nkb; Tue, 15 Mar 2022 03:49:48 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzqIPZpZ/3fwPC4qdStCv9tNkZIUpGCBNU391H0Iws7OfiBLJhLvWdvUT9IKC0U0Xl6Fewx X-Received: by 2002:a17:907:1c10:b0:6da:6316:d009 with SMTP id nc16-20020a1709071c1000b006da6316d009mr22132716ejc.621.1647341388010; Tue, 15 Mar 2022 03:49:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1647341388; cv=none; d=google.com; s=arc-20160816; b=A3H19PfoZhvrt2tcSA/9724kC1wvHA5+6Ej3gRfU0t+eIX8MSlgjkAp8jkRJDRkjXq 63QCxRvjFP6db6988l2NA1rYM4SkQQNw7zHzN5slUjFq+FqF9374ntDm94zdl+PYU1Xg e5lGdO+Ucsk96rwH4zJd39V6m3OOPc53vwyIZ25tDv5NI2QuU7WnpotzIFDV7P1qLtmU PVoJQIMohB9PcLhPr15qLslD7IacCYsbMjgFPuXm6kM8H6L9suyLqaGMnwHT9PBrXlM2 csje9FSJAvDShRnBaWrKNta+sBBG+VtntcWKuIEJecNV4hqFE4tvnUpOTQnrbLKLWHF0 xlgw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:reply-to:list-subscribe :list-help:list-post:list-archive:list-unsubscribe:list-id :precedence:subject:mime-version:references:in-reply-to:message-id :date:to:from:delivered-to; bh=KLPOIhUjzDO1KTgQhKu9E5mQDOOlb+2XBiiGNvCd4xk=; b=XGuHAkTOhzr+5X9YKhDRZsP5jXQKAOl5YPrTj8KuGFDr0vBEnCwtXc7sgmIoDlgk+d aXgc5+rHhcTZBm9NUedWNlQ9NzmhbIfetIKIrdj8mI2KT8MylN1vQtAKvbfFod1Cy1n2 h5YVd6q+peGF8Fz9Q1sUoCoFFhs/0cJNEOQZZPmU96AB5EXZwnxpK3DixyV5eeZl5sFB 6bwOEK6135HbP35xK3q9P3GkDETwXwh4P3W5AknVM5wQpIudNJ0BGGU8pCSm7b7vgFc1 ShS8WcGTzb1JZ/58hs/sFNiUPivcE/EklXlLBoKGGURy/2XEoIgZs19PmxNKCQlp09PU VrzQ== ARC-Authentication-Results: i=1; mx.google.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 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id 4-20020a508e04000000b004021db7d2b3si11511869edw.2.2022.03.15.03.49.46; Tue, 15 Mar 2022 03:49:47 -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; 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 Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1DDAF68B1A3; Tue, 15 Mar 2022 12:49:43 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E768468AF37 for ; Tue, 15 Mar 2022 12:49:36 +0200 (EET) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 1DDF224017C for ; Tue, 15 Mar 2022 11:49:36 +0100 (CET) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id 1ZeRN7T9e9RJ for ; Tue, 15 Mar 2022 11:49:35 +0100 (CET) Received: from libav.khirnov.net (libav.khirnov.net [IPv6:2a00:c500:561:201::7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "libav.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 829DA240179 for ; Tue, 15 Mar 2022 11:49:35 +0100 (CET) Received: by libav.khirnov.net (Postfix, from userid 1000) id 60F483A0189; Tue, 15 Mar 2022 11:49:35 +0100 (CET) From: Anton Khirnov To: ffmpeg-devel@ffmpeg.org Date: Tue, 15 Mar 2022 11:49:32 +0100 Message-Id: <20220315104932.25496-2-anton@khirnov.net> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220315104932.25496-1-anton@khirnov.net> References: <20220315104932.25496-1-anton@khirnov.net> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/2] lavc/vp9: fix the race in exporting video enc params X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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" X-TUID: 2Rmxiei0sRNN The parameters are currently attached directly to the decoded frame after it is decoded. This is racy, since the frame is shared with other threads, as reported e.g. by the fate-vp9-encparams test in TSAN build. Instead, write the params to a refcounted AVBuffer. Each output frame then gets its own reference to this buffer, which should be thread-safe. --- libavcodec/vp9.c | 93 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 83 insertions(+), 10 deletions(-) diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c index 7ef10f7a80..f9df898733 100644 --- a/libavcodec/vp9.c +++ b/libavcodec/vp9.c @@ -41,6 +41,10 @@ #define VP9_SYNCCODE 0x498342 +typedef struct VP9TFPriv { + AVBufferRef *enc_par; +} VP9TFPriv; + #if HAVE_THREADS DEFINE_OFFSET_ARRAY(VP9Context, vp9_context, pthread_init_cnt, (offsetof(VP9Context, progress_mutex)), @@ -100,6 +104,13 @@ static void vp9_frame_unref(AVCodecContext *avctx, VP9Frame *f) f->hwaccel_picture_private = NULL; } +static void vp9_tf_priv_free(void *opaque, uint8_t *data) +{ + VP9TFPriv *p = (VP9TFPriv*)data; + av_buffer_unref(&p->enc_par); + av_freep(&data); +} + static int vp9_frame_alloc(AVCodecContext *avctx, VP9Frame *f) { VP9Context *s = avctx->priv_data; @@ -139,6 +150,19 @@ static int vp9_frame_alloc(AVCodecContext *avctx, VP9Frame *f) } } + if (avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS) { + VP9TFPriv *p = av_mallocz(sizeof(*p)); + if (!p) + goto fail; + + f->tf.priv_buf = av_buffer_create((uint8_t*)p, sizeof(*p), vp9_tf_priv_free, + NULL, AV_BUFFER_FLAG_READONLY); + if (!f->tf.priv_buf) { + av_freep(&p); + goto fail; + } + } + return 0; fail: @@ -1497,7 +1521,9 @@ int loopfilter_proc(AVCodecContext *avctx) static int vp9_export_enc_params(VP9Context *s, VP9Frame *frame) { + VP9TFPriv *p = (VP9TFPriv*)frame->tf.priv_buf->data; AVVideoEncParams *par; + size_t par_size; unsigned int tile, nb_blocks = 0; if (s->s.h.segmentation.enabled) { @@ -1505,11 +1531,16 @@ static int vp9_export_enc_params(VP9Context *s, VP9Frame *frame) nb_blocks += s->td[tile].nb_block_structure; } - par = av_video_enc_params_create_side_data(frame->tf.f, - AV_VIDEO_ENC_PARAMS_VP9, nb_blocks); + par = av_video_enc_params_alloc(AV_VIDEO_ENC_PARAMS_VP9, nb_blocks, &par_size); if (!par) return AVERROR(ENOMEM); + p->enc_par = av_buffer_create((uint8_t *)par, par_size, NULL, NULL, 0); + if (!p->enc_par) { + av_freep(&par); + return AVERROR(ENOMEM); + } + par->qp = s->s.h.yac_qi; par->delta_qp[0][0] = s->s.h.ydc_qdelta; par->delta_qp[1][0] = s->s.h.uvdc_qdelta; @@ -1547,6 +1578,38 @@ static int vp9_export_enc_params(VP9Context *s, VP9Frame *frame) return 0; } +static int vp9_output_frame(AVFrame *dst, const ThreadFrame *src) +{ + int ret = 0; + + ret = av_frame_ref(dst, src->f); + if (ret < 0) + return ret; + + if (src->priv_buf) { + VP9TFPriv *p = (VP9TFPriv*)src->priv_buf->data; + + if (p->enc_par) { + AVBufferRef *buf = av_buffer_ref(p->enc_par); + if (!buf) { + ret = AVERROR(ENOMEM); + goto finish; + } + + if (!av_frame_new_side_data_from_buf(dst, AV_FRAME_DATA_VIDEO_ENC_PARAMS, buf)) { + av_buffer_unref(&buf); + ret = AVERROR(ENOMEM); + goto finish; + } + } + } + +finish: + if (ret < 0) + av_frame_unref(dst); + return ret; +} + static int vp9_decode_frame(AVCodecContext *avctx, void *frame, int *got_frame, AVPacket *pkt) { @@ -1561,11 +1624,14 @@ static int vp9_decode_frame(AVCodecContext *avctx, void *frame, if ((ret = decode_frame_header(avctx, data, size, &ref)) < 0) { return ret; } else if (ret == 0) { - if (!s->s.refs[ref].f->buf[0]) { + ThreadFrame *tf = &s->s.refs[ref]; + if (!tf->f->buf[0]) { av_log(avctx, AV_LOG_ERROR, "Requested reference %d not available\n", ref); return AVERROR_INVALIDDATA; } - if ((ret = av_frame_ref(frame, s->s.refs[ref].f)) < 0) + ff_thread_await_progress(tf, INT_MAX, 0); + ret = vp9_output_frame(frame, tf); + if (ret < 0) return ret; ((AVFrame *)frame)->pts = pkt->pts; ((AVFrame *)frame)->pkt_dts = pkt->dts; @@ -1743,6 +1809,17 @@ static int vp9_decode_frame(AVCodecContext *avctx, void *frame, ff_thread_finish_setup(avctx); } } while (s->pass++ == 1); + + /* this must be done before report_progress(INT_MAX), since the exported + * data may be read in other threads */ + if (avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS) { + ret = vp9_export_enc_params(s, &s->s.frames[CUR_FRAME]); + if (ret < 0) { + ff_thread_report_progress(&s->s.frames[CUR_FRAME].tf, INT_MAX, 0); + return ret; + } + } + ff_thread_report_progress(&s->s.frames[CUR_FRAME].tf, INT_MAX, 0); if (s->td->error_info < 0) { @@ -1750,11 +1827,6 @@ static int vp9_decode_frame(AVCodecContext *avctx, void *frame, s->td->error_info = 0; return AVERROR_INVALIDDATA; } - if (avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS) { - ret = vp9_export_enc_params(s, &s->s.frames[CUR_FRAME]); - if (ret < 0) - return ret; - } finish: // ref frame setup @@ -1767,7 +1839,8 @@ finish: } if (!s->s.h.invisible) { - if ((ret = av_frame_ref(frame, s->s.frames[CUR_FRAME].tf.f)) < 0) + ret = vp9_output_frame(frame, &s->s.frames[CUR_FRAME].tf); + if (ret < 0) return ret; *got_frame = 1; }