[vlc-devel] [PATCH] Added IVTC deinterlacer (NTSC film mode)

Laurent Aimar fenrir at elivagar.org
Wed Jan 5 00:13:18 CET 2011


Hi,

Here is a preliminary review:

On Mon, Jan 03, 2011 at 02:33:52AM +0200, Juha Jeronen wrote:
> Anyway, here is the updated IVTC patch (in the message body this time).  
 It seems that the patch got broken by that.

> +struct ivtc_sys_t
> +{
> +    bool b_possible_cadence_break_detected;
> +    int i_telecine_field_dominance; /* Whether TFF or BFF telecine;  
> detected from the video */
> +    int i_cadence_pos; /* Cadence counter (starts once the filter locks  
> on). */
> +    int i_ivtc_filter_mode; /* Telecined or not. See RenderIVTC. */
> +    int pi_interlace_scores[IVTC_NUM_FIELD_PAIRS];
> +    int pi_cadence_pos_history[IVTC_DETECTION_HISTORY_SIZE]; /*  
> Detected cadence data (raw) */
> +    picture_t *pp_ivtc_working_area[IVTC_WORKING_AREA_SIZE]; /* Temp  
> frames; stored here to avoid reallocation */
> +    /* These are used for detecting need for reallocation of the  
> working area. */
> +    int i_old_chroma;
> +    int i_old_size_x;
> +    int i_old_size_y;
> +};
> +typedef struct ivtc_sys_t ivtc_sys_t;
 You can use a shorter way:
typedef struct
{
 [blah]
} ivtc_sys_t

