From 389d89b3f1f8d3c0eb9b6447f239ad81766dd348 Mon Sep 17 00:00:00 2001
From: Li-Heng Chen <lihengc@netflix.com>
Date: Sat, 10 Sep 2022 22:58:40 -0700
Subject: [PATCH] libavfilter/f_select: switch to activate and properly handle
 EOF pts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch solves a potential EOF pts bug that can be triggered with other filters:
when placing select filter before fps filter, the EOF pts in `f_select` always
indicate the last input frame regardless of the frame selected. This may cause
unwanted duplication of the last-selected-frame in `vf_fps`. Switching the
filtering process from `filter_frame` to `activate` allows to properly set
EOF pts by ff_outlink_set_status.

This bug can be reproduced by the ffmpeg cmd below (bitstreams in fate-suite
can reproduce this issue):

ffmpeg -y -i /path/ffmpeg/fate-suite/h264/bbc2.sample.h264 -vf select='gte(n\,0)*gte(24\,n)',fps=25/1 out.y4m

w/o fps=25/1: outputs 26 frames
w/  fps=25/1: outputs 47 frames, where frame #25 is duplicated 22 times.

Signed-off-by: Li-Heng Chen <lihengc@netflix.com>
---
 libavfilter/f_select.c | 55 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 10 deletions(-)

diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
index 1cfe2d59e5..f83891c618 100644
--- a/libavfilter/f_select.c
+++ b/libavfilter/f_select.c
@@ -33,6 +33,7 @@
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
 #include "avfilter.h"
+#include "filters.h"
 #include "audio.h"
 #include "formats.h"
 #include "internal.h"
@@ -159,6 +160,7 @@ typedef struct SelectContext {
     double select;
     int select_out;                 ///< mark the selected output pad index
     int nb_outputs;
+    int64_t eof_pts;
 } SelectContext;
 
 #define OFFSET(x) offsetof(SelectContext, x)
@@ -215,6 +217,7 @@ static int config_input(AVFilterLink *inlink)
 
     select->bitdepth = desc->comp[0].depth;
     select->nb_planes = is_yuv ? 1 : av_pix_fmt_count_planes(inlink->format);
+    select->eof_pts = AV_NOPTS_VALUE;
 
     for (int plane = 0; plane < select->nb_planes; plane++) {
         ptrdiff_t line_size = av_image_get_linesize(inlink->format, inlink->w, plane);
@@ -336,7 +339,7 @@ static void select_frame(AVFilterContext *ctx, AVFrame *frame)
     if (isnan(select->var_values[VAR_START_T]))
         select->var_values[VAR_START_T] = TS2D(frame->pts) * av_q2d(inlink->time_base);
 
-    select->var_values[VAR_N  ] = inlink->frame_count_out;
+    select->var_values[VAR_N  ] = inlink->frame_count_out - 1;
     select->var_values[VAR_PTS] = TS2D(frame->pts);
     select->var_values[VAR_T  ] = TS2D(frame->pts) * av_q2d(inlink->time_base);
     select->var_values[VAR_POS] = frame->pkt_pos == -1 ? NAN : frame->pkt_pos;
@@ -409,17 +412,49 @@ static void select_frame(AVFilterContext *ctx, AVFrame *frame)
     select->var_values[VAR_PREV_T]   = select->var_values[VAR_T];
 }
 
-static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
+static int activate(AVFilterContext *ctx)
 {
-    AVFilterContext *ctx = inlink->dst;
+    int ret, status;
     SelectContext *select = ctx->priv;
+    AVFilterLink *inlink = ctx->inputs[0];
+    AVFilterLink *outlink = ctx->outputs[0];
+    AVFrame *in;
+    int64_t pts;
 
-    select_frame(ctx, frame);
-    if (select->select)
-        return ff_filter_frame(ctx->outputs[select->select_out], frame);
+    FF_FILTER_FORWARD_STATUS_BACK(outlink, inlink);
 
-    av_frame_free(&frame);
-    return 0;
+    ret = ff_inlink_consume_frame(inlink, &in);
+    if (ret < 0)
+        return ret;
+    if (ret > 0) {
+        select_frame(ctx, in);
+        if (select->select)
+            return ff_filter_frame(ctx->outputs[select->select_out], in);
+    }
+    av_frame_free(&in);
+
+    ret = ff_inlink_acknowledge_status(inlink, &status, &pts);
+
+    /* Two conditions that current pts could be the EOF pts:
+     * - If the current frame N is not selected while the previous
+     *   frame (N-1) is selected, frame N-1 could be the last frame,
+     *   hence we update select->eof_pts.
+     * - Last input frame: we have EOF status, and the current (last)
+     *   frame is selected */
+    if ((select->var_values[VAR_N] - select->var_values[VAR_PREV_SELECTED_N] == 1.0) ||
+        (status == AVERROR_EOF && select->select))
+        select->eof_pts = pts;
+
+    if (ret) {
+        av_log(ctx, AV_LOG_TRACE, "N:EOF PTS:%"PRId64"\n",
+               select->eof_pts);
+        ff_outlink_set_status(outlink, status, select->eof_pts);
+        return 0;
+    }
+
+    FF_FILTER_FORWARD_WANTED(outlink, inlink);
+
+    return FFERROR_NOT_READY;
 }
 
 static int request_frame(AVFilterLink *outlink)
@@ -467,7 +502,6 @@ static const AVFilterPad avfilter_af_aselect_inputs[] = {
         .name         = "default",
         .type         = AVMEDIA_TYPE_AUDIO,
         .config_props = config_input,
-        .filter_frame = filter_frame,
     },
 };
 
@@ -475,6 +509,7 @@ const AVFilter ff_af_aselect = {
     .name        = "aselect",
     .description = NULL_IF_CONFIG_SMALL("Select audio frames to pass in output."),
     .init        = aselect_init,
+    .activate    = activate,
     .uninit      = uninit,
     .priv_size   = sizeof(SelectContext),
     FILTER_INPUTS(avfilter_af_aselect_inputs),
@@ -522,7 +557,6 @@ static const AVFilterPad avfilter_vf_select_inputs[] = {
         .name         = "default",
         .type         = AVMEDIA_TYPE_VIDEO,
         .config_props = config_input,
-        .filter_frame = filter_frame,
     },
 };
 
@@ -530,6 +564,7 @@ const AVFilter ff_vf_select = {
     .name          = "select",
     .description   = NULL_IF_CONFIG_SMALL("Select video frames to pass in output."),
     .init          = select_init,
+    .activate      = activate,
     .uninit        = uninit,
     .priv_size     = sizeof(SelectContext),
     .priv_class    = &select_class,
-- 
2.32.1 (Apple Git-133)

