[FFmpeg-devel] [PATCH 1/4] lavfi: add frame counter into AVFilterLink and use it in filters.

Clément Bœsch ubitux at gmail.com
Tue Apr 16 03:14:59 CEST 2013


On Mon, Apr 15, 2013 at 11:10:47PM +0200, Stefano Sabatini wrote:
[...]
> > diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> > index 0b970d0..38bc5ee 100644
> > --- a/libavfilter/avfilter.h
> > +++ b/libavfilter/avfilter.h
> > @@ -718,6 +718,11 @@ struct AVFilterLink {
> >       * Link processing flags.
> >       */
> >      unsigned flags;
> > +
> > +    /**
> > +     * Number of frames a link has seen
> 
> nit: number of frames
> 
> Also I don't really like ambiguous expressions like "has seen", what
> about:
> 
> number of frames have seen sent through the link
> 

I have trouble understanding that sentence... "number of frames passed
through that link"?

[...]
> > -static int draw_text(AVFilterContext *ctx, AVFrame *frame,
> > -                     int width, int height)
> > +static int draw_text(AVFilterContext *ctx, AVFilterLink *inlink,
> > +                     AVFrame *frame, int width, int height)
> 
> Nit:
> inlink = ctx->inputs[0];
> 
> no need to change the signature
> 

Changed locally.

[...]
> > diff --git a/libavfilter/vf_framestep.c b/libavfilter/vf_framestep.c
> > index bd079cc..fb20411 100644
> > --- a/libavfilter/vf_framestep.c
> > +++ b/libavfilter/vf_framestep.c
> > @@ -30,7 +30,7 @@
> >  
> >  typedef struct {
> >      const AVClass *class;
> > -    int frame_step, frame_count;
> > +    int frame_step;
> >  } FrameStepContext;
> >  
> >  #define OFFSET(x) offsetof(FrameStepContext, x)
> > @@ -64,7 +64,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *ref)
> >  {
> >      FrameStepContext *framestep = inlink->dst->priv;
> >  
> > -    if (!(framestep->frame_count++ % framestep->frame_step)) {
> > +    if (!(inlink->frame_count % framestep->frame_step)) {
> 
> Seems wrong. If you use inlink->frame_count, since it will be already
> set to 1, while it was previously 0 for the first frame (and
> post-incremented).
> 

No, it will be set to 0 since inlink->frame_count is incremented just
*after* the filter_frame() call.

[...]

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130416/36082b36/attachment.asc>


More information about the ffmpeg-devel mailing list