diff mbox

[FFmpeg-devel,14/17] avformat/hdsenc: Add explicit deinit function

Message ID 20191226105342.11175-14-andreas.rheinhardt@gmail.com
State Accepted
Headers show

Commit Message

Andreas Rheinhardt Dec. 26, 2019, 10:53 a.m. UTC 
hdsenc already had an explicit function to free all allocations in case
of an error, but it was not marked as deinit function, so that it was
not called automatically when the AVFormatContext for muxing gets freed.

Using an explicit deinit function also makes the code cleaner by
allowing to return immediately without "goto fail".

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
Unchanged since last time.

 libavformat/hdsenc.c | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

Comments

Andreas Rheinhardt April 28, 2020, 11:21 p.m. UTC | #1 
Andreas Rheinhardt:
> hdsenc already had an explicit function to free all allocations in case
> of an error, but it was not marked as deinit function, so that it was
> not called automatically when the AVFormatContext for muxing gets freed.
> 
> Using an explicit deinit function also makes the code cleaner by
> allowing to return immediately without "goto fail".
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> Unchanged since last time.
> 
>  libavformat/hdsenc.c | 33 +++++++++++----------------------
>  1 file changed, 11 insertions(+), 22 deletions(-)
> 
> diff --git a/libavformat/hdsenc.c b/libavformat/hdsenc.c
> index 46f0026bce..353a45f6df 100644
> --- a/libavformat/hdsenc.c
> +++ b/libavformat/hdsenc.c
> @@ -317,21 +317,18 @@ static int hds_write_header(AVFormatContext *s)
>      ff_const59 AVOutputFormat *oformat;
>  
>      if (mkdir(s->url, 0777) == -1 && errno != EEXIST) {
> -        ret = AVERROR(errno);
>          av_log(s, AV_LOG_ERROR , "Failed to create directory %s\n", s->url);
> -        goto fail;
> +        return AVERROR(errno);
>      }
>  
>      oformat = av_guess_format("flv", NULL, NULL);
>      if (!oformat) {
> -        ret = AVERROR_MUXER_NOT_FOUND;
> -        goto fail;
> +        return AVERROR_MUXER_NOT_FOUND;
>      }
>  
>      c->streams = av_mallocz_array(s->nb_streams, sizeof(*c->streams));
>      if (!c->streams) {
> -        ret = AVERROR(ENOMEM);
> -        goto fail;
> +        return AVERROR(ENOMEM);
>      }
>  
>      for (i = 0; i < s->nb_streams; i++) {
> @@ -341,8 +338,7 @@ static int hds_write_header(AVFormatContext *s)
>  
>          if (!st->codecpar->bit_rate) {
>              av_log(s, AV_LOG_ERROR, "No bit rate set for stream %d\n", i);
> -            ret = AVERROR(EINVAL);
> -            goto fail;
> +            return AVERROR(EINVAL);
>          }
>          if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
>              if (os->has_video) {
> @@ -358,8 +354,7 @@ static int hds_write_header(AVFormatContext *s)
>              os->has_audio = 1;
>          } else {
>              av_log(s, AV_LOG_ERROR, "Unsupported stream type in stream %d\n", i);
> -            ret = AVERROR(EINVAL);
> -            goto fail;
> +            return AVERROR(EINVAL);
>          }
>          os->bitrate += s->streams[i]->codecpar->bit_rate;
>  
> @@ -367,8 +362,7 @@ static int hds_write_header(AVFormatContext *s)
>              os->first_stream = i;
>              ctx = avformat_alloc_context();
>              if (!ctx) {
> -                ret = AVERROR(ENOMEM);
> -                goto fail;
> +                return AVERROR(ENOMEM);
>              }
>              os->ctx = ctx;
>              ctx->oformat = oformat;
> @@ -379,8 +373,7 @@ static int hds_write_header(AVFormatContext *s)
>                                           AVIO_FLAG_WRITE, os,
>                                           NULL, hds_write, NULL);
>              if (!ctx->pb) {
> -                ret = AVERROR(ENOMEM);
> -                goto fail;
> +                return AVERROR(ENOMEM);
>              }
>          } else {
>              ctx = os->ctx;
> @@ -388,8 +381,7 @@ static int hds_write_header(AVFormatContext *s)
>          s->streams[i]->id = c->nb_streams;
>  
>          if (!(st = avformat_new_stream(ctx, NULL))) {
> -            ret = AVERROR(ENOMEM);
> -            goto fail;
> +            return AVERROR(ENOMEM);
>          }
>          avcodec_parameters_copy(st->codecpar, s->streams[i]->codecpar);
>          st->codecpar->codec_tag = 0;
> @@ -403,7 +395,7 @@ static int hds_write_header(AVFormatContext *s)
>          OutputStream *os = &c->streams[i];
>          int j;
>          if ((ret = avformat_write_header(os->ctx, NULL)) < 0) {
> -             goto fail;
> +             return ret;
>          }
>          os->ctx_inited = 1;
>          avio_flush(os->ctx->pb);
> @@ -414,7 +406,7 @@ static int hds_write_header(AVFormatContext *s)
>                   "%s/stream%d_temp", s->url, i);
>          ret = init_file(s, os, 0);
>          if (ret < 0)
> -            goto fail;
> +            return ret;
>  
>          if (!os->has_video && c->min_frag_duration <= 0) {
>              av_log(s, AV_LOG_WARNING,
> @@ -425,9 +417,6 @@ static int hds_write_header(AVFormatContext *s)
>      }
>      ret = write_manifest(s, 0);
>  
> -fail:
> -    if (ret)
> -        hds_free(s);
>      return ret;
>  }
>  
> @@ -557,7 +546,6 @@ static int hds_write_trailer(AVFormatContext *s)
>          rmdir(s->url);
>      }
>  
> -    hds_free(s);
>      return 0;
>  }
>  
> @@ -588,5 +576,6 @@ AVOutputFormat ff_hds_muxer = {
>      .write_header   = hds_write_header,
>      .write_packet   = hds_write_packet,
>      .write_trailer  = hds_write_trailer,
> +    .deinit         = hds_free,
>      .priv_class     = &hds_class,
>  };
> 
Will apply this tomorrow unless there are objections.

