Re: [FFmpeg-devel] [PATCH 2/2] hlsenc: write playlist into a temp file and replace the original atomically
On Fri, Feb 20, 2015 at 12:55:14PM +0100, Hendrik Leppkes wrote: --- libavformat/hlsenc.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 1831c17..0f14e90 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -242,10 +242,12 @@ static int hls_window(AVFormatContext *s, int last) int target_duration = 0; int ret = 0; AVIOContext *out = NULL; +char temp_filename[1024]; int64_t sequence = FFMAX(hls-start_sequence, hls-sequence - hls-nb_entries); int version = hls-flags HLS_SINGLE_FILE ? 4 : 3; -if ((ret = avio_open2(out, s-filename, AVIO_FLAG_WRITE, +snprintf(temp_filename, sizeof(temp_filename), %s.tmp, s-filename); +if ((ret = avio_open2(out, temp_filename, AVIO_FLAG_WRITE, s-interrupt_callback, NULL)) 0) goto fail; @@ -280,6 +282,8 @@ static int hls_window(AVFormatContext *s, int last) fail: avio_closep(out); +if (ret = 0) +ff_rename(temp_filename, s-filename, s); what if s-filename is not a local file ? or am i missing something that prevents that ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] hlsenc: write playlist into a temp file and replace the original atomically
On Fri, Feb 20, 2015 at 1:38 PM, Michael Niedermayer michae...@gmx.at wrote: On Fri, Feb 20, 2015 at 12:55:14PM +0100, Hendrik Leppkes wrote: --- libavformat/hlsenc.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 1831c17..0f14e90 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -242,10 +242,12 @@ static int hls_window(AVFormatContext *s, int last) int target_duration = 0; int ret = 0; AVIOContext *out = NULL; +char temp_filename[1024]; int64_t sequence = FFMAX(hls-start_sequence, hls-sequence - hls-nb_entries); int version = hls-flags HLS_SINGLE_FILE ? 4 : 3; -if ((ret = avio_open2(out, s-filename, AVIO_FLAG_WRITE, +snprintf(temp_filename, sizeof(temp_filename), %s.tmp, s-filename); +if ((ret = avio_open2(out, temp_filename, AVIO_FLAG_WRITE, s-interrupt_callback, NULL)) 0) goto fail; @@ -280,6 +282,8 @@ static int hls_window(AVFormatContext *s, int last) fail: avio_closep(out); +if (ret = 0) +ff_rename(temp_filename, s-filename, s); what if s-filename is not a local file ? or am i missing something that prevents that ? What else could it be? It writes to separate segment files as well, not sure that would work on anything but local files. For the record, this is the same method used in dashenc for writing its manifest file, and it can avoid a sort-of race condition when serving the files through a web server directly (ie. web server reading while in the middle of re-writing it). - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] hlsenc: write playlist into a temp file and replace the original atomically
On Fri, Feb 20, 2015 at 02:15:30PM +0100, Hendrik Leppkes wrote: On Fri, Feb 20, 2015 at 1:38 PM, Michael Niedermayer michae...@gmx.at wrote: On Fri, Feb 20, 2015 at 12:55:14PM +0100, Hendrik Leppkes wrote: --- libavformat/hlsenc.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 1831c17..0f14e90 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -242,10 +242,12 @@ static int hls_window(AVFormatContext *s, int last) int target_duration = 0; int ret = 0; AVIOContext *out = NULL; +char temp_filename[1024]; int64_t sequence = FFMAX(hls-start_sequence, hls-sequence - hls-nb_entries); int version = hls-flags HLS_SINGLE_FILE ? 4 : 3; -if ((ret = avio_open2(out, s-filename, AVIO_FLAG_WRITE, +snprintf(temp_filename, sizeof(temp_filename), %s.tmp, s-filename); +if ((ret = avio_open2(out, temp_filename, AVIO_FLAG_WRITE, s-interrupt_callback, NULL)) 0) goto fail; @@ -280,6 +282,8 @@ static int hls_window(AVFormatContext *s, int last) fail: avio_closep(out); +if (ret = 0) +ff_rename(temp_filename, s-filename, s); what if s-filename is not a local file ? or am i missing something that prevents that ? What else could it be? http, ftp, who knows also ff_rename() would interpret a http://; differently from avio_open2(), so something more is needed to not end up with odd rename() calls It writes to separate segment files as well, not sure that would work on anything but local files. i dont know either but if someone passes ftp://...; it should either work or fail with a error message or mostly work while printing a warning For the record, this is the same method used in dashenc for writing its manifest file, and it can avoid a sort-of race condition when serving the files through a web server directly (ie. web server reading while in the middle of re-writing it). - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Let us carefully observe those good qualities wherein our enemies excel us and endeavor to excel them, by avoiding what is faulty, and imitating what is excellent in them. -- Plutarch signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] hlsenc: write playlist into a temp file and replace the original atomically
On 20.02.2015 14:30, Michael Niedermayer wrote: On Fri, Feb 20, 2015 at 02:15:30PM +0100, Hendrik Leppkes wrote: On Fri, Feb 20, 2015 at 1:38 PM, Michael Niedermayer michae...@gmx.at wrote: On Fri, Feb 20, 2015 at 12:55:14PM +0100, Hendrik Leppkes wrote: What else could it be? http, ftp, who knows also ff_rename() would interpret a http://; differently from avio_open2(), so something more is needed to not end up with odd rename() calls It writes to separate segment files as well, not sure that would work on anything but local files. i dont know either but if someone passes ftp://...; it should either work or fail with a error message or mostly work while printing a warning Some ftp servers claim thay allow to upload atomically. Who knows whats the true. I wouldn't do that probably. But real regression is in case someone doesn't really care to have live stream, but just want to generate manifest and chunks on remote server directly for later use. For me you patch is OK, but maybe it is worth to add rename as part of protocol implementation to support these cases too. Assuming tempfile could have hardcoded name like .filename instead of filename and rename it when is ready. For the record, this is the same method used in dashenc for writing its manifest file, and it can avoid a sort-of race condition when serving the files through a web server directly (ie. web server reading while in the middle of re-writing it). And the same in smoothstreaming encoder. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
