[FFmpeg-devel] [PATCH] lavfi/select: make select_frame() return void

[Stefano Sabatini]

On date Tuesday 2013-04-16 22:28:50 +0200, Clément Bœsch encoded:
> On Mon, Apr 15, 2013 at 09:02:56PM +0200, Stefano Sabatini wrote:
> > Set select->select internally to the function, avoid lossy double->int
> > conversion causing spurious frame drop (e.g. in case the evaluation
> > result is positive and less than 1).
> > ---
> >  libavfilter/f_select.c |    8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
> > index 9a5666f..ca9bdc9 100644
> > --- a/libavfilter/f_select.c
> > +++ b/libavfilter/f_select.c
> > @@ -247,7 +247,7 @@ static double get_scene_score(AVFilterContext *ctx, AVFrame *frame)
> >  #define D2TS(d)  (isnan(d) ? AV_NOPTS_VALUE : (int64_t)(d))
> >  #define TS2D(ts) ((ts) == AV_NOPTS_VALUE ? NAN : (double)(ts))
> >  
> > -static int select_frame(AVFilterContext *ctx, AVFrame *frame)
> > +static void select_frame(AVFilterContext *ctx, AVFrame *frame)
> >  {
> >      SelectContext *select = ctx->priv;
> >      AVFilterLink *inlink = ctx->inputs[0];
> > @@ -284,7 +284,7 @@ static int select_frame(AVFilterContext *ctx, AVFrame *frame)
> >          break;
> >      }
> >  
> > -    res = av_expr_eval(select->expr, select->var_values, NULL);
> > +    select->select = res = av_expr_eval(select->expr, select->var_values, NULL);
> >      av_log(inlink->dst, AV_LOG_DEBUG,
> >             "n:%f pts:%f t:%f key:%d",
> >             select->var_values[VAR_N],
> > @@ -322,15 +322,13 @@ static int select_frame(AVFilterContext *ctx, AVFrame *frame)
> >      select->var_values[VAR_N] += 1.0;
> >      select->var_values[VAR_PREV_PTS] = select->var_values[VAR_PTS];
> >      select->var_values[VAR_PREV_T]   = select->var_values[VAR_T];
> > -
> > -    return res;
> >  }
> >  
> >  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> >  {
> >      SelectContext *select = inlink->dst->priv;
> >  
> > -    select->select = select_frame(inlink->dst, frame);
> > +    select_frame(inlink->dst, frame);
> >      if (select->select)
> >          return ff_filter_frame(inlink->dst->outputs[0], frame);
> >  
> 

> Fine, even though you could just return double (and not set select->select
> in that function).

Doesn't make much sense not to set that and only that value in the context.

> 
> (Totally unrelated note: some constness for the frame pointer might make
> sense).

Unrelated and there are other consting issues to be fixed.

Will push soon, thanks.
-- 
FFmpeg = Fabulous Forgiving Mysterious Powered Exuberant Gadget