[FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code

[Nicolas George]

Anton Khirnov (12023-01-27):
> ---
>  libavfilter/framesync.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
> index ee91e4cf68..153db4fa21 100644
> --- a/libavfilter/framesync.c
> +++ b/libavfilter/framesync.c
> @@ -201,24 +201,23 @@ static int framesync_advance(FFFrameSync *fs)
>              break;
>          }
>          for (i = 0; i < fs->nb_in; i++) {
> -            if (fs->in[i].pts_next == pts ||
> -                (fs->in[i].ts_mode == TS_NEAREST &&
> -                 fs->in[i].have_next &&
> -                 fs->in[i].pts_next != INT64_MAX && fs->in[i].pts != AV_NOPTS_VALUE &&
> -                 fs->in[i].pts_next - pts < pts - fs->in[i].pts) ||
> -                (fs->in[i].before == EXT_INFINITY &&
> -                 fs->in[i].state == STATE_BOF)) {
> -                av_frame_free(&fs->in[i].frame);
> -                fs->in[i].frame      = fs->in[i].frame_next;
> -                fs->in[i].pts        = fs->in[i].pts_next;
> -                fs->in[i].frame_next = NULL;
> -                fs->in[i].pts_next   = AV_NOPTS_VALUE;
> -                fs->in[i].have_next  = 0;
> -                fs->in[i].state      = fs->in[i].frame ? STATE_RUN : STATE_EOF;
> -                if (fs->in[i].sync == fs->sync_level && fs->in[i].frame)

> +            FFFrameSyncIn * const in = &fs->in[i];

Get rid of the const, since the rest of this code does not use anything
similar and the benefit is dubious.

> +
> +            if (in->pts_next == pts ||
> +                (in->ts_mode == TS_NEAREST && in->have_next             &&
> +                 in->pts_next != INT64_MAX && in->pts != AV_NOPTS_VALUE &&
> +                 in->pts_next - pts < pts - in->pts) ||
> +                (in->before == EXT_INFINITY && in->state == STATE_BOF)) {
> +                av_frame_free(&in->frame);
> +                in->frame      = in->frame_next;
> +                in->pts        = in->pts_next;
> +                in->frame_next = NULL;
> +                in->pts_next   = AV_NOPTS_VALUE;
> +                in->have_next  = 0;
> +                in->state      = in->frame ? STATE_RUN : STATE_EOF;
> +                if (in->sync == fs->sync_level && in->frame)
>                      fs->frame_ready = 1;
> -                if (fs->in[i].state == STATE_EOF &&
> -                    fs->in[i].after == EXT_STOP)
> +                if (in->state == STATE_EOF && in->after == EXT_STOP)
>                      framesync_eof(fs);
>              }
>          }

I do not like this change. In fact, the more I think about it the more I
remember that I specifically considered using intermediate pointers for
inputs and deciding against: too much extra noise if done everywhere,
too little benefit for the inconsistency if done only where there are
many.

So: thanks but no.

-- 
  Nicolas George