>  #define HISTORY_SIZE (3)
>  struct filter_sys_t
>  {
> @@ -138,8 +163,11 @@ struct filter_sys_t
>
>      mtime_t i_last_date;
>
> -    /* Yadif */
> +    /* Yadif, IVTC */
>      picture_t *pp_history[HISTORY_SIZE];
> +
> +    /* IVTC */
> +    ivtc_sys_t *p_ivtc;
Using
 ivtc_sys_t ivtc;
here would simplify a bit.

> +/* Helper function: compose frame from given field pair.
> +
> +   The inputs are full pictures (frames); only one field will be used  
> from each.
> +   Caller must manage allocation/deallocation of p_outpic.
> +   Pitches of the inputs must match!
> +*/
 Using /** will make it doxygen compatible.

> +static void ComposeFrame( filter_t *p_filter, picture_t *p_outpic,  
> picture_t *p_inpic_top, picture_t *p_inpic_bottom )
> +{
> +    assert( p_filter != NULL );
> +    assert( p_outpic != NULL );
> +    assert( p_inpic_top != NULL );
> +    assert( p_inpic_bottom != NULL );
> +
> +    int i_plane;
> +    for( i_plane = 0 ; i_plane < p_inpic_top->i_planes ; i_plane++ )
> +    {
> +        uint8_t *p_in_top, *p_in_bottom, *p_out_end, *p_out;
> +
> +        p_in_top    = p_inpic_top->p[i_plane].p_pixels;
> +        p_in_bottom = p_inpic_bottom->p[i_plane].p_pixels;
> +
> +        p_out = p_outpic->p[i_plane].p_pixels;
> +        p_out_end = p_out + p_outpic->p[i_plane].i_pitch
> +                             * p_outpic->p[i_plane].i_visible_lines;
> +
> +        assert( p_inpic_top->p[i_plane].i_pitch ==  
> p_inpic_bottom->p[i_plane].i_pitch );
> +        switch( p_filter->fmt_in.video.i_chroma )
> +        {
> +            case VLC_CODEC_I422:
> +            case VLC_CODEC_J422:
> +                assert( p_outpic->p[i_plane].i_visible_lines % 2 == 0 );
> +
> +                /* in the frame for bottom field, skip first line,  
> which belongs to the top field */
> +                p_in_bottom += p_inpic_bottom->p[i_plane].i_pitch;
> +
> +                /* copy luma or chroma, alternating between input fields */
> +                for( ; p_out < p_out_end ; )
> +                {
> +                    vlc_memcpy( p_out, p_in_top,  
> p_inpic_top->p[i_plane].i_pitch );
> +                    p_out += p_outpic->p[i_plane].i_pitch;
> +                    vlc_memcpy( p_out, p_in_bottom,  
> p_inpic_bottom->p[i_plane].i_pitch );
> +                    p_out += p_outpic->p[i_plane].i_pitch;
> +
> +                    p_in_top    += 2 * p_inpic_top->p[i_plane].i_pitch;
> +                    p_in_bottom += 2 * p_inpic_bottom->p[i_plane].i_pitch;
> +                }
> +                break;
> +
> +            case VLC_CODEC_I420:
> +            case VLC_CODEC_J420:
> +            case VLC_CODEC_YV12:
> +                assert( p_outpic->p[i_plane].i_visible_lines % 2 == 0 );
> +
> +                /* in the frame for bottom field, skip first line,  
> which belongs to the top field */
> +                p_in_bottom += p_inpic_bottom->p[i_plane].i_pitch;
> +
> +                /* copy luma or chroma, alternating between input fields */
> +                /* FIXME: why does this strategy work the best for 420  
> chroma, too?
> +                          I would have thought we'd need to average the  
> subsampled chroma... -JJ */
 If I correctly understand your code, I would say it probably depends on
how the telecine frames where generated. And I bet they are usually created
by using one line of every two from the original I420 progressive source. If
so, you simply revert it without any loss.

> +                for( ; p_out < p_out_end ; )
> +                {
> +                    vlc_memcpy( p_out, p_in_top,  
> p_inpic_top->p[i_plane].i_pitch );
> +                    p_out += p_outpic->p[i_plane].i_pitch;
> +                    vlc_memcpy( p_out, p_in_bottom,  
> p_inpic_bottom->p[i_plane].i_pitch );
> +                    p_out += p_outpic->p[i_plane].i_pitch;
> +
> +                    p_in_top    += 2 * p_inpic_top->p[i_plane].i_pitch;
> +                    p_in_bottom += 2 * p_inpic_bottom->p[i_plane].i_pitch;
> +                }
> +                break;
 I may have missed something, but I believe the switch() is in fact useless.

> +                /* Original development version for these cases -  
> causes chroma stripes */
> +                if( 0 )
> +                {
> +                    if( i_plane == Y_PLANE )
> +                    {
> +                        /* in the frame for bottom field, skip first  
> line, which belongs to the top field */
> +                        p_in_bottom += p_inpic_bottom->p[i_plane].i_pitch;
> +
> +                        /* copy luma, alternating between input fields */
> +                        for( ; p_out < p_out_end ; )
> +                        {
> +                            vlc_memcpy( p_out, p_in_top,  
> p_inpic_top->p[i_plane].i_pitch );
> +                            p_out += p_outpic->p[i_plane].i_pitch;
> +                            vlc_memcpy( p_out, p_in_bottom,  
> p_inpic_bottom->p[i_plane].i_pitch );
> +                            p_out += p_outpic->p[i_plane].i_pitch;
> +
> +                            p_in_top    += 2 *  
> p_inpic_top->p[i_plane].i_pitch;
> +                            p_in_bottom += 2 *  
> p_inpic_bottom->p[i_plane].i_pitch;
> +                        }
> +                    }
> +                    else
> +                    {
> +                        /* now we don't skip anything - we need full  
> chroma from both frames for averaging */
> +
> +                        /* average the chroma */
> +                        for( ; p_out < p_out_end ; )
> +                        {
> +                            p_filter->p_sys->pf_merge( p_out, p_in_top,  
> p_in_bottom,
> +                                                        
> p_inpic_top->p[i_plane].i_pitch );
> +
> +                            p_out       += p_outpic->p[i_plane].i_pitch;
> +                            p_in_top    += p_inpic_top->p[i_plane].i_pitch;
> +                            p_in_bottom +=  
> p_inpic_bottom->p[i_plane].i_pitch;
> +                        }
> +                    }
> +                    break;
> +                } /* end of disabled development version */
 You should simply remove it as it is dead code.

> +        }
> +    }
> +}
> +
> +/* Helper function: estimates "how much interlaced" the given picture is.
> +
> +   We use the comb metric from Transcode. RenderX()'s comb metric was  
> also tested
> +   during development, and for this particular purpose, Transcode's  
> metric won.
> +
> +   Note that we *must not* subsample at all in order to catch  
> interlacing in telecined frames
> +   with localized motion (e.g. anime with characters talking, where  
> only mouths move
> +   and everything else stays still. These can be just a few pixels  
> wide, and located
> +   anywhere in the frame!)
> +*/
> +static int CalculateInterlaceScore(filter_t *p_filter, picture_t* p_pic)
> +{
> +    assert( p_filter != NULL );
> +    assert( p_pic != NULL );
> +
> +    /* This is based on the comb detector used in the IVTC filter of  
> Transcode 1.1.5. */
 Is it your own reimplementation of the algorithm?

> +    int i_plane;
> +    int i_score = 0;
> +    int y, x;
  Moving x/y inside the loop is a tiny bit better.

> +    for( i_plane = 0 ; i_plane < p_pic->i_planes ; i_plane++ )
> +    {
> +        const int i_lasty = p_pic->p[i_plane].i_visible_lines-1;
> +        const int w = p_pic->p[i_plane].i_pitch;
> +        for( y = 1; y < i_lasty; y+=2 )
> +        {
> +            uint8_t *src_c = &p_pic->p[i_plane].p_pixels[y*w];     /*  
> current line */
> +            uint8_t *src_p = &p_pic->p[i_plane].p_pixels[(y-1)*w]; /*  
> previous line */
> +            uint8_t *src_n = &p_pic->p[i_plane].p_pixels[(y+1)*w]; /*  
> next line */
> +
> +#define T 100
> +#ifdef CAN_COMPILE_MMXEXT
> +            /* FIXME: this MMX version probably requires some tuning.  
> My MMX-fu is somewhat lacking :)
> +               See below for the C-only version to see more quickly  
> what this does. It's very simple. -JJ */
> +            if( vlc_CPU() & CPU_CAPABILITY_MMXEXT )
> +            {
> +                assert( w % 4 == 0 );
> +
> +                pxor_r2r( mm7, mm7 ); /* we need one register full of  
> zeros */
> +                for( x = 0; x < w; x+=4 )
> +                {
> +                    /* clear working registers */
> +                    pxor_r2r( mm0, mm0 );
> +                    pxor_r2r( mm1, mm1 );
> +                    pxor_r2r( mm2, mm2 );
> +
> +                    /* move in four bytes from each row */
> +                    movd_m2r( *((int32_t*)src_c), mm0 );
> +                    movd_m2r( *((int32_t*)src_p), mm1 );
> +                    movd_m2r( *((int32_t*)src_n), mm2 );
> +
> +                    /* pad with zeros to make room for computation */
> +                    punpcklbw_r2r( mm7, mm0 );
> +                    punpcklbw_r2r( mm7, mm1 );
> +                    punpcklbw_r2r( mm7, mm2 );
> +
> +                                          /* Let's follow here what  
> happens to each component word: */
> +                    movq_r2r( mm1, mm3 ); /* P */
> +                    psubsw_r2r( mm0, mm3 ); /* mm3 = P - C  (signed) */
> +                    movq_r2r( mm2, mm4 ); /* N */
> +                    psubsw_r2r( mm0, mm4 ); /* mm4 = N - C  (signed) */
> +
> +                    /* (P - C) * (N - C) */
> +                    movq_r2r( mm4, mm0);
> +                    pmullw_r2r( mm3, mm4 ); /* mm4 = lw( mm3 * mm4 )  
> (this is now lw of "comb") */
> +                    pmulhw_r2r( mm3, mm0 ); /* mm0 = hw( mm3 * mm0 )  
> (this is now hw of "comb") */
> +
> +                    /* unpack the two low-order results into mm1 (its  
> old value is no longer needed) */
> +                    movq_r2r( mm4, mm1 );
> +                    punpcklbw_r2r( mm0, mm1 );
> +
> +                    /* unpack the two high-order results into mm2 (its  
> old value is no longer needed) */
> +                    movq_r2r( mm4, mm2 );
> +                    punpckhbw_r2r( mm0, mm2 );
> +
> +                    /* FIXME: we need a signed comparison here. Get rid  
> of the negative values first?
> +                              But how, since there is no paddsd  
> instruction?
> +
> +                              Let's stop with the MMX here, transfer  
> the results to memory and
> +                              do the rest the old-fashioned way. */
> +
> +                    /* Let's "evacuate" mm1 first. */
> +                    /* Low-order part: */
> +                    movq_r2r( mm1, mm0 );
> +                    punpckldq_r2r( mm7, mm0 );
> +                    int32_t result1;
> +                    movq_r2m( mm0, result1 );
> +                    /* High-order part: */
> +                    movq_r2r( mm1, mm0 );
> +                    punpckhdq_r2r( mm7, mm0 );
> +                    int32_t result2;
> +                    movq_r2m( mm0, result2 );
> +                    /* Then same for mm2. */
> +                    /* Low-order part: */
> +                    movq_r2r( mm2, mm0 );
> +                    punpckldq_r2r( mm7, mm0 );
> +                    int32_t result3;
> +                    movq_r2m( mm0, result3 );
> +                    /* High-order part: */
> +                    movq_r2r( mm2, mm0 );
> +                    punpckhdq_r2r( mm7, mm0 );
> +                    int32_t result4;
> +                    movq_r2m( mm0, result4 );
 movq (for the 4 assignments) cannot do what you want, making the whole
function always returning 0 (at least for me)... If you also end up with
a non valid value, I fear that some of the (quality) tests you did are not
meaningful.

> +
> +                    if( result1 > T )
> +                        ++i_score;
> +                    if( result2 > T )
> +                        ++i_score;
> +                    if( result3 > T )
> +                        ++i_score;
> +                    if( result4 > T )
> +                        ++i_score;
> +
> +                    src_c += 4;
> +                    src_p += 4;
> +                    src_n += 4;
> +                }
> +            }
> +            else
> +            {
> +#endif
> +                for( x = 0; x < w; ++x )
> +                {
> +                    /* worst case: need 17 bits for "comb" */
> +                    int_fast32_t C = *src_c;
> +                    int_fast32_t P = *src_p;
> +                    int_fast32_t N = *src_n;
> +                    /* Comments in Transcode's filter_ivtc.c attribute  
> this combing metric to Gunnar Thalin.
> +
> +                       It seems the idea is that if the picture is  
> interlaced, both expressions will have
> +                       the same signs, and this comes up positive. The  
> original author has chosen the
> +                       "T = 100" carefully... -JJ
> +                    */
> +                    int_fast32_t comb = (P - C) * (N - C);
> +                    if( comb > T )
> +                        ++i_score;
> +
> +                    ++src_c;
> +                    ++src_p;
> +                    ++src_n;
> +                }
> +#ifdef CAN_COMPILE_MMXEXT
> +            }
> +#endif
> +        }
> +    }
> +
> +#ifdef CAN_COMPILE_MMXEXT
> +    if( vlc_CPU() & CPU_CAPABILITY_MMXEXT )
> +        emms();
> +#endif
> +
> +    return i_score;
> +}
> +#undef T
> +
> +
> +/* IVTC filter modes */
> +#define IVTC_MODE_DETECTING      0
> +#define IVTC_MODE_TELECINED_NTSC 1
> +
> +/* Field pair combinations from successive frames in the PCN stencil
> +   (T = top, B = bottom, P = previous, C = current, N = next).
> +   We will use these as array indices; hence the explicit numbering. */
> +enum ivtc_field_pair { FIELD_PAIR_TPBP = 0, FIELD_PAIR_TPBC = 1,
> +                       FIELD_PAIR_TCBP = 2, FIELD_PAIR_TCBC = 3,
> +                       FIELD_PAIR_TCBN = 4, FIELD_PAIR_TNBC = 5,
> +                       FIELD_PAIR_TNBN = 6 };
> +typedef enum ivtc_field_pair ivtc_field_pair;
typedef enum {
 [blah]
} ivtc_field_pair;
is enough.

