[FFmpeg-devel] [PATCH] avutil/frame: move wipe_side_data counter to its utilized scope

[Jan Ekström]

On Mon, Mar 13, 2023 at 12:10 AM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
>
> Jan Ekström:
> > Originally in 77b2cd7b41d7ec8008b6fac753c04f77824c514c this
> > counter was separate in av_frame_unref, in which the same counter
> > was re-utilized multiple times over multiple loops.
> >
> > This code was then refactored into wipe_side_data as-is in
> > 5d839778b9f3edb682b7f71dde4f80f07c75b098 , keeping the location of
> > counter initialization. This was unnecessary, as the counter was
> > no longer utilized outside of the for loop's scope and thus could
> > reside within the loop itself.
> > ---
> >  libavutil/frame.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index 9545477acc..d06a673779 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -74,9 +74,7 @@ static void free_side_data(AVFrameSideData **ptr_sd)
> >
> >  static void wipe_side_data(AVFrame *frame)
> >  {
> > -    int i;
> > -
> > -    for (i = 0; i < frame->nb_side_data; i++) {
> > +    for (int i = 0; i < frame->nb_side_data; i++) {
> >          free_side_data(&frame->side_data[i]);
> >      }
> >      frame->nb_side_data = 0;
>
> Don't create a patch for a single for loop; do this for all for loops in
> this file where it is possible.

Alright, went through the loops in avutil/frame.c. Just to understand
your preferences I made two separate commits available in
https://github.com/jeeb/ffmpeg/commits/avutil_wipe_side_data_counter_fixup

First that moves the initialization in cases where there is just a
single loop utilizing that counter.
Second one that moves the initialization also in cases where the same
counter is reset and reused for multiple loops.

So yea, just note which of these I should post on ML and I'll do that :) .

Jan