[FFmpeg-devel] Unsubscribe
Sam Davis
codehounded at gmail.com
Wed Sep 14 22:02:14 EEST 2022
Unsubscribe
On Wed, Sep 14, 2022, 11:14 Nicolas George <george at nsup.org> 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 <lihengc at netflix.com>
> > ---
> > 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(&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);
> > +
>
> > + 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))
> > + select->eof_pts = pts;
> > +
>
> > + if (ret && status == AVERROR_EOF) {
>
> The status has to be forwarded even if it is other than EOF.
>
> > + 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 +496,6 @@ static const AVFilterPad
> avfilter_af_aselect_inputs[] = {
> > .name = "default",
> > .type = AVMEDIA_TYPE_AUDIO,
> > .config_props = config_input,
> > - .filter_frame = filter_frame,
> > },
> > };
> >
> > @@ -475,6 +503,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 +551,6 @@ static const AVFilterPad avfilter_vf_select_inputs[]
> = {
> > .name = "default",
> > .type = AVMEDIA_TYPE_VIDEO,
> > .config_props = config_input,
> > - .filter_frame = filter_frame,
> > },
> > };
> >
> > @@ -530,6 +558,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,
>
> Regards,
>
> --
> Nicolas George
> _______________________________________________
> 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