> +static int RenderIVTC( filter_t *p_filter, picture_t *p_dst, picture_t  
> *p_src )
> +{
> +    filter_sys_t *p_sys = p_filter->p_sys;
> +    ivtc_sys_t *p_ivtc  = p_sys->p_ivtc;
> +    mtime_t t_final = VLC_TS_INVALID; /* for timestamp mangling */
> +
> +    /* We keep a three-frame cache like Yadif does.
> +       We use this for telecine sequence detection and film frame  
> reconstruction. */
> +
> +    picture_t *p_dup = picture_NewFromFormat( &p_src->format );
> +    if( p_dup )
> +        picture_Copy( p_dup, p_src );
> +
> +    /* Slide the history */
> +    if( p_sys->pp_history[0] )
> +        picture_Release( p_sys->pp_history[0]  );
> +    for( int i = 1; i < HISTORY_SIZE; i++ )
> +        p_sys->pp_history[i-1] = p_sys->pp_history[i];
> +    p_sys->pp_history[HISTORY_SIZE-1] = p_dup;
> +
> +    picture_t *p_prev = p_sys->pp_history[0];
> +    picture_t *p_curr = p_sys->pp_history[1];
> +    picture_t *p_next = p_sys->pp_history[2];
> +
> +    /* Slide history of detected cadence positions */
> +    for( int i = 1; i < IVTC_DETECTION_HISTORY_SIZE; i++ )
> +        p_ivtc->pi_cadence_pos_history[i-1] =  
> p_ivtc->pi_cadence_pos_history[i];
> +    /* The latest position has not been detected yet. */
> +    p_ivtc->pi_cadence_pos_history[IVTC_DETECTION_HISTORY_SIZE-1] =  
> CADENCE_POS_INVALID;
> +
> +    /* Slide history of field pair interlace scores */
> +    p_ivtc->pi_interlace_scores[FIELD_PAIR_TPBP] =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TCBC];
> +    p_ivtc->pi_interlace_scores[FIELD_PAIR_TPBC] =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TCBN];
> +    p_ivtc->pi_interlace_scores[FIELD_PAIR_TCBP] =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBC];
> +    p_ivtc->pi_interlace_scores[FIELD_PAIR_TCBC] =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBN];
> +
> +    /* Allocate/reallocate working area used for composition of  
> artificial frames.
> +       We avoid reallocation if possible, and only do that if the video  
> size or chroma type changes.
> +       Size is detected from the luma plane. The final deallocation is  
> eventually done by Flush().
> +    */
> +    assert( p_next != NULL );
> +    int i_chroma = p_filter->fmt_in.video.i_chroma;
> +    int i_size_x = p_next->p[Y_PLANE].i_pitch;
> +    int i_size_y = p_next->p[Y_PLANE].i_visible_lines;
> +    /* Note that both frames in the working area are always allocated  
> at the same time,
> +       so it is enough to check [0]. */
 As you fill p_history with pictures created by picture_NewFromFormat()
