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

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Jan Ekström:
> On Mon, Mar 13, 2023 at 12:10 AM Andreas Rheinhardt
> <andreas.rheinhardt@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 :) .
> 

The second one (or rather: one that combines both).

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".