From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: stop using av_stream_get_parser()
Date: Thu, 18 Aug 2022 15:58:37 +0200
Message-ID: <DB6PR0101MB2214CF976284082BF0BBB96D8F6D9@DB6PR0101MB2214.eurprd01.prod.exchangelabs.com> (raw)
In-Reply-To: <20220818134605.12583-2-anton@khirnov.net>
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?
- 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".
next prev parent reply other threads:[~2022-08-18 13:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-18 13:46 [FFmpeg-devel] [PATCH 1/4] lavf: deprecate av_stream_get_end_pts() Anton Khirnov
2022-08-18 13:46 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: stop using av_stream_get_parser() Anton Khirnov
2022-08-18 13:58 ` Andreas Rheinhardt [this message]
2022-08-18 16:44 ` James Almer
2022-08-18 17:08 ` Andreas Rheinhardt
2022-08-18 14:14 ` Anton Khirnov
2022-08-18 14:34 ` Andreas Rheinhardt
2022-08-18 15:19 ` Michael Niedermayer
2022-08-18 13:46 ` [FFmpeg-devel] [PATCH 3/4] lavf: deprecate av_stream_get_parser() Anton Khirnov
2022-08-18 13:46 ` [FFmpeg-devel] [PATCH 4/4] fftools/ffmpeg: call av_guess_frame_rate() when opening the file Anton Khirnov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DB6PR0101MB2214CF976284082BF0BBB96D8F6D9@DB6PR0101MB2214.eurprd01.prod.exchangelabs.com \
--to=andreas.rheinhardt@outlook.com \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git