[vlc-devel] Fwd: Re: [PATCH 07/11] video_filter:deinterlace: use the same signature for all render methods
Steve Lhomme
robux4 at gmail.com
Fri Jun 30 07:17:45 CEST 2017
On Wed, Jun 28, 2017 at 1:56 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
>
>
> ________________________________
> De : "Rémi Denis-Courmont" <remi at remlab.net>
> Envoyé : 27 juin 2017 12:35:08 GMT+03:00
> À : Mailing list for VLC media player developers <vlc-devel at videolan.org>
> Objet : Re: [vlc-devel] [PATCH 07/11] video_filter:deinterlace: use the same
> signature for all render methods
>
> Le 27 juin 2017 10:21:43 GMT+02:00, Steve Lhomme <robux4 at videolabs.io> a
> écrit :
>>
>> This way we can use a generic callback.
>> ---
>> modules/video_filter/deinterlace/algo_basic.c | 38
>> +++++++++++++++++-------
>> modules/video_filter/deinterlace/algo_basic.h | 18 ++++++-----
>> modules/video_filter/deinterlace/algo_ivtc.c | 6 +++-
>> modules/video_filter/deinterlace/algo_ivtc.h | 3 +-
>> modules/video_filter/deinterlace/algo_phosphor.c | 3 +-
>> modules/video_filter/deinterlace/algo_phosphor.h | 2 +-
>> modules/video_filter/deinterlace/algo_x.c | 7 ++++-
>> modules/video_filter/deinterlace/algo_x.h | 3 +-
>> modules/video_filter/deinterlace/algo_yadif.c | 2 +-
>> modules/video_filter/deinterlace/deinterlace.c | 34
>> ++++++++++++---------
>> 10 files changed, 76 insertions(+), 40 deletions(-)
>>
>> diff --git a/modules/video_filter/deinterlace/algo_basic.c
>> b/modules/video_filter/deinterlace/algo_basic.c
>> index e2cfdf63fc..c77847083a 100644
>> --- a/modules/video_filter/deinterlace/algo_basic.c
>> +++ b/modules/video_filter/deinterlace/algo_basic.c
>> @@ -43,11 +43,13 @@
>> * RenderDiscard: only keep TOP or BOTTOM field, discard the other.
>>
>> *****************************************************************************/
>>
>> -void RenderDiscard( picture_t *p_outpic, picture_t *p_pic, int i_field )
>> +int RenderDiscard( filter_t *p_filter, picture_t *p_outpic, picture_t
>> *p_pic,
>> + int order, int i_field )
>> {
>> + VLC_UNUSED(p_filter);
>> + VLC_UNUSED(order);
>> assert(i_field == 0);
>> int i_plane;
>> -
>> /* Copy image and skip lines */
>> for( i_plane = 0 ; i_plane < p_pic->i_planes ; i_plane++ )
>> {
>> @@ -67,14 +69,18 @@ void RenderDiscard( picture_t *p_outpic, picture_t
>> *p_pic, int i_field )
>> p_in += 2 * p_pic->p[i_plane].i_pitch;
>> }
>> }
>> + return VLC_SUCCESS;
>> }
>>
>>
>> /*****************************************************************************
>> * RenderBob: renders a BOB picture - simple copy
>>
>> *****************************************************************************/
>>
>> -void RenderBob( picture_t *p_outpic, picture_t *p_pic, int i_field )
>> +int RenderBob( filter_t *p_filter, picture_t *p_outpic, picture_t *p_pic,
>> + int order, int i_field )
>> {
>> + VLC_UNUSED(p_filter);
>> + VLC_UNUSED(order);
>> int i_plane;
>>
>> /* Copy image and skip lines */
>> @@ -119,15 +125,18 @@ void RenderBob( picture_t *p_outpic, picture_t
>> *p_pic, int i_field )
>> memcpy( p_out, p_in, p_pic->p[i_plane].i_pitch );
>> }
>> }
>> + return VLC_SUCCESS;
>> }
>>
>>
>> /*****************************************************************************
>> * RenderLinear: BOB with linear interpolation
>>
>> *****************************************************************************/
>>
>> -void RenderLinear( filter_t *p_filter,
>> - picture_t *p_outpic, picture_t *p_pic, int i_field )
>> +int RenderLinear( filter_t *p_filter,
>> + picture_t *p_outpic, picture_t *p_pic, int order, int
>> i_field )
>> {
>> + VLC_UNUSED(p_filter);
>> + VLC_UNUSED(order);
>> int i_plane;
>>
>> /* Copy image and skip lines */
>> @@ -174,17 +183,20 @@ void RenderLinear( filter_t *p_filter,
>> }
>> }
>> EndMerge();
>> + return VLC_SUCCESS;
>> }
>>
>>
>> /*****************************************************************************
>> * RenderMean: Half-resolution blender
>>
>> *****************************************************************************/
>>
>> -void RenderMean( filter_t *p_filter,
>> - picture_t *p_outpic, picture_t *p_pic )
>> +int RenderMean( filter_t *p_filter,
>> + picture_t *p_outpic, picture_t *p_pic, int order, int
>> i_field )
>> {
>> + VLC_UNUSED(p_filter);
>> + VLC_UNUSED(order);
>> + VLC_UNUSED(i_field);
>> int i_plane;
>> -
>> /* Copy image and skip lines */
>> for( i_plane = 0 ; i_plane < p_pic->i_planes ; i_plane++ )
>> {
>> @@ -207,17 +219,20 @@ void RenderMean( filter_t *p_filter,
>> }
>> }
>> EndMerge();
>> + return VLC_SUCCESS;
>> }
>>
>>
>> /*****************************************************************************
>> * RenderBlend: Full-resolution blender
>>
>> *****************************************************************************/
>>
>> -void RenderBlend( filter_t *p_filter,
>> - picture_t *p_outpic, picture_t *p_pic )
>> +int RenderBlend( filter_t *p_filter,
>> + picture_t *p_outpic, picture_t *p_pic, int order, int
>> i_field )
>> {
>> + VLC_UNUSED(p_filter);
>> + VLC_UNUSED(order);
>> + VLC_UNUSED(i_field);
>> int i_plane;
>> -
>> /* Copy image and skip lines */
>> for( i_plane = 0 ; i_plane < p_pic->i_planes ; i_plane++ )
>> {
>> @@ -244,4 +259,5 @@ void RenderBlend( filter_t *p_filter,
>>
>> }
>> }
>> EndMerge();
>> + return VLC_SUCCESS;
>> }
>> diff --git a/modules/video_filter/deinterlace/algo_basic.h
>> b/modules/video_filter/deinterlace/algo_basic.h
>> index 2b1d9e3c81..aaeec3d4c5 100644
>> --- a/modules/video_filter/deinterlace/algo_basic.h
>> +++ b/modules/video_filter/deinterlace/algo_basic.h
>> @@ -50,7 +50,8 @@ struct picture_t;
>> * @see RenderBob()
>> * @see Deinterlace()
>> */
>> -void RenderDiscard( picture_t *p_outpic, picture_t *p_pic, int i_field );
>> +int RenderDiscard( filter_t *p_filter,
>> + picture_t *p_outpic, picture_t *p_pic, int order, int
>> i_field );
>>
>> /**
>> * RenderBob: basic framerate doubler.
>> @@ -65,7 +66,8 @@ void RenderDiscard( picture_t *p_outpic, picture_t
>> *p_pic, int i_field );
>> * @see RenderLinear()
>> * @see Deinterlace()
>> */
>> -void RenderBob( picture_t *p_outpic, picture_t *p_pic, int i_field );
>> +int RenderBob( filter_t *p_filter,
>> + picture_t *p_outpic, picture_t *p_pic, int order, int
>> i_field );
>>
>> /**
>> * RenderLinear: Bob with linear interpolation.
>> @@ -79,8 +81,8 @@ void RenderBob( picture_t *p_outpic, picture_t *p_pic,
>> int i_field );
>> * @see RenderBob()
>> * @see Deinterlace()
>> */
>> -void RenderLinear( filter_t *p_filter,
>> - picture_t *p_outpic, picture_t *p_pic, int i_field );
>> +int RenderLinear( filter_t *p_filter,
>> + picture_t *p_outpic, picture_t *p_pic, int order, int
>> i_field );
>>
>> /**
>> * RenderMean: half-resolution blender.
>> @@ -94,8 +96,8 @@ void RenderLinear( filter_t *p_filter,
>> * @param p_pic Input frame. Must exist.
>> * @see Deinterlace()
>> */
>> -void RenderMean( filter_t *p_filter,
>> - picture_t *p_outpic, picture_t *p_pic );
>> +int RenderMean( filter_t *p_filter,
>> + picture_t *p_outpic, picture_t *p_pic, int order, int
>> i_field );
>>
>> /**
>> * RenderBlend: full-resolution blender.
>> @@ -110,7 +112,7 @@ void RenderMean( filter_t *p_filter,
>> * @param p_pic Input frame. Must exist.
>> * @see Deinterlace()
>> */
>> -void RenderBlend( filter_t *p_filter,
>> - picture_t *p_outpic, picture_t *p_pic );
>> +int RenderBlend( filter_t *p_filter,
>> + picture_t *p_outpic, picture_t *p_pic, int order, int
>> i_field );
>>
>> #endif
>> diff --git a/modules/video_filter/deinterlace/algo_ivtc.c
>> b/modules/video_filter/deinterlace/algo_ivtc.c
>> index 35f920beeb..5603828f0e 100644
>> --- a/modules/video_filter/deinterlace/algo_ivtc.c
>> +++ b/modules/video_filter/deinterlace/algo_ivtc.c
>> @@ -1467,8 +1467,12 @@ static bool IVTCOutputOrDropFrame( filter_t
>> *p_filter, picture_t *p_dst )
>>
>> *****************************************************************************/
>>
>> /* See function doc in header. */
>> -int RenderIVTC( filter_t *p_filter, picture_t *p_dst )
>> +int RenderIVTC( filter_t *p_filter, picture_t *p_dst, picture_t *p_pic,
>> + int order, int i_field )
>> {
>> + VLC_UNUSED(p_pic);
>> + VLC_UNUSED(order);
>> + VLC_UNUSED(i_field);
>> assert( p_filter != NULL );
>> assert( p_dst != NULL );
>>
>> diff --git a/modules/video_filter/deinterlace/algo_ivtc.h
>> b/modules/video_filter/deinterlace/algo_ivtc.h
>> index 3bc4ff3f0a..69334ae0ee 100644
>> --- a/modules/video_filter/deinterlace/algo_ivtc.h
>> +++ b/modules/video_filter/deinterlace/algo_ivtc.h
>> @@ -141,7 +141,8 @@ typedef struct
>> * @see CalculateInterlaceScore()
>> * @see EstimateNumBlocksWithMotion()
>> */
>> -int RenderIVTC( filter_t *p_filter, picture_t *p_dst );
>> +int RenderIVTC( filter_t *p_filter, picture_t *p_dst, picture_t *p_pic,
>> + int i_field, int order);
>>
>> /**
>> * Clears the inverse telecine subsystem state.
>> diff --git a/modules/video_filter/deinterlace/algo_phosphor.c
>> b/modules/video_filter/deinterlace/algo_phosphor.c
>> index 0240e0bce5..cc90ec13a6 100644
>> --- a/modules/video_filter/deinterlace/algo_phosphor.c
>> +++ b/modules/video_filter/deinterlace/algo_phosphor.c
>> @@ -278,9 +278,10 @@ static void DarkenFieldMMX( picture_t *p_dst,
>>
>> /* See header for function doc. */
>> int RenderPhosphor( filter_t *p_filter,
>> - picture_t *p_dst,
>> + picture_t *p_dst, picture_t *p_pic,
>> int i_order, int i_field )
>> {
>> + VLC_UNUSED(p_pic);
>> assert( p_filter != NULL );
>> assert( p_dst != NULL );
>> assert( i_order >= 0 && i_order <= 2 ); /* 2 = soft field repeat */
>> diff --git a/modules/video_filter/deinterlace/algo_phosphor.h
>> b/modules/video_filter/deinterlace/algo_phosphor.h
>> index 8159adb374..14aa252502 100644
>> --- a/modules/video_filter/deinterlace/algo_phosphor.h
>> +++ b/modules/video_filter/deinterlace/algo_phosphor.h
>> @@ -101,7 +101,7 @@ typedef struct
>> * @see Deinterlace()
>> */
>> int RenderPhosphor( filter_t *p_filter,
>> - picture_t *p_dst,
>> + picture_t *p_dst, picture_t *p_pic,
>> int i_order, int i_field );
>>
>>
>> /*****************************************************************************
>> diff --git a/modules/video_filter/deinterlace/algo_x.c
>> b/modules/video_filter/deinterlace/algo_x.c
>> index 70633a6fc0..4e6dee782c 100644
>> --- a/modules/video_filter/deinterlace/algo_x.c
>> +++ b/modules/video_filter/deinterlace/algo_x.c
>> @@ -512,8 +512,12 @@ static inline void XDeintBand8x8MMXEXT( uint8_t *dst,
>> int i_dst,
>> * Public functions
>>
>> *****************************************************************************/
>>
>> -void RenderX( picture_t *p_outpic, picture_t *p_pic )
>> +int RenderX( filter_t *p_filter, picture_t *p_outpic, picture_t *p_pic,
>> + int order, int i_field )
>> {
>> + VLC_UNUSED(p_filter);
>> + VLC_UNUSED(order);
>> + VLC_UNUSED(i_field);
>> int i_plane;
>> #if defined (CAN_COMPILE_MMXEXT)
>> const bool mmxext = vlc_CPU_MMXEXT();
>> @@ -569,4 +573,5 @@ void RenderX( picture_t *p_outpic, picture_t *p_pic )
>> if( mmxext )
>> emms();
>> #endif
>> + return VLC_SUCCESS;
>> }
>> diff --git a/modules/video_filter/deinterlace/algo_x.h
>> b/modules/video_filter/deinterlace/algo_x.h
>> index 16b7cd1bde..99aadf99dc 100644
>> --- a/modules/video_filter/deinterlace/algo_x.h
>> +++ b/modules/video_filter/deinterlace/algo_x.h
>> @@ -50,6 +50,7 @@ struct picture_t;
>> * @param[out] p_outpic Output frame. Must be allocated by caller.
>> * @see Deinterlace()
>> */
>> -void RenderX( picture_t *p_outpic, picture_t *p_pic );
>> +int RenderX( filter_t *p_filter, picture_t *p_outpic, picture_t *p_pic,
>> + int order, int i_field );
>>
>> #endif
>> diff --git a/modules/video_filter/deinterlace/algo_yadif.c
>> b/modules/video_filter/deinterlace/algo_yadif.c
>> index da3470dde6..b326a17736 100644
>> --- a/modules/video_filter/deinterlace/algo_yadif.c
>> +++ b/modules/video_filter/deinterlace/algo_yadif.c
>> @@ -182,7 +182,7 @@ int RenderYadif( filter_t *p_filter, picture_t *p_dst,
>> picture_t *p_src,
>> as set by Open() or SetFilterMethod(). It is always 0.
>> */
>>
>> /* FIXME not good as it does not use i_order/i_field */
>> - RenderX( p_dst, p_next );
>> + RenderX( p_filter, p_dst, p_next, i_order, i_field );
>> return VLC_SUCCESS;
>> }
>> else
>> diff --git a/modules/video_filter/deinterlace/deinterlace.c
>> b/modules/video_filter/deinterlace/deinterlace.c
>> index 2df1e278bd..17ab199c50 100644
>> --- a/modules/video_filter/deinterlace/deinterlace.c
>> +++ b/modules/video_filter/deinterlace/deinterlace.c
>> @@ -435,35 +435,41 @@ picture_t *Deinterlace( filter_t *p_filter,
>> picture_t *p_pic )
>>
>> switch( p_sys->i_mode )
>> {
>> case DEINTERLACE_DISCARD:
>> - RenderDiscard( p_dst[0], p_pic, 0 );
>> + RenderDiscard( p_filter, p_dst[0], p_pic, 0, 0 );
>> break;
>>
>> case DEINTERLACE_BOB:
>> - RenderBob( p_dst[0], p_pic, !b_top_field_first );
>> + RenderBob( p_filter, p_dst[0], p_pic, 0,
>> + !b_top_field_first );
>> if( p_dst[1] )
>> - RenderBob( p_dst[1], p_pic, b_top_field_first );
>> + RenderBob( p_filter, p_dst[1], p_pic, 1,
>> + b_top_field_first );
>> if( p_dst[2] )
>> - RenderBob( p_dst[2], p_pic, !b_top_field_first );
>> + RenderBob( p_filter, p_dst[2], p_pic, 2,
>> + !b_top_field_first );
>> break;;
>>
>> case DEINTERLACE_LINEAR:
>> - RenderLinear( p_filter, p_dst[0], p_pic, !b_top_field_first
>> );
>> + RenderLinear( p_filter, p_dst[0], p_pic, 0,
>> + !b_top_field_first );
>> if( p_dst[1] )
>> - RenderLinear( p_filter, p_dst[1], p_pic,
>> b_top_field_first );
>> + RenderLinear( p_filter, p_dst[1], p_pic, 1,
>> + b_top_field_first );
>> if( p_dst[2] )
>> - RenderLinear( p_filter, p_dst[2], p_pic,
>> !b_top_field_first );
>> + RenderLinear( p_filter, p_dst[2], p_pic, 2,
>> + !b_top_field_first );
>> break;
>>
>> case DEINTERLACE_MEAN:
>> - RenderMean( p_filter, p_dst[0], p_pic );
>> + RenderMean( p_filter, p_dst[0], p_pic, 0, 0 );
>> break;
>>
>> case DEINTERLACE_BLEND:
>> - RenderBlend( p_filter, p_dst[0], p_pic );
>> + RenderBlend( p_filter, p_dst[0], p_pic, 0, 0 );
>> break;
>>
>> case DEINTERLACE_X:
>> - RenderX( p_dst[0], p_pic );
>> + RenderX( p_filter, p_dst[0], p_pic, 0, 0 );
>> break;
>>
>> case DEINTERLACE_YADIF:
>> @@ -481,21 +487,21 @@ picture_t *Deinterlace( filter_t *p_filter,
>> picture_t *p_pic )
>> break;
>>
>> case DEINTERLACE_PHOSPHOR:
>> - if( RenderPhosphor( p_filter, p_dst[0], 0,
>> + if( RenderPhosphor( p_filter, p_dst[0], p_pic, 0,
>> !b_top_field_first ) )
>> goto drop;
>> if( p_dst[1] )
>> - RenderPhosphor( p_filter, p_dst[1], 1,
>> + RenderPhosphor( p_filter, p_dst[1], p_pic, 1,
>> b_top_field_first );
>> if( p_dst[2] )
>> - RenderPhosphor( p_filter, p_dst[2], 2,
>> + RenderPhosphor( p_filter, p_dst[2], p_pic, 2,
>> !b_top_field_first );
>> break;
>>
>> case DEINTERLACE_IVTC:
>> /* Note: RenderIVTC will automatically drop the duplicate
>> frames
>> produced by IVTC. This is part of normal operation.
>> */
>> - if( RenderIVTC( p_filter, p_dst[0] ) )
>> + if( RenderIVTC( p_filter, p_dst[0], p_pic, 0, 0 ) )
>> goto drop;
>> break;
>> }
>
>
> This is questionable, as the later patch still has two distinct ways to
> invoke the callbacks.
>
> You could just as well have two signatures, one for each cases, and a union
> to store the callback pointer. *Or* you could actually factor everything as
> a single callback, with a little bit of duplication in the dual-fields
> algorithms.
I like the dual signature. It makes errors for the callee impossible.
And we already have a flag (b_single_field) to tell which variant we
should call. I'll make new patches.
> --
> Rémi Denis-Courmont
> Typed on an inconvenient virtual keyboard
> --
> Rémi Denis-Courmont
> Typed on an inconvenient virtual keyboard
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list