diff mbox

[FFmpeg-devel,v2] avformat/hlsenc: fix hlsenc bug at windows system

Message ID 20170112165531.3495-1-lq@chinaffmpeg.org
State Accepted
Commit b97e9cba0b3b926aa3f9940519e93f2ade599af2
Headers show

Commit Message

Liu Steven Jan. 12, 2017, 4:55 p.m. UTC 
when hlsenc use flag second_level_segment_index,
second_level_segment_size and second_level_segment_duration,
the rename is ok but the output filename always use the old filename
so move the rename operation after the close the ts file and
before open new segment

Reported-by: Christian Johannesen <chrisjohannesen@gmail.com>
Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/hlsenc.c | 49 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 14 deletions(-)

Comments

Steven Liu Jan. 13, 2017, midnight UTC | #1 
2017-01-13 0:55 GMT+08:00 Steven Liu <lq@chinaffmpeg.org>:

> when hlsenc use flag second_level_segment_index,
> second_level_segment_size and second_level_segment_duration,
> the rename is ok but the output filename always use the old filename
> so move the rename operation after the close the ts file and
> before open new segment
>
> Reported-by: Christian Johannesen <chrisjohannesen@gmail.com>
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  libavformat/hlsenc.c | 49 ++++++++++++++++++++++++++++++
> +++++--------------
>  1 file changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index a2c606c..f662913 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -453,16 +453,10 @@ static int hls_append_segment(struct AVFormatContext
> *s, HLSContext *hls, double
>
>      if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
> HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
>          strlen(hls->current_segment_final_filename_fmt)) {
> -        char * old_filename = av_strdup(hls->avf->filename);  // %%s will
> be %s after strftime
> -        if (!old_filename) {
> -            av_free(en);
> -            return AVERROR(ENOMEM);
> -        }
>          av_strlcpy(hls->avf->filename, hls->current_segment_final_filename_fmt,
> sizeof(hls->avf->filename));
>          if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) {
>              char * filename = av_strdup(hls->avf->filename);  // %%s will
> be %s after strftime
>              if (!filename) {
> -                av_free(old_filename);
>                  av_free(en);
>                  return AVERROR(ENOMEM);
>              }
> @@ -473,7 +467,6 @@ static int hls_append_segment(struct AVFormatContext
> *s, HLSContext *hls, double
>                          "you can try to remove second_level_segment_size
> flag\n",
>                         filename);
>                  av_free(filename);
> -                av_free(old_filename);
>                  av_free(en);
>                  return AVERROR(EINVAL);
>              }
> @@ -482,7 +475,6 @@ static int hls_append_segment(struct AVFormatContext
> *s, HLSContext *hls, double
>          if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_DURATION) {
>              char * filename = av_strdup(hls->avf->filename);  // %%t will
> be %t after strftime
>              if (!filename) {
> -                av_free(old_filename);
>                  av_free(en);
>                  return AVERROR(ENOMEM);
>              }
> @@ -493,14 +485,11 @@ static int hls_append_segment(struct AVFormatContext
> *s, HLSContext *hls, double
>                          "you can try to remove second_level_segment_time
> flag\n",
>                         filename);
>                  av_free(filename);
> -                av_free(old_filename);
>                  av_free(en);
>                  return AVERROR(EINVAL);
>              }
>              av_free(filename);
>          }
> -        ff_rename(old_filename, hls->avf->filename, hls);
> -        av_free(old_filename);
>      }
>
>
> @@ -1239,14 +1228,22 @@ static int hls_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>      if (can_split && av_compare_ts(pkt->pts - hls->start_pts,
> st->time_base,
>                                     end_pts, AV_TIME_BASE_Q) >= 0) {
>          int64_t new_start_pos;
> +        char *old_filename = av_strdup(hls->avf->filename);
> +
> +        if (!old_filename) {
> +            return AVERROR(ENOMEM);
> +        }
> +
>          av_write_frame(oc, NULL); /* Flush any buffered data */
>
>          new_start_pos = avio_tell(hls->avf->pb);
>          hls->size = new_start_pos - hls->start_pos;
>          ret = hls_append_segment(s, hls, hls->duration, hls->start_pos,
> hls->size);
>          hls->start_pos = new_start_pos;
> -        if (ret < 0)
> +        if (ret < 0) {
> +            av_free(old_filename);
>              return ret;
> +        }
>
>          hls->end_pts = pkt->pts;
>          hls->duration = 0;
> @@ -1261,6 +1258,10 @@ static int hls_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>              if (hls->start_pos >= hls->max_seg_size) {
>                  hls->sequence++;
>                  ff_format_io_close(s, &oc->pb);
> +                if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
> HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
> +                     strlen(hls->current_segment_final_filename_fmt)) {
> +                    ff_rename(old_filename, hls->avf->filename, hls);
> +                }
>                  if (hls->vtt_avf)
>                      ff_format_io_close(s, &hls->vtt_avf->pb);
>                  ret = hls_start(s);
> @@ -1272,22 +1273,30 @@ static int hls_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>              hls->number++;
>          } else {
>              ff_format_io_close(s, &oc->pb);
> +            if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
> HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
> +                strlen(hls->current_segment_final_filename_fmt)) {
> +                ff_rename(old_filename, hls->avf->filename, hls);
> +            }
>              if (hls->vtt_avf)
>                  ff_format_io_close(s, &hls->vtt_avf->pb);
>
>              ret = hls_start(s);
>          }
>
> -        if (ret < 0)
> +        if (ret < 0) {
> +            av_free(old_filename);
>              return ret;
> +        }
>
>          if( st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE )
>              oc = hls->vtt_avf;
>          else
>          oc = hls->avf;
>
> -        if ((ret = hls_window(s, 0)) < 0)
> +        if ((ret = hls_window(s, 0)) < 0) {
> +            av_free(old_filename);
>              return ret;
> +        }
>      }
>
>      ret = ff_write_chained(oc, stream_index, pkt, s, 0);
> @@ -1300,6 +1309,12 @@ static int hls_write_trailer(struct AVFormatContext
> *s)
>      HLSContext *hls = s->priv_data;
>      AVFormatContext *oc = hls->avf;
>      AVFormatContext *vtt_oc = hls->vtt_avf;
> +    char *old_filename = av_strdup(hls->avf->filename);
> +
> +    if (!old_filename) {
> +        return AVERROR(ENOMEM);
> +    }
> +
>
>      av_write_trailer(oc);
>      if (oc->pb) {
> @@ -1309,6 +1324,11 @@ static int hls_write_trailer(struct AVFormatContext
> *s)
>          hls_append_segment(s, hls, hls->duration + hls->dpp,
> hls->start_pos, hls->size);
>      }
>
> +    if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
> HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
> +         strlen(hls->current_segment_final_filename_fmt)) {
> +         ff_rename(old_filename, hls->avf->filename, hls);
> +    }
> +
>      if (vtt_oc) {
>          if (vtt_oc->pb)
>              av_write_trailer(vtt_oc);
> @@ -1329,6 +1349,7 @@ static int hls_write_trailer(struct AVFormatContext
> *s)
>
>      hls_free_segments(hls->segments);
>      hls_free_segments(hls->old_segments);
> +    av_free(old_filename);
>      return 0;
>  }
>
> --
> 2.10.1.382.ga23ca1b.dirty
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

