[FFmpeg-devel] [PATCH] lavfi: Port mp=pp7 to libavfilter

[Michael Niedermayer]

On Sat, Dec 27, 2014 at 06:07:53PM +0530, arwa arif wrote:
[...]
> +#define OFFSET(x) offsetof(PP7Context, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +static const AVOption pp7_options[] = {
> +    { "qp", "force a constant quantizer parameter", OFFSET(qp), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 64, FLAGS },
> +    { "mode", "set thresholding mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64 = MODE_MEDIUM}, 0, 2, FLAGS, "mode" },
> +        { "hard",   "hard thresholding",   0, AV_OPT_TYPE_CONST, {.i64 = MODE_HARD},   INT_MIN, INT_MAX, FLAGS, "mode" },
> +        { "soft",   "soft thresholding",   0, AV_OPT_TYPE_CONST, {.i64 = MODE_SOFT},   INT_MIN, INT_MAX, FLAGS, "mode" },
> +        { "medium", "medium thresholding", 0, AV_OPT_TYPE_CONST, {.i64 = MODE_MEDIUM}, INT_MIN, INT_MAX, FLAGS, "mode" },
> +    { "use_bframe_qp", "use B-frames' QP", OFFSET(use_bframe_qp), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, FLAGS },
> +    { NULL }
> +};
> +
> +AVFILTER_DEFINE_CLASS(pp7);
> +

> +#if HAVE_MMX_INLINE
> +DECLARE_ALIGNED(8, static const uint8_t, dither)[8][8] = {

dither is used outside HAVE_MMX_INLINE code, it thus cannot be
under HAVE_MMX_INLINE or build would break on non-x86 platforms


    > +    {  0,  48,  12,  60,   3,  51,  15,  63, },
> +    { 32,  16,  44,  28,  35,  19,  47,  31, },
> +    {  8,  56,   4,  52,  11,  59,   7,  55, },
> +    { 40,  24,  36,  20,  43,  27,  39,  23, },
> +    {  2,  50,  14,  62,   1,  49,  13,  61, },
> +    { 34,  18,  46,  30,  33,  17,  45,  29, },
> +    { 10,  58,   6,  54,   9,  57,   5,  53, },
> +    { 42,  26,  38,  22,  41,  25,  37,  21, },
> +};
> +#endif


[...]
> diff --git a/libavfilter/x86/vf_pp7.c b/libavfilter/x86/vf_pp7.c
> new file mode 100644
> index 0000000..fe33d7f
> --- /dev/null
> +++ b/libavfilter/x86/vf_pp7.c
> @@ -0,0 +1,68 @@
> +/*
> + * Copyright (c) 2005 Michael Niedermayer <michaelni at gmx.at>
> + *
> + * 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.
> + */
> +
> +#include "libavutil/attributes.h"
> +#include "libavutil/cpu.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/x86/asm.h"
> +#include "libavfilter/vf_pp7.h"
> +
> +static void dctB_mmx(int16_t *dst, int16_t *src)
> +{
> +    __asm__ volatile (
> +        "movq  (%0), %%mm0      \n\t"
> +        "movq  1*4*2(%0), %%mm1 \n\t"
> +        "paddw 6*4*2(%0), %%mm0 \n\t"
> +        "paddw 5*4*2(%0), %%mm1 \n\t"
> +        "movq  2*4*2(%0), %%mm2 \n\t"
> +        "movq  3*4*2(%0), %%mm3 \n\t"
> +        "paddw 4*4*2(%0), %%mm2 \n\t"
> +        "paddw %%mm3, %%mm3     \n\t" //s
> +        "movq %%mm3, %%mm4      \n\t" //s
> +        "psubw %%mm0, %%mm3     \n\t" //s-s0
> +        "paddw %%mm0, %%mm4     \n\t" //s+s0
> +        "movq %%mm2, %%mm0      \n\t" //s2
> +        "psubw %%mm1, %%mm2     \n\t" //s2-s1
> +        "paddw %%mm1, %%mm0     \n\t" //s2+s1
> +        "movq %%mm4, %%mm1      \n\t" //s0'
> +        "psubw %%mm0, %%mm4     \n\t" //s0'-s'
> +        "paddw %%mm0, %%mm1     \n\t" //s0'+s'
> +        "movq %%mm3, %%mm0      \n\t" //s3'
> +        "psubw %%mm2, %%mm3     \n\t"
> +        "psubw %%mm2, %%mm3     \n\t"
> +        "paddw %%mm0, %%mm2     \n\t"
> +        "paddw %%mm0, %%mm2     \n\t"
> +        "movq %%mm1, (%1)       \n\t"
> +        "movq %%mm4, 2*4*2(%1)  \n\t"
> +        "movq %%mm2, 1*4*2(%1)  \n\t"
> +        "movq %%mm3, 3*4*2(%1)  \n\t"
> +        :: "r" (src), "r"(dst)
> +    );
> +}
> +
> +av_cold void ff_pp7_init_x86(PP7Context *p)
> +{

> +#if HAVE_MMX_INLINE
> +    int cpu_flags = av_get_cpu_flags();
> +
> +    if (HAVE_MMX_INLINE && cpu_flags & AV_CPU_FLAG_MMX)
> +        p->dctB = dctB_mmx;
> +#endif

the 2 HAVE_MMX_INLINE are redundant relative to each other

also the actual asm should possibly be under HAVE_MMX_INLINE as well

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141227/1cd6b638/attachment.asc>