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

[Steve Lhomme]

On Thu, Jul 20, 2017 at 2:26 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> 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.

Perfect. We agree.

> Filters return either their input picture, or pictures
> allocated by the callback.

Read the other patches if you don't understand why it's needed.

> --
> Rémi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel