[FFmpeg-devel,v2] lavf/dashenc: Write media trailers when DASH trailer is written.

Submitted by Andrey Semashev on Nov. 29, 2018, 6:28 p.m.

Details

Message ID 20181129182832.13771-1-andrey.semashev@gmail.com
State New
Headers show

Commit Message

Andrey Semashev Nov. 29, 2018, 6:28 p.m.
This commit ensures that all (potentially, long) filesystem activity is
performed when the user calls av_write_trailer on the DASH libavformat
context, not when freeing the context. Also, this defers media segment
deletion until after the media trailers are written.
---
 libavformat/dashenc.c | 82 ++++++++++++++++++++++++++++++-------------
 1 file changed, 58 insertions(+), 24 deletions(-)

Comments

kjeyapal@akamai.com Nov. 30, 2018, 6:12 a.m.
On 11/29/18 11:58 PM, Andrey Semashev wrote:
> This commit ensures that all (potentially, long) filesystem activity is

> performed when the user calls av_write_trailer on the DASH libavformat

> context, not when freeing the context. Also, this defers media segment

> deletion until after the media trailers are written.

> ---

>  libavformat/dashenc.c | 82 ++++++++++++++++++++++++++++++-------------

>  1 file changed, 58 insertions(+), 24 deletions(-)

>

> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c

> index 6ce70e0076..ecfd84a32c 100644

> --- a/libavformat/dashenc.c

> +++ b/libavformat/dashenc.c

> @@ -424,8 +424,6 @@ static void dash_free(AVFormatContext *s)

>          return;

>      for (i = 0; i < s->nb_streams; i++) {

>          OutputStream *os = &c->streams[i];

> -        if (os->ctx && os->ctx_inited)

> -            av_write_trailer(os->ctx);

>          if (os->ctx && os->ctx->pb)

>              ffio_free_dyn_buf(&os->ctx->pb);

>          ff_format_io_close(s, &os->out);

> @@ -1331,6 +1329,47 @@ static void dashenc_delete_file(AVFormatContext *s, char *filename) {

>      }

>  }

>  

> +static int dashenc_delete_segment_file(AVFormatContext *s, const char* file)

> +{

> +    DASHContext *c = s->priv_data;

> +    size_t dirname_len, file_len;

> +    char filename[1024];

> +

> +    dirname_len = strlen(c->dirname);

> +    if (dirname_len >= sizeof(filename)) {

> +        av_log(s, AV_LOG_WARNING, "Cannot delete segments as the directory path is too long: %"PRIu64" characters: %s\n",

> +            (uint64_t)dirname_len, c->dirname);

> +        return AVERROR(ENAMETOOLONG);

> +    }

> +

> +    memcpy(filename, c->dirname, dirname_len);

> +

> +    file_len = strlen(file);

> +    if ((dirname_len + file_len) >= sizeof(filename)) {

> +        av_log(s, AV_LOG_WARNING, "Cannot delete segments as the path is too long: %"PRIu64" characters: %s%s\n",

> +            (uint64_t)(dirname_len + file_len), c->dirname, file);

> +        return AVERROR(ENAMETOOLONG);

> +    }

> +

> +    memcpy(filename + dirname_len, file, file_len + 1); // include the terminating zero

> +    dashenc_delete_file(s, filename);

> +

> +    return 0;

> +}

> +

> +static inline void dashenc_delete_media_segments(AVFormatContext *s, OutputStream *os, int remove_count)

> +{

> +    for (int i = 0; i < remove_count; ++i) {

> +        dashenc_delete_segment_file(s, os->segments[i]->file);

> +

> +        // Delete the segment regardless of whether the file was successfully deleted

> +        av_free(os->segments[i]);

> +    }

> +

> +    os->nb_segments -= remove_count;

> +    memmove(os->segments, os->segments + remove_count, os->nb_segments * sizeof(*os->segments));

> +}

> +

>  static int dash_flush(AVFormatContext *s, int final, int stream)

>  {

>      DASHContext *c = s->priv_data;

> @@ -1420,23 +1459,12 @@ static int dash_flush(AVFormatContext *s, int final, int stream)

>          os->pos += range_length;

>      }

