[FFmpeg-devel] [PATCH] avfilter: add mergeplanes

Stefano Sabatini stefasab at gmail.com
Fri Oct 25 18:41:30 CEST 2013


On date Friday 2013-10-25 08:50:16 +0000, Paul B Mahol encoded:
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
>  MAINTAINERS                  |   1 +
>  doc/filters.texi             |  54 ++++++++
>  libavfilter/Makefile         |   1 +
>  libavfilter/allfilters.c     |   1 +
>  libavfilter/vf_mergeplanes.c | 313 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 370 insertions(+)
>  create mode 100644 libavfilter/vf_mergeplanes.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 677451c..f017eb1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -333,6 +333,7 @@ Filters:
>    vf_extractplanes.c                    Paul B Mahol
>    vf_histogram.c                        Paul B Mahol
>    vf_il.c                               Paul B Mahol
> +  vf_mergeplanes.c                      Paul B Mahol
>    vf_psnr.c                             Paul B Mahol
>    vf_scale.c                            Michael Niedermayer
>    vf_separatefields.c                   Paul B Mahol
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 7783807..b112416 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -5258,6 +5258,60 @@ lutyuv=y='bitand(val, 128+64+32)'
>  @end example
>  @end itemize
>  
> + at section mergeplanes
> +
> +Merge color channel components from several video streams.

Also add something like the following, which clarifies the use of the
filter:

The filter accepts up to 4 input streams, and merge selected input
planes to the output video.

Also I'm not sure if you require the filter to only accept planar
formats, if that's not the case maybe mergecomponents would be more
appropriate (and general).

> +
> +This filter accepts the following options:
> + at table @option

> + at item mapping
> +Set input to output plane mapping. Default is @code{0}.
> +
> +This sets mapping from input stream planes to output stream planes.
> +

> +For example for 0xAa[Bb[Cc[Dd]]], 'Aa' describe mapping for first plane of output
> +stream. 'A' sets what input stream to pick, and 'a' number of plane of that
> +input stream to use. For rest mappings it is similar, 'Bb' describe mapping
> +for second plane of output stream, 'Cc' for third and 'Dd' for fourth plane.

The mappings is specified as a bitmap. It should be specified as a
hexadecimal number in the form 0xAa[Bb[Cc[Dd]]]. 'Aa' describes the
mapping for the first plane of the output stream. A sets the number of
the input stream to use (from 0 to 3), and 'a' the plane number of the
corresponding input to use (from 0 to 3). The rest of the mappings is
similar, 'Bb' describes the mapping for the output stream second
plane, ...

Not that I'm a fan of hexadecimal specs, I'd prefer something like
0.0|1.2|..., but that can be extended later if needed.

> +
> + at item format
> +Set output pixel format. Default is @code{yuva444p}.
> + at end table
> +
> + at subsection Examples
> +
> + at itemize
> + at item
> +Merge three gray video streams of same width and height into single video stream:
> + at example
> +mergeplanes=0x001020:yuv444p

here and below, I suggest something like:
[a0][a1][a2]mergeplanes=0x001020:yuv444p

to clarify the example

> + at end example
> +
> + at item
> +Merge 1st yuv444p stream and 2nd gray video stream into yuva444p video stream:
> + at example
> +mergeplanes=0x00010210:yuva444p
> + at end example
> +
> + at item
> +Swap Y and A plane in yuva444p stream:
> + at example
> +format=yuva444p,mergeplanes=0x03010200:yuva444p
> + at end example
> +
> + at item
> +Swap U and V plane in yuv420p stream:
> + at example
> +format=yuv420p,mergeplanes=0x000201:yuv420p
> + at end example
> +

> + at item
> +Cast a rgb24 clip to yuv444p:
> + at example
> +format=rgb24,mergeplanes=0x000102:yuv444p
> + at end example

I wonder if this makes sense...

