[vlc-devel] [PATCH v2 08/20] fps: implement draining of extra pictures

Romain Vimont rom1v at videolabs.io
Thu Oct 15 21:43:21 CEST 2020


On Wed, Oct 14, 2020 at 02:39:08PM +0200, Steve Lhomme wrote:
> The p_previous_pic keeps the picture that was returned by Filter(), which is
> the basis for the extra pictures created during the drain, by incrementing by
> one "framerate tick" each output. We need to keep a reference as the output
> picture might be released by the caller, before the drain is called.
> 
> We keep the original date of the incoming picture so we can compare to the
> "framerate" ticks to know if we need extra pictures or not.
> 
> When expanding the frame rate, we have the following output:
> - pic0 -> original date
>        -> original date + tick 1
> - pic1 -> original date + tick 2
>        -> original date + tick 3
> - pic2 -> original date + tick 4
>        -> original date + tick 5
> 
> The date of the incoming picture is always included in the last output
> frame date + duration.
> 
> The code assumes that enough drain calls will be done until the next filter
> call. (A flush in between is good as well)
> 
> We no longer return pictures chained using vlc_picture_chain_AppendChain().
> ---
>  modules/video_filter/fps.c | 53 ++++++++++++++++++++------------------
>  1 file changed, 28 insertions(+), 25 deletions(-)
> 
> diff --git a/modules/video_filter/fps.c b/modules/video_filter/fps.c
> index 67c1bd86e23..cfc711e60bf 100644
> --- a/modules/video_filter/fps.c
> +++ b/modules/video_filter/fps.c
> @@ -61,24 +61,27 @@ static const char *const ppsz_filter_options[] = {
>  typedef struct
>  {
>      date_t          next_output_pts; /**< output calculated PTS */
> -    picture_t       *p_previous_pic; /**< kept source picture used to produce filter output */
> +    picture_t       *p_previous_pic; /**< picture used to produce drain outputs */
> +    vlc_tick_t      src_date;
>      vlc_tick_t      i_output_frame_interval;
>  } filter_sys_t;
>  
> -static void SetOutputDate(filter_sys_t *p_sys, picture_t *pic)
> +static picture_t *SetOutputDate(filter_sys_t *p_sys, picture_t *pic)

nit: This is odd, the caller knows the picture (it passed it by
argument, and it's always the same), there is no need to return it.
And semantically, it's confusing that SetOutputDate() returns a picture.

>  {
>      pic->date = date_Get( &p_sys->next_output_pts );
>      date_Increment( &p_sys->next_output_pts, 1 );
> +    return pic;
>  }
>  
>  static picture_t *Filter( filter_t *p_filter, picture_t *p_picture)
>  {
>      filter_sys_t *p_sys = p_filter->p_sys;
> -    const vlc_tick_t src_date = p_picture->date;
> +    assert(p_sys->p_previous_pic == NULL); // missing drain ?
> +    p_sys->src_date = p_picture->date;
>      /* If input picture doesn't have actual valid timestamp,
>          we don't really have currently a way to know what else
>          to do with it other than drop it for now*/
> -    if( unlikely( src_date == VLC_TICK_INVALID) )
> +    if( unlikely( p_sys->src_date == VLC_TICK_INVALID) )
>      {
>          msg_Dbg( p_filter, "skipping non-dated picture");
>          picture_Release( p_picture );
> @@ -93,43 +96,43 @@ static picture_t *Filter( filter_t *p_filter, picture_t *p_picture)
>      if( unlikely( date_Get( &p_sys->next_output_pts ) == VLC_TICK_INVALID ) )
>      {
>          msg_Dbg( p_filter, "Resetting timestamps" );
> -        date_Set( &p_sys->next_output_pts, src_date );
> -        if( p_sys->p_previous_pic )
> -            picture_Release( p_sys->p_previous_pic );
> -        // p_picture will be returned so we need a reference
> -        p_sys->p_previous_pic = picture_Hold( p_picture );
> +        date_Set( &p_sys->next_output_pts, p_sys->src_date );
>      }
>  
>      /* Check if we can skip input as better should follow */
> -    else if( src_date <
> +    else if( p_sys->src_date <
>          ( date_Get( &p_sys->next_output_pts ) - p_sys->i_output_frame_interval ) )
>      {
> -        if( p_sys->p_previous_pic )
> -            picture_Release( p_sys->p_previous_pic );
> -        p_sys->p_previous_pic = p_picture;
> +        picture_Release( p_picture );
>          return NULL;
>      }
>  
> -    SetOutputDate( p_sys, p_sys->p_previous_pic );
> +    p_sys->p_previous_pic = picture_Hold( p_picture );
> +    return SetOutputDate( p_sys, p_picture );
> +}
> +
> +static picture_t *Drain( filter_t *p_filter )
> +{
> +    filter_sys_t *p_sys = p_filter->p_sys;
> +
> +    if ( p_sys->p_previous_pic == NULL )
> +        return NULL;
>  
> -    picture_t *last_pic = p_sys->p_previous_pic;
>      /* Duplicating pictures are not that effective and framerate increase
>          should be avoided, it's only here as filter should work in that direction too*/
> -    while( unlikely( (date_Get( &p_sys->next_output_pts ) + p_sys->i_output_frame_interval ) < src_date ) )
> +    if ( unlikely( date_Get( &p_sys->next_output_pts ) < p_sys->src_date ) )
>      {
>          picture_t *p_tmp = NULL;
>          p_tmp = picture_NewFromFormat( &p_filter->fmt_out.video );
>  
> -        picture_Copy( p_tmp, p_sys->p_previous_pic);
> -        SetOutputDate( p_sys, p_tmp );
> -
> -        vlc_picture_chain_AppendChain( last_pic, p_tmp );
> -        last_pic = p_tmp;
> +        picture_Copy( p_tmp, p_sys->p_previous_pic );
> +        return SetOutputDate( p_sys, p_tmp );
>      }
>  
> -    last_pic = p_sys->p_previous_pic;
> -    p_sys->p_previous_pic = p_picture;
> -    return last_pic;
> +    // no more draining possible
> +    picture_Release( p_sys->p_previous_pic );
> +    p_sys->p_previous_pic = NULL;
> +    return NULL;
>  }
>  
>  static void Flush( filter_t *p_filter )
> @@ -153,7 +156,7 @@ static void Close( filter_t *p_filter )
>  
>  static const struct vlc_filter_operations filter_ops =
>  {
> -    .filter_video = Filter, .close = Close,
> +    .filter_video = Filter, .drain_video = Drain, .close = Close,
>      .flush = Flush,
>  };
>  
> -- 
> 2.26.2
> 
> _______________________________________________
> 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