From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 40D0844524 for ; Wed, 14 Sep 2022 15:14:46 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 84EB668BB2D; Wed, 14 Sep 2022 18:14:44 +0300 (EEST) Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B6C3C68B972 for ; Wed, 14 Sep 2022 18:14:37 +0300 (EEST) X-ENS-nef-client: 129.199.129.80 ( name = phare.normalesup.org ) Received: from phare.normalesup.org (phare.normalesup.org [129.199.129.80]) by nef.ens.fr (8.14.4/1.01.28121999) with ESMTP id 28EFEaXs011365 for ; Wed, 14 Sep 2022 17:14:37 +0200 Received: by phare.normalesup.org (Postfix, from userid 1001) id C7215EB5B9; Wed, 14 Sep 2022 17:14:36 +0200 (CEST) Date: Wed, 14 Sep 2022 17:14:36 +0200 From: Nicolas George To: FFmpeg development discussions and patches Message-ID: References: MIME-Version: 1.0 In-Reply-To: X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (nef.ens.fr [129.199.96.32]); Wed, 14 Sep 2022 17:14:37 +0200 (CEST) Subject: Re: [FFmpeg-devel] [PATCH] libavfilter/f_select: switch to activate and properly handle EOF pts X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: multipart/mixed; boundary="===============7217580361747604508==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============7217580361747604508== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="RxRb1yoAvAGQWTw4" Content-Disposition: inline --RxRb1yoAvAGQWTw4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Li-Heng Chen (12022-09-13): > This patch solves a potential EOF pts bug that can be triggered with othe= r filters: when placing select filter before fps filter, the EOF pts in `f_= select` always indicate the last input frame regardless of the frame select= ed. This may cause unwanted duplication of the last-selected-frame in `vf_f= ps`. Switching the filtering process from `filter_frame` to `activate` allo= ws 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): >=20 > ffmpeg -y -i /path/ffmpeg/fate-suite/h264/bbc2.sample.h264 -vf select=3D'= gte(n\,0)*gte(24\,n)',fps=3D25/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. >=20 > Signed-off-by: Li-Heng Chen > --- > libavfilter/f_select.c | 49 +++++++++++++++++++++++++++++++++--------- > 1 file changed, 39 insertions(+), 10 deletions(-) >=20 > 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 ind= ex > int nb_outputs; > + int64_t eof_pts; > } SelectContext; >=20 > #define OFFSET(x) offsetof(SelectContext, x) > @@ -215,6 +217,7 @@ static int config_input(AVFilterLink *inlink) >=20 > select->bitdepth =3D desc->comp[0].depth; > select->nb_planes =3D is_yuv ? 1 : av_pix_fmt_count_planes(inlink->fo= rmat); > + select->eof_pts =3D AV_NOPTS_VALUE; >=20 > for (int plane =3D 0; plane < select->nb_planes; plane++) { > ptrdiff_t line_size =3D av_image_get_linesize(inlink->format, inl= ink->w, plane); > @@ -336,7 +339,7 @@ static void select_frame(AVFilterContext *ctx, AVFram= e *frame) > if (isnan(select->var_values[VAR_START_T])) > select->var_values[VAR_START_T] =3D TS2D(frame->pts) * av_q2d(inl= ink->time_base); >=20 > - select->var_values[VAR_N ] =3D inlink->frame_count_out; > + select->var_values[VAR_N ] =3D inlink->frame_count_out - 1; > select->var_values[VAR_PTS] =3D TS2D(frame->pts); > select->var_values[VAR_T ] =3D TS2D(frame->pts) * av_q2d(inlink->tim= e_base); > select->var_values[VAR_POS] =3D frame->pkt_pos =3D=3D -1 ? NAN : fram= e->pkt_pos; > @@ -409,17 +412,43 @@ static void select_frame(AVFilterContext *ctx, AVFr= ame *frame) > select->var_values[VAR_PREV_T] =3D select->var_values[VAR_T]; > } >=20 > -static int filter_frame(AVFilterLink *inlink, AVFrame *frame) > +static int activate(AVFilterContext *ctx) > { > - AVFilterContext *ctx =3D inlink->dst; > + int ret, status; > SelectContext *select =3D ctx->priv; > + AVFilterLink *inlink =3D ctx->inputs[0]; > + AVFilterLink *outlink =3D ctx->outputs[0]; > + AVFrame *in; > + int64_t pts; >=20 > - select_frame(ctx, frame); > - if (select->select) > - return ff_filter_frame(ctx->outputs[select->select_out], frame); > + FF_FILTER_FORWARD_STATUS_BACK(outlink, inlink); >=20 > - av_frame_free(&frame); > - return 0; > + ret =3D 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 =3D ff_inlink_acknowledge_status(inlink, &status, &pts); > + > + if (((int64_t)select->var_values[VAR_N] - (int64_t)select->var_value= s[VAR_PREV_SELECTED_N] =3D=3D 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 =3D=3D AVERROR_EOF && select->select)) > + select->eof_pts =3D pts; > + > + if (ret && status =3D=3D 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; > } >=20 > static int request_frame(AVFilterLink *outlink) > @@ -467,7 +496,6 @@ static const AVFilterPad avfilter_af_aselect_inputs[]= =3D { > .name =3D "default", > .type =3D AVMEDIA_TYPE_AUDIO, > .config_props =3D config_input, > - .filter_frame =3D filter_frame, > }, > }; >=20 > @@ -475,6 +503,7 @@ const AVFilter ff_af_aselect =3D { > .name =3D "aselect", > .description =3D NULL_IF_CONFIG_SMALL("Select audio frames to pass in= output."), > .init =3D aselect_init, > + .activate =3D activate, > .uninit =3D uninit, > .priv_size =3D sizeof(SelectContext), > FILTER_INPUTS(avfilter_af_aselect_inputs), > @@ -522,7 +551,6 @@ static const AVFilterPad avfilter_vf_select_inputs[] = =3D { > .name =3D "default", > .type =3D AVMEDIA_TYPE_VIDEO, > .config_props =3D config_input, > - .filter_frame =3D filter_frame, > }, > }; >=20 > @@ -530,6 +558,7 @@ const AVFilter ff_vf_select =3D { > .name =3D "select", > .description =3D NULL_IF_CONFIG_SMALL("Select video frames to pass = in output."), > .init =3D select_init, > + .activate =3D activate, > .uninit =3D uninit, > .priv_size =3D sizeof(SelectContext), > .priv_class =3D &select_class, Regards, --=20 Nicolas George --RxRb1yoAvAGQWTw4 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE6ooRQGBoNzw0KnwPcZVLI8pNxgwFAmMh79oACgkQcZVLI8pN xgy9zRAAvAMlI4oCbStilMcQhJ+jjhHgFPLvNB48JcnqqN96RH3UMBlECckUlOu3 AJhkKz/a4Qx9dJ3ikBpbdXHtgoxVF9UPtaj/vzD+5oKqxLB79MHy0E5dbcvlD0yR 8Xnr4XS5BQkHOpwyikb17DNogLjyjzLGbRNm5i+E7Xfxl+YNpGQfAftGk6oWY3hM 5oteDO147DW74UfRLdXRhrnKYwp/aoset53GPHhGUdXAY3mYL4VawhhFa3Menr/Z UUtQtxP3FuUWDNWQ+w3zvfGtfX970B0a36D0waGbq/jYVmFGwXd6fM+kmYOitKwM +TpBFRrf42/ciPrMviv6UkOSsWjqvJDV/Dcg/UKxF9BmGvvpzaMASlIE2xPwCDSF 7qGq+NRM9GQ6T6QZMKuwvQkbVL+v7gNM3C0Urs4bS97vSM9XsTYCoCPxmbVFra9w w+jRWNkklvW5j2QS35A3TQCsAbtj4GgNHNmQBI5Zc7YCh14cxo/HTTSKwKzO9OUY JQg+aW9jMrNBkRB9ClQqEzOGMEBUuYWpSpu0PueN3KpiShtoo8GJASijAxJuYiLO V/u4znhe//mfW92ikJquINghZoyJMwzZ5i4NUbGePzuiyQ0ykSsFijCrYMHl0zvg BSUDa1PWFryFE2OhbS36ZruYP9WUDVte5zt9Zxd4l5O4Ez+RDOo= =rnEE -----END PGP SIGNATURE----- --RxRb1yoAvAGQWTw4-- --===============7217580361747604508== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". --===============7217580361747604508==--