>  

> -    if (c->window_size || (final && c->remove_at_exit)) {

> +    if (c->window_size) {

>          for (i = 0; i < s->nb_streams; i++) {

>              OutputStream *os = &c->streams[i];

> -            int j;

> -            int remove = os->nb_segments - c->window_size - c->extra_window_size;

> -            if (final && c->remove_at_exit)

> -                remove = os->nb_segments;

> -            if (remove > 0) {

> -                for (j = 0; j < remove; j++) {

> -                    char filename[1024];

> -                    snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->segments[j]->file);

> -                    dashenc_delete_file(s, filename);

> -                    av_free(os->segments[j]);

> -                }

> -                os->nb_segments -= remove;

> -                memmove(os->segments, os->segments + remove, os->nb_segments * sizeof(*os->segments));

> -            }

> +            int remove_count = os->nb_segments - c->window_size - c->extra_window_size;

> +            if (remove_count > 0)

> +                dashenc_delete_media_segments(s, os, remove_count);

>          }

>      }

>  

> @@ -1584,6 +1612,7 @@ static int dash_write_packet(AVFormatContext *s, AVPacket *pkt)

>  static int dash_write_trailer(AVFormatContext *s)

>  {

>      DASHContext *c = s->priv_data;

> +    int i;

>  

>      if (s->nb_streams > 0) {

>          OutputStream *os = &c->streams[0];

> @@ -1599,14 +1628,19 @@ static int dash_write_trailer(AVFormatContext *s)

>      }

>      dash_flush(s, 1, -1);

>  

> -    if (c->remove_at_exit) {

> -        char filename[1024];

> -        int i;

> -        for (i = 0; i < s->nb_streams; i++) {

> -            OutputStream *os = &c->streams[i];

> -            snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->initfile);

> -            dashenc_delete_file(s, filename);

> +    for (i = 0; i < s->nb_streams; ++i) {

> +        OutputStream *os = &c->streams[i];

> +        if (os->ctx && os->ctx_inited) {

> +            av_write_trailer(os->ctx);

> +        }

> +

> +        if (c->remove_at_exit) {

> +            dashenc_delete_media_segments(s, os, os->nb_segments);

> +            dashenc_delete_segment_file(s, os->initfile);

>          }

> +    }

> +

> +    if (c->remove_at_exit) {

>          dashenc_delete_file(s, s->url);

>      }

>  

LGTM

Regards,
Karthick
kjeyapal@akamai.com Dec. 3, 2018, 5:45 a.m.
On 11/30/18 11:42 AM, Jeyapal, Karthick wrote:
>

> On 11/29/18 11:58 PM, Andrey Semashev wrote:

>> This commit ensures that all (potentially, long) filesystem activity is

>> performed when the user calls av_write_trailer on the DASH libavformat

>> context, not when freeing the context. Also, this defers media segment

>> deletion until after the media trailers are written.

>> ---

>>  libavformat/dashenc.c | 82 ++++++++++++++++++++++++++++++-------------

>>  1 file changed, 58 insertions(+), 24 deletions(-)

>>

>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c

>> index 6ce70e0076..ecfd84a32c 100644

>> --- a/libavformat/dashenc.c

>> +++ b/libavformat/dashenc.c

>> @@ -424,8 +424,6 @@ static void dash_free(AVFormatContext *s)

>>          return;

>>      for (i = 0; i < s->nb_streams; i++) {

>>          OutputStream *os = &c->streams[i];

>> -        if (os->ctx && os->ctx_inited)

>> -            av_write_trailer(os->ctx);

>>          if (os->ctx && os->ctx->pb)

>>              ffio_free_dyn_buf(&os->ctx->pb);

>>          ff_format_io_close(s, &os->out);

>> @@ -1331,6 +1329,47 @@ static void dashenc_delete_file(AVFormatContext *s, char *filename) {

>>      }

>>  }

>>  

>> +static int dashenc_delete_segment_file(AVFormatContext *s, const char* file)

>> +{

>> +    DASHContext *c = s->priv_data;

>> +    size_t dirname_len, file_len;

>> +    char filename[1024];

>> +

>> +    dirname_len = strlen(c->dirname);

>> +    if (dirname_len >= sizeof(filename)) {

>> +        av_log(s, AV_LOG_WARNING, "Cannot delete segments as the directory path is too long: %"PRIu64" characters: %s\n",

>> +            (uint64_t)dirname_len, c->dirname);

>> +        return AVERROR(ENAMETOOLONG);

>> +    }

>> +

>> +    memcpy(filename, c->dirname, dirname_len);

>> +

>> +    file_len = strlen(file);

>> +    if ((dirname_len + file_len) >= sizeof(filename)) {

>> +        av_log(s, AV_LOG_WARNING, "Cannot delete segments as the path is too long: %"PRIu64" characters: %s%s\n",

>> +            (uint64_t)(dirname_len + file_len), c->dirname, file);

>> +        return AVERROR(ENAMETOOLONG);

>> +    }

>> +

>> +    memcpy(filename + dirname_len, file, file_len + 1); // include the terminating zero

>> +    dashenc_delete_file(s, filename);

>> +

>> +    return 0;

>> +}

>> +

>> +static inline void dashenc_delete_media_segments(AVFormatContext *s, OutputStream *os, int remove_count)

>> +{

>> +    for (int i = 0; i < remove_count; ++i) {

>> +        dashenc_delete_segment_file(s, os->segments[i]->file);

>> +

>> +        // Delete the segment regardless of whether the file was successfully deleted

>> +        av_free(os->segments[i]);

>> +    }

>> +

>> +    os->nb_segments -= remove_count;

>> +    memmove(os->segments, os->segments + remove_count, os->nb_segments * sizeof(*os->segments));

>> +}

>> +

>>  static int dash_flush(AVFormatContext *s, int final, int stream)

>>  {

>>      DASHContext *c = s->priv_data;

>> @@ -1420,23 +1459,12 @@ static int dash_flush(AVFormatContext *s, int final, int stream)

>>          os->pos += range_length;

>>      }

>>  

>> -    if (c->window_size || (final && c->remove_at_exit)) {

>> +    if (c->window_size) {

>>          for (i = 0; i < s->nb_streams; i++) {

>>              OutputStream *os = &c->streams[i];

>> -            int j;

>> -            int remove = os->nb_segments - c->window_size - c->extra_window_size;

>> -            if (final && c->remove_at_exit)

>> -                remove = os->nb_segments;

>> -            if (remove > 0) {

>> -                for (j = 0; j < remove; j++) {

>> -                    char filename[1024];

>> -                    snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->segments[j]->file);

>> -                    dashenc_delete_file(s, filename);

>> -                    av_free(os->segments[j]);

>> -                }

>> -                os->nb_segments -= remove;

>> -                memmove(os->segments, os->segments + remove, os->nb_segments * sizeof(*os->segments));

>> -            }

>> +            int remove_count = os->nb_segments - c->window_size - c->extra_window_size;

>> +            if (remove_count > 0)

>> +                dashenc_delete_media_segments(s, os, remove_count);

>>          }

>>      }

>>  

>> @@ -1584,6 +1612,7 @@ static int dash_write_packet(AVFormatContext *s, AVPacket *pkt)

>>  static int dash_write_trailer(AVFormatContext *s)

>>  {

>>      DASHContext *c = s->priv_data;

>> +    int i;

>>  

>>      if (s->nb_streams > 0) {

>>          OutputStream *os = &c->streams[0];

>> @@ -1599,14 +1628,19 @@ static int dash_write_trailer(AVFormatContext *s)

>>      }

>>      dash_flush(s, 1, -1);

>>  

>> -    if (c->remove_at_exit) {

>> -        char filename[1024];

>> -        int i;

>> -        for (i = 0; i < s->nb_streams; i++) {

>> -            OutputStream *os = &c->streams[i];

>> -            snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->initfile);

>> -            dashenc_delete_file(s, filename);

>> +    for (i = 0; i < s->nb_streams; ++i) {

>> +        OutputStream *os = &c->streams[i];

>> +        if (os->ctx && os->ctx_inited) {

>> +            av_write_trailer(os->ctx);

>> +        }

>> +

>> +        if (c->remove_at_exit) {

>> +            dashenc_delete_media_segments(s, os, os->nb_segments);

>> +            dashenc_delete_segment_file(s, os->initfile);

>>          }

>> +    }

>> +

>> +    if (c->remove_at_exit) {

>>          dashenc_delete_file(s, s->url);

>>      }

>>  

> LGTM

Hi Andrey,

Could you please rebase this patch with the latest git master. It is throwing some build errors with the latest version.

Regards,
Karthick
>

> Regards,

> Karthick

>

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Andrey Semashev Dec. 3, 2018, 11:20 a.m.
On 12/3/18 8:45 AM, Jeyapal, Karthick wrote:
> 
> On 11/30/18 11:42 AM, Jeyapal, Karthick wrote:
>>
>> On 11/29/18 11:58 PM, Andrey Semashev wrote:
>>> This commit ensures that all (potentially, long) filesystem activity is
>>> performed when the user calls av_write_trailer on the DASH libavformat
>>> context, not when freeing the context. Also, this defers media segment
>>> deletion until after the media trailers are written.
>>> ---
>>>   libavformat/dashenc.c | 82 ++++++++++++++++++++++++++++++-------------
>>>   1 file changed, 58 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>>> index 6ce70e0076..ecfd84a32c 100644
>>> --- a/libavformat/dashenc.c
>>> +++ b/libavformat/dashenc.c
>>> @@ -424,8 +424,6 @@ static void dash_free(AVFormatContext *s)
>>>           return;
>>>       for (i = 0; i < s->nb_streams; i++) {
>>>           OutputStream *os = &c->streams[i];
>>> -        if (os->ctx && os->ctx_inited)
>>> -            av_write_trailer(os->ctx);
>>>           if (os->ctx && os->ctx->pb)
>>>               ffio_free_dyn_buf(&os->ctx->pb);
>>>           ff_format_io_close(s, &os->out);
>>> @@ -1331,6 +1329,47 @@ static void dashenc_delete_file(AVFormatContext *s, char *filename) {
>>>       }
>>>   }
>>>   
>>> +static int dashenc_delete_segment_file(AVFormatContext *s, const char* file)
>>> +{
>>> +    DASHContext *c = s->priv_data;
>>> +    size_t dirname_len, file_len;
>>> +    char filename[1024];
>>> +
>>> +    dirname_len = strlen(c->dirname);
>>> +    if (dirname_len >= sizeof(filename)) {
>>> +        av_log(s, AV_LOG_WARNING, "Cannot delete segments as the directory path is too long: %"PRIu64" characters: %s\n",
>>> +            (uint64_t)dirname_len, c->dirname);
>>> +        return AVERROR(ENAMETOOLONG);
>>> +    }
>>> +
>>> +    memcpy(filename, c->dirname, dirname_len);
>>> +
>>> +    file_len = strlen(file);
>>> +    if ((dirname_len + file_len) >= sizeof(filename)) {
>>> +        av_log(s, AV_LOG_WARNING, "Cannot delete segments as the path is too long: %"PRIu64" characters: %s%s\n",
>>> +            (uint64_t)(dirname_len + file_len), c->dirname, file);
>>> +        return AVERROR(ENAMETOOLONG);
>>> +    }
>>> +
>>> +    memcpy(filename + dirname_len, file, file_len + 1); // include the terminating zero
>>> +    dashenc_delete_file(s, filename);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static inline void dashenc_delete_media_segments(AVFormatContext *s, OutputStream *os, int remove_count)
>>> +{
>>> +    for (int i = 0; i < remove_count; ++i) {
>>> +        dashenc_delete_segment_file(s, os->segments[i]->file);
>>> +
>>> +        // Delete the segment regardless of whether the file was successfully deleted
>>> +        av_free(os->segments[i]);
>>> +    }
>>> +
>>> +    os->nb_segments -= remove_count;
>>> +    memmove(os->segments, os->segments + remove_count, os->nb_segments * sizeof(*os->segments));
>>> +}
>>> +
>>>   static int dash_flush(AVFormatContext *s, int final, int stream)
>>>   {
>>>       DASHContext *c = s->priv_data;
>>> @@ -1420,23 +1459,12 @@ static int dash_flush(AVFormatContext *s, int final, int stream)
>>>           os->pos += range_length;
>>>       }
>>>   
>>> -    if (c->window_size || (final && c->remove_at_exit)) {
>>> +    if (c->window_size) {
>>>           for (i = 0; i < s->nb_streams; i++) {
>>>               OutputStream *os = &c->streams[i];
>>> -            int j;
>>> -            int remove = os->nb_segments - c->window_size - c->extra_window_size;
>>> -            if (final && c->remove_at_exit)
>>> -                remove = os->nb_segments;
>>> -            if (remove > 0) {
>>> -                for (j = 0; j < remove; j++) {
>>> -                    char filename[1024];
>>> -                    snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->segments[j]->file);
>>> -                    dashenc_delete_file(s, filename);
>>> -                    av_free(os->segments[j]);
>>> -                }
>>> -                os->nb_segments -= remove;
>>> -                memmove(os->segments, os->segments + remove, os->nb_segments * sizeof(*os->segments));
>>> -            }
>>> +            int remove_count = os->nb_segments - c->window_size - c->extra_window_size;
>>> +            if (remove_count > 0)
>>> +                dashenc_delete_media_segments(s, os, remove_count);
>>>           }
>>>       }
>>>   
>>> @@ -1584,6 +1612,7 @@ static int dash_write_packet(AVFormatContext *s, AVPacket *pkt)
>>>   static int dash_write_trailer(AVFormatContext *s)
>>>   {
>>>       DASHContext *c = s->priv_data;
>>> +    int i;
>>>   
>>>       if (s->nb_streams > 0) {
>>>           OutputStream *os = &c->streams[0];
>>> @@ -1599,14 +1628,19 @@ static int dash_write_trailer(AVFormatContext *s)
>>>       }
>>>       dash_flush(s, 1, -1);
>>>   
>>> -    if (c->remove_at_exit) {
>>> -        char filename[1024];
>>> -        int i;
>>> -        for (i = 0; i < s->nb_streams; i++) {
>>> -            OutputStream *os = &c->streams[i];
>>> -            snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->initfile);
>>> -            dashenc_delete_file(s, filename);
>>> +    for (i = 0; i < s->nb_streams; ++i) {
>>> +        OutputStream *os = &c->streams[i];
>>> +        if (os->ctx && os->ctx_inited) {
>>> +            av_write_trailer(os->ctx);
>>> +        }
>>> +
>>> +        if (c->remove_at_exit) {
>>> +            dashenc_delete_media_segments(s, os, os->nb_segments);
>>> +            dashenc_delete_segment_file(s, os->initfile);
>>>           }
>>> +    }
>>> +
>>> +    if (c->remove_at_exit) {
>>>           dashenc_delete_file(s, s->url);
>>>       }
>>>   
>> LGTM
> Hi Andrey,
> 
> Could you please rebase this patch with the latest git master. It is throwing some build errors with the latest version.

Yes, some code was made invalid by other patches. I've send the updated 
version.

Patch hide | download patch | download mbox

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..ecfd84a32c 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -424,8 +424,6 @@  static void dash_free(AVFormatContext *s)
         return;
     for (i = 0; i < s->nb_streams; i++) {
         OutputStream *os = &c->streams[i];
-        if (os->ctx && os->ctx_inited)
-            av_write_trailer(os->ctx);
         if (os->ctx && os->ctx->pb)
             ffio_free_dyn_buf(&os->ctx->pb);
         ff_format_io_close(s, &os->out);
@@ -1331,6 +1329,47 @@  static void dashenc_delete_file(AVFormatContext *s, char *filename) {
     }
 }
 
+static int dashenc_delete_segment_file(AVFormatContext *s, const char* file)
+{
+    DASHContext *c = s->priv_data;
+    size_t dirname_len, file_len;
+    char filename[1024];
+
+    dirname_len = strlen(c->dirname);
+    if (dirname_len >= sizeof(filename)) {
+        av_log(s, AV_LOG_WARNING, "Cannot delete segments as the directory path is too long: %"PRIu64" characters: %s\n",
+            (uint64_t)dirname_len, c->dirname);
+        return AVERROR(ENAMETOOLONG);
+    }
+
+    memcpy(filename, c->dirname, dirname_len);
+
+    file_len = strlen(file);
+    if ((dirname_len + file_len) >= sizeof(filename)) {
+        av_log(s, AV_LOG_WARNING, "Cannot delete segments as the path is too long: %"PRIu64" characters: %s%s\n",
+            (uint64_t)(dirname_len + file_len), c->dirname, file);
+        return AVERROR(ENAMETOOLONG);
+    }
+
+    memcpy(filename + dirname_len, file, file_len + 1); // include the terminating zero
+    dashenc_delete_file(s, filename);
+
+    return 0;
+}
+
+static inline void dashenc_delete_media_segments(AVFormatContext *s, OutputStream *os, int remove_count)
+{
+    for (int i = 0; i < remove_count; ++i) {
+        dashenc_delete_segment_file(s, os->segments[i]->file);
+
+        // Delete the segment regardless of whether the file was successfully deleted
+        av_free(os->segments[i]);
+    }
+
+    os->nb_segments -= remove_count;
+    memmove(os->segments, os->segments + remove_count, os->nb_segments * sizeof(*os->segments));
+}
+
 static int dash_flush(AVFormatContext *s, int final, int stream)
 {
     DASHContext *c = s->priv_data;
@@ -1420,23 +1459,12 @@  static int dash_flush(AVFormatContext *s, int final, int stream)
         os->pos += range_length;
     }
 
-    if (c->window_size || (final && c->remove_at_exit)) {
+    if (c->window_size) {
         for (i = 0; i < s->nb_streams; i++) {
             OutputStream *os = &c->streams[i];
-            int j;
-            int remove = os->nb_segments - c->window_size - c->extra_window_size;
-            if (final && c->remove_at_exit)
-                remove = os->nb_segments;
-            if (remove > 0) {
-                for (j = 0; j < remove; j++) {
-                    char filename[1024];
-                    snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->segments[j]->file);
-                    dashenc_delete_file(s, filename);
-                    av_free(os->segments[j]);
-                }
-                os->nb_segments -= remove;
-                memmove(os->segments, os->segments + remove, os->nb_segments * sizeof(*os->segments));
-            }
+            int remove_count = os->nb_segments - c->window_size - c->extra_window_size;
+            if (remove_count > 0)
+                dashenc_delete_media_segments(s, os, remove_count);
         }
     }
 
@@ -1584,6 +1612,7 @@  static int dash_write_packet(AVFormatContext *s, AVPacket *pkt)
 static int dash_write_trailer(AVFormatContext *s)
 {
     DASHContext *c = s->priv_data;
+    int i;
 
     if (s->nb_streams > 0) {
         OutputStream *os = &c->streams[0];
@@ -1599,14 +1628,19 @@  static int dash_write_trailer(AVFormatContext *s)
     }
     dash_flush(s, 1, -1);
 
-    if (c->remove_at_exit) {
-        char filename[1024];
-        int i;
-        for (i = 0; i < s->nb_streams; i++) {
-            OutputStream *os = &c->streams[i];
-            snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->initfile);
-            dashenc_delete_file(s, filename);
+    for (i = 0; i < s->nb_streams; ++i) {
+        OutputStream *os = &c->streams[i];
+        if (os->ctx && os->ctx_inited) {
+            av_write_trailer(os->ctx);
+        }
+
+        if (c->remove_at_exit) {
+            dashenc_delete_media_segments(s, os, os->nb_segments);
+            dashenc_delete_segment_file(s, os->initfile);
         }
+    }
+
+    if (c->remove_at_exit) {
         dashenc_delete_file(s, s->url);
     }