Re: [FFmpeg-devel] [PATCH 2/2] hlsenc: write playlist into a temp file and replace the original atomically
On Sat, Feb 21, 2015 at 12:10:09AM +0100, Lukasz Marek wrote: > 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 > >>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, ok, applied ive also made rename optional and disabled it if its not the file protocol > 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. yes, though this seems too much work if its for just this > > >>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 > -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Into a blind darkness they enter who follow after the Ignorance, they as if into a greater darkness enter who devote themselves to the Knowledge alone. -- Isha Upanishad 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 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
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 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 Fri, Feb 20, 2015 at 1:38 PM, Michael Niedermayer 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 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
[FFmpeg-devel] [PATCH 2/2] hlsenc: write playlist into a temp file and replace the original atomically
--- 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); return ret; } -- 1.9.5.msysgit.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel