[FFmpeg-devel] [PATCH] lavfi/select: add support to branch option
Clément Bœsch
ubitux at gmail.com
Tue Apr 16 22:50:55 CEST 2013
On Mon, Apr 15, 2013 at 09:05:55PM +0200, Stefano Sabatini wrote:
> On date Monday 2013-04-15 00:52:26 +0200, Stefano Sabatini encoded:
> > On date Monday 2013-04-15 00:08:56 +0200, Nicolas George encoded:
> > > Le sextidi 26 germinal, an CCXXI, Stefano Sabatini a écrit :
> > > > Assertion !link->frame_requested || link->flags & FF_LINK_FLAG_REQUEST_LOOP failed at libavfilter/avfilter.c:344
> > >
> > > That means that request_frame() returned 0 but did not actually push a
> > > frame. Possibly, in this case, it pushed a frame but on the wrong pad.
> >
> > Yes, it was a pre-exising double -> int silly bug.
> >
> > Updated.
> > --
> > FFmpeg = Formidable & Friendly Most Portentous Exuberant Game
>
> > From 5581748d3350829e6da4146e7a018723d015e059 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Mon, 8 Apr 2013 12:58:56 +0200
> > Subject: [PATCH] lavfi/select: add support for dynamic number of outputs
> >
> > TODO: bump micro, add docs
> > ---
> > doc/filters.texi | 23 +++++++++++++---
> > libavfilter/f_select.c | 69 +++++++++++++++++++++++++++---------------------
> > 2 files changed, 59 insertions(+), 33 deletions(-)
>
> Patch split, only relevant patch in attachment.
> --
> FFmpeg = Frenzy and Free MultiPurpose Exuberant God
> From e3c80434ad414c976eeb6ed0e5ac7b7ed058b8ea Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Mon, 8 Apr 2013 12:58:56 +0200
> Subject: [PATCH] lavfi/select: add support for dynamic number of outputs
>
> TODO: bump micro, add docs
> ---
> doc/filters.texi | 23 +++++++++++++++---
> libavfilter/f_select.c | 63 ++++++++++++++++++++++++++++--------------------
> 2 files changed, 57 insertions(+), 29 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 868f2ae..9f7e22b 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -6767,10 +6767,21 @@ This filter accepts the following options:
> @table @option
>
> @item expr, e
> -An expression, which is evaluated for each input frame. If the expression is
> -evaluated to a non-zero value, the frame is selected and passed to the output,
> -otherwise it is discarded.
> +Set expression, which is evaluated for each input frame.
>
> +If the expression is evaluated to zero, the frame is discarded.
> +
> +If the evaluation result is non-zero, if the value is negative or NaN
> +the frame is sent to the first output; otherwise the frame is sent to
> +the output with index @code{ceil(val)-1}, assuming that the input
> +index starts from 0.
> +
> +For example a value of @code{1.2} corresponds to the output
> + at code{ceil(1.2)-1 = 2-1=1}, that is the second output.
Weird '=' spacing.
> +
> + at item outputs, n
> +Set the number of outputs. The output to which to send the selected
> +frame is based on the result of the evaluation.
Missing default value.
> @end table
>
> The expression can contain the following constants:
> @@ -6924,6 +6935,12 @@ ffmpeg -i video.avi -vf select='gt(scene\,0.4)',scale=160:120,tile -frames:v 1 p
>
> Comparing @var{scene} against a value between 0.3 and 0.5 is generally a sane
> choice.
> +
> + at item
> +Send even and odd frames to separate outputs, and compose them:
> + at example
> +[in] select=n=2:e='mod(n, 2)+1'[odd][even]; [odd] pad=h=2*ih [tmp]; [tmp][even] overlay=y=h [out]
^^
a space here would be nice
Also, [in] and [out] make the example unnecessarily complex.
> + at end example
> @end itemize
>
> @section asendcmd, sendcmd
> diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
> index ca9bdc9..5888f97 100644
> --- a/libavfilter/f_select.c
> +++ b/libavfilter/f_select.c
> @@ -23,6 +23,7 @@
> * filter for selecting which frame passes in the filterchain
> */
>
> +#include "libavutil/avstring.h"
> #include "libavutil/eval.h"
> #include "libavutil/fifo.h"
> #include "libavutil/internal.h"
> @@ -136,13 +137,16 @@ typedef struct {
> #endif
> AVFrame *prev_picref; ///< previous frame (scene detect only)
> double select;
> + int select_out;
nit: a short doxy for this one may help
> + int nb_outputs;
> } SelectContext;
>
> +static int request_frame(AVFilterLink *outlink);
>
> static av_cold int init(AVFilterContext *ctx)
> {
> SelectContext *select = ctx->priv;
> - int ret;
> + int i, ret;
>
> if ((ret = av_expr_parse(&select->expr, select->expr_str,
> var_names, NULL, NULL, NULL, NULL, 0, ctx)) < 0) {
> @@ -152,6 +156,17 @@ static av_cold int init(AVFilterContext *ctx)
> }
> select->do_scene_detect = !!strstr(select->expr_str, "scene");
>
> + for (i = 0; i < select->nb_outputs; i++) {
> + AVFilterPad pad = { 0 };
> +
> + pad.name = av_asprintf("output%d", i);
> + if (!pad.name)
> + return AVERROR(ENOMEM);
> + pad.type = ctx->filter->inputs[0].type;
> + pad.request_frame = request_frame;
> + ff_insert_outpad(ctx, i, &pad);
> + }
> +
> return 0;
> }
>
> @@ -308,7 +323,11 @@ static void select_frame(AVFilterContext *ctx, AVFrame *frame)
> break;
> }
>
> - av_log(inlink->dst, AV_LOG_DEBUG, " -> select:%f\n", res);
> + select->select_out =
> + res == 0 ? -1 : /* drop */
> + isnan(res) || res < 0 ? 0 : /* first output */
> + FFMIN(ceilf(res)-1, select->nb_outputs); /* other outputs */
select->nb_outputs - 1?
Also, if you want a compact if/else code, please use something like
if (res == 0) select->select_out = -1; // drop
else if (isnan(res) || res < 0) select->select_out = 0; // first output
else select->select_out = FFMIN(...); // other outputs
...instead of hardly readable nested ?:
> + av_log(inlink->dst, AV_LOG_DEBUG, " -> select:%f out:%d\n", res, select->select_out);
>
nit: put "out" into parenthesis to show that it's directly related to the
select value.
> if (res) {
> select->var_values[VAR_PREV_SELECTED_N] = select->var_values[VAR_N];
> @@ -326,11 +345,12 @@ static void select_frame(AVFilterContext *ctx, AVFrame *frame)
>
> static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> {
> - SelectContext *select = inlink->dst->priv;
> + AVFilterContext *ctx = inlink->dst;
> + SelectContext *select = ctx->priv;
>
> - select_frame(inlink->dst, frame);
> + select_frame(ctx, frame);
> if (select->select)
> - return ff_filter_frame(inlink->dst->outputs[0], frame);
> + return ff_filter_frame(ctx->outputs[select->select_out], frame);
>
> av_frame_free(&frame);
> return 0;
> @@ -341,13 +361,13 @@ static int request_frame(AVFilterLink *outlink)
> AVFilterContext *ctx = outlink->src;
> SelectContext *select = ctx->priv;
> AVFilterLink *inlink = outlink->src->inputs[0];
> - select->select = 0;
> + int out_no = FF_OUTLINK_IDX(outlink);
>
> do {
> int ret = ff_request_frame(inlink);
> if (ret < 0)
> return ret;
> - } while (!select->select);
> + } while (select->select_out != out_no);
>
Is this code still required with the new request frame system? (if you
have the flag set)
> return 0;
> }
> @@ -355,10 +375,14 @@ static int request_frame(AVFilterLink *outlink)
> static av_cold void uninit(AVFilterContext *ctx)
> {
> SelectContext *select = ctx->priv;
> + int i;
>
> av_expr_free(select->expr);
> select->expr = NULL;
>
> + for (i = 0; i < ctx->nb_outputs; i++)
> + av_freep(&ctx->output_pads[i].name);
> +
> #if CONFIG_AVCODEC
> if (select->do_scene_detect) {
> av_frame_free(&select->prev_picref);
> @@ -393,6 +417,8 @@ static int query_formats(AVFilterContext *ctx)
> static const AVOption aselect_options[] = {
> { "expr", "An expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, .flags = AFLAGS },
> { "e", "An expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, .flags = AFLAGS },
> + { "outputs", "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, AFLAGS },
> + { "n", "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, AFLAGS },
> { NULL },
> };
> AVFILTER_DEFINE_CLASS(aselect);
> @@ -424,14 +450,6 @@ static const AVFilterPad avfilter_af_aselect_inputs[] = {
> { NULL }
> };
>
> -static const AVFilterPad avfilter_af_aselect_outputs[] = {
> - {
> - .name = "default",
> - .type = AVMEDIA_TYPE_AUDIO,
> - },
> - { NULL }
> -};
> -
> AVFilter avfilter_af_aselect = {
> .name = "aselect",
> .description = NULL_IF_CONFIG_SMALL("Select audio frames to pass in output."),
> @@ -439,8 +457,8 @@ AVFilter avfilter_af_aselect = {
> .uninit = uninit,
> .priv_size = sizeof(SelectContext),
> .inputs = avfilter_af_aselect_inputs,
> - .outputs = avfilter_af_aselect_outputs,
> .priv_class = &aselect_class,
> + .flags = AVFILTER_FLAG_DYNAMIC_OUTPUTS,
> };
> #endif /* CONFIG_ASELECT_FILTER */
>
> @@ -451,6 +469,8 @@ AVFilter avfilter_af_aselect = {
> static const AVOption select_options[] = {
> { "expr", "An expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, .flags = FLAGS },
> { "e", "An expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, .flags = FLAGS },
> + { "outputs", "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, FLAGS },
> + { "n", "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, FLAGS },
> { NULL },
> };
>
> @@ -483,15 +503,6 @@ static const AVFilterPad avfilter_vf_select_inputs[] = {
> { NULL }
> };
>
> -static const AVFilterPad avfilter_vf_select_outputs[] = {
> - {
> - .name = "default",
> - .type = AVMEDIA_TYPE_VIDEO,
> - .request_frame = request_frame,
> - },
> - { NULL }
> -};
> -
> AVFilter avfilter_vf_select = {
> .name = "select",
> .description = NULL_IF_CONFIG_SMALL("Select video frames to pass in output."),
> @@ -503,6 +514,6 @@ AVFilter avfilter_vf_select = {
> .priv_class = &select_class,
>
> .inputs = avfilter_vf_select_inputs,
> - .outputs = avfilter_vf_select_outputs,
> + .flags = AVFILTER_FLAG_DYNAMIC_OUTPUTS,
Rest LGTM
--
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130416/a67dbd15/attachment.asc>
More information about the ffmpeg-devel
mailing list