James Almer: > On 11/19/2021 1:43 PM, Andreas Rheinhardt wrote: >> send_frame_to_filters() sends a frame to all the filters that >> need said frame; for every filter except the last one this involves >> creating a reference to the frame, because >> av_buffersrc_add_frame_flags() by default takes ownership of >> the supplied references. Yet said function has a flag which >> changes its behaviour to create a reference itself. >> This commit uses this flag and stops creating the references itself; >> this allows to remove the spare AVFrame holding the temporary >> references; it also avoids unreferencing said frame. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com> >> --- >> fftools/ffmpeg.c | 22 +++++++--------------- >> fftools/ffmpeg.h | 1 - >> fftools/ffmpeg_opt.c | 4 ---- >> 3 files changed, 7 insertions(+), 20 deletions(-) >> >> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >> index e88ca554ae..bdbbc23cfd 100644 >> --- a/fftools/ffmpeg.c >> +++ b/fftools/ffmpeg.c >> @@ -633,7 +633,6 @@ static void ffmpeg_cleanup(int ret) >> InputStream *ist = input_streams[i]; >> av_frame_free(&ist->decoded_frame); >> - av_frame_free(&ist->filter_frame); >> av_packet_free(&ist->pkt); >> av_dict_free(&ist->decoder_opts); >> avsubtitle_free(&ist->prev_sub.subtitle); >> @@ -2171,7 +2170,7 @@ static int >> ifilter_has_all_input_formats(FilterGraph *fg) >> return 1; >> } >> -static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame) >> +static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame, >> int buffersrc_flags) >> { >> FilterGraph *fg = ifilter->graph; >> AVFrameSideData *sd; >> @@ -2217,7 +2216,6 @@ static int ifilter_send_frame(InputFilter >> *ifilter, AVFrame *frame) >> AVFrame *tmp = av_frame_clone(frame); >> if (!tmp) >> return AVERROR(ENOMEM); >> - av_frame_unref(frame); >> if (!av_fifo_space(ifilter->frame_queue)) { >> ret = av_fifo_realloc2(ifilter->frame_queue, 2 * >> av_fifo_size(ifilter->frame_queue)); >> @@ -2243,7 +2241,8 @@ static int ifilter_send_frame(InputFilter >> *ifilter, AVFrame *frame) >> } >> } >> - ret = av_buffersrc_add_frame_flags(ifilter->filter, frame, >> AV_BUFFERSRC_FLAG_PUSH); >> + ret = av_buffersrc_add_frame_flags(ifilter->filter, frame, >> + AV_BUFFERSRC_FLAG_PUSH | >> buffersrc_flags); > > nit: It may look nicer if you instead make ifilter_send_frame() like > > static int ifilter_send_frame(InputStream *ist, AVFrame *frame, int idx) > { > InputFilter *ifilter = ist->filters[idx]; > int flags = AV_BUFFERSRC_FLAG_PUSH; > [...] > if (idx < ist->nb_filters - 1) > flags |= AV_BUFFERSRC_FLAG_KEEP_REF; > ret = av_buffersrc_add_frame_flags(ifilter->filter, frame, flags); > [...] > } > > Which will simplify the code in send_frame_to_filters() below with a simple > > ret = ifilter_send_frame(ist, decoded_frame, i); >
I do not really like the thought of ifilter_send_frame() knowing the idx of the filter it is processing. If you don't like send_frame_to_filters() setting the buffersrc flags itself, one could use static int ifilter_send_frame(InputStream *ist, AVFrame *frame, int const_frame) with the semantics being that if const_frame is set, the frame is to be treated as const, otherwise ifilter_send_frame() may use the supplied references. The call would then be ifilter_send_frame(ist, decoded_frame, i != ist->nb_filters - 1). (Honestly, the reason I didn't do it in this way is because I didn't like the const_frame name; but I couldn't come up with something better.) >> if (ret < 0) { >> if (ret != AVERROR_EOF) >> av_log(NULL, AV_LOG_ERROR, "Error while filtering: >> %s\n", av_err2str(ret)); >> @@ -2306,18 +2305,13 @@ static int decode(AVCodecContext *avctx, >> AVFrame *frame, int *got_frame, AVPacke >> static int send_frame_to_filters(InputStream *ist, AVFrame >> *decoded_frame) >> { >> int i, ret; >> - AVFrame *f; >> av_assert1(ist->nb_filters > 0); /* ensure ret is initialized */ >> for (i = 0; i < ist->nb_filters; i++) { >> - if (i < ist->nb_filters - 1) { >> - f = ist->filter_frame; >> - ret = av_frame_ref(f, decoded_frame); >> - if (ret < 0) >> - break; >> - } else >> - f = decoded_frame; >> - ret = ifilter_send_frame(ist->filters[i], f); >> + int flags = 0; >> + if (i < ist->nb_filters - 1) >> + flags = AV_BUFFERSRC_FLAG_KEEP_REF; >> + ret = ifilter_send_frame(ist->filters[i], decoded_frame, flags); >> if (ret == AVERROR_EOF) >> ret = 0; /* ignore */ >> if (ret < 0) { >> @@ -2385,7 +2379,6 @@ static int decode_audio(InputStream *ist, >> AVPacket *pkt, int *got_output, >> ist->nb_samples = decoded_frame->nb_samples; >> err = send_frame_to_filters(ist, decoded_frame); >> - av_frame_unref(ist->filter_frame); >> av_frame_unref(decoded_frame); >> return err < 0 ? err : ret; >> } >> @@ -2511,7 +2504,6 @@ static int decode_video(InputStream *ist, >> AVPacket *pkt, int *got_output, int64_ >> err = send_frame_to_filters(ist, decoded_frame); >> fail: >> - av_frame_unref(ist->filter_frame); >> av_frame_unref(decoded_frame); >> return err < 0 ? err : ret; >> } >> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h >> index 545ff1c8e7..ce790c4d6f 100644 >> --- a/fftools/ffmpeg.h >> +++ b/fftools/ffmpeg.h >> @@ -311,7 +311,6 @@ typedef struct InputStream { >> AVCodecContext *dec_ctx; >> const AVCodec *dec; >> AVFrame *decoded_frame; >> - AVFrame *filter_frame; /* a ref of decoded_frame, to be sent to >> filters */ >> AVPacket *pkt; >> int64_t prev_pkt_pts; >> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c >> index 98bd3b47b6..6732a29625 100644 >> --- a/fftools/ffmpeg_opt.c >> +++ b/fftools/ffmpeg_opt.c >> @@ -893,10 +893,6 @@ static void add_input_streams(OptionsContext *o, >> AVFormatContext *ic) >> if (!ist->decoded_frame) >> exit_program(1); >> - ist->filter_frame = av_frame_alloc(); >> - if (!ist->filter_frame) >> - exit_program(1); >> - >> ist->pkt = av_packet_alloc(); >> if (!ist->pkt) >> exit_program(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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".