and as you can consider that the video size will not dynamically change,
it is not needed to support size changes here. It will remove a lot of code.

> +    /* The artificial frames TNBC and TCBN will actually be generated a  
> few lines further down;
> +       we just give the pointers more descriptive names now. */
> +    picture_t *p_tnbc = p_ivtc->pp_ivtc_working_area[0];
> +    picture_t *p_tcbn = p_ivtc->pp_ivtc_working_area[1];
> +
> +    /* Filter if we have all the pictures we need */
> +    if( p_prev && p_curr && p_next )
> +    {
> +        assert( p_tnbc != NULL );
> +        assert( p_tcbn != NULL );
> +
> +        /* Start of cadence detection. */
> +
> +        /* Generate artificial frames TNBC and TCBN into the temporary  
> working area. */
> +        ComposeFrame( p_filter, p_tnbc, p_next, p_curr );
> +        ComposeFrame( p_filter, p_tcbn, p_curr, p_next );
> +
> +        /* Compute interlace scores for TNBN, TNBC and TCBN. Note that  
> p_next contains TNBN. */
> +        p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBN] =  
> CalculateInterlaceScore( p_filter, p_next );
> +        p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBC] =  
> CalculateInterlaceScore( p_filter, p_tnbc );
> +        p_ivtc->pi_interlace_scores[FIELD_PAIR_TCBN] =  
> CalculateInterlaceScore( p_filter, p_tcbn );
> +
> +        /* Detect likely cadence position according to the tables,  
> using the tabulated combinations
> +            of all 7 available interlace scores. */
> +        int pi_ivtc_scores[NUM_CADENCE_POS];
> +        for( int i = 0; i < NUM_CADENCE_POS; i++ )
> +            pi_ivtc_scores[i] = p_ivtc->pi_interlace_scores[  
> pi_best_field_pairs[i][0] ]
> +                              + p_ivtc->pi_interlace_scores[  
> pi_best_field_pairs[i][1] ]
> +                              + p_ivtc->pi_interlace_scores[  
> pi_best_field_pairs[i][2] ];
> +        /* Find minimum */
> +        int j = 0;
> +        int minscore = pi_ivtc_scores[j];
> +        for( int i = 1; i < NUM_CADENCE_POS; i++ )
> +        {
> +            if( pi_ivtc_scores[i] < minscore )
> +            {
> +                minscore = pi_ivtc_scores[i];
> +                j = i;
> +            }
> +        }
> +        /* The current raw detected cadence position is: */
> +        p_ivtc->pi_cadence_pos_history[IVTC_DETECTION_HISTORY_SIZE-1] = j;
> +
> +        /* End of cadence detection. */
> +        /* Start of cadence analysis. */
> +
> +        /* Remember old state of "potential cadence break", so that we  
> can detect two breaks in a row
> +           after the cadence analysis is done. We allow for one error  
> (which could be a fluke), and
> +           only exit film mode if two breaks are detected in a row.
> +        */
> +        const bool b_old_possible_cadence_break_detected =  
> p_ivtc->b_possible_cadence_break_detected;
> +
> +        bool b_film_mode = p_ivtc->i_ivtc_filter_mode ==  
> IVTC_MODE_TELECINED_NTSC;
> +
> +        /* If the detection history has been completely filled (three  
> positions of the stencil), start analysis.
> +           See if the picked up sequenceis a valid NTSC telecine. The  
> history is complete if its farthest past
> +           element has been filled.
> +        */
> +        if( p_ivtc->pi_cadence_pos_history[0] != CADENCE_POS_INVALID )
> +        {
> +            /* Convert the history elements to cadence position and TFD. */
> +            int pi_tfd[IVTC_DETECTION_HISTORY_SIZE];
> +            int pi_pos[IVTC_DETECTION_HISTORY_SIZE];
> +            for( int i = 0; i < IVTC_DETECTION_HISTORY_SIZE; i++)
> +            {
> +                const int i_detected_pos =  
> p_ivtc->pi_cadence_pos_history[i];
> +                pi_pos[i] = pi_detected_pos_to_cadence_pos[i_detected_pos];
> +                pi_tfd[i] = pi_detected_pos_to_tfd[i_detected_pos];
> +            }
> +
> +            /* See if the sequence is valid. The cadence positions must  
> be successive mod 5.
> +               We can't say anything about TFF/BFF yet, because the  
> progressive-looking position "dea"
> +               may be there. If the sequence otherwise looks valid, we  
> handle that last by voting. */
> +
> +            bool b_sequence_valid = true;
> +            int j = pi_pos[0];
> +            for( int i = 1; i < IVTC_DETECTION_HISTORY_SIZE; i++ )
> +            {
> +                if( pi_pos[i] != (++j % 5) )
> +                {
> +                    b_sequence_valid = false;
> +                    break;
> +                }
> +            }
> +
> +            if( b_sequence_valid )
> +            {
> +                /* Determine TFF/BFF. */
> +                int i_vote_invalid = 0;
> +                int i_vote_tff     = 0;
> +                int i_vote_bff     = 0;
> +                for( int i = 0; i < IVTC_DETECTION_HISTORY_SIZE; i++ )
> +                {
> +                    if( pi_tfd[i] == TFD_INVALID )
> +                        i_vote_invalid++;
> +                    else if( pi_tfd[i] == TFD_TFF )
> +                        i_vote_tff++;
> +                    else if( pi_tfd[i] == TFD_BFF )
> +                        i_vote_bff++;
> +                }
> +                int i_telecine_field_dominance = TFD_INVALID;
> +
> +                /* With three entries, two votes for any one item are  
> enough to decide this conclusively. */
> +                if( i_vote_tff >= 2)
> +                    i_telecine_field_dominance = TFD_TFF;
> +                else if( i_vote_bff >= 2)
> +                    i_telecine_field_dominance = TFD_BFF;
> +                /* in all other cases, "invalid" won or no winner - no  
> NTSC telecine detected. */
> +
> +                /* Reset the cadence break flag if this round came up  
> ok. */
> +                if( i_telecine_field_dominance != TFD_INVALID )
> +                    p_ivtc->b_possible_cadence_break_detected = false;
> +
> +                /* Update the cadence counter from detected data  
> whenever we can.
> +                   Upon testing several strategies, this was found to  
> be the most reliable one.
> +                */
> +                if( i_telecine_field_dominance == TFD_TFF )
> +                {
> +                    if( p_ivtc->i_ivtc_filter_mode !=  
> IVTC_MODE_TELECINED_NTSC )
> +                        msg_Dbg( p_filter, "IVTC: 3:2 pulldown: NTSC  
> TFF telecine detected. Film mode on." );
> +                    p_ivtc->i_ivtc_filter_mode = IVTC_MODE_TELECINED_NTSC;
> +                    p_ivtc->i_cadence_pos =  
> pi_pos[IVTC_DETECTION_HISTORY_SIZE-1];
> +                    p_ivtc->i_telecine_field_dominance = TFD_TFF;
> +                    p_ivtc->b_possible_cadence_break_detected = false;
> +                }
> +                else if( i_telecine_field_dominance == TFD_BFF )
> +                {
> +                    if( p_ivtc->i_ivtc_filter_mode !=  
> IVTC_MODE_TELECINED_NTSC )
> +                        msg_Dbg( p_filter, "IVTC: 3:2 pulldown: NTSC  
> BFF telecine detected. Film mode on." );
> +                    p_ivtc->i_ivtc_filter_mode = IVTC_MODE_TELECINED_NTSC;
> +                    p_ivtc->i_cadence_pos =  
> pi_pos[IVTC_DETECTION_HISTORY_SIZE-1];
> +                    p_ivtc->i_telecine_field_dominance = TFD_BFF;
> +                    p_ivtc->b_possible_cadence_break_detected = false;
> +                }
> +                else if( b_film_mode && i_telecine_field_dominance ==  
> TFD_INVALID )
> +                {
> +                    msg_Dbg( p_filter, "IVTC: 3:2 pulldown: telecine  
> field dominance not found. Possible cadence break detected." );
> +                    p_ivtc->b_possible_cadence_break_detected = true;
> +                }
> +
> +                /* In case we are in NTSC film mode, but did not detect  
> a possible cadence break,
> +                   make one final sanity check. Detect a case where the  
> sequence is still valid,
> +                   but the predicted (counted) position does not match  
> the latest detected one.
> +
> +                   During development it was found that this check is  
> very important. If this is
> +                   left out, weaving might continue on a broken cadence  
> and the detector won't
> +                   notice anything wrong.
> +                */
> +                if( p_ivtc->i_ivtc_filter_mode ==  
> IVTC_MODE_TELECINED_NTSC &&
> +                    !p_ivtc->b_possible_cadence_break_detected &&
> +                    p_ivtc->i_cadence_pos !=  
> pi_pos[IVTC_DETECTION_HISTORY_SIZE-1] )
> +                {
> +                    msg_Dbg( p_filter, "IVTC: 3:2 pulldown: predicted  
> and detected position do not match. Possible cadence break detected." );
> +                    p_ivtc->b_possible_cadence_break_detected = true;
> +                }
> +            }
> +            else /* No valid NTSC telecine sequence detected.
> +                    Either there is no NTSC telecine, or there is no  
> motion at all. */
> +            {
> +                ; /* Currently, do nothing. During development,  
> strategies for detecting
> +                     true progressive, true interlaced and PAL  
> telecined were tested,
> +                     but in practice these caused more harm than good.  
> Their main effect
> +                     was that they confused the NTSC telecine detector.
> +                  */
> +            }
> +        }
> +
> +        /* Detect cadence breaks. If we see two possible breaks in a  
> row, we consider the cadence broken.
> +           Note that this is the only reason to exit film mode.
> +        */
> +        if( p_ivtc->i_ivtc_filter_mode == IVTC_MODE_TELECINED_NTSC &&
> +            p_ivtc->b_possible_cadence_break_detected &&  
> b_old_possible_cadence_break_detected )
> +        {
> +            msg_Dbg( p_filter, "IVTC: 3:2 pulldown: cadence break  
> detected. Film mode off." );
> +            p_ivtc->i_ivtc_filter_mode = IVTC_MODE_DETECTING;
> +            p_ivtc->i_cadence_pos = CADENCE_POS_INVALID;
> +            p_ivtc->i_telecine_field_dominance = TFD_INVALID;
> +            p_ivtc->b_possible_cadence_break_detected = false;
> +        }
> +
> +        /* End of cadence analysis. */
> +
> +        /* Perform IVTC if we're (still) in film mode. This means that  
> the cadence is locked on. */
> +        picture_t *p_ivtc_result = NULL; /* this will become either  
> TCBC, TNBC, TCBN or TNBN */
> +        int i_result_interlace_score = -1;
> +        if( p_ivtc->i_ivtc_filter_mode == IVTC_MODE_TELECINED_NTSC )
> +        {
> +            assert( p_ivtc->i_telecine_field_dominance != TFD_INVALID );
> +            assert( p_ivtc->i_cadence_pos != CADENCE_POS_INVALID );
> +
> +            /* Apply film frame reconstruction. */
> +
> +            /* Decide what to do. Always use the cadence counter do  
> decide whether to drop this frame. */
> +            int op =  
> pi_reconstruction_ops[p_ivtc->i_telecine_field_dominance][p_ivtc->i_cadence_pos];
> +            if( op == IVTC_OP_DROP_FRAME )
> +            {
> +                /* Bump cadence counter into the next expected position */
> +                p_ivtc->i_cadence_pos = ++p_ivtc->i_cadence_pos % 5;
> +
> +                /* Drop frame. We're done. */
> +                return VLC_EGENERIC; /* Not an error. Tell the caller  
> not to expect an output frame. */
> +            }
> +            /* During development, a strategy trusting the cadence  
> counter for frame reconstruction,
> +               in cases when no break has been detected was tried, but  
> in practice always using the
> +               Transcode strategy works better on average, if we also  
> use the Transcode comb metric.
> +
> +               The Transcode strategy catches localized motion better,  
> while the cadence counter
> +               is better at following slow vertical camera pans (which  
> won't show combing,
> +               but will look just slightly messed up). In my opinion  
> (JJ), it's more important
> +               to catch localized motion reliably.
> +
> +               The old code has been left here so it can be enabled  
> later, if desired.
> +            */
> +            else if( 0 && !p_ivtc->b_possible_cadence_break_detected )
> +            {
> +                if( op == IVTC_OP_COPY_N )
> +                {
> +                    p_ivtc_result = p_next;
> +                    i_result_interlace_score =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBN];
> +                }
> +                else if( op == IVTC_OP_COPY_C )
> +                {
> +                    p_ivtc_result = p_curr;
> +                    i_result_interlace_score =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TCBC];
> +                }
> +                else if( op == IVTC_OP_COMPOSE_TNBC )
> +                {
> +                    p_ivtc_result = p_tnbc;
> +                    i_result_interlace_score =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBC];
> +                }
> +                else if( op == IVTC_OP_COMPOSE_TCBN )
> +                {
> +                    p_ivtc_result = p_tcbn;
> +                    i_result_interlace_score =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TCBN];
> +                }
> +            }
> +            else /* Transcode strategy for producing film frames */
> +            {
> +                /* We check which field paired with TN or BN gives the  
> smallest interlace score,
> +                   and declare that combination the resulting  
> progressive frame.
> +
> +                   The difference to Transcode is that we still use our  
> cadence counter to decide
> +                   the fate of the timestamps, and which frames get  
> dropped. We can't be sure
> +                   that our frame drops hit the duplicates instead of  
> any useful frames, but
> +                   that's the best (I think) that can be done with this  
> realtime strategy.
> +
> +                   (Another approach would be to use a five-frame  
> future buffer, compare all
> +                    consecutive frames, and drop the one that looks the  
> most like a duplicate
> +                    (e.g. smallest absolute value of signed  
> pixel-by-pixel difference summed over the picture)
> +                    Transcode's "decimate" filter works this way.)
> +                */
> +                int tnbn =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBN]; /* may match on both TFF  
> and BFF */
> +                int tnbc =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBC]; /* may match on TFF  
> material */
> +                int tcbn =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TCBN]; /* may match on BFF  
> material */
> +
> +                int i_chosen = -1;
> +                /* This is the right choice 50% of the time (see the  
> rightmost entries in the "best field pairs"
> +                   column of the cadence tables). */
> +                if( tnbn <= tnbc && tnbn <= tcbn )
> +                {
> +                    p_ivtc_result = p_next;
> +                    i_result_interlace_score = tnbn;
> +                    i_chosen = FIELD_PAIR_TNBN;
> +                }
> +                else if( tnbc <= tnbn && tnbc <= tcbn )
> +                {
> +                    p_ivtc_result = p_tnbc;
> +                    i_result_interlace_score = tnbc;
> +                    i_chosen = FIELD_PAIR_TNBC;
> +                }
> +                else if( tcbn <= tnbn && tcbn <= tnbc )
> +                {
> +                    p_ivtc_result = p_tcbn;
> +                    i_result_interlace_score = tcbn;
> +                    i_chosen = FIELD_PAIR_TCBN;
> +                }
> +                /* The above was an exhaustive list of possibilities.  
> No "else" is needed. */
> +            }
> +
> +            /* Note that we get to this point only if we didn't drop  
> the frame.
> +               Mangle the presentation timestamp to convert 30 -> 24 fps.
> +            */
> +            int i_timestamp_delta =  
> pi_timestamp_deltas[p_ivtc->i_cadence_pos];
> +            assert( i_timestamp_delta >= 0); /* When we get here, we  
> must always have a valid adjustment. */
> +
> +            /* "Current" is the frame that is being extracted now. Use  
> its original timestamp as the base. */
> +            t_final = p_curr->date + (p_next->date -  
> p_curr->date)*i_timestamp_delta/4;
> +
> +            /* Bump cadence counter into the next expected position. */
> +            p_ivtc->i_cadence_pos = ++p_ivtc->i_cadence_pos % 5;
> +        }
> +        else /* Not film mode, IVTC bypassed. Just use the latest frame  
> as the "IVTC result". */
> +        {
> +            p_ivtc_result = p_next;
> +            i_result_interlace_score =  
> p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBN];
> +        }
> +        assert( p_ivtc_result != NULL );
> +
> +        /* We're almost done. Estimate whether the resulting frame is  
> interlaced or not.
> +           Deinterlace if it is, and return that to caller. Otherwise  
> just pass the frame through to caller.
> +
> +           Note that we shouldn't run all frames through RenderX() just  
> in case, because it tends to
> +           mangle (especially Japanese) text in some opening credits.  
> For an example, play the opening
> +           sequence from the anime Utakata using RenderX() to see this  
> effect.
> +        */
> +        if( i_result_interlace_score > RENDERIVTC_INTERLACE_THRESHOLD)
> +        {
> +            char temp[1000];
> +            sprintf(temp, "IVTC: removing residual interlacing (score  
> %d > %d)", i_result_interlace_score, RENDERIVTC_INTERLACE_THRESHOLD );
> +            msg_Dbg( p_filter, temp );
 Why using a temporary buffer? msg_Dbg() support printf like arguments.

> +
> +            RenderX( p_dst, p_ivtc_result );
> +        }
> +        else /* Not interlaced, just copy the frame. */
> +        {
> +            picture_Copy( p_dst, p_ivtc_result );
> +        }
> +
> +        /* Note that picture_Copy() copies the presentation timestamp, too.
> +           Apply timestamp mangling now, if any was needed.
> +        */
> +        if( t_final > VLC_TS_INVALID )
> +            p_dst->date = t_final;
> +
> +        return VLC_SUCCESS;
> +    }
> +    else if( !p_prev && !p_curr && p_next ) /* first frame */
> +    {
> +        /* Render the first frame using any means necessary, so that a  
> picture appears immediately. */
> +
> +        /* Let's do some init for the filter. This score will become  
> TPBP by the time the actual filter starts. */
> +        p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBN] =  
> CalculateInterlaceScore( p_filter, p_next );
> +
> +        /* Do the usual conditional area-based deinterlacing (see  
> above). */
> +        if( p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBN] >  
> RENDERIVTC_INTERLACE_THRESHOLD)
> +            RenderX( p_dst, p_next );
> +        else
> +            picture_Copy( p_dst, p_next );
> +
> +        return VLC_SUCCESS;
> +    }
> +    else /* now the only possibility is (!p_prev && p_curr && p_next) */
 It's better to add an assert() than a comment in such cases.