> + at end itemize
> +
>  @section mcdeint
>  
>  Apply motion-compensation deinterlacing.
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 9eae3f7..3fcccf8 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -158,6 +158,7 @@ OBJS-$(CONFIG_LUT_FILTER)                    += vf_lut.o
>  OBJS-$(CONFIG_LUTRGB_FILTER)                 += vf_lut.o
>  OBJS-$(CONFIG_LUTYUV_FILTER)                 += vf_lut.o
>  OBJS-$(CONFIG_MCDEINT_FILTER)                += vf_mcdeint.o
> +OBJS-$(CONFIG_MERGEPLANES_FILTER)            += vf_mergeplanes.o framesync.o
>  OBJS-$(CONFIG_MP_FILTER)                     += vf_mp.o
>  OBJS-$(CONFIG_MPDECIMATE_FILTER)             += vf_mpdecimate.o
>  OBJS-$(CONFIG_NEGATE_FILTER)                 += vf_lut.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index e73ea62..ad3a470 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -154,6 +154,7 @@ void avfilter_register_all(void)
>      REGISTER_FILTER(LUTRGB,         lutrgb,         vf);
>      REGISTER_FILTER(LUTYUV,         lutyuv,         vf);
>      REGISTER_FILTER(MCDEINT,        mcdeint,        vf);
> +    REGISTER_FILTER(MERGEPLANES,    mergeplanes,    vf);
>      REGISTER_FILTER(MP,             mp,             vf);
>      REGISTER_FILTER(MPDECIMATE,     mpdecimate,     vf);
>      REGISTER_FILTER(NEGATE,         negate,         vf);
> diff --git a/libavfilter/vf_mergeplanes.c b/libavfilter/vf_mergeplanes.c
> new file mode 100644
> index 0000000..923e16b
> --- /dev/null
> +++ b/libavfilter/vf_mergeplanes.c
> @@ -0,0 +1,313 @@
> +/*
> + * Copyright (c) 2013 Paul B Mahol
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/imgutils.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "avfilter.h"
> +#include "internal.h"
> +#include "framesync.h"
> +
> +typedef struct InputParam {
> +    int depth[4];
> +    int nb_planes;
> +    int planewidth[4];
> +    int planeheight[4];
> +} InputParam;
> +
> +typedef struct MergePlanesContext {
> +    const AVClass *class;
> +    int64_t mapping;
> +    const enum AVPixelFormat out_fmt;
> +    int nb_inputs;
> +    int nb_planes;
> +    int planewidth[4];
> +    int planeheight[4];
> +    int map[4][2];
> +    const AVPixFmtDescriptor *outdesc;
> +
> +    FFFrameSync fs;
> +    FFFrameSyncIn fsin[3]; /* must be immediately after fs */
> +} MergePlanesContext;
> +
> +#define OFFSET(x) offsetof(MergePlanesContext, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +static const AVOption mergeplanes_options[] = {
> +    { "mapping", "set input to output plane mapping", OFFSET(mapping), AV_OPT_TYPE_INT, {.i64=0}, 0, 0x33333333, FLAGS },
> +    { "format", "set output pixel format", OFFSET(out_fmt), AV_OPT_TYPE_PIXEL_FMT, {.i64=AV_PIX_FMT_YUVA444P}, .flags=FLAGS },
> +    { NULL }
> +};
> +
> +AVFILTER_DEFINE_CLASS(mergeplanes);
> +

> +static int query_formats(AVFilterContext *ctx)

init() is called before query_formats(), thus it should be defined
before this (indeed I had to jump back and forth in this and in other
reviews).

