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 A10F043F41 for ; Thu, 18 Aug 2022 16:44:22 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 946FF68B9B0; Thu, 18 Aug 2022 19:44:19 +0300 (EEST) Received: from mail-vs1-f43.google.com (mail-vs1-f43.google.com [209.85.217.43]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B1B3368B20B for ; Thu, 18 Aug 2022 19:44:13 +0300 (EEST) Received: by mail-vs1-f43.google.com with SMTP id m66so2021155vsm.12 for ; Thu, 18 Aug 2022 09:44:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc; bh=9hqZ3rR066EvFRu184tEOzbqo2MBt1Nsbha+cshfxUU=; b=eaczWit/AwDtfXva1Bxl7j5zCJhcYhqTjR6IvC816lhzpHg/zqjDSg/a4CXenHSruw o6DkUafxRMWHWkV5tdx9XvHkIEoqs0UygwXG6DOPZDr7KdtSp38heXdfvrSJtn4zOjuE P0ZBA3fFTgO6NyCVsolSGso2YmTR8QuK6Z3Vj4s6ObRF728iOKIZ6Aukc3avWnRSPkeJ JsVltd7hvInfYeTgOp64rhEBFWQnkNuj/bSY8sb+TuuPR3Mt3XKGrHly0Si5h+JoKub0 3MMPeF26FU4nXeJ0FPH78sfrjlaoMA/gEMSadgCO5sYiELkPfD0e2W04OKpR5tn3bQaZ SlJA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc; bh=9hqZ3rR066EvFRu184tEOzbqo2MBt1Nsbha+cshfxUU=; b=ul6QjBsMO5CPE5Y9Vgs9jtuJB6VCqc1hFSXBhp5YtLHvslHHswHBBSt8RnmvE5qXvW z/XDY+Te72kstc1/zsiIkAEu5tDcBTD1co+uQ83dympD87S2ro9G4X5cI6NtYRj/8pUt 5jVEVJSnaFCyP7iXGZ3PDWaH2ijJQTZxaZzyP5kO5JAag6XitxwbXssRgYDqZvRMP1Sk 9Vtyf8fAROh/pTEGCdkVvy884zgKQYaPmzn6t7DKYhCc8FNcUwEfgQqNmzPqrYBvKeNo XM6CmVr1pYKRN74gzJBh/Yby29SdLjPr9gA9D8LmU9t5Y3f0VbrMI+VOucex07Zz9QEu 9UcQ== X-Gm-Message-State: ACgBeo17gBMuDqLswUdfbnr5zLalwTQ3i4R2Y/S9ik4IuJ/fjBcrOBX6 20nMQpqV/RQwtmAdmMqsGnf+hw594xw= X-Google-Smtp-Source: AA6agR5wD3ZCTerJmmAZH/D0V+MN1Ux6OXjzVMKhjdl91td1zt8r6lhyLD3M06w/NYTqeP8NmvxLVA== X-Received: by 2002:a67:e0d2:0:b0:38f:9db4:4f53 with SMTP id m18-20020a67e0d2000000b0038f9db44f53mr933306vsl.49.1660841051325; Thu, 18 Aug 2022 09:44:11 -0700 (PDT) Received: from [192.168.0.13] ([191.97.187.183]) by smtp.gmail.com with ESMTPSA id v12-20020a67ff8c000000b0038712af0dbesm1048551vsq.22.2022.08.18.09.44.09 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 18 Aug 2022 09:44:10 -0700 (PDT) Message-ID: <3124274f-90c3-1ddf-9b4c-bf07570e245c@gmail.com> Date: Thu, 18 Aug 2022 13:44:10 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220818134605.12583-1-anton@khirnov.net> <20220818134605.12583-2-anton@khirnov.net> From: James Almer In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: stop using av_stream_get_parser() 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 8/18/2022 10:58 AM, Andreas Rheinhardt wrote: > Anton Khirnov: >> The parser is internal to the demuxer, so its state at any particular >> point is not well-defined for the caller. Additionally, it is being >> accessed from the main thread, while demuxing runs in a separate thread. >> >> Use a separate parser owned by ffmpeg.c to retrieve the same >> information. >> >> Fixes races, e.g. in: >> - fate-h264-brokensps-2580 >> - fate-h264-extradata-reload >> - fate-iv8-demux >> - fate-m4v-cfr >> - fate-m4v >> --- >> fftools/ffmpeg.c | 33 +++++++++++++++++++++++++++++++-- >> fftools/ffmpeg.h | 9 +++++++++ >> fftools/ffmpeg_opt.c | 9 +++++++++ >> 3 files changed, 49 insertions(+), 2 deletions(-) >> >> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >> index ef7177fc33..580df0443a 100644 >> --- a/fftools/ffmpeg.c >> +++ b/fftools/ffmpeg.c >> @@ -610,6 +610,9 @@ static void ffmpeg_cleanup(int ret) >> avcodec_free_context(&ist->dec_ctx); >> avcodec_parameters_free(&ist->par); >> >> + avcodec_free_context(&ist->parser_dec); >> + av_parser_close(ist->parser); >> + >> av_freep(&input_streams[i]); >> } >> >> @@ -2421,6 +2424,15 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo >> ret = av_packet_ref(avpkt, pkt); >> if (ret < 0) >> return ret; >> + >> + if (ist->parser) { >> + // we do not use the parsed output, only the >> + // filled codec context fields >> + uint8_t *dummy; >> + int dummy_size; >> + av_parser_parse2(ist->parser, ist->parser_dec, &dummy, &dummy_size, >> + pkt->data, pkt->size, pkt->pts, pkt->dts, pkt->pos); >> + } >> } >> >> if (pkt && pkt->dts != AV_NOPTS_VALUE) { >> @@ -2452,7 +2464,8 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo >> if (pkt && pkt->duration) { >> duration_dts = av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q); >> } else if(ist->dec_ctx->framerate.num != 0 && ist->dec_ctx->framerate.den != 0) { >> - int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict+1 : ist->dec_ctx->ticks_per_frame; >> + int ticks = ist->parser ? ist->parser->repeat_pict + 1 : >> + ist->dec_ctx->ticks_per_frame; >> duration_dts = ((int64_t)AV_TIME_BASE * >> ist->dec_ctx->framerate.den * ticks) / >> ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame; >> @@ -2555,7 +2568,8 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo >> } else if (pkt->duration) { >> ist->next_dts += av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q); >> } else if(ist->dec_ctx->framerate.num != 0) { >> - int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict + 1 : ist->dec_ctx->ticks_per_frame; >> + int ticks = ist->parser ? ist->parser->repeat_pict + 1 : >> + ist->dec_ctx->ticks_per_frame; >> ist->next_dts += ((int64_t)AV_TIME_BASE * >> ist->dec_ctx->framerate.den * ticks) / >> ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame; >> @@ -2688,6 +2702,21 @@ static int init_input_stream(int ist_index, char *error, int error_len) >> assert_avoptions(ist->decoder_opts); >> } >> >> + if (ist->parser) { >> + AVCodecParameters *par_tmp; >> + >> + par_tmp = avcodec_parameters_alloc(); >> + if (!par_tmp) >> + return AVERROR(ENOMEM); >> + >> + ret = avcodec_parameters_from_context(par_tmp, ist->dec_ctx); >> + if (ret >= 0) >> + ret = avcodec_parameters_to_context(ist->parser_dec, par_tmp); >> + avcodec_parameters_free(&par_tmp); >> + if (ret < 0) >> + return ret; >> + } >> + >> ist->next_pts = AV_NOPTS_VALUE; >> ist->next_dts = AV_NOPTS_VALUE; >> >> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h >> index 44cc23fa84..f67a2f1d1d 100644 >> --- a/fftools/ffmpeg.h >> +++ b/fftools/ffmpeg.h >> @@ -334,6 +334,15 @@ typedef struct InputStream { >> AVFrame *decoded_frame; >> AVPacket *pkt; >> >> + /** >> + * Parser for timestamp processing. >> + */ >> + AVCodecParserContext *parser; >> + /** >> + * Codec context needed by parsing. >> + */ >> + AVCodecContext *parser_dec; >> + >> int64_t prev_pkt_pts; >> int64_t start; /* time when read started */ >> /* predicted dts of the next packet read for this stream or (when there are >> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c >> index 30ca5cd609..a505c7b26f 100644 >> --- a/fftools/ffmpeg_opt.c >> +++ b/fftools/ffmpeg_opt.c >> @@ -1065,6 +1065,15 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic) >> ist->top_field_first = -1; >> MATCH_PER_STREAM_OPT(top_field_first, i, ist->top_field_first, ic, st); >> >> + ist->parser = av_parser_init(ist->dec->id); >> + if (ist->parser) { >> + ist->parser->flags |= PARSER_FLAG_COMPLETE_FRAMES; >> + >> + ist->parser_dec = avcodec_alloc_context3(NULL); >> + if (!ist->parser_dec) >> + exit_program(1); >> + } >> + >> break; >> case AVMEDIA_TYPE_AUDIO: >> ist->guess_layout_max = INT_MAX; > > Are you aware that some parsers (e.g. the HEVC one) are still slow even > with the PARSER_FLAG_COMPLETE_FRAMES flag and consume non-negligible > amounts of memory? Is it because it uses ff_h2645_packet_split()? It used to do like h264_parser and feature a custom NAL splitting implementation in order to not bother unscaping entire slice NALs (For raw annexb) since it only cares about their headers. Maybe adding a max_read_size parameter to it so it stops parsing NALs after that point would help. It would also allow us to use it in h264_parser, simplifying it considerably. > > - Andreas > _______________________________________________ > 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". _______________________________________________ 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".