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.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.)

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@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".


_______________________________________________
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".

Reply via email to