From patchwork Wed Sep 9 14:58:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas George X-Patchwork-Id: 22230 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 01C7C44A754 for ; Wed, 9 Sep 2020 17:58:45 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CC5C868B8C1; Wed, 9 Sep 2020 17:58:44 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 017F168B89D for ; Wed, 9 Sep 2020 17:58:38 +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 089Ewb6q014898 for ; Wed, 9 Sep 2020 16:58:38 +0200 Received: by phare.normalesup.org (Postfix, from userid 1001) id DA170E6C30; Wed, 9 Sep 2020 16:58:37 +0200 (CEST) From: Nicolas George To: ffmpeg-devel@ffmpeg.org Date: Wed, 9 Sep 2020 16:58:35 +0200 Message-Id: <20200909145835.1456596-1-george@nsup.org> X-Mailer: git-send-email 2.28.0 MIME-Version: 1.0 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (nef.ens.fr [129.199.96.32]); Wed, 09 Sep 2020 16:58:38 +0200 (CEST) Subject: [FFmpeg-devel] [PATCH] lavfi: flag incorrect uses of ff_outlink APIs. X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" The link API provides an abstraction, where a filter sees on an input a stream of frames or samples terminated by a status change. Calling ff_outlink functions on an input bypasses this abstraction to peek inside implementation details; it may work now, but it can break at any point if the implementation is changed or if new conditions are triggered. I do not know how to fix these filters, but adding these comments will avoid new code to imitate the pattern and require fixing too. Signed-off-by: Nicolas George --- libavfilter/af_afade.c | 2 ++ libavfilter/af_afir.c | 3 +++ libavfilter/af_headphone.c | 1 + libavfilter/f_interleave.c | 3 +++ libavfilter/vf_framepack.c | 2 ++ libavfilter/vf_xfade.c | 2 ++ libavfilter/vf_xfade_opencl.c | 2 ++ 7 files changed, 15 insertions(+) diff --git a/libavfilter/af_afade.c b/libavfilter/af_afade.c index 4edfd27a3c..fc549522a3 100644 --- a/libavfilter/af_afade.c +++ b/libavfilter/af_afade.c @@ -555,9 +555,11 @@ static int activate(AVFilterContext *ctx) return ff_filter_frame(outlink, out); } } else if (ff_outlink_frame_wanted(ctx->outputs[0])) { + /* FIXME calling ff_outlink_something on an input is invalid */ if (!s->cf0_eof && ff_outlink_get_status(ctx->inputs[0])) { s->cf0_eof = 1; } + /* FIXME calling ff_outlink_something on an input is invalid */ if (ff_outlink_get_status(ctx->inputs[1])) { ff_outlink_set_status(ctx->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE); return 0; diff --git a/libavfilter/af_afir.c b/libavfilter/af_afir.c index ca90a158a1..56725064b5 100644 --- a/libavfilter/af_afir.c +++ b/libavfilter/af_afir.c @@ -642,6 +642,7 @@ static int activate(AVFilterContext *ctx) if (ret < 0) return ret; + /* FIXME calling ff_outlink_something on an input is invalid */ if (ff_outlink_get_status(ctx->inputs[1 + s->selir]) == AVERROR_EOF) s->eof_coeffs[s->selir] = 1; @@ -698,6 +699,7 @@ static int activate(AVFilterContext *ctx) } if (ff_outlink_frame_wanted(ctx->outputs[0]) && + /* FIXME calling ff_outlink_something on an input is invalid */ !ff_outlink_get_status(ctx->inputs[0])) { ff_inlink_request_frame(ctx->inputs[0]); return 0; @@ -705,6 +707,7 @@ static int activate(AVFilterContext *ctx) if (s->response && ff_outlink_frame_wanted(ctx->outputs[1]) && + /* FIXME calling ff_outlink_something on an input is invalid */ !ff_outlink_get_status(ctx->inputs[0])) { ff_inlink_request_frame(ctx->inputs[0]); return 0; diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c index edf8e773d7..317e631de5 100644 --- a/libavfilter/af_headphone.c +++ b/libavfilter/af_headphone.c @@ -520,6 +520,7 @@ static int activate(AVFilterContext *ctx) if ((ret = check_ir(input, i)) < 0) return ret; + /* FIXME calling ff_outlink_something on an input is invalid */ if (ff_outlink_get_status(input) == AVERROR_EOF) { if (!ff_inlink_queued_samples(input)) { av_log(ctx, AV_LOG_ERROR, "No samples provided for " diff --git a/libavfilter/f_interleave.c b/libavfilter/f_interleave.c index a18bbe79b3..638252f28e 100644 --- a/libavfilter/f_interleave.c +++ b/libavfilter/f_interleave.c @@ -116,10 +116,12 @@ static int activate(AVFilterContext *ctx) } for (i = 0; i < ctx->nb_inputs; i++) + /* FIXME calling ff_outlink_something on an input is invalid */ nb_eofs += !!ff_outlink_get_status(ctx->inputs[i]); if ((nb_eofs > 0 && s->duration_mode == DURATION_SHORTEST) || (nb_eofs == ctx->nb_inputs && s->duration_mode == DURATION_LONGEST) || + /* FIXME calling ff_outlink_something on an input is invalid */ (ff_outlink_get_status(ctx->inputs[0]) && s->duration_mode == DURATION_FIRST)) { ff_outlink_set_status(outlink, AVERROR_EOF, s->pts); return 0; @@ -129,6 +131,7 @@ static int activate(AVFilterContext *ctx) if (ff_inlink_queued_frames(ctx->inputs[i])) continue; if (ff_outlink_frame_wanted(outlink) && + /* FIXME calling ff_outlink_something on an input is invalid */ !ff_outlink_get_status(ctx->inputs[i])) { ff_inlink_request_frame(ctx->inputs[i]); return 0; diff --git a/libavfilter/vf_framepack.c b/libavfilter/vf_framepack.c index b5d877ca99..e3f0b6b820 100644 --- a/libavfilter/vf_framepack.c +++ b/libavfilter/vf_framepack.c @@ -354,6 +354,7 @@ static int activate(AVFilterContext *ctx) FF_FILTER_FORWARD_STATUS(ctx->inputs[1], outlink); if (ff_outlink_frame_wanted(ctx->outputs[0]) && + /* FIXME calling ff_outlink_something on an input is invalid */ !ff_outlink_get_status(ctx->inputs[0]) && !s->input_views[0]) { ff_inlink_request_frame(ctx->inputs[0]); @@ -361,6 +362,7 @@ static int activate(AVFilterContext *ctx) } if (ff_outlink_frame_wanted(ctx->outputs[0]) && + /* FIXME calling ff_outlink_something on an input is invalid */ !ff_outlink_get_status(ctx->inputs[1]) && !s->input_views[1]) { ff_inlink_request_frame(ctx->inputs[1]); diff --git a/libavfilter/vf_xfade.c b/libavfilter/vf_xfade.c index 7412709587..0eebfe457d 100644 --- a/libavfilter/vf_xfade.c +++ b/libavfilter/vf_xfade.c @@ -1824,10 +1824,12 @@ static int xfade_activate(AVFilterContext *ctx) } if (ff_outlink_frame_wanted(outlink)) { + /* FIXME calling ff_outlink_something on an input is invalid */ if (!s->eof[0] && ff_outlink_get_status(ctx->inputs[0])) { s->eof[0] = 1; s->xfade_is_over = 1; } + /* FIXME calling ff_outlink_something on an input is invalid */ if (!s->eof[1] && ff_outlink_get_status(ctx->inputs[1])) { s->eof[1] = 1; } diff --git a/libavfilter/vf_xfade_opencl.c b/libavfilter/vf_xfade_opencl.c index 4736043147..18f0ce29d8 100644 --- a/libavfilter/vf_xfade_opencl.c +++ b/libavfilter/vf_xfade_opencl.c @@ -323,10 +323,12 @@ static int xfade_opencl_activate(AVFilterContext *avctx) } if (ff_outlink_frame_wanted(outlink)) { + /* FIXME calling ff_outlink_something on an input is invalid */ if (!ctx->eof[0] && ff_outlink_get_status(avctx->inputs[0])) { ctx->eof[0] = 1; ctx->xfade_is_over = 1; } + /* FIXME calling ff_outlink_something on an input is invalid */ if (!ctx->eof[1] && ff_outlink_get_status(avctx->inputs[1])) { ctx->eof[1] = 1; }