applied


Thanks
wm4 Jan. 13, 2017, 5:34 a.m. UTC | #2 
On Fri, 13 Jan 2017 08:00:34 +0800
Steven Liu <lingjiujianke@gmail.com> wrote:

> 2017-01-13 0:55 GMT+08:00 Steven Liu <lq@chinaffmpeg.org>:
> 
> > when hlsenc use flag second_level_segment_index,
> > second_level_segment_size and second_level_segment_duration,
> > the rename is ok but the output filename always use the old filename
> > so move the rename operation after the close the ts file and
> > before open new segment
> >
> > Reported-by: Christian Johannesen <chrisjohannesen@gmail.com>
> > Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> > ---
> >  libavformat/hlsenc.c | 49 ++++++++++++++++++++++++++++++
> > +++++--------------
> >  1 file changed, 35 insertions(+), 14 deletions(-)
> >
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index a2c606c..f662913 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -453,16 +453,10 @@ static int hls_append_segment(struct AVFormatContext
> > *s, HLSContext *hls, double
> >
> >      if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
> > HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
> >          strlen(hls->current_segment_final_filename_fmt)) {
> > -        char * old_filename = av_strdup(hls->avf->filename);  // %%s will
> > be %s after strftime
> > -        if (!old_filename) {
> > -            av_free(en);
> > -            return AVERROR(ENOMEM);
> > -        }
> >          av_strlcpy(hls->avf->filename, hls->current_segment_final_filename_fmt,
> > sizeof(hls->avf->filename));
> >          if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) {
> >              char * filename = av_strdup(hls->avf->filename);  // %%s will
> > be %s after strftime
> >              if (!filename) {
> > -                av_free(old_filename);
> >                  av_free(en);
> >                  return AVERROR(ENOMEM);
> >              }
> > @@ -473,7 +467,6 @@ static int hls_append_segment(struct AVFormatContext
> > *s, HLSContext *hls, double
> >                          "you can try to remove second_level_segment_size
> > flag\n",
> >                         filename);
> >                  av_free(filename);
> > -                av_free(old_filename);
> >                  av_free(en);
> >                  return AVERROR(EINVAL);
> >              }
> > @@ -482,7 +475,6 @@ static int hls_append_segment(struct AVFormatContext
> > *s, HLSContext *hls, double
> >          if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_DURATION) {
> >              char * filename = av_strdup(hls->avf->filename);  // %%t will
> > be %t after strftime
> >              if (!filename) {
> > -                av_free(old_filename);
> >                  av_free(en);
> >                  return AVERROR(ENOMEM);
> >              }
> > @@ -493,14 +485,11 @@ static int hls_append_segment(struct AVFormatContext
> > *s, HLSContext *hls, double
> >                          "you can try to remove second_level_segment_time
> > flag\n",
> >                         filename);
> >                  av_free(filename);
> > -                av_free(old_filename);
> >                  av_free(en);
> >                  return AVERROR(EINVAL);
> >              }
> >              av_free(filename);
> >          }
> > -        ff_rename(old_filename, hls->avf->filename, hls);
> > -        av_free(old_filename);
> >      }
> >
> >
> > @@ -1239,14 +1228,22 @@ static int hls_write_packet(AVFormatContext *s,
> > AVPacket *pkt)
> >      if (can_split && av_compare_ts(pkt->pts - hls->start_pts,
> > st->time_base,
> >                                     end_pts, AV_TIME_BASE_Q) >= 0) {
> >          int64_t new_start_pos;
> > +        char *old_filename = av_strdup(hls->avf->filename);
> > +
> > +        if (!old_filename) {
> > +            return AVERROR(ENOMEM);
> > +        }
> > +
> >          av_write_frame(oc, NULL); /* Flush any buffered data */
> >
> >          new_start_pos = avio_tell(hls->avf->pb);
> >          hls->size = new_start_pos - hls->start_pos;
> >          ret = hls_append_segment(s, hls, hls->duration, hls->start_pos,
> > hls->size);
> >          hls->start_pos = new_start_pos;
> > -        if (ret < 0)
> > +        if (ret < 0) {
> > +            av_free(old_filename);
> >              return ret;
> > +        }
> >
> >          hls->end_pts = pkt->pts;
> >          hls->duration = 0;
> > @@ -1261,6 +1258,10 @@ static int hls_write_packet(AVFormatContext *s,
> > AVPacket *pkt)
> >              if (hls->start_pos >= hls->max_seg_size) {
> >                  hls->sequence++;
> >                  ff_format_io_close(s, &oc->pb);
> > +                if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
> > HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
> > +                     strlen(hls->current_segment_final_filename_fmt)) {
> > +                    ff_rename(old_filename, hls->avf->filename, hls);
> > +                }
> >                  if (hls->vtt_avf)
> >                      ff_format_io_close(s, &hls->vtt_avf->pb);
> >                  ret = hls_start(s);
> > @@ -1272,22 +1273,30 @@ static int hls_write_packet(AVFormatContext *s,
> > AVPacket *pkt)
> >              hls->number++;
> >          } else {
> >              ff_format_io_close(s, &oc->pb);
> > +            if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
> > HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
> > +                strlen(hls->current_segment_final_filename_fmt)) {
> > +                ff_rename(old_filename, hls->avf->filename, hls);
> > +            }
> >              if (hls->vtt_avf)
> >                  ff_format_io_close(s, &hls->vtt_avf->pb);
> >
> >              ret = hls_start(s);
> >          }
> >
> > -        if (ret < 0)
> > +        if (ret < 0) {
> > +            av_free(old_filename);
> >              return ret;
> > +        }
> >
> >          if( st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE )
> >              oc = hls->vtt_avf;
> >          else
> >          oc = hls->avf;
> >
> > -        if ((ret = hls_window(s, 0)) < 0)
> > +        if ((ret = hls_window(s, 0)) < 0) {
> > +            av_free(old_filename);
> >              return ret;
> > +        }
> >      }
> >
> >      ret = ff_write_chained(oc, stream_index, pkt, s, 0);
> > @@ -1300,6 +1309,12 @@ static int hls_write_trailer(struct AVFormatContext
> > *s)
> >      HLSContext *hls = s->priv_data;
> >      AVFormatContext *oc = hls->avf;
> >      AVFormatContext *vtt_oc = hls->vtt_avf;
> > +    char *old_filename = av_strdup(hls->avf->filename);
> > +
> > +    if (!old_filename) {
> > +        return AVERROR(ENOMEM);
> > +    }
> > +
> >
> >      av_write_trailer(oc);
> >      if (oc->pb) {
> > @@ -1309,6 +1324,11 @@ static int hls_write_trailer(struct AVFormatContext
> > *s)
> >          hls_append_segment(s, hls, hls->duration + hls->dpp,
> > hls->start_pos, hls->size);
> >      }
> >
> > +    if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
> > HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
> > +         strlen(hls->current_segment_final_filename_fmt)) {
> > +         ff_rename(old_filename, hls->avf->filename, hls);
> > +    }
> > +
> >      if (vtt_oc) {
> >          if (vtt_oc->pb)
> >              av_write_trailer(vtt_oc);
> > @@ -1329,6 +1349,7 @@ static int hls_write_trailer(struct AVFormatContext
> > *s)
> >
> >      hls_free_segments(hls->segments);
> >      hls_free_segments(hls->old_segments);
> > +    av_free(old_filename);
> >      return 0;
> >  }
> >
> > --
> > 2.10.1.382.ga23ca1b.dirty
> >
> >
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >  
> 
> applied

