[FFmpeg-devel,06/16] avformat/hlsenc: Use smaller scope for some variables

Several variables which are only used when the HLS_SINGLE_FILE bit is
not set have nevertheless been set regardless of whether this bit is set.
This has been changed; futhermore, these variables have been moved into
a smaller scope.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/hlsenc.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

> 在 2019年12月16日，08:04，Andreas Rheinhardt <andreas.rheinhardt@gmail.com> 写道：
> 
> Several variables which are only used when the HLS_SINGLE_FILE bit is
> not set have nevertheless been set regardless of whether this bit is set.
> This has been changed; futhermore, these variables have been moved into
> a smaller scope.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> libavformat/hlsenc.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 5b3856099c..62f66c4c65 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -2237,8 +2237,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>     int ret = 0, can_split = 1, i, j;
>     int stream_index = 0;
>     int range_length = 0;
> -    const char *proto = NULL;
> -    int use_temp_file = 0;
>     uint8_t *buffer = NULL;
>     VariantStream *vs = NULL;
>     char *old_filename = NULL;
> @@ -2337,11 +2335,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>             }
>         }
> 
> -        if (oc->url[0]) {
> -            proto = avio_find_protocol_name(oc->url);
> -            use_temp_file = proto && !strcmp(proto, "file") && (hls->flags & HLS_TEMP_FILE);
> -        }
> -
>         if (hls->flags & HLS_SINGLE_FILE) {
>             ret = flush_dynbuf(vs, &range_length);
>             av_freep(&vs->temp_buffer);
> @@ -2350,6 +2343,14 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>             }
>             vs->size = range_length;
>         } else {
> +            int use_temp_file = 0;
> +
> +            if (oc->url[0]) {
> +                const char *proto = avio_find_protocol_name(oc->url);
> +                use_temp_file = proto && !strcmp(proto, "file")
> +                                      && (hls->flags & HLS_TEMP_FILE);
> +            }
> +
>             if ((hls->max_seg_size > 0 && (vs->size >= hls->max_seg_size)) || !byterange_mode) {
>                 AVDictionary *options = NULL;
>                 char *filename = NULL;
> @@ -2399,10 +2400,9 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>                 av_freep(&vs->temp_buffer);
>                 av_freep(&filename);
>             }
> -        }
> 
> -        if (use_temp_file && !(hls->flags & HLS_SINGLE_FILE)) {
> -            hls_rename_temp_file(s, oc);
> +            if (use_temp_file)
> +                hls_rename_temp_file(s, oc);
>         }
> 
>         old_filename = av_strdup(oc->url);
> -- 
> 2.20.1
> 
> _______________________________________________
> 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”.

There maybe need and description in developer document about the CodeStyle.
It’s looks good to me about move the vars to small scope, but i cannot sure, 
because when somebody review my dash patch told me i should declaration the variables name at the the start of the function.
i think there have lots of module can “Use smaller scope for some variables.” if the code style don’t make a standard description about “variables declaration".

Thanks
Steven

On Mon, Dec 16, 2019 at 4:07 AM Steven Liu <lq@chinaffmpeg.org> wrote:

>
>
> > 在 2019年12月16日，08:04，Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> 写道：
> >
> > Several variables which are only used when the HLS_SINGLE_FILE bit is
> > not set have nevertheless been set regardless of whether this bit is set.
> > This has been changed; futhermore, these variables have been moved into
> > a smaller scope.
> >
> > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> > ---
> > libavformat/hlsenc.c | 20 ++++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index 5b3856099c..62f66c4c65 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -2237,8 +2237,6 @@ static int hls_write_packet(AVFormatContext *s,
> AVPacket *pkt)
> >     int ret = 0, can_split = 1, i, j;
> >     int stream_index = 0;
> >     int range_length = 0;
> > -    const char *proto = NULL;
> > -    int use_temp_file = 0;
> >     uint8_t *buffer = NULL;
> >     VariantStream *vs = NULL;
> >     char *old_filename = NULL;
> > @@ -2337,11 +2335,6 @@ static int hls_write_packet(AVFormatContext *s,
> AVPacket *pkt)
> >             }
> >         }
> >
> > -        if (oc->url[0]) {
> > -            proto = avio_find_protocol_name(oc->url);
> > -            use_temp_file = proto && !strcmp(proto, "file") &&
> (hls->flags & HLS_TEMP_FILE);
> > -        }
> > -
> >         if (hls->flags & HLS_SINGLE_FILE) {
> >             ret = flush_dynbuf(vs, &range_length);
> >             av_freep(&vs->temp_buffer);
> > @@ -2350,6 +2343,14 @@ static int hls_write_packet(AVFormatContext *s,
> AVPacket *pkt)
> >             }
> >             vs->size = range_length;
> >         } else {
> > +            int use_temp_file = 0;
> > +
> > +            if (oc->url[0]) {
> > +                const char *proto = avio_find_protocol_name(oc->url);
> > +                use_temp_file = proto && !strcmp(proto, "file")
> > +                                      && (hls->flags & HLS_TEMP_FILE);
> > +            }
> > +
> >             if ((hls->max_seg_size > 0 && (vs->size >=
> hls->max_seg_size)) || !byterange_mode) {
> >                 AVDictionary *options = NULL;
> >                 char *filename = NULL;
> > @@ -2399,10 +2400,9 @@ static int hls_write_packet(AVFormatContext *s,
> AVPacket *pkt)
> >                 av_freep(&vs->temp_buffer);
> >                 av_freep(&filename);
> >             }
> > -        }
> >
> > -        if (use_temp_file && !(hls->flags & HLS_SINGLE_FILE)) {
> > -            hls_rename_temp_file(s, oc);
> > +            if (use_temp_file)
> > +                hls_rename_temp_file(s, oc);
> >         }
> >
> >         old_filename = av_strdup(oc->url);
> > --
> > 2.20.1
> >
> > _______________________________________________
> > 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”.
>
> There maybe need and description in developer document about the CodeStyle.
> It’s looks good to me about move the vars to small scope, but i cannot
> sure,
> because when somebody review my dash patch told me i should declaration
> the variables name at the the start of the function.
> i think there have lots of module can “Use smaller scope for some
> variables.” if the code style don’t make a standard description about
> “variables declaration".
>
> FFmpeg uses the C90 standard with some features from C99 (like designated
initializers, for loops with variable definition etc.) and one part of C90
is the rule that declarations always have to be at the beginning of blocks,
before any statement; but this goes for all blocks, not only for a function
body block. So it is allowed to declare variables at the beginning of each
block, as I do here. Is it possible that your reviewer mentioned this rule?

- Andreas