[FFmpeg-soc] libavfilter audio work - qualification task

Stefano Sabatini stefano.sabatini-lala at poste.it
Mon Apr 19 21:17:55 CEST 2010


On date Monday 2010-04-19 00:42:37 -0700, S.N. Hemanth Meenakshisundaram encoded:
> On 04/18/2010 07:44 PM, S.N. Hemanth Meenakshisundaram wrote:
> >On 04/18/2010 05:10 PM, Stefano Sabatini wrote:
> >>On date Sunday 2010-04-18 16:10:35 -0700, S.N. Hemanth
> >>Meenakshisundaram encoded:
> >>>On 04/13/2010 12:05 AM, Stefano Sabatini wrote:
> >>>>On date Monday 2010-04-12 22:52:44 -0700, S.N. Hemanth
> >>>>Meenakshisundaram encoded:
> >>>>[...]
> >>>>>I had one (dumb?) question about yadif :
> >>>>>
> >>>>>Since the PicRef structure in libavfilter has no 'fields' member,
> >>>>>how do I find out if the image data in the buffer is
> >>>>>MP_IMGFIELD_ORDERED, MP_IMGFIELD_TOP_FIRST etc. Is it indicated in
> >>>>>the PixelFormat?
> >>>>Ehm no, I believe we need to add it to the AVFilterPicRef struct.
> >>>>
> >>>Hi,
> >>>
> >>>I have a couple more questions :
> >>>
> >>>1. It looks like we output two frames for every input frame/field if
> >>>mode is 1 or 3, else we output a single frame. Is this right?
> >>> From the manual page of mplayer:
> >>
> >>        yadif=[mode[:field_dominance]]
> >>               Yet another deinterlacing filter
> >><mode>
> >>                       0: Output 1 frame for each frame.
> >>                       1: Output 1 frame for each field.
> >>                       2: Like 0 but skips spatial interlacing check.
> >>                       3: Like 1 but skips spatial interlacing check.
> >>
> >>So yes you look right.
> >>
> >>>2. The mplayer code has a correct_pts variable that seems to allow
> >>>doubling of frame rate. Here is it safe to assume this is always
> >>>allowed/true?
> >>I suppose so, that will also depend on the mode, from what I can
> >>understand you'll need to duplicate the frame rate only with mode 1/3,
> >>this will be done manipulating the output pts.
> >>
> >>>3. If I add a 'fields' member to AVFilterPicRef struct, how do I
> >>>make sure it will be set to the right value (i.e. TOP_FIRST etc)?
> >>It is a problem of the calling application to set this, for example an
> >>application requests a picref with avfilter_get_video_buffer() and
> >>sets the field accordingly. In the case of ffmpeg.c/ffplay.c this
> >>depends on the options set in the codec context or by the user
> >>(e.g. -top_field 1).
> >>
> >>>Thanks
> >>Regards.
> >Thank you. Attached is an initial vf_yadif.c - I have only
> >compiled it yet and will start testing it shortly. A couple of
> >questions :
> >
> >1. I wasn't sure how to modify the header since we wanted to keep
> >yadif under the GPL while the rest of FFmpeg is under the LGPL.

Add:
yadif_filter_deps=gpl

in configure.

> >
> >2. Is the PixelFormat YUV420P equivalent to YV12 or IYUV/I420. The
> >description for YUV420P doesn't seem to say if the 2nd and 3rd
> >planes are VU or UV. Where can I find the other PixelFormat
> >equivalent.

As for the ffmpeg pixfmt ones, you can find a description of each
format in libavutil/pixfmt.h and libavutil/pixdesc.h.

