[FFmpeg-devel] [PATCH] lavfi: Port fspp to FFmpeg

Stefano Sabatini stefasab at gmail.com
Thu Dec 18 08:20:51 CET 2014


On date Thursday 2014-12-18 12:06:04 +0530, arwa arif encoded:
> >
> > AVFILTER_DEFINE_CLASS(fspp)
> 
> 
> Why do we need this? Is it not defined already?
> 
> 
> > > +static int config_input(AVFilterLink *inlink)
> > > +{
> > > +
> > > +    AVFilterContext *ctx = inlink->dst;
> > > +    FSPPContext *fspp = ctx->priv;
> > > +    const int h = FFALIGN(inlink->h + 16, 16);
> > > +    const AVPixFmtDescriptor *desc =
> > av_pix_fmt_desc_get(inlink->format);
> > > +
> > > +    fspp->hsub = desc->log2_chroma_w;
> > > +    fspp->vsub = desc->log2_chroma_h;
> > > +
> > > +    fspp->temp_stride = FFALIGN(inlink->w + 16, 16);
> >
> > > +    fspp->temp = av_malloc_array(fspp->temp_stride, h *
> > sizeof(*fspp->temp));
> > > +    fspp->src  = av_malloc_array(fspp->temp_stride, h *
> > sizeof(*fspp->src));
> >
> > missing null checks
> >
> 
> NULL checks are done here :-
> 
> 
> >
> > > +
> > > +    if (!fspp->temp || !fspp->src)
> > > +        return AVERROR(ENOMEM);
> 
> > +
> > > +    if (ARCH_X86)
> > > +        ff_fspp_init_x86(fspp);
> > > +
> > > +    else {
> > > +        fspp->store_slice  = store_slice_c;
> > > +        fspp->store_slice2 = store_slice2_c;
> > > +        fspp->mul_thrmat   = mul_thrmat_c;
> > > +        fspp->column_fidct = column_fidct_c;
> > > +        fspp->row_idct     = row_idct_c;
> > > +        fspp->row_fdct     = row_fdct_c;
> > > +    }
> > > +
> >
> >
> > +typedef struct fsppContext {
> >
> > typedef struct {
> >
> > > +    uint64_t threshold_mtx_noq[8 * 2];
> > > +    uint64_t threshold_mtx[8 * 2];        //used in both C & MMX (&
> > later SSE2) versions
> > > +
> > > +    int log2_count;
> > > +    int hsub;
> > > +    int vsub;
> > > +    int temp_stride;
> > > +    int qp;
> > > +    int qscale_type;
> > > +    int prev_q;
> > > +    uint8_t *src;
> > > +    int16_t *temp;
> > > +    int bframes;
> > > +    uint8_t *non_b_qp_table;
> > > +    int non_b_qp_alloc_size;
> > > +    int use_bframe_qp;
> > > +
> > > +    void (*store_slice)(uint8_t *dst, int16_t *src,
> > > +                     int dst_stride, int src_stride,
> > > +                     int width, int height, int log2_scale);
> > > +
> > > +    void (*store_slice2)(uint8_t *dst, int16_t *src,
> > > +                     int dst_stride, int src_stride,
> > > +                     int width, int height, int log2_scale);
> > > +
> > > +    void (*mul_thrmat)(struct fsppContext *fspp, int q);
> >
> 

> This function uses a pointer to the struct, so can't remove that.
> 

Please keep my comments and some context, otherwise it's hard to
understand what you're referring to.

> > > +
> > > +    void (*column_fidct)(int16_t* thr_adr, int16_t *data,
> > > +                     int16_t *output, int cnt);
> > > +
> > > +    void (*row_idct)(int16_t* workspace, int16_t* output_adr,
> > > +                     int output_stride, int cnt);
> > > +
> > > +    void (*row_fdct)(int16_t *data, const uint8_t *pixels,
> > > +                     int line_size, int cnt);
> > > +
> > > +} FSPPContext;
> > > +
> 
> 
> I checked the filter ouputs by using :-  ffmpeg -f lavfi -i testsrc,mp=fspp
> -t 5 -f md5 ../out_old.
> They are coming out to be same.


> From 5be218818b389bafe75eda6abef66f4d0a80a995 Mon Sep 17 00:00:00 2001
> From: Arwa Arif <arwaarif1994 at gmail.com>
> Date: Sun, 14 Dec 2014 12:03:31 +0530
> Subject: [PATCH] lavfi: port mp=uspp to a native libavfilter filter
> 
> ---
>  LICENSE.md                        |    1 +
>  configure                         |    1 +
>  doc/filters.texi                  |   30 +
>  libavfilter/Makefile              |    1 +
>  libavfilter/allfilters.c          |    1 +
>  libavfilter/libmpcodecs/vf_fspp.c |    4 +-
>  libavfilter/version.h             |    4 +-
>  libavfilter/vf_fspp.c             |  678 ++++++++++++++++++
>  libavfilter/vf_fspp.h             |   96 +++
>  libavfilter/x86/Makefile          |    1 +
>  libavfilter/x86/vf_fspp.c         | 1405 +++++++++++++++++++++++++++++++++++++
>  11 files changed, 2218 insertions(+), 4 deletions(-)
>  create mode 100644 libavfilter/vf_fspp.c
>  create mode 100644 libavfilter/vf_fspp.h
>  create mode 100644 libavfilter/x86/vf_fspp.c
> 
> diff --git a/LICENSE.md b/LICENSE.md
> index cf9955f..a91f090 100644
> --- a/LICENSE.md
> +++ b/LICENSE.md
> @@ -32,6 +32,7 @@ Specifically, the GPL parts of FFmpeg are:
>      - vf_decimate.c
>      - vf_delogo.c
>      - vf_geq.c
> +    - vf_fspp.c

nit: alphabetical order

>      - vf_histeq.c
>      - vf_hqdn3d.c
>      - vf_interlace.c
> diff --git a/configure b/configure
> index e37285a..29f5534 100755
> --- a/configure
> +++ b/configure
> @@ -2575,6 +2575,7 @@ ebur128_filter_deps="gpl"
>  flite_filter_deps="libflite"
>  frei0r_filter_deps="frei0r dlopen"
>  frei0r_src_filter_deps="frei0r dlopen"
> +fspp_filter_deps="gpl"
>  geq_filter_deps="gpl"
>  histeq_filter_deps="gpl"
>  hqdn3d_filter_deps="gpl"
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 882caa0..30d8c43 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -4997,6 +4997,35 @@ frei0r=perspective:0.2/0.2|0.8/0.2
>  For more information, see
>  @url{http://frei0r.dyne.org}
>  
> + at section fspp
> +
> +Apply fast and simple postprocessing. It is a faster version of the simple
> +postprocessing filter - @ref{spp}.
> +
> +It splits (I)DCT into horizontal/vertical passes. Unlike Simple postprocessing
> +filter, one of them is performed once per block, not pixel. This allows for

nit: not per pixel

reads better to my non native English mind

> +much better speed.
> +
> +The filter accepts the following options:
> +
> + at table @option
> + at item quality
> +Set quality. This option defines the number of levels for averaging. It accepts
> +an integer in the range 0-5. If set to @code{0}, the filter will have no
> +effect. A value of @code{5} means the higher quality. For each increment of
> +that value the speed drops by a factor of approximately 2.  Default value is
> + at code{4}.
> +
> + at item qp
> +Force a constant quantization parameter. It accepts 0 and 1. If not set
> +(@code{0}), the filter will use the QP from the video stream (if available).
> +
> + at item use_bframe_qp
> +Enable the use of the QP from the B-Frames if set to @code{1}. Using this
> +option may cause flicker since the B-Frames have often larger QP. Default is
> + at code{0} (not enabled).
> + at end table
> +
>  @section geq
>  
>  The filter accepts the following options:
> @@ -8292,6 +8321,7 @@ stereo3d=abl:sbsr
>  @end example
>  @end itemize
>  
> + at anchor{spp}
>  @section spp
>  
>  Apply a simple postprocessing filter that compresses and decompresses the image
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 6b7291e..8c523b4 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -125,6 +125,7 @@ OBJS-$(CONFIG_FRAMESTEP_FILTER)              += vf_framestep.o
>  OBJS-$(CONFIG_FPS_FILTER)                    += vf_fps.o
>  OBJS-$(CONFIG_FRAMEPACK_FILTER)              += vf_framepack.o
>  OBJS-$(CONFIG_FREI0R_FILTER)                 += vf_frei0r.o
> +OBJS-$(CONFIG_FSPP_FILTER)                   += vf_fspp.o
>  OBJS-$(CONFIG_GEQ_FILTER)                    += vf_geq.o
>  OBJS-$(CONFIG_GRADFUN_FILTER)                += vf_gradfun.o
>  OBJS-$(CONFIG_HALDCLUT_FILTER)               += vf_lut3d.o dualinput.o framesync.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index adb86be..4a915c7 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -141,6 +141,7 @@ void avfilter_register_all(void)
>      REGISTER_FILTER(FRAMEPACK,      framepack,      vf);
>      REGISTER_FILTER(FRAMESTEP,      framestep,      vf);
>      REGISTER_FILTER(FREI0R,         frei0r,         vf);
> +    REGISTER_FILTER(FSPP,           fspp,           vf);
>      REGISTER_FILTER(GEQ,            geq,            vf);
>      REGISTER_FILTER(GRADFUN,        gradfun,        vf);
>      REGISTER_FILTER(HALDCLUT,       haldclut,       vf);
> diff --git a/libavfilter/libmpcodecs/vf_fspp.c b/libavfilter/libmpcodecs/vf_fspp.c
> index d457859..3a80dc2 100644
> --- a/libavfilter/libmpcodecs/vf_fspp.c
> +++ b/libavfilter/libmpcodecs/vf_fspp.c
> @@ -710,8 +710,8 @@ const vf_info_t ff_vf_info_fspp = {
>  #if HAVE_MMX_INLINE
>  
>  DECLARE_ASM_CONST(8, uint64_t, MM_FIX_0_382683433)=FIX64(0.382683433, 14);
> -DECLARE_ALIGNED(8, uint64_t, ff_MM_FIX_0_541196100)=FIX64(0.541196100, 14);
> -DECLARE_ALIGNED(8, uint64_t, ff_MM_FIX_0_707106781)=FIX64(0.707106781, 14);
> +extern uint64_t ff_MM_FIX_0_707106781;
> +extern uint64_t ff_MM_FIX_0_541196100;

still looks weird

[...]
> diff --git a/libavfilter/vf_fspp.c b/libavfilter/vf_fspp.c
> new file mode 100644
> index 0000000..032e635
> --- /dev/null
> +++ b/libavfilter/vf_fspp.c
> @@ -0,0 +1,678 @@
> +/*
> + * Copyright (c) 2003 Michael Niedermayer <michaelni at gmx.at>
> + * Copyright (C) 2005 Nikolaj Poroshin <porosh3 at psu.ru>
> + * Copyright (c) 2014 Arwa Arif <arwaarif1994 at gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU 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.
> + */
> +
> +/**
> + * @file
> + * Fast Simple Post-processing filter
> + * This implementation is based on an algorithm described in
> + * "Aria Nosratinia Embedded Post-Processing for
> + * Enhancement of Compressed Images (1999)"
> + * (http://www.utdallas.edu/~aria/papers/vlsisp99.pdf)
> + * Further, with splitting (I)DCT into horizontal/vertical passes, one of
> + * them can be performed once per block, not pixel. This allows for much
> + * better speed.
> + *
> + * Originally written by Michael Niedermayer and Nikolaj for the MPlayer
> + * project, and ported by Arwa Arif for FFmpeg.
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/imgutils.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "internal.h"
> +#include "libavcodec/avcodec.h" //for reference to FF_QSCALE_TYPE
> +#include "vf_fspp.h"
> +
> +#define OFFSET(x) offsetof(FSPPContext, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +static const AVOption fspp_options[] = {
> +    { "quality",       "set quality",                          OFFSET(log2_count),    AV_OPT_TYPE_INT, {.i64 = 4}, 0, MAX_LEVEL, FLAGS },
> +    { "qp",            "force a constant quantizer parameter", OFFSET(qp),            AV_OPT_TYPE_INT, {.i64 = 0}, 0, 63,        FLAGS },
> +    { "use_bframe_qp", "use B-frames' QP",                     OFFSET(use_bframe_qp), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1,         FLAGS },
> +    { NULL }
> +};
> +
> +static const AVClass fspp_class = {
> +    .class_name       = "fspp",
> +    .item_name        = av_default_item_name,
> +    .option           = fspp_options,
> +    .version          = LIBAVUTIL_VERSION_INT,
> +    .category         = AV_CLASS_CATEGORY_FILTER,
> +};

Use AVFILTER_DEFINE_CLASS()

Please don't disregard my comments, reply inline if you can't
implement them or disagree with them (and explain why).

[...]
> diff --git a/libavfilter/vf_fspp.h b/libavfilter/vf_fspp.h
> new file mode 100644
> index 0000000..78a9ff9
> --- /dev/null
> +++ b/libavfilter/vf_fspp.h
> @@ -0,0 +1,96 @@
> +/*
> + * Copyright (c) 2003 Michael Niedermayer <michaelni at gmx.at>
> + * Copyright (C) 2005 Nikolaj Poroshin <porosh3 at psu.ru>
> + * Copyright (c) 2014 Arwa Arif <arwaarif1994 at gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU 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.
> + */
> +
> +#ifndef AVFILTER_FSPP_H
> +#define AVFILTER_FSPP_H
> +
> +#include "avfilter.h"
> +
> +#define BLOCKSZ 12
> +#define MAX_LEVEL 5
> +
> +#define DCTSIZE 8
> +#define DCTSIZE_S "8"
> +
> +#define FIX(x,s)  ((int) ((x) * (1 << s) + 0.5) & 0xffff)
> +#define C64(x)    ((uint64_t)((x) | (x) << 16)) <<32 | (uint64_t)(x) | (uint64_t)(x) << 16
> +#define FIX64(x,s)  C64(FIX(x,s))
> +
> +#define MULTIPLY16H(x,k)   (((x) * (k)) >> 16)
> +#define THRESHOLD(r,x,t)                         \
> +    if(((unsigned)((x) + t)) > t * 2) r = (x);   \
> +    else r = 0;
> +#define DESCALE(x,n)  (((x) + (1 << ((n) - 1))) >> n)
> +
> +typedef int32_t int_simd16_t;
> +static const int16_t FIX_0_382683433   = FIX(0.382683433, 14);
> +static const int16_t FIX_0_541196100   = FIX(0.541196100, 14);
> +static const int16_t FIX_0_707106781   = FIX(0.707106781, 14);
> +static const int16_t FIX_1_306562965   = FIX(1.306562965, 14);
> +static const int16_t FIX_1_414213562_A = FIX(1.414213562, 14);
> +static const int16_t FIX_1_847759065   = FIX(1.847759065, 13);
> +static const int16_t FIX_2_613125930   = FIX(-2.613125930, 13);
> +static const int16_t FIX_1_414213562   = FIX(1.414213562, 13);
> +static const int16_t FIX_1_082392200   = FIX(1.082392200, 13);
> +

> +typedef struct fsppContext {

typedef struct {

should be enough

[...]
-- 
FFmpeg = Fabulous and Faithless Monstrous Picky Extended God


More information about the ffmpeg-devel mailing list