diff mbox series

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

Message ID 20230127131639.4928-1-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,1/4] lavfi/framesync: use a local variable to shorten code | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov Jan. 27, 2023, 1:16 p.m. UTC 
---
 libavfilter/framesync.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

Comments

Nicolas George Jan. 27, 2023, 2:39 p.m. UTC | #1 
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.
diff mbox series

Patch

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];
+
+            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);
             }
         }