> >
> >Regards,
> 
> Hi,
> 
> Videos do play with vf_yadif in the filter chain, but there are a
> couple of problems right now. Am working on fixing them.
> 
> The first is a memory corruption reported by valgrind :
> 
> ==30358== Conditional jump or move depends on uninitialised value(s)
> ==30358==    at 0x418218: filter_line_c (vf_yadif.c:331)
> ==30358==    by 0x418762: end_frame (vf_yadif.c:381)
> 
> ==30358==  Uninitialised value was created by a heap allocation
> [...]
> ==30358==    by 0x90D187: av_malloc (mem.c:83)
> ==30358==    by 0x4183D1: config_props_input (vf_yadif.c:405)
> 
> The offending piece of code seems to be :
> 
> filter_line_c :
> 
>         int spatial_score= FFABS(cur[-refs-1] - cur[+refs-1]) + FFABS(c-e)
>                          + FFABS(cur[-refs+1] - cur[+refs+1]) - 1;
> 
> #define CHECK(j)\
>     {   int score= FFABS(cur[-refs-1+j] - cur[+refs-1-j])\
>                  + FFABS(cur[-refs  +j] - cur[+refs  -j])\
>                  + FFABS(cur[-refs+1+j] - cur[+refs+1-j]);\
>         if(score < spatial_score){\
>             spatial_score= score;\
>             spatial_pred= (cur[-refs  +j] + cur[+refs  -j])>>1;\
> 
> Line 331:     CHECK(-1) CHECK(-2) }} }}
> 
> The allocation is here :
> 
>     for(i=0; i<3; i++) {
>         int is_chroma= !!i;
>         int w= ((link->w     + 31) & (~31))>>is_chroma;
>         int h= ((link->h + 6 + 31) & (~31))>>is_chroma;
> 
>         av_log(NULL, AV_LOG_ERROR, "Alloc dims are : '%d, %d, %d,
> %ld'\n", w, h, i, yadif);
>         yadif->stride[i]= w;
>         for(j=0; j<3; j++)
>             yadif->ref[j][i]= (uint8_t
> *)(av_malloc(w*h*sizeof(uint8_t)))+3*w;
> 
> 
> Am wondering if this is because the store_refs function does a :
> 
>     memcpy (p->ref[3], p->ref[0], sizeof(uint8_t *)*3);
>     memmove(p->ref[0], p->ref[1], sizeof(uint8_t *)*3*3);
> 
> Isn't ref[3] unallocated at this time? This code is from the mplayer
> yadif, so I guess am doing something wrong here.
> 
> 
> The 2nd issue is strange and might be related :
> 
> The av_log statement above
> 
>         av_log(NULL, AV_LOG_ERROR, "Alloc dims are : '%d, %d, %d,
> %ld'\n", w, h, i, yadif);
> 
> throws a segmentation fault in av_log code if I use yadif (which is
> a YadifContext * pointer) instead of NULL as the logging context. I
> tried logging the context pointer and it is not NULL and appears
> valid.
> 
> Am working on this, but please let me know if there's something I am
> missing. Full diff is again attached.
> 
> Thanks,

> Index: vf_yadif.c
> ===================================================================
> --- vf_yadif.c	(revision 0)
> +++ vf_yadif.c	(revision 0)
> @@ -0,0 +1,534 @@
> +/*
> + * Copyright (C) 2006 Michael Niedermayer <michaelni at gmx.at>
> + *
> + * This file is part of MPlayer.
> + *
> + * MPlayer 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.
> + *
> + * MPlayer 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 MPlayer; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +/**
> + * @file libavfilter/vf_yadif.c
> + * Yet Another Deinterlacing Filter
> + *
> + */
> +
> +#include "avfilter.h"
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <inttypes.h>
> +#include <math.h>
> +#include "config.h"

many of these are not required if you include "avfilter.h", also
"config.h" should be removed.

> +typedef struct {
> +    int mode;
> +    int parity;
> +    int buffered_tff;
> +    double buffered_pts;
> +    AVFilterPicRef *buffered_pic;
> +    int stride[3];
> +    uint8_t *ref[4][3];
> +} YadifContext;

Please document all these fields.

> +static void (*filter_line)(YadifContext *p, uint8_t *dst, uint8_t *prev, uint8_t *cur, uint8_t *next, int w, int refs, int parity);
> +
> +static inline void * memcpy_pic(void * dst, const void * src,
> +                                int bytesPerLine, int height,
> +                                int dstStride, int srcStride)

nits: use K&R style, plain_style variables.

> +{
> +        int i;
> +        void *retval=dst;
> +
> +        if(dstStride == srcStride)
> +        {
> +                if (srcStride < 0) {
> +                        src = (const uint8_t*)src + (height-1)*srcStride;
> +                        dst = (uint8_t*)dst + (height-1)*dstStride;
> +                        srcStride = -srcStride;
> +                }
> +
> +                memcpy(dst, src, srcStride*height);
> +        }
> +        else
> +        {
> +                for(i=0; i<height; i++)
> +                {
> +                        memcpy(dst, src, bytesPerLine);
> +                        src = (const uint8_t*)src + srcStride;
> +                        dst = (uint8_t*)dst + dstStride;
> +                }
> +        }
> +
> +        return retval;
> +}

> +static void store_ref(YadifContext *p, uint8_t *src[3], int src_stride[3], int width, int height) {

p is not a good name, yadif or something like that is better.

> +    int i;
> +
> +    memcpy (p->ref[3], p->ref[0], sizeof(uint8_t *)*3);
> +    memmove(p->ref[0], p->ref[1], sizeof(uint8_t *)*3*3);
> +
> +    for(i=0; i<3; i++){
> +        int is_chroma= !!i;
> +
> +        memcpy_pic(p->ref[2][i], src[i], width>>is_chroma, height>>is_chroma, p->stride[i], src_stride[i]);
> +    }
> +}
> +
> +#if HAVE_MMX && defined(NAMED_ASM_ARGS)
> +
> +#define LOAD4(mem,dst) \
> +            "movd      "mem", "#dst" \n\t"\
> +            "punpcklbw %%mm7, "#dst" \n\t"
> +
> +#define PABS(tmp,dst) \
> +            "pxor     "#tmp", "#tmp" \n\t"\
> +            "psubw    "#dst", "#tmp" \n\t"\
> +            "pmaxsw   "#tmp", "#dst" \n\t"
> +
> +#define CHECK(pj,mj) \
> +            "movq "#pj"(%[cur],%[mrefs]), %%mm2 \n\t" /* cur[x-refs-1+j] */\
> +            "movq "#mj"(%[cur],%[prefs]), %%mm3 \n\t" /* cur[x+refs-1-j] */\
> +            "movq      %%mm2, %%mm4 \n\t"\
> +            "movq      %%mm2, %%mm5 \n\t"\
> +            "pxor      %%mm3, %%mm4 \n\t"\
> +            "pavgb     %%mm3, %%mm5 \n\t"\
> +            "pand     %[pb1], %%mm4 \n\t"\
> +            "psubusb   %%mm4, %%mm5 \n\t"\
> +            "psrlq     $8,    %%mm5 \n\t"\
> +            "punpcklbw %%mm7, %%mm5 \n\t" /* (cur[x-refs+j] + cur[x+refs-j])>>1 */\
> +            "movq      %%mm2, %%mm4 \n\t"\
> +            "psubusb   %%mm3, %%mm2 \n\t"\
> +            "psubusb   %%mm4, %%mm3 \n\t"\
> +            "pmaxub    %%mm3, %%mm2 \n\t"\
> +            "movq      %%mm2, %%mm3 \n\t"\
> +            "movq      %%mm2, %%mm4 \n\t" /* ABS(cur[x-refs-1+j] - cur[x+refs-1-j]) */\
> +            "psrlq      $8,   %%mm3 \n\t" /* ABS(cur[x-refs  +j] - cur[x+refs  -j]) */\
> +            "psrlq     $16,   %%mm4 \n\t" /* ABS(cur[x-refs+1+j] - cur[x+refs+1-j]) */\
> +            "punpcklbw %%mm7, %%mm2 \n\t"\
> +            "punpcklbw %%mm7, %%mm3 \n\t"\
> +            "punpcklbw %%mm7, %%mm4 \n\t"\
> +            "paddw     %%mm3, %%mm2 \n\t"\
> +            "paddw     %%mm4, %%mm2 \n\t" /* score */
> +
> +#define CHECK1 \
> +            "movq      %%mm0, %%mm3 \n\t"\
> +            "pcmpgtw   %%mm2, %%mm3 \n\t" /* if(score < spatial_score) */\
> +            "pminsw    %%mm2, %%mm0 \n\t" /* spatial_score= score; */\
> +            "movq      %%mm3, %%mm6 \n\t"\
> +            "pand      %%mm3, %%mm5 \n\t"\
> +            "pandn     %%mm1, %%mm3 \n\t"\
> +            "por       %%mm5, %%mm3 \n\t"\
> +            "movq      %%mm3, %%mm1 \n\t" /* spatial_pred= (cur[x-refs+j] + cur[x+refs-j])>>1; */
> +
> +#define CHECK2 /* pretend not to have checked dir=2 if dir=1 was bad.\
> +                  hurts both quality and speed, but matches the C version. */\
> +            "paddw    %[pw1], %%mm6 \n\t"\
> +            "psllw     $14,   %%mm6 \n\t"\
> +            "paddsw    %%mm6, %%mm2 \n\t"\
> +            "movq      %%mm0, %%mm3 \n\t"\
> +            "pcmpgtw   %%mm2, %%mm3 \n\t"\
> +            "pminsw    %%mm2, %%mm0 \n\t"\
> +            "pand      %%mm3, %%mm5 \n\t"\
> +            "pandn     %%mm1, %%mm3 \n\t"\
> +            "por       %%mm5, %%mm3 \n\t"\
> +            "movq      %%mm3, %%mm1 \n\t"
> +
> +static void filter_line_mmx2(YadifContext *p, uint8_t *dst, uint8_t *prev, uint8_t *cur, uint8_t *next, int w, int refs, int parity){
> +    static const uint64_t pw_1 = 0x0001000100010001ULL;
> +    static const uint64_t pb_1 = 0x0101010101010101ULL;
> +    const int mode = p->mode;
> +    uint64_t tmp0, tmp1, tmp2, tmp3;
> +    int x;
> +
> +#define FILTER\
> +    for(x=0; x<w; x+=4){\
> +        __asm__ volatile(\
> +            "pxor      %%mm7, %%mm7 \n\t"\
> +            LOAD4("(%[cur],%[mrefs])", %%mm0) /* c = cur[x-refs] */\
> +            LOAD4("(%[cur],%[prefs])", %%mm1) /* e = cur[x+refs] */\
> +            LOAD4("(%["prev2"])", %%mm2) /* prev2[x] */\
> +            LOAD4("(%["next2"])", %%mm3) /* next2[x] */\
> +            "movq      %%mm3, %%mm4 \n\t"\
> +            "paddw     %%mm2, %%mm3 \n\t"\
> +            "psraw     $1,    %%mm3 \n\t" /* d = (prev2[x] + next2[x])>>1 */\
> +            "movq      %%mm0, %[tmp0] \n\t" /* c */\
> +            "movq      %%mm3, %[tmp1] \n\t" /* d */\
> +            "movq      %%mm1, %[tmp2] \n\t" /* e */\
> +            "psubw     %%mm4, %%mm2 \n\t"\
> +            PABS(      %%mm4, %%mm2) /* temporal_diff0 */\
> +            LOAD4("(%[prev],%[mrefs])", %%mm3) /* prev[x-refs] */\
> +            LOAD4("(%[prev],%[prefs])", %%mm4) /* prev[x+refs] */\
> +            "psubw     %%mm0, %%mm3 \n\t"\
> +            "psubw     %%mm1, %%mm4 \n\t"\
> +            PABS(      %%mm5, %%mm3)\
> +            PABS(      %%mm5, %%mm4)\
> +            "paddw     %%mm4, %%mm3 \n\t" /* temporal_diff1 */\
> +            "psrlw     $1,    %%mm2 \n\t"\
> +            "psrlw     $1,    %%mm3 \n\t"\
> +            "pmaxsw    %%mm3, %%mm2 \n\t"\
> +            LOAD4("(%[next],%[mrefs])", %%mm3) /* next[x-refs] */\
> +            LOAD4("(%[next],%[prefs])", %%mm4) /* next[x+refs] */\
> +            "psubw     %%mm0, %%mm3 \n\t"\
> +            "psubw     %%mm1, %%mm4 \n\t"\
> +            PABS(      %%mm5, %%mm3)\
> +            PABS(      %%mm5, %%mm4)\
> +            "paddw     %%mm4, %%mm3 \n\t" /* temporal_diff2 */\
> +            "psrlw     $1,    %%mm3 \n\t"\
> +            "pmaxsw    %%mm3, %%mm2 \n\t"\
> +            "movq      %%mm2, %[tmp3] \n\t" /* diff */\
> +\
> +            "paddw     %%mm0, %%mm1 \n\t"\
> +            "paddw     %%mm0, %%mm0 \n\t"\
> +            "psubw     %%mm1, %%mm0 \n\t"\
> +            "psrlw     $1,    %%mm1 \n\t" /* spatial_pred */\
> +            PABS(      %%mm2, %%mm0)      /* ABS(c-e) */\
> +\
> +            "movq -1(%[cur],%[mrefs]), %%mm2 \n\t" /* cur[x-refs-1] */\
> +            "movq -1(%[cur],%[prefs]), %%mm3 \n\t" /* cur[x+refs-1] */\
> +            "movq      %%mm2, %%mm4 \n\t"\
> +            "psubusb   %%mm3, %%mm2 \n\t"\
> +            "psubusb   %%mm4, %%mm3 \n\t"\
> +            "pmaxub    %%mm3, %%mm2 \n\t"\
> +            "pshufw $9,%%mm2, %%mm3 \n\t"\
> +            "punpcklbw %%mm7, %%mm2 \n\t" /* ABS(cur[x-refs-1] - cur[x+refs-1]) */\
> +            "punpcklbw %%mm7, %%mm3 \n\t" /* ABS(cur[x-refs+1] - cur[x+refs+1]) */\
> +            "paddw     %%mm2, %%mm0 \n\t"\
> +            "paddw     %%mm3, %%mm0 \n\t"\
> +            "psubw    %[pw1], %%mm0 \n\t" /* spatial_score */\
> +\
> +            CHECK(-2,0)\
> +            CHECK1\
> +            CHECK(-3,1)\
> +            CHECK2\
> +            CHECK(0,-2)\
> +            CHECK1\
> +            CHECK(1,-3)\
> +            CHECK2\
> +\
> +            /* if(p->mode<2) ... */\
> +            "movq    %[tmp3], %%mm6 \n\t" /* diff */\
> +            "cmp       $2, %[mode] \n\t"\
> +            "jge       1f \n\t"\
> +            LOAD4("(%["prev2"],%[mrefs],2)", %%mm2) /* prev2[x-2*refs] */\
> +            LOAD4("(%["next2"],%[mrefs],2)", %%mm4) /* next2[x-2*refs] */\
> +            LOAD4("(%["prev2"],%[prefs],2)", %%mm3) /* prev2[x+2*refs] */\
> +            LOAD4("(%["next2"],%[prefs],2)", %%mm5) /* next2[x+2*refs] */\
> +            "paddw     %%mm4, %%mm2 \n\t"\
> +            "paddw     %%mm5, %%mm3 \n\t"\
> +            "psrlw     $1,    %%mm2 \n\t" /* b */\
> +            "psrlw     $1,    %%mm3 \n\t" /* f */\
> +            "movq    %[tmp0], %%mm4 \n\t" /* c */\
> +            "movq    %[tmp1], %%mm5 \n\t" /* d */\
> +            "movq    %[tmp2], %%mm7 \n\t" /* e */\
> +            "psubw     %%mm4, %%mm2 \n\t" /* b-c */\
> +            "psubw     %%mm7, %%mm3 \n\t" /* f-e */\
> +            "movq      %%mm5, %%mm0 \n\t"\
> +            "psubw     %%mm4, %%mm5 \n\t" /* d-c */\
> +            "psubw     %%mm7, %%mm0 \n\t" /* d-e */\
> +            "movq      %%mm2, %%mm4 \n\t"\
> +            "pminsw    %%mm3, %%mm2 \n\t"\
> +            "pmaxsw    %%mm4, %%mm3 \n\t"\
> +            "pmaxsw    %%mm5, %%mm2 \n\t"\
> +            "pminsw    %%mm5, %%mm3 \n\t"\
> +            "pmaxsw    %%mm0, %%mm2 \n\t" /* max */\
> +            "pminsw    %%mm0, %%mm3 \n\t" /* min */\
> +            "pxor      %%mm4, %%mm4 \n\t"\
> +            "pmaxsw    %%mm3, %%mm6 \n\t"\
> +            "psubw     %%mm2, %%mm4 \n\t" /* -max */\
> +            "pmaxsw    %%mm4, %%mm6 \n\t" /* diff= MAX3(diff, min, -max); */\
> +            "1: \n\t"\
> +\
> +            "movq    %[tmp1], %%mm2 \n\t" /* d */\
> +            "movq      %%mm2, %%mm3 \n\t"\
> +            "psubw     %%mm6, %%mm2 \n\t" /* d-diff */\
> +            "paddw     %%mm6, %%mm3 \n\t" /* d+diff */\
> +            "pmaxsw    %%mm2, %%mm1 \n\t"\
> +            "pminsw    %%mm3, %%mm1 \n\t" /* d = clip(spatial_pred, d-diff, d+diff); */\
> +            "packuswb  %%mm1, %%mm1 \n\t"\
> +\
> +            :[tmp0]"=m"(tmp0),\
> +             [tmp1]"=m"(tmp1),\
> +             [tmp2]"=m"(tmp2),\
> +             [tmp3]"=m"(tmp3)\
> +            :[prev] "r"(prev),\
> +             [cur]  "r"(cur),\
> +             [next] "r"(next),\
> +             [prefs]"r"((x86_reg)refs),\
> +             [mrefs]"r"((x86_reg)-refs),\
> +             [pw1]  "m"(pw_1),\
> +             [pb1]  "m"(pb_1),\
> +             [mode] "g"(mode)\
> +        );\
> +        __asm__ volatile("movd %%mm1, %0" :"=m"(*dst));\
> +        dst += 4;\
> +        prev+= 4;\
> +        cur += 4;\
> +        next+= 4;\
> +    }
> +
> +    if(parity){
> +#define prev2 "prev"
> +#define next2 "cur"
> +        FILTER
> +#undef prev2
> +#undef next2
> +    }else{
> +#define prev2 "cur"
> +#define next2 "next"
> +        FILTER
> +#undef prev2
> +#undef next2
> +    }
> +}
> +#undef LOAD4
> +#undef PABS
> +#undef CHECK
> +#undef CHECK1
> +#undef CHECK2
> +#undef FILTER
> +
> +#endif /* HAVE_MMX && defined(NAMED_ASM_ARGS) */

can't comment on asm code, hint: you can skip it for the first review
if you like, this can be added when we have the first working C
implementation.

> +static void filter_line_c(YadifContext *p, uint8_t *dst, uint8_t *prev, uint8_t *cur, uint8_t *next, int w, int refs, int parity){

Split this overly long line, also p is not a good name.

> +    int x;
> +    uint8_t *prev2= parity ? prev : cur ;
> +    uint8_t *next2= parity ? cur  : next;
> +    for(x=0; x<w; x++){
> +        int c= cur[-refs];
> +        int d= (prev2[0] + next2[0])>>1;
> +        int e= cur[+refs];
> +        int temporal_diff0= FFABS(prev2[0] - next2[0]);
> +        int temporal_diff1=( FFABS(prev[-refs] - c) + FFABS(prev[+refs] - e) )>>1;
> +        int temporal_diff2=( FFABS(next[-refs] - c) + FFABS(next[+refs] - e) )>>1;
> +        int diff= FFMAX3(temporal_diff0>>1, temporal_diff1, temporal_diff2);
> +        int spatial_pred= (c+e)>>1;
> +        int spatial_score= FFABS(cur[-refs-1] - cur[+refs-1]) + FFABS(c-e)
> +                         + FFABS(cur[-refs+1] - cur[+refs+1]) - 1;
> +        av_log(p, AV_LOG_ERROR, "Params are : '%d'\n", refs);

AV_LOG_DEBUG, also this will crash as YadifContext is not a proper log
context.

> +#define CHECK(j)\
> +    {   int score= FFABS(cur[-refs-1+j] - cur[+refs-1-j])\
> +                 + FFABS(cur[-refs  +j] - cur[+refs  -j])\
> +                 + FFABS(cur[-refs+1+j] - cur[+refs+1-j]);\
> +        if(score < spatial_score){\
> +            spatial_score= score;\
> +            spatial_pred= (cur[-refs  +j] + cur[+refs  -j])>>1;\
> +
> +        CHECK(-1) CHECK(-2) }} }}
> +        CHECK( 1) CHECK( 2) }} }}
> +
> +        if(p->mode<2){
> +            int b= (prev2[-2*refs] + next2[-2*refs])>>1;
> +            int f= (prev2[+2*refs] + next2[+2*refs])>>1;
> +#if 0
> +            int a= cur[-3*refs];
> +            int g= cur[+3*refs];
> +            int max= FFMAX3(d-e, d-c, FFMIN3(FFMAX(b-c,f-e),FFMAX(b-c,b-a),FFMAX(f-g,f-e)) );
> +            int min= FFMIN3(d-e, d-c, FFMAX3(FFMIN(b-c,f-e),FFMIN(b-c,b-a),FFMIN(f-g,f-e)) );
> +#else
> +            int max= FFMAX3(d-e, d-c, FFMIN(b-c, f-e));
> +            int min= FFMIN3(d-e, d-c, FFMAX(b-c, f-e));
> +#endif
> +
> +            diff= FFMAX3(diff, min, -max);
> +        }
> +
> +        if(spatial_pred > d + diff)
> +           spatial_pred = d + diff;
> +        else if(spatial_pred < d - diff)
> +           spatial_pred = d - diff;
> +
> +        dst[0] = spatial_pred;
> +
> +        dst++;
> +        cur++;
> +        prev++;
> +        next++;
> +        prev2++;
> +        next2++;
> +    }
> +}
> +
> +static void filter(YadifContext *p, uint8_t *dst[3], int dst_stride[3], int width, int height, int parity, int tff){
> +    int y, i;
> +
> +    for(i=0; i<3; i++){
> +        int is_chroma= !!i;
> +        int w= width >>is_chroma;
> +        int h= height>>is_chroma;
> +        int refs= p->stride[i];
> +
> +        for(y=0; y<h; y++){
> +            if((y ^ parity) & 1){
> +                uint8_t *prev= &p->ref[0][i][y*refs];
> +                uint8_t *cur = &p->ref[1][i][y*refs];
> +                uint8_t *next= &p->ref[2][i][y*refs];
> +                uint8_t *dst2= &dst[i][y*dst_stride[i]];
> +                filter_line(p, dst2, prev, cur, next, w, refs, parity ^ tff);
> +            }else{
> +                memcpy(&dst[i][y*dst_stride[i]], &p->ref[1][i][y*refs], w);
> +            }
> +        }
> +    }

> +#if HAVE_MMX && defined(NAMED_ASM_ARGS)
> +    if(gCpuCaps.hasMMX2) __asm__ volatile("emms \n\t" : : : "memory");
> +#endif

skip this, FFmpeg has no gCpuCaps.

> +}
> +
> +static int config_props_input(AVFilterLink *link)
> +{
> +    YadifContext *yadif = link->dst->priv;
> +    int i, j;
> +
> +    for(i=0; i<3; i++) {
> +        int is_chroma= !!i;
> +        int w= ((link->w     + 31) & (~31))>>is_chroma;
> +        int h= ((link->h + 6 + 31) & (~31))>>is_chroma;
> +
> +        av_log(NULL, AV_LOG_ERROR, "Alloc dims are : '%d, %d, %d, %ld'\n", w, h, i, yadif);

use link->dst as the log context, replace AV_LOG_ERROR -> AV_LOG_INFO
and use a notation such as: w:%d h:%d

> +        yadif->stride[i]= w;
> +        for(j=0; j<3; j++)
> +            yadif->ref[j][i]= (uint8_t *)(av_malloc(w*h*sizeof(uint8_t)))+3*w;
> +    }
> +    return 0;
> +}
> +
> +static void continue_buffered_image(AVFilterContext *ctx);