Please wait a bit longer with applying such patches (maybe about 24
hours), so everyone gets a chance to review it.
diff mbox

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index a2c606c..f662913 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -453,16 +453,10 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
 
     if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE | HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
         strlen(hls->current_segment_final_filename_fmt)) {
-        char * old_filename = av_strdup(hls->avf->filename);  // %%s will be %s after strftime
-        if (!old_filename) {
-            av_free(en);
-            return AVERROR(ENOMEM);
-        }
         av_strlcpy(hls->avf->filename, hls->current_segment_final_filename_fmt, sizeof(hls->avf->filename));
         if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) {
             char * filename = av_strdup(hls->avf->filename);  // %%s will be %s after strftime
             if (!filename) {
-                av_free(old_filename);
                 av_free(en);
                 return AVERROR(ENOMEM);
             }
@@ -473,7 +467,6 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
                         "you can try to remove second_level_segment_size flag\n",
                        filename);
                 av_free(filename);
-                av_free(old_filename);
                 av_free(en);
                 return AVERROR(EINVAL);
             }
@@ -482,7 +475,6 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
         if (hls->flags & HLS_SECOND_LEVEL_SEGMENT_DURATION) {
             char * filename = av_strdup(hls->avf->filename);  // %%t will be %t after strftime
             if (!filename) {
-                av_free(old_filename);
                 av_free(en);
                 return AVERROR(ENOMEM);
             }
@@ -493,14 +485,11 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double
                         "you can try to remove second_level_segment_time flag\n",
                        filename);
                 av_free(filename);
