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 <arwaarif1...@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}
>
> +@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:
> +
> +@table @option
> +@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
> +@code{4}.
> +
> +@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).
> +
> +@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
> +@code{0} (not enabled).
> +@end table
> +
> @section geq
>
> The filter accepts the following options:
> @@ -8292,6 +8321,7 @@ stereo3d=abl:sbsr
> @end example
> @end itemize
>
> +@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 <michae...@gmx.at>
> + * Copyright (C) 2005 Nikolaj Poroshin <poro...@psu.ru>
> + * Copyright (c) 2014 Arwa Arif <arwaarif1...@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 <michae...@gmx.at>
> + * Copyright (C) 2005 Nikolaj Poroshin <poro...@psu.ru>
> + * Copyright (c) 2014 Arwa Arif <arwaarif1...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
