From patchwork Sat Jul 28 16:25:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 9835 Delivered-To: ffmpegpatchwork@gmail.com Received: by 2002:a02:104:0:0:0:0:0 with SMTP id c4-v6csp2074050jad; Sat, 28 Jul 2018 09:25:51 -0700 (PDT) X-Google-Smtp-Source: AAOMgpdjERQ3/PqGlqnH4YFjweyMoLCcI9vPJXtp9xQp/0OoJRsL3Y1uiuSqHp2xJCz1WOFUCXYZ X-Received: by 2002:adf:e3c5:: with SMTP id k5-v6mr10171248wrm.94.1532795151590; Sat, 28 Jul 2018 09:25:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1532795151; cv=none; d=google.com; s=arc-20160816; b=PBWKXG/XFpqULRbzzAwUrr5jv0BCNgsAhJvQSxPBzaNVsqbw2SVwXOQbSZojfoxey4 H3vgE3kqCO54INhG1EtG0Owajd+igcbkk5UOmvK8I6ccJPRiYr310uRWDsR+D/URwfCT V6jSvzTIW9Q0gLpPOl8BgDqNH9n1Mgc6nGl+D73s0Wto/UuQm8MAQHSBgw5BjP+SYDgq rrxEcnSBvL00frz5wzgp5P+FbrvHONMARW3vOI3f9a3Q+8TqCm0cKh2ULN/0gEbuTdFP 0cjViF6K81wthVcJKXE/vWKxnxHo0tadxnc6tDFP7vxYD3Tjirg/z9Ds3h+IpQRb9+Ex j+Vg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject :content-language:in-reply-to:mime-version:user-agent:date :message-id:openpgp:from:references:to:dkim-signature:delivered-to :arc-authentication-results; bh=AwNw9VkaEFHsmuGjwLywu6z06mOARv2vaO7a0ONO9H0=; b=GfGfk8tv2NIWkyE9gVHqdauQOM0e85j5fjqL+1yOXpF82GRA76Tprt/LUDZcRKytD5 q6CkatEKZwiumYP9q7hAHTahWoJ5sKf4nb/CfHE+NCSLEjNJn9NIOGwSwHQMX/LwGYPP u1TMoYiU/CuT168Nex+BsmoVj0a33BuAY15X0L1o9Lw2qwhJv0ZwGlXIx5W1fYj3A/oF fjGtnWr5ieS7Oph7uSYTPT2Y0Su5/oXsMZqUn99mNQkwINokya+ySUD8/X5PoY2lFvZS njAWDzD1V9upImEBGr3RkqXNaqoVVw7OUHVws6Tes1Ofr2VCMj/6IVE7K9lAphPcMRVm wcVg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=F45d0gCt; 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=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id o80-v6si6469458wme.154.2018.07.28.09.25.50; Sat, 28 Jul 2018 09:25:51 -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; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=F45d0gCt; 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=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id AB59C689E18; Sat, 28 Jul 2018 19:25:33 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qt0-f196.google.com (mail-qt0-f196.google.com [209.85.216.196]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5BF93689CBD for ; Sat, 28 Jul 2018 19:25:27 +0300 (EEST) Received: by mail-qt0-f196.google.com with SMTP id c15-v6so8177760qtp.0 for ; Sat, 28 Jul 2018 09:25:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:from:openpgp:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=lDa5pXY5u6oMU7xqBbfmSC9oDiTY/1k4f3oVqmfovPQ=; b=F45d0gCtJNVtE4p9Fk1WvDUHSXz3idyTOVOKExI0jtbVZHGgplsSPxWIOHzmS0Ixdp NXgpvgIih9blkc1uHIEHhs67bRXGF8+K0mPpIiRZHgUU29jiJOTgmBDFfn/bgR+tcdYa v/SwLbrI+Es/Cgf2fJ2+fN2eZgcNg+SJ3RtOWMHAc+1PABNZPFDAGXT88y5+hx3+cWdp 35pKJbSFPhx87craqz5jYlyiGRSmc2bEKrJs+ccfhwIdLDZ/dMp6VEYSpug2WwVDaCgG 1hSvk6Rd4rvljvuCuTkKBUicvk6zpzUS07nJxq80VDf3TX5eh10//e+e/tNOmt8yqZ48 bI1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language; bh=lDa5pXY5u6oMU7xqBbfmSC9oDiTY/1k4f3oVqmfovPQ=; b=fAkQgUtnSZpXuky6CAICXcEp2jksksmUK0X7TYYmWuIS5Ee+PcHRQyVmpZI13il8tN HG6ZDptTVopFycWyMsmtOaQW16Wx1uzwPA73bavhMoOMubU+aaoXaZRl/fSNHICSe7Yu RLZHwz3REV+lH2Ye/EewWgw+1vcoFyRRG2pNzJZE/tqyvDGd55pcTL0XZVn9vArblziU 6BSKl9fpaDBNAx/fP8uke1XEPPD3yBQWW0pgo0eVHBtS0R3dJHdY7aHPbtGR9PEI3WMm kffvukLtPzr6mN1gRA6dYZU2S81q2YzYklS3VdSmD7YExWH/HoL5fKOCt2IEbejBrtD+ ARgA== X-Gm-Message-State: AOUpUlEkzeVic5lmQd4aIzcmHfh6oovoQ7BR9YFMRDlDIa1hUWYCHbSQ KqN6Y2HtrwxLbYPUIOtXQukRcmA6 X-Received: by 2002:ac8:3159:: with SMTP id h25-v6mr10630663qtb.305.1532795141759; Sat, 28 Jul 2018 09:25:41 -0700 (PDT) Received: from [192.168.0.4] ([190.188.171.140]) by smtp.gmail.com with ESMTPSA id a66-v6sm4072582qkf.96.2018.07.28.09.25.40 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 28 Jul 2018 09:25:41 -0700 (PDT) To: ffmpeg-devel@ffmpeg.org References: <20180727145749.9436-1-jamrial@gmail.com> <20180727145749.9436-8-jamrial@gmail.com> <20180728015811.GN19650@michaelspb> <8d800ccb-9b98-94c0-348b-b54ac5311cbb@gmail.com> <20180728070907.GO19650@michaelspb> From: James Almer Openpgp: preference=signencrypt Message-ID: Date: Sat, 28 Jul 2018 13:25:36 -0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180728070907.GO19650@michaelspb> Content-Language: en-US Subject: Re: [FFmpeg-devel] [PATCH 8/8] avcodec/decode: copy the output parameters from the last bsf in the chain back to the AVCodecContext 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On 7/28/2018 4:09 AM, Michael Niedermayer wrote: > On Fri, Jul 27, 2018 at 11:11:47PM -0300, James Almer wrote: >> On 7/27/2018 10:58 PM, Michael Niedermayer wrote: >>> On Fri, Jul 27, 2018 at 11:57:49AM -0300, James Almer wrote: >>>> Certain AVCodecParameters, like the contents of the extradata, may be changed >>>> by the init() function of any of the bitstream filters in the chain. >>>> >>>> Signed-off-by: James Almer >>>> --- >>>> Now it's not going to be called after the codec has been opened. >>>> >>>> libavcodec/decode.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>> >>> This breaks: >>> ffmpeg -i https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png -bitexact -pix_fmt rgba -f framecrc - >> >> Is any other decoder failing the same way? Because the apng decoder >> threading code may be faulty otherwise. Plenty of avctx fields are >> changed after ff_thread_init() is called within avcodec_open2(). There >> should not be a race at this point. > > I found a failure with mpeg4 (with bframes) decoding + pcm_alaw from mkv but it > does not want to reproduce. The slightest change i do makes this not happen > even just duplicating a command line parameter (which should have no effect) > simply adding the -threads parameter to it independant of value makes it go away > too > > > in the png case > this hits teh issue: > -threads 2 -i https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png -f framecrc - > > this does not: > -threads 1 -i https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png -f framecrc - > > also odly the bitexact flag made a differnce in how it fails outside valgrind > last i tried. (doesnt make a difference in valgrind it seems) A solution may be moving the ff_decode_bsfs_init call in patch 7/8 right above the call to ff_thread_init (See attachment), hopefully preventing this race once this patch is applied afterwards, but it will result in the bsfs initialized before the decoder, and some of the avctx fields that are changed later in avcodec_open2 like channels and bit_rate not being reflected during said bsfs initialization. I can't say if the former is correct or ideal, but for now the latter would not be an issue. I don't know what may happen if we were to autoinsert a filter that does care about such fields in the future, though. If the above change doesn't solve it, or is not ideal, then this patch 8/8 can be dropped or postponed, and the rest of the set pushed without it. From f0e852857b3d218ca5e483ac47df8cb58ff2a362 Mon Sep 17 00:00:00 2001 From: James Almer Date: Thu, 26 Jul 2018 20:42:27 -0300 Subject: [PATCH 7/8 v2] avcodec/decode: flush the internal bsfs instead of constantly reinitalizing them Initialize the bsfs once when opening the codec and uninitialize them once when closing it, instead of at every codec flush/seek. Signed-off-by: James Almer --- libavcodec/decode.c | 20 ++++++++++---------- libavcodec/decode.h | 2 ++ libavcodec/utils.c | 7 +++++++ 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index db364ca700..2e82f6b506 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -182,7 +182,7 @@ static int unrefcount_frame(AVCodecInternal *avci, AVFrame *frame) return 0; } -static int bsfs_init(AVCodecContext *avctx) +int ff_decode_bsfs_init(AVCodecContext *avctx) { AVCodecInternal *avci = avctx->internal; DecodeFilterContext *s = &avci->filter; @@ -688,10 +688,6 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke if (avpkt && !avpkt->size && avpkt->data) return AVERROR(EINVAL); - ret = bsfs_init(avctx); - if (ret < 0) - return ret; - av_packet_unref(avci->buffer_pkt); if (avpkt && (avpkt->data || avpkt->side_data_elems)) { ret = av_packet_ref(avci->buffer_pkt, avpkt); @@ -751,10 +747,6 @@ int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr if (!avcodec_is_open(avctx) || !av_codec_is_decoder(avctx->codec)) return AVERROR(EINVAL); - ret = bsfs_init(avctx); - if (ret < 0) - return ret; - if (avci->buffer_frame->buf[0]) { av_frame_move_ref(frame, avci->buffer_frame); } else { @@ -1978,6 +1970,14 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame *frame) return ret; } +static void bsfs_flush(AVCodecContext *avctx) +{ + DecodeFilterContext *s = &avctx->internal->filter; + + for (int i = 0; i < s->nb_bsfs; i++) + av_bsf_flush(s->bsfs[i]); +} + void avcodec_flush_buffers(AVCodecContext *avctx) { avctx->internal->draining = 0; @@ -1998,7 +1998,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx) avctx->pts_correction_last_pts = avctx->pts_correction_last_dts = INT64_MIN; - ff_decode_bsfs_uninit(avctx); + bsfs_flush(avctx); if (!avctx->refcounted_frames) av_frame_unref(avctx->internal->to_free); diff --git a/libavcodec/decode.h b/libavcodec/decode.h index 15271c529a..c3e0e82f4c 100644 --- a/libavcodec/decode.h +++ b/libavcodec/decode.h @@ -64,6 +64,8 @@ typedef struct FrameDecodeData { */ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt); +int ff_decode_bsfs_init(AVCodecContext *avctx); + void ff_decode_bsfs_uninit(AVCodecContext *avctx); /** diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 4f9a2b76ef..285bfdbc63 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -727,6 +727,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code goto free_and_end; } + if (av_codec_is_decoder(avctx->codec)) { + ret = ff_decode_bsfs_init(avctx); + if (ret < 0) + goto free_and_end; + } + if (HAVE_THREADS && !(avctx->internal->frame_thread_encoder && (avctx->active_thread_type&FF_THREAD_FRAME))) { ret = ff_thread_init(avctx); @@ -1032,6 +1038,7 @@ FF_ENABLE_DEPRECATION_WARNINGS av_packet_free(&avctx->internal->last_pkt_props); av_packet_free(&avctx->internal->ds.in_pkt); + ff_decode_bsfs_uninit(avctx); av_freep(&avctx->internal->pool); }