-                av_free(old_filename);
                 av_free(en);
                 return AVERROR(EINVAL);
             }
             av_free(filename);
         }
-        ff_rename(old_filename, hls->avf->filename, hls);
-        av_free(old_filename);
     }
 
 
@@ -1239,14 +1228,22 @@  static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
     if (can_split && av_compare_ts(pkt->pts - hls->start_pts, st->time_base,
                                    end_pts, AV_TIME_BASE_Q) >= 0) {
         int64_t new_start_pos;
+        char *old_filename = av_strdup(hls->avf->filename);
+
+        if (!old_filename) {
+            return AVERROR(ENOMEM);
+        }
+
         av_write_frame(oc, NULL); /* Flush any buffered data */
 
         new_start_pos = avio_tell(hls->avf->pb);
         hls->size = new_start_pos - hls->start_pos;
         ret = hls_append_segment(s, hls, hls->duration, hls->start_pos, hls->size);
         hls->start_pos = new_start_pos;
-        if (ret < 0)
+        if (ret < 0) {
+            av_free(old_filename);
             return ret;
+        }
 
         hls->end_pts = pkt->pts;
         hls->duration = 0;
@@ -1261,6 +1258,10 @@  static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
             if (hls->start_pos >= hls->max_seg_size) {
                 hls->sequence++;
                 ff_format_io_close(s, &oc->pb);
+                if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE | HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
+                     strlen(hls->current_segment_final_filename_fmt)) {
+                    ff_rename(old_filename, hls->avf->filename, hls);
+                }
                 if (hls->vtt_avf)
                     ff_format_io_close(s, &hls->vtt_avf->pb);
                 ret = hls_start(s);
@@ -1272,22 +1273,30 @@  static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
             hls->number++;
         } else {
             ff_format_io_close(s, &oc->pb);
+            if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE | HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
+                strlen(hls->current_segment_final_filename_fmt)) {
+                ff_rename(old_filename, hls->avf->filename, hls);
+            }
             if (hls->vtt_avf)
                 ff_format_io_close(s, &hls->vtt_avf->pb);
 
             ret = hls_start(s);
         }
 
