From: "Jan Ekström" <jeebjp@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] avutil/frame: move wipe_side_data counter to its utilized scope
Date: Sat, 18 Mar 2023 21:51:34 +0200
Message-ID: <CAEu79SY14qLhCz6Tjk1jFx9diFRNkaXCOEs9nT6-O8ZziTprbA@mail.gmail.com> (raw)
In-Reply-To: <AS8P250MB0744CBA2C9CFD6CFD442786D8FB89@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM>
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 :) .
Jan
_______________________________________________
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".
next prev parent reply other threads:[~2023-03-18 19:51 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-12 19:42 Jan Ekström
2023-03-12 22:11 ` Andreas Rheinhardt
2023-03-13 9:53 ` Jan Ekström
2023-03-18 19:51 ` Jan Ekström [this message]
2023-03-18 21:48 ` Andreas Rheinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAEu79SY14qLhCz6Tjk1jFx9diFRNkaXCOEs9nT6-O8ZziTprbA@mail.gmail.com \
--to=jeebjp@gmail.com \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git