> +    {
> +        /* This is the second frame. We need three for the detector to  
> work, so we drop this one.
> +           We will only do some initialization for the detector here. */
> +
> +        /* Generate artificial frames TNBC and TCBN into the temporary  
> working area */
> +        ComposeFrame( p_filter, p_tnbc, p_next, p_curr );
> +        ComposeFrame( p_filter, p_tcbn, p_curr, p_next );
> +
> +        /* These scores will become TCBC, TCBP and TPBC when the filter  
> starts.
> +           The score for the current TCBC has already been computed at  
> the first frame,
> +           and slid into place at the start of this frame.
> +        */
> +        p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBN] =  
> CalculateInterlaceScore( p_filter, p_next );
> +        p_ivtc->pi_interlace_scores[FIELD_PAIR_TNBC] =  
> CalculateInterlaceScore( p_filter, p_tnbc );
> +        p_ivtc->pi_interlace_scores[FIELD_PAIR_TCBN] =  
> CalculateInterlaceScore( p_filter, p_tcbn );
> +
> +        return VLC_EGENERIC; /* Not really an error. This is expected,  
> but we must
> +                                signal the caller not to expect an  
> output frame. */
> +    }
> +}
> +
> +/*****************************************************************************
>   * video filter2 functions
>    
> *****************************************************************************/
>  static picture_t *Deinterlace( filter_t *p_filter, picture_t *p_pic )
> @@ -1609,6 +2799,13 @@ static picture_t *Deinterlace( filter_t  
> *p_filter, picture_t *p_pic )
>              if( p_dst[1] )
>                  RenderYadif( p_filter, p_dst[1], p_pic, 1,  
> p_pic->b_top_field_first );
>              break;
> +
> +        case DEINTERLACE_IVTC:
> +            /* Note: RenderIVTC will automatically drop the duplicate  
> frames produced by IVTC.
> +                     This is part of normal IVTC operation. */
> +            if( RenderIVTC( p_filter, p_dst[0], p_pic ) )
> +                goto drop;
> +            break;
>      }
>
>      p_dst[0]->b_progressive = true;
> @@ -1629,6 +2826,7 @@ drop:
>  static void Flush( filter_t *p_filter )
>  {
>      filter_sys_t *p_sys = p_filter->p_sys;
> +    ivtc_sys_t *p_ivtc = p_sys->p_ivtc;
>
>      p_sys->i_last_date = VLC_TS_INVALID;
>      for( int i = 0; i < HISTORY_SIZE; i++ )
> @@ -1637,6 +2835,25 @@ static void Flush( filter_t *p_filter )
>              picture_Release( p_sys->pp_history[i] );
>          p_sys->pp_history[i] = NULL;
>      }
> +
> +    /* IVTC */
> +    for( int i = 0; i < IVTC_WORKING_AREA_SIZE; i++ )
> +    {
> +        if( p_ivtc->pp_ivtc_working_area[i] )
> +            picture_Release( p_ivtc->pp_ivtc_working_area[i] );
> +        p_ivtc->pp_ivtc_working_area[i] = NULL;
> +    }
> +    p_ivtc->b_possible_cadence_break_detected = false;
> +    p_ivtc->i_cadence_pos = CADENCE_POS_INVALID;
> +    p_ivtc->i_telecine_field_dominance = TFD_INVALID;
> +    p_ivtc->i_ivtc_filter_mode = IVTC_MODE_DETECTING;
> +    for( int i = 0; i < IVTC_NUM_FIELD_PAIRS; i++ )
> +        p_ivtc->pi_interlace_scores[i] = 0;
> +    for( int i = 0; i < IVTC_DETECTION_HISTORY_SIZE; i++ )
> +        p_ivtc->pi_cadence_pos_history[i] = CADENCE_POS_INVALID; /*  
> detected positions */
> +    p_ivtc->i_old_chroma = -1;
> +    p_ivtc->i_old_size_x = -1;
> +    p_ivtc->i_old_size_y = -1;
 It seems that some part could be factorized with the init code.
