[vlc-devel] [PATCH 1/3] deinterlace: add a callback so calls to filter_NewPicture can be overridden

[Rémi Denis-Courmont]

On jeudi 20 juillet 2017 13:37:24 EEST Steve Lhomme wrote:
> On Thu, Jul 20, 2017 at 1:16 PM, Rémi Denis-Courmont <remi at remlab.net> 
wrote:
> > Le 20 juillet 2017 17:04:26 GMT+08:00, Steve Lhomme <robux4 at videolabs.io>
> > a
> > 
> > écrit :
> >> ---
> >> 
> >>  modules/video_filter/deinterlace/common.c | 6 ++++--
> >>  modules/video_filter/deinterlace/common.h | 5 +++++
> >>  2 files changed, 9 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/modules/video_filter/deinterlace/common.c
> >> b/modules/video_filter/deinterlace/common.c
> >> index ea9c6af464..b1f7e39db3 100644
> >> --- a/modules/video_filter/deinterlace/common.c
> >> +++ b/modules/video_filter/deinterlace/common.c
> >> @@ -50,6 +50,8 @@ void InitDeinterlacingContext( struct deinterlace_ctx
> >> *p_context )
> >> 
> >>                                    cannot have offset) */
> >>      
> >>      for( int i = 0; i < HISTORY_SIZE; i++ )
> >>      
> >>          p_context->pp_history[i] = NULL;
> >> 
> >> +
> >> +    p_context->pf_new_output_picture = filter_NewPicture;
> >> 
> >>  }
> >>  
> >>  void FlushDeinterlacing(struct deinterlace_ctx *p_context)
> >> 
> >> @@ -139,7 +141,7 @@ picture_t *DoDeinterlacing( filter_t *p_filter,
> >> 
> >>      bool b_top_field_first;
> >>      
> >>      /* Request output picture */
> >> 
> >> -    p_dst[0] = filter_NewPicture( p_filter );
> >> +    p_dst[0] = p_context->pf_new_output_picture( p_filter );
> >> 
> >>      if( p_dst[0] == NULL )
> >>      {
> >>      
> >>          picture_Release( p_pic );
> >> 
> >> @@ -235,7 +237,7 @@ picture_t *DoDeinterlacing( filter_t *p_filter,
> >> 
> >>          for( int i = 1; i < i_double_rate_alloc_end ; ++i )
> >>          {
> >>          
> >>              p_dst[i-1]->p_next =
> >> 
> >> -            p_dst[i]           = filter_NewPicture( p_filter );
> >> +            p_dst[i]           = p_context->pf_new_output_picture(
> >> p_filter );
> >> 
> >>              if( p_dst[i] )
> >>              {
> >>              
> >>                  picture_CopyProperties( p_dst[i], p_pic );
> >> 
> >> diff --git a/modules/video_filter/deinterlace/common.h
> >> b/modules/video_filter/deinterlace/common.h
> >> index 940a87e2a9..4f46eb293e 100644
> >> --- a/modules/video_filter/deinterlace/common.h
> >> +++ b/modules/video_filter/deinterlace/common.h
> >> @@ -89,6 +89,11 @@ struct deinterlace_ctx
> >> 
> >>                                   int order, int i_field);
> >>          
> >>          int (*pf_render_single_pic)(filter_t *, picture_t *p_dst,
> >> 
> >> picture_t *p_pic);
> >> 
> >>      };
> >> 
> >> +
> >> +    /**
> >> +     * Callback to override calls to filter_NewPicture()
> >> +     */
> >> +    picture_t *(*pf_new_output_picture)(filter_t *);
> >> 
> >>  };
> >>  
> >>  #define DEINTERLACE_DST_SIZE 3
> > 
> > That does not make sense. Filter_newPicture is already a callback.
> 
> Call it what you like. I don't think a filter is allowed to change the
> values set in p_filter->owner.
> And even if it was, we'd still need to store the original callback to
> call it afterwards. My solution makes a lot more sense.

No, it makes zero sense.

It doesn't make sense for a filter to allocate pictures through another means 
than that callback. Filters return either their input picture, or pictures 
allocated by the callback.

-- 
Rémi Denis-Courmont