From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 7BF17423B5 for ; Tue, 15 Mar 2022 10:49:45 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CE23468B19D; Tue, 15 Mar 2022 12:49:42 +0200 (EET) 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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; } -- 2.34.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".