| Message ID | 20181129182832.13771-1-andrey.semashev@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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.
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); }