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 28FFA44537 for ; Wed, 14 Sep 2022 19:02:36 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4C6F768BB69; Wed, 14 Sep 2022 22:02:35 +0300 (EEST) Received: from mail-oa1-f47.google.com (mail-oa1-f47.google.com [209.85.160.47]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7D2D568B9C0 for ; Wed, 14 Sep 2022 22:02:28 +0300 (EEST) Received: by mail-oa1-f47.google.com with SMTP id 586e51a60fabf-1278624b7c4so43620875fac.5 for ; Wed, 14 Sep 2022 12:02:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date; bh=62BHqVP9NKU3ru0c+iU85bpR8gd0qDBkRPfL3WNPVx0=; b=HGoEdCLleYy2w/icnrHiQcB8sWd1v5TfPmH6hKFlMo5vBVKETqsq7Q4rgxxzJnhveC ltUPE0lVXRQHG/jWfzU9sio34YiHwk4kDmRabS9wbi63UZX7nAcm/qzULBozDJ85CAIe M3YXjhH5nTBU0eo+nZrNrkCcHM/iNTQu//exxIxQddst5DbHjnjK3xgDue7r40uR+jVf MMsz1S6loxHTxlyqYPjjVOFp+X+/2h9oUltaFA6kqWO09xoFXxFeoy7LUq2kIBThLMnl +0pqORB29K/6HSFq998e3HYVADOnJfHV5E+2gnmHn+B8PPoHig1DW/HbPR1Iu0x9laBj mknw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date; bh=62BHqVP9NKU3ru0c+iU85bpR8gd0qDBkRPfL3WNPVx0=; b=kfLWprsc/Re+bVdbARFaXVXYxeQr1OiRwq4OBJFlg8fQw/1E7PGlTcVSy2RGSVmxfw jesTPUywbrNzpwimAiL36nWrubkB7OCJHkoFw4Cg7aOcFLkyDbZTvgyLNaiHJ7/36ix0 6ejdqHEhfSITfP/xU53c0qUl7SA43jN/IwCyxar2IS3+YY7khHQPBLR/azC3yS2GmGh1 CxCibFbGwiY8/AZ/CTYju+zfE8oBVtm6mF2U4y5+hf757yXWpIYZdnww+JVsLaUcprun aj1UJRBWK+5Twywn8aEXMDf6JCrExxlwP2XkbJv44bAEs5dKulIRVzBJbTOlXXuftX6Y eznQ== X-Gm-Message-State: ACgBeo2pkYXh1d+4ZL7hybdW3FkQZG5Fz2ierMeQcfrKb9iIJ+M2b8X+ 4ZAyDWL84yO5dLxk19jksO4YCZiqBVkA3jtIovogaxZb X-Google-Smtp-Source: AA6agR6GYElsW5yn0scfVmT9WGtNSIxjA+oTPr2B1bpi+JucvjV4ibZxWEyyVP7fOXGgjQEzM4ujtRRvFgNhfHtaWqE= X-Received: by 2002:aca:c008:0:b0:350:2a7:5ff4 with SMTP id q8-20020acac008000000b0035002a75ff4mr2423581oif.161.1663182146084; Wed, 14 Sep 2022 12:02:26 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Sam Davis Date: Wed, 14 Sep 2022 15:02:14 -0400 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: [FFmpeg-devel] Unsubscribe 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: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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(&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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".