- Andreas
Andreas Rheinhardt April 30, 2020, 3:51 p.m. UTC | #2 
Andreas Rheinhardt:
> Andreas Rheinhardt:
>> hdsenc already had an explicit function to free all allocations in case
>> of an error, but it was not marked as deinit function, so that it was
>> not called automatically when the AVFormatContext for muxing gets freed.
>>
>> Using an explicit deinit function also makes the code cleaner by
>> allowing to return immediately without "goto fail".
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>> Unchanged since last time.
>>
>>  libavformat/hdsenc.c | 33 +++++++++++----------------------
>>  1 file changed, 11 insertions(+), 22 deletions(-)
>>
>> diff --git a/libavformat/hdsenc.c b/libavformat/hdsenc.c
>> index 46f0026bce..353a45f6df 100644
>> --- a/libavformat/hdsenc.c
>> +++ b/libavformat/hdsenc.c
>> @@ -317,21 +317,18 @@ static int hds_write_header(AVFormatContext *s)
>>      ff_const59 AVOutputFormat *oformat;
>>  
>>      if (mkdir(s->url, 0777) == -1 && errno != EEXIST) {
>> -        ret = AVERROR(errno);
>>          av_log(s, AV_LOG_ERROR , "Failed to create directory %s\n", s->url);
>> -        goto fail;
>> +        return AVERROR(errno);
>>      }
>>  
>>      oformat = av_guess_format("flv", NULL, NULL);
>>      if (!oformat) {
>> -        ret = AVERROR_MUXER_NOT_FOUND;
>> -        goto fail;
>> +        return AVERROR_MUXER_NOT_FOUND;
>>      }
>>  
>>      c->streams = av_mallocz_array(s->nb_streams, sizeof(*c->streams));
>>      if (!c->streams) {
>> -        ret = AVERROR(ENOMEM);
>> -        goto fail;
>> +        return AVERROR(ENOMEM);
>>      }
>>  
>>      for (i = 0; i < s->nb_streams; i++) {
>> @@ -341,8 +338,7 @@ static int hds_write_header(AVFormatContext *s)
>>  
>>          if (!st->codecpar->bit_rate) {
>>              av_log(s, AV_LOG_ERROR, "No bit rate set for stream %d\n", i);
>> -            ret = AVERROR(EINVAL);
>> -            goto fail;
>> +            return AVERROR(EINVAL);
>>          }
>>          if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
>>              if (os->has_video) {
>> @@ -358,8 +354,7 @@ static int hds_write_header(AVFormatContext *s)
>>              os->has_audio = 1;
>>          } else {
>>              av_log(s, AV_LOG_ERROR, "Unsupported stream type in stream %d\n", i);
>> -            ret = AVERROR(EINVAL);
>> -            goto fail;
>> +            return AVERROR(EINVAL);
>>          }
>>          os->bitrate += s->streams[i]->codecpar->bit_rate;
>>  
>> @@ -367,8 +362,7 @@ static int hds_write_header(AVFormatContext *s)
>>              os->first_stream = i;
>>              ctx = avformat_alloc_context();
>>              if (!ctx) {
>> -                ret = AVERROR(ENOMEM);
>> -                goto fail;
>> +                return AVERROR(ENOMEM);
>>              }
>>              os->ctx = ctx;
>>              ctx->oformat = oformat;
>> @@ -379,8 +373,7 @@ static int hds_write_header(AVFormatContext *s)
>>                                           AVIO_FLAG_WRITE, os,
>>                                           NULL, hds_write, NULL);
>>              if (!ctx->pb) {
>> -                ret = AVERROR(ENOMEM);
>> -                goto fail;
>> +                return AVERROR(ENOMEM);
>>              }
>>          } else {
>>              ctx = os->ctx;
>> @@ -388,8 +381,7 @@ static int hds_write_header(AVFormatContext *s)
>>          s->streams[i]->id = c->nb_streams;
>>  
>>          if (!(st = avformat_new_stream(ctx, NULL))) {
>> -            ret = AVERROR(ENOMEM);
>> -            goto fail;
>> +            return AVERROR(ENOMEM);
>>          }
>>          avcodec_parameters_copy(st->codecpar, s->streams[i]->codecpar);
>>          st->codecpar->codec_tag = 0;
>> @@ -403,7 +395,7 @@ static int hds_write_header(AVFormatContext *s)
>>          OutputStream *os = &c->streams[i];
>>          int j;
>>          if ((ret = avformat_write_header(os->ctx, NULL)) < 0) {
>> -             goto fail;
>> +             return ret;
>>          }
>>          os->ctx_inited = 1;
>>          avio_flush(os->ctx->pb);
>> @@ -414,7 +406,7 @@ static int hds_write_header(AVFormatContext *s)
>>                   "%s/stream%d_temp", s->url, i);
>>          ret = init_file(s, os, 0);
>>          if (ret < 0)
>> -            goto fail;
>> +            return ret;
>>  
>>          if (!os->has_video && c->min_frag_duration <= 0) {
>>              av_log(s, AV_LOG_WARNING,
>> @@ -425,9 +417,6 @@ static int hds_write_header(AVFormatContext *s)
>>      }
>>      ret = write_manifest(s, 0);
>>  
>> -fail:
>> -    if (ret)
>> -        hds_free(s);
>>      return ret;
>>  }
>>  
>> @@ -557,7 +546,6 @@ static int hds_write_trailer(AVFormatContext *s)
>>          rmdir(s->url);
>>      }
>>  
>> -    hds_free(s);
>>      return 0;
>>  }
>>  
>> @@ -588,5 +576,6 @@ AVOutputFormat ff_hds_muxer = {
>>      .write_header   = hds_write_header,
>>      .write_packet   = hds_write_packet,
>>      .write_trailer  = hds_write_trailer,
>> +    .deinit         = hds_free,
>>      .priv_class     = &hds_class,
>>  };
>>
> Will apply this tomorrow unless there are objections.
> 
> - Andreas
> 
Applied.

