[FFmpeg-devel] [PATCH] fftools/ffmpeg: Avoid temporary frame
James Almer
jamrial at gmail.com
Fri Nov 19 20:04:39 EET 2021
On 11/19/2021 2:02 PM, Andreas Rheinhardt wrote:
> 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.)
In that case maybe just set flags to AV_BUFFERSRC_FLAG_PUSH instead of
zero below in send_frame_to_filters(), and pass buffersrc_flags alone to
av_buffersrc_add_frame_flags().
What i wanted to avoid is doing "AV_BUFFERSRC_FLAG_PUSH |
buffersrc_flags", which to me looks really ugly.
>
>>> 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".
>
> _______________________________________________
> 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