[FFmpeg-devel] [PATCH] fftools/ffmpeg: Avoid temporary frame
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Fri Nov 19 19:02:59 EET 2021
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.rheinhardt at 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list