diff mbox

[FFmpeg-devel] avformat/hlsenc: fix handling of delete_segments when %v is present

Message ID e1692e68-2580-e2f3-7f85-59d470fd7af0@vivanet.hu
State Accepted
Commit 9825f77ac7ab102783aad4d2e0c42584a0dde466
Headers show

Commit Message

Bodecs Bela April 6, 2018, 10:27 a.m. UTC 
Dear All,

when var_stream_map option is used, %v must appear either in segment 
name template or in the directory path. This latter case currently is 
not handled and using delete_segments flag of hls_flags is broken now. 
This patch fixes this issue.
The root cause of the bug was that HLSSegment struct only stores the 
final filename part, but not the final directory path. Most of the 
cases, final path info is unneded, It only necessary when you want to 
delete old segments (e.g in case of live streaming).
Without variant streams it was unnecessary to store the final directory 
path, because all segment were stored into the same directory. But 
introducing %v in directory names either require to store the final 
directory path into HLSSegment or associate segments with their variant 
streams to be able deleting them later. I have choosen the second 
solution and introduced a variant index data member into the segment struct.

please review this patch.

thank you in advance,

Bela Bodecs
From ad97bcd1c4ff0b734de3bffc58e7421192a33e43 Mon Sep 17 00:00:00 2001
From: Bela Bodecs <bodecsb@vivanet.hu>
Date: Fri, 6 Apr 2018 12:21:59 +0200
Subject: [PATCH] avformat/hlsenc: fix handling of delete_segments when %v is
 present

When var_stream_map option is used, %v must appear either in segment
name template or in the directory path. This latter case currently is
not handled and delete_segments flag of hls_flags is broken now. This
patch fix this. The root cause of the bug was that HLSSegment struct
only stores the final filename part, but not the final directory path.
Most of the cases, final path info is unneded, It only necessary when
you want to delete old segments (e.g in case of live streaming).
Without variant streams it was unnecessary to store the final directory
path, because all segment were stored into the same directory. But
introducing %v in directory names either require to store the final
directory path into HLSSegment or associate segments with their variant
streams to be able deleting them later. I have choosen the second
solution and introduced a variant index data member into the segment
struct.

Signed-off-by: Bela Bodecs <bodecsb@vivanet.hu>
---
 libavformat/hlsenc.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

Comments

Liu Steven April 8, 2018, 6:41 a.m. UTC | #1 
> On 6 Apr 2018, at 18:27, Bodecs Bela <bodecsb@vivanet.hu> wrote:
> 
> Dear All,
> 
> when var_stream_map option is used, %v must appear either in segment name template or in the directory path. This latter case currently is not handled and using delete_segments flag of hls_flags is broken now. This patch fixes this issue.
> The root cause of the bug was that HLSSegment struct only stores the final filename part, but not the final directory path. Most of the cases, final path info is unneded, It only necessary when you want to delete old segments (e.g in case of live streaming).
> Without variant streams it was unnecessary to store the final directory path, because all segment were stored into the same directory. But introducing %v in directory names either require to store the final directory path into HLSSegment or associate segments with their variant streams to be able deleting them later. I have choosen the second solution and introduced a variant index data member into the segment struct.
> 
> please review this patch.
> 
> thank you in advance,
> 
> Bela Bodecs
> 
> 
> <0001-avformat-hlsenc-fix-handling-of-delete_segments-when.patch>_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

LGTM
Thanks
Steven
Liu Steven April 11, 2018, 4:03 a.m. UTC | #2 
> On 6 Apr 2018, at 18:27, Bodecs Bela <bodecsb@vivanet.hu> wrote:
> 
> Dear All,
> 
> when var_stream_map option is used, %v must appear either in segment name template or in the directory path. This latter case currently is not handled and using delete_segments flag of hls_flags is broken now. This patch fixes this issue.
> The root cause of the bug was that HLSSegment struct only stores the final filename part, but not the final directory path. Most of the cases, final path info is unneded, It only necessary when you want to delete old segments (e.g in case of live streaming).
> Without variant streams it was unnecessary to store the final directory path, because all segment were stored into the same directory. But introducing %v in directory names either require to store the final directory path into HLSSegment or associate segments with their variant streams to be able deleting them later. I have choosen the second solution and introduced a variant index data member into the segment struct.
> 
> please review this patch.
> 
> thank you in advance,
> 
> Bela Bodecs
> 
> 
> <0001-avformat-hlsenc-fix-handling-of-delete_segments-when.patch>_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
pushed

Thanks
Steven
diff mbox

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 2a54b43..8eb8421 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -75,6 +75,7 @@  typedef struct HLSSegment {
     int discont;
     int64_t pos;
     int64_t size;
+    unsigned var_stream_idx;
 
     char key_uri[LINE_BUFFER_SIZE + 1];
     char iv_string[KEYSIZE*2 + 1];
@@ -106,6 +107,7 @@  typedef enum {
 } SegmentType;
 
 typedef struct VariantStream {
+    unsigned var_stream_idx;
     unsigned number;
     int64_t sequence;
     AVOutputFormat *oformat;
@@ -478,9 +480,23 @@  static int hls_delete_old_segments(AVFormatContext *s, HLSContext *hls,
         }
         p = (char *)av_basename(dirname);
         *p = '\0';
+
     }
 
     while (segment) {
+        char * r_dirname = dirname;
+
+        /* if %v is present in the file's directory */
+        if (av_stristr(dirname, "%v")) {
+
+            if (replace_int_data_in_filename(&r_dirname, dirname, 'v', segment->var_stream_idx) < 1) {
+                ret = AVERROR(EINVAL);
+                goto fail;
+            }
+            av_free(dirname);
+            dirname = r_dirname;
+        }
+
         av_log(hls, AV_LOG_DEBUG, "deleting old segment %s\n",
                                   segment->filename);
         path_size =  (hls->use_localtime_mkdir ? 0 : strlen(dirname)) + strlen(segment->filename) + 1;
@@ -965,6 +981,7 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
     if (!en)
         return AVERROR(ENOMEM);
 
+    en->var_stream_idx = vs->var_stream_idx;
     ret = sls_flags_filename_process(s, hls, vs, en, duration, pos, size);
     if (ret < 0) {
         return ret;
@@ -1824,9 +1841,11 @@  static int parse_variant_stream_mapstring(AVFormatContext *s)
     while (varstr = av_strtok(p, " \t", &saveptr1)) {
         p = NULL;
 
-        if (nb_varstreams < hls->nb_varstreams)
-            vs = &(hls->var_streams[nb_varstreams++]);
-        else
+        if (nb_varstreams < hls->nb_varstreams) {
+            vs = &(hls->var_streams[nb_varstreams]);
+            vs->var_stream_idx = nb_varstreams;
+            nb_varstreams++;
+        } else
             return AVERROR(EINVAL);
 
         q = varstr;
@@ -1984,6 +2003,7 @@  static int update_variant_stream_info(AVFormatContext *s) {
         if (!hls->var_streams)
             return AVERROR(ENOMEM);
 
+        hls->var_streams[0].var_stream_idx = 0;
         hls->var_streams[0].nb_streams = s->nb_streams;
         hls->var_streams[0].streams = av_mallocz(sizeof(AVStream *) *
                                             hls->var_streams[0].nb_streams);