- Andreas
diff mbox

Patch

diff --git a/libavformat/hdsenc.c b/libavformat/hdsenc.c
index 46f0026bce..353a45f6df 100644
--- a/libavformat/hdsenc.c
+++ b/libavformat/hdsenc.c
@@ -317,21 +317,18 @@  static int hds_write_header(AVFormatContext *s)
     ff_const59 AVOutputFormat *oformat;
 
     if (mkdir(s->url, 0777) == -1 && errno != EEXIST) {
-        ret = AVERROR(errno);
         av_log(s, AV_LOG_ERROR , "Failed to create directory %s\n", s->url);
-        goto fail;
+        return AVERROR(errno);
     }
 
     oformat = av_guess_format("flv", NULL, NULL);
     if (!oformat) {
-        ret = AVERROR_MUXER_NOT_FOUND;
-        goto fail;
+        return AVERROR_MUXER_NOT_FOUND;
     }
 
     c->streams = av_mallocz_array(s->nb_streams, sizeof(*c->streams));
     if (!c->streams) {
-        ret = AVERROR(ENOMEM);
-        goto fail;
+        return AVERROR(ENOMEM);
     }
 
     for (i = 0; i < s->nb_streams; i++) {
@@ -341,8 +338,7 @@  static int hds_write_header(AVFormatContext *s)
 
         if (!st->codecpar->bit_rate) {
             av_log(s, AV_LOG_ERROR, "No bit rate set for stream %d\n", i);
-            ret = AVERROR(EINVAL);
-            goto fail;
+            return AVERROR(EINVAL);
         }
         if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
             if (os->has_video) {
@@ -358,8 +354,7 @@  static int hds_write_header(AVFormatContext *s)
             os->has_audio = 1;
         } else {
             av_log(s, AV_LOG_ERROR, "Unsupported stream type in stream %d\n", i);
-            ret = AVERROR(EINVAL);
-            goto fail;
+            return AVERROR(EINVAL);
         }
         os->bitrate += s->streams[i]->codecpar->bit_rate;
 
@@ -367,8 +362,7 @@  static int hds_write_header(AVFormatContext *s)
             os->first_stream = i;
             ctx = avformat_alloc_context();
             if (!ctx) {
-                ret = AVERROR(ENOMEM);
-                goto fail;
+                return AVERROR(ENOMEM);
             }
             os->ctx = ctx;
             ctx->oformat = oformat;
@@ -379,8 +373,7 @@  static int hds_write_header(AVFormatContext *s)
                                          AVIO_FLAG_WRITE, os,
                                          NULL, hds_write, NULL);
             if (!ctx->pb) {
-                ret = AVERROR(ENOMEM);
-                goto fail;
+                return AVERROR(ENOMEM);
             }
         } else {
             ctx = os->ctx;
@@ -388,8 +381,7 @@  static int hds_write_header(AVFormatContext *s)
         s->streams[i]->id = c->nb_streams;
 
         if (!(st = avformat_new_stream(ctx, NULL))) {
-            ret = AVERROR(ENOMEM);
-            goto fail;
+            return AVERROR(ENOMEM);
         }
         avcodec_parameters_copy(st->codecpar, s->streams[i]->codecpar);
         st->codecpar->codec_tag = 0;
@@ -403,7 +395,7 @@  static int hds_write_header(AVFormatContext *s)
         OutputStream *os = &c->streams[i];
         int j;
         if ((ret = avformat_write_header(os->ctx, NULL)) < 0) {
-             goto fail;
+             return ret;
         }
         os->ctx_inited = 1;
         avio_flush(os->ctx->pb);
@@ -414,7 +406,7 @@  static int hds_write_header(AVFormatContext *s)
                  "%s/stream%d_temp", s->url, i);
         ret = init_file(s, os, 0);
         if (ret < 0)
-            goto fail;
+            return ret;
 
         if (!os->has_video && c->min_frag_duration <= 0) {
             av_log(s, AV_LOG_WARNING,
@@ -425,9 +417,6 @@  static int hds_write_header(AVFormatContext *s)
     }
     ret = write_manifest(s, 0);
 
-fail:
-    if (ret)
-        hds_free(s);
     return ret;
 }
 
@@ -557,7 +546,6 @@  static int hds_write_trailer(AVFormatContext *s)
         rmdir(s->url);
     }
 
-    hds_free(s);
     return 0;
 }
 
@@ -588,5 +576,6 @@  AVOutputFormat ff_hds_muxer = {
     .write_header   = hds_write_header,
     .write_packet   = hds_write_packet,
     .write_trailer  = hds_write_trailer,
+    .deinit         = hds_free,
     .priv_class     = &hds_class,
 };