-        if (ret < 0)
+        if (ret < 0) {
+            av_free(old_filename);
             return ret;
+        }
 
         if( st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE )
             oc = hls->vtt_avf;
         else
         oc = hls->avf;
 
-        if ((ret = hls_window(s, 0)) < 0)
+        if ((ret = hls_window(s, 0)) < 0) {
+            av_free(old_filename);
             return ret;
+        }
     }
 
     ret = ff_write_chained(oc, stream_index, pkt, s, 0);
@@ -1300,6 +1309,12 @@  static int hls_write_trailer(struct AVFormatContext *s)
     HLSContext *hls = s->priv_data;
     AVFormatContext *oc = hls->avf;
     AVFormatContext *vtt_oc = hls->vtt_avf;
+    char *old_filename = av_strdup(hls->avf->filename);
+
+    if (!old_filename) {
+        return AVERROR(ENOMEM);
+    }
+
 
     av_write_trailer(oc);
     if (oc->pb) {
@@ -1309,6 +1324,11 @@  static int hls_write_trailer(struct AVFormatContext *s)
         hls_append_segment(s, hls, hls->duration + hls->dpp, hls->start_pos, hls->size);
     }
 
+    if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE | HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
+         strlen(hls->current_segment_final_filename_fmt)) {
+         ff_rename(old_filename, hls->avf->filename, hls);
+    }
+
     if (vtt_oc) {
         if (vtt_oc->pb)
             av_write_trailer(vtt_oc);
@@ -1329,6 +1349,7 @@  static int hls_write_trailer(struct AVFormatContext *s)
 
     hls_free_segments(hls->segments);
     hls_free_segments(hls->old_segments);
+    av_free(old_filename);
     return 0;
 }