Unsubscribe
On Wed, Sep 14, 2022, 11:14 Nicolas George wrote:
> Li-Heng Chen (12022-09-13):
> > 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.
>
> Thanks for the patch. A few comments.
>
> First, please wrap this paragraph. Also, the patch seems to have been
> corrupted by the mail software, please look into it.
>
> Also, please explain the logic you used to set the end timestamp. It
> seems you are using some kind of weird test to use the timestamp of the
> fist not-selected frame.
>
> In fact, I have doubt about the validity of this patch at all. You will
> notice that the select filters does not alter the timestamps of the
> frames. The frames that are not selected are skipped, but they count for
> the timing. I do not think the final frames should behave differently.
>
> > 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
>
> I do not think it shows a bug. If you want to truncate the stream, the
> select filter is not the recommended choice, use trim.
>
> >
> > Signed-off-by: Li-Heng Chen
> > ---
> > libavfilter/f_select.c | 49 +-
> > 1 file changed, 39 insertions(+), 10 deletions(-)
> >
> > diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
> > index 1cfe2d59e5..e01f476b5b 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,43 @@ 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();
> > -return 0;
> > +ret = ff_inlink_consume_frame(inlink, );
> > +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();
> > +
> > +ret = ff_inlink_acknowledge_status(inlink, , );
> > +
>
> > +if (((int64_t)select->var_values[VAR_N] -
> (int64_t)select->var_values[VAR_PREV_SELECTED_N] == 1) ||
>
> Do not bring back integer values that have been converted to float. The
> real value of var_values[VAR_N] is available; if
> var_values[VAR_PREV_SELECTED_N] is necessary too introduce a field to
> keep track of its exact value.
>
> > +(status == AVERROR_EOF && select->select))
> > +