> +{
> +    MergePlanesContext *s = ctx->priv;

I'm not a fun of meaningless variable names as well.

> +    AVFilterFormats *formats = NULL;
> +    int i;
> +
> +    s->outdesc = av_pix_fmt_desc_get(s->out_fmt);
> +    for (i = 0; i < AV_PIX_FMT_NB; i++) {
> +        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(i);

> +        if (desc->comp[0].depth_minus1 == s->outdesc->comp[0].depth_minus1 &&
> +            (desc->flags & AV_PIX_FMT_FLAG_PLANAR || desc->nb_components == 1))

Note: I hate PLANAR, the information is redundant and leads to
inconsistencies since it could be derived from the pixdesc
description. It should be replaced by something like
av_pix_desc_is_planar().

> +            ff_add_format(&formats, i);
> +    }
> +
> +    for (i = 0; i < s->nb_inputs; i++)
> +        ff_formats_ref(formats, &ctx->inputs[i]->out_formats);
> +
> +    formats = NULL;
> +    ff_add_format(&formats, s->out_fmt);
> +    ff_formats_ref(formats, &ctx->outputs[0]->in_formats);
> +
> +    return 0;
> +}
> +
> +static int process_frame(FFFrameSync *fs)
> +{
> +    AVFilterContext *ctx = fs->parent;
> +    AVFilterLink *outlink = ctx->outputs[0];
> +    MergePlanesContext *s = fs->opaque;
> +    AVFrame *in[4] = { NULL };
> +    AVFrame *out;
> +    int i, ret;
> +
> +    for (i = 0; i < s->nb_inputs; i++) {
> +        if ((ret = ff_framesync_get_frame(&s->fs, i, &in[i], 0)) < 0)
> +            return ret;
> +    }
> +
> +    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> +    if (!out)
> +        return AVERROR(ENOMEM);
> +    out->pts = av_rescale_q(s->fs.pts, s->fs.time_base, outlink->time_base);
> +
> +    for (i = 0; i < s->nb_planes; i++) {
> +        const int input = s->map[i][1];
> +        const int plane = s->map[i][0];
> +
> +        av_image_copy_plane(out->data[i], out->linesize[i],
> +                            in[input]->data[plane], in[input]->linesize[plane],
> +                            s->planewidth[i], s->planeheight[i]);
> +    }
> +
> +    return ff_filter_frame(outlink, out);
> +}
> +
> +static int config_output(AVFilterLink *outlink)
> +{
> +    AVFilterContext *ctx = outlink->src;
> +    MergePlanesContext *s = ctx->priv;
> +    InputParam inputsp[4];
> +    FFFrameSyncIn *in;
> +    int i;
> +
> +    ff_framesync_init(&s->fs, ctx, s->nb_inputs);
> +    in = s->fs.in;
> +    s->fs.opaque = s;
> +    s->fs.on_event = process_frame;
> +
> +    outlink->w = ctx->inputs[0]->w;
> +    outlink->h = ctx->inputs[0]->h;
> +    outlink->time_base = ctx->inputs[0]->time_base;
> +    outlink->frame_rate = ctx->inputs[0]->frame_rate;
> +    outlink->sample_aspect_ratio = ctx->inputs[0]->sample_aspect_ratio;
> +
> +    s->planewidth[1]  =
> +    s->planewidth[2]  = FF_CEIL_RSHIFT(outlink->w, s->outdesc->log2_chroma_w);
> +    s->planewidth[0]  =
> +    s->planewidth[3]  = outlink->w;
> +    s->planeheight[1] =
> +    s->planeheight[2] = FF_CEIL_RSHIFT(outlink->h, s->outdesc->log2_chroma_h);
> +    s->planeheight[0] =
> +    s->planeheight[3] = outlink->h;
> +
> +    for (i = 0; i < s->nb_inputs; i++) {
> +        InputParam *inputp = &inputsp[i];
> +        AVFilterLink *inlink = ctx->inputs[i];
> +        const AVPixFmtDescriptor *indesc = av_pix_fmt_desc_get(inlink->format);
> +        int j;
> +
> +        if (outlink->sample_aspect_ratio.num != inlink->sample_aspect_ratio.num ||
> +            outlink->sample_aspect_ratio.den != inlink->sample_aspect_ratio.den) {
> +            av_log(ctx, AV_LOG_ERROR, "input #%d link %s SAR %d:%d "
> +                                      "do not match output link %s SAR %d:%d\n",

does not

> +                                      i, ctx->input_pads[i].name,
> +                                      inlink->sample_aspect_ratio.num,
> +                                      inlink->sample_aspect_ratio.den,
> +                                      ctx->output_pads[0].name,
> +                                      outlink->sample_aspect_ratio.num,
> +                                      outlink->sample_aspect_ratio.den);
> +            return AVERROR(EINVAL);
> +        }
> +
> +        inputp->planewidth[1]  =
> +        inputp->planewidth[2]  = FF_CEIL_RSHIFT(inlink->w, indesc->log2_chroma_w);
> +        inputp->planewidth[0]  =
> +        inputp->planewidth[3]  = inlink->w;
> +        inputp->planeheight[1] =
> +        inputp->planeheight[2] = FF_CEIL_RSHIFT(inlink->h, indesc->log2_chroma_h);
> +        inputp->planeheight[0] =
> +        inputp->planeheight[3] = inlink->h;
> +        inputp->nb_planes = av_pix_fmt_count_planes(inlink->format);
> +
> +        for (j = 0; j < inputp->nb_planes; j++)
> +            inputp->depth[j] = indesc->comp[j].depth_minus1 + 1;
> +
> +        in[i].time_base = inlink->time_base;
> +        in[i].sync   = 1;
> +        in[i].before = EXT_STOP;
> +        in[i].after  = EXT_STOP;
> +    }
> +
> +    for (i = 0; i < s->nb_planes; i++) {
> +        const int input = s->map[i][1];
> +        const int plane = s->map[i][0];
> +        InputParam *inputp = &inputsp[input];
> +
> +        if (plane + 1 > inputp->nb_planes) {
> +            av_log(ctx, AV_LOG_ERROR, "input %d does not have %d plane\n",
> +                                      input, plane);
> +            goto fail;
> +        }
> +        if (s->outdesc->comp[i].depth_minus1 + 1 != inputp->depth[plane]) {
> +            av_log(ctx, AV_LOG_ERROR, "output plane %d depth %d does not "
> +                                      "match input %d plane %d depth %d\n",
> +                                      i, s->outdesc->comp[i].depth_minus1 + 1,
> +                                      input, plane, inputp->depth[plane]);
> +            goto fail;
> +        }
> +        if (s->planewidth[i] != inputp->planewidth[plane]) {
> +            av_log(ctx, AV_LOG_ERROR, "output plane %d width %d does not "
> +                                      "match input %d plane %d width %d\n",
> +                                      i, s->planewidth[i],
> +                                      input, plane, inputp->planewidth[plane]);
> +            goto fail;
> +        }
> +        if (s->planeheight[i] != inputp->planeheight[plane]) {
> +            av_log(ctx, AV_LOG_ERROR, "output plane %d height %d does not "
> +                                      "match input %d plane %d height %d\n",
> +                                      i, s->planeheight[i],
> +                                      input, plane, inputp->planeheight[plane]);
> +            goto fail;
> +        }
> +    }
> +
> +    return ff_framesync_configure(&s->fs);
> +fail:
> +    return AVERROR(EINVAL);
> +}
> +

LGTM otherwise if tested, nice job.
-- 
FFmpeg = Faithful & Freak Mortal Problematic Esoteric Gorilla


More information about the ffmpeg-devel mailing list