Maybe you can avoid this forward declaration re-ordering a little the
code.

> +extern int correct_pts;

Well this shouldn't be an extern int, maybe this should be a parameter
of the yadif context.

> +
> +static void start_frame(AVFilterLink *link, AVFilterPicRef *picref)
> +{
> +    AVFilterLink *out = link->dst->outputs[0];
> +
> +    out->outpic      = avfilter_get_video_buffer(out, AV_PERM_WRITE, picref->w, picref->h);
> +    out->outpic->pts = picref->pts;
> +    // out->outpic->fields = picref->fields;

Try to provide also a patch for this, it basically requires to add a 
pixel format field flags to AVFilterPicRef, check
libmpcodecs/mp_image.h for ideas, that may be something like:
#define AV_PIX_FMT_FIELD_ORDERED      0x01
#define AV_PIX_FMT_FIELD_TOP_FIRST    0x02
#define AV_PIX_FMT_FIELD_REPEAT_FIRST 0x04
...

> +    out->outpic->pos = picref->pos;
> +    avfilter_start_frame(out, avfilter_ref_pic(out->outpic, ~0)); 
> +}

> +static void end_frame(AVFilterLink *link)
> +{
> +    YadifContext *yadif = link->dst->priv;
> +    AVFilterPicRef *picref  = link->cur_pic;
> +    int tff;
> +
> +    if(yadif->parity < 0) {
> +        // TODO : Add 'fields' to PicRef and use it here
> +        /* if (picref->fields & MP_IMGFIELD_ORDERED)
> +         *     tff = !!(picref->fields & MP_IMGFIELD_TOP_FIRST);
> +         * else
> +         */
> +            tff = 1;
> +    }
> +    else
> +         tff = (yadif->parity&1)^1;
> +
> +    store_ref(yadif, picref->data, picref->linesize, picref->w, picref->h);
> +
> +    yadif->buffered_pic = picref;
> +    yadif->buffered_tff = tff;
> +    yadif->buffered_pts = picref->pts;
> +
> +    return continue_buffered_image(link->dst);
> +}
> +
> +static void continue_buffered_image(AVFilterContext *ctx)
> +{
> +    YadifContext *yadif = ctx->priv;
> +    AVFilterPicRef *picref = yadif->buffered_pic;
> +    AVFilterPicRef *dpicref = ctx->outputs[0]->outpic;
> +    AVFilterLink *out = ctx->outputs[0];
> +    int tff = yadif->buffered_tff;
> +    int i;
> +
> +    for(i = 0; i<=(yadif->mode&1); i++) {
> +        filter(yadif, dpicref->data, dpicref->linesize, picref->w, picref->h, i ^ tff ^ 1, tff);
> +        avfilter_draw_slice(out, 0, dpicref->h, 1);
> +        avfilter_end_frame(out);
> +        if (i < (yadif->mode & 1)) {
> +            dpicref      = avfilter_get_video_buffer(out, AV_PERM_WRITE, picref->w, picref->h);
> +            dpicref->pts = picref->pts + 0.02; // FIXME : Assuming 50fps here
> +            // out->outpic->fields = picref->fields;
> +            dpicref->pos = picref->pos;
> +            avfilter_start_frame(out, avfilter_ref_pic(dpicref, ~0));
> +        }
> +    }
> +    return;
> +}
> +
> +static void uninit(AVFilterContext *ctx){
> +    int i;
> +    YadifContext *yadif = ctx->priv;
> +    if(!yadif) return;
> +
> +    for(i=0; i<3*3; i++){
> +        uint8_t **p= &yadif->ref[i%3][i/3];
> +        if(*p) av_free(*p - 3*yadif->stride[i/3]);
> +        *p= NULL;
> +    }
> +}
> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> +    enum PixelFormat pix_fmts[] = {
> +        PIX_FMT_YUV420P, PIX_FMT_GRAY8, PIX_FMT_NONE
> +    };
> +
> +    avfilter_set_common_formats(ctx, avfilter_make_format_list(pix_fmts));
> +    return 0;
> +}
> +
> +static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> +{
> +    YadifContext *yadif = ctx->priv;
> +    yadif->mode = 0;
> +    yadif->parity = -1;
> +
> +    if (args)
> +        sscanf(args, "%d:%d", &yadif->mode, &yadif->parity);
> +
> +    filter_line = filter_line_c;

> +#if HAVE_MMX && defined(NAMED_ASM_ARGS)
> +    if(gCpuCaps.hasMMX2) filter_line = filter_line_mmx2;
> +#endif

same as before

> +
> +    return 0;
> +}
> +
> +AVFilter avfilter_vf_yadif =
> +{
> +    .name      = "yadif",
> +    .description = "Yet Another DeInterlacing Filter",
> +    .priv_size = sizeof(YadifContext),
> +    .init      = init,
> +    .uninit      = uninit,
> +
> +    .query_formats = query_formats,
> +
> +    .inputs    = (AVFilterPad[]) {{ .name            = "default",
> +                                    .type            = AVMEDIA_TYPE_VIDEO,
> +                                    .start_frame     = start_frame,
> +                                    .end_frame       = end_frame,
> +                                    .config_props    = config_props_input,
> +                                    .min_perms       = AV_PERM_READ, },
> +                                  { .name = NULL}},
> +    .outputs   = (AVFilterPad[]) {{ .name            = "default",
> +                                    .type            = AVMEDIA_TYPE_VIDEO, },
> +                                  { .name = NULL}},
> +};
> +

Regards.


More information about the FFmpeg-soc mailing list