>  }
>
>  static int Mouse( filter_t *p_filter,
> @@ -1657,6 +2874,7 @@ static int Open( vlc_object_t *p_this )
>  {
>      filter_t *p_filter = (filter_t*)p_this;
>      filter_sys_t *p_sys;
> +    ivtc_sys_t *p_ivtc;
>
>      if( !IsChromaSupported( p_filter->fmt_in.video.i_chroma ) )
>          return VLC_EGENERIC;
> @@ -1665,6 +2883,13 @@ static int Open( vlc_object_t *p_this )
>      p_sys = p_filter->p_sys = malloc( sizeof( *p_sys ) );
>      if( !p_sys )
>          return VLC_ENOMEM;
> +    p_ivtc = p_filter->p_sys->p_ivtc = malloc( sizeof( *p_ivtc ) );
> +    if( !p_ivtc )
> +    {
> +        free( p_filter->p_sys );
> +        p_filter->p_sys = NULL;
> +        return VLC_ENOMEM;
> +    }
>
>      p_sys->i_mode = DEINTERLACE_BLEND;
>      p_sys->b_double_rate = false;
> @@ -1673,6 +2898,21 @@ static int Open( vlc_object_t *p_this )
>      for( int i = 0; i < HISTORY_SIZE; i++ )
>          p_sys->pp_history[i] = NULL;
>
> +    /* IVTC */
> +    for( int i = 0; i < IVTC_WORKING_AREA_SIZE; i++ )
> +        p_ivtc->pp_ivtc_working_area[i] = NULL;
> +    p_ivtc->b_possible_cadence_break_detected = false;
> +    p_ivtc->i_cadence_pos = CADENCE_POS_INVALID;
> +    p_ivtc->i_telecine_field_dominance = TFD_INVALID;
> +    p_ivtc->i_ivtc_filter_mode = IVTC_MODE_DETECTING;
> +    for( int i = 0; i < IVTC_NUM_FIELD_PAIRS; i++ )
> +        p_ivtc->pi_interlace_scores[i] = 0;
> +    for( int i = 0; i < IVTC_DETECTION_HISTORY_SIZE; i++ )
> +        p_ivtc->pi_cadence_pos_history[i] = CADENCE_POS_INVALID;
> +    p_ivtc->i_old_chroma = -1;
> +    p_ivtc->i_old_size_x = -1;
> +    p_ivtc->i_old_size_y = -1;
> +
>  #if defined(CAN_COMPILE_C_ALTIVEC)
>      if( vlc_CPU() & CPU_CAPABILITY_ALTIVEC )
>      {
> @@ -1755,6 +2995,7 @@ static void Close( vlc_object_t *p_this )
>      filter_t *p_filter = (filter_t*)p_this;
>
>      Flush( p_filter );
> +    free( p_filter->p_sys->p_ivtc );
>      free( p_filter->p_sys );
>  }

Regards,

-- 
fenrir



More information about the vlc-devel mailing list