diff mbox

[FFmpeg-devel] avformat/hlsenc: add hls_flag option to create segments atomically

Message ID 20170131202942.62185-1-ffmpeg@tmm1.net
State New
Headers show

Commit Message

Aman Karmani Jan. 31, 2017, 8:29 p.m. UTC
From: Aman Gupta <aman@tmm1.net>

Adds a `-hls_flags +temp_file` which will write segment data to
filename.tmp, and then rename to filename when the segment is complete
and before the file is added to the m3u8 playlist.

This patch is similar in spirit to one used in Plex's ffmpeg fork, and
allows a transcoding webserver to ensure incomplete segment files are
never served up accidentally.
---
 libavformat/hlsenc.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Steven Liu Feb. 1, 2017, 9:23 a.m. UTC | #1
2017-02-01 4:29 GMT+08:00 Aman Gupta <ffmpeg@tmm1.net>:

> From: Aman Gupta <aman@tmm1.net>
>
> Adds a `-hls_flags +temp_file` which will write segment data to
> filename.tmp, and then rename to filename when the segment is complete
> and before the file is added to the m3u8 playlist.
>
> This patch is similar in spirit to one used in Plex's ffmpeg fork, and
> allows a transcoding webserver to ensure incomplete segment files are
> never served up accidentally.
> ---
>  libavformat/hlsenc.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index bd1e684..23b9011 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -76,6 +76,7 @@ typedef enum HLSFlags {
>      HLS_SECOND_LEVEL_SEGMENT_INDEX = (1 << 8), // include segment index
> in segment filenames when use_localtime  e.g.: %%03d
>      HLS_SECOND_LEVEL_SEGMENT_DURATION = (1 << 9), // include segment
> duration (microsec) in segment filenames when use_localtime  e.g.: %%09t
>      HLS_SECOND_LEVEL_SEGMENT_SIZE = (1 << 10), // include segment size
> (bytes) in segment filenames when use_localtime  e.g.: %%014s
> +    HLS_TEMP_FILE = (1 << 11),
>  } HLSFlags;
>
>  typedef enum {
> @@ -915,6 +916,10 @@ static int hls_start(AVFormatContext *s)
>
>      set_http_options(&options, c);
>
> +    if (c->flags & HLS_TEMP_FILE) {
> +        av_strlcat(oc->filename, ".tmp", sizeof(oc->filename));
> +    }
> +
>      if (c->key_info_file) {
>          if ((err = hls_encryption_start(s)) < 0)
>              goto fail;
> @@ -1276,6 +1281,15 @@ static int hls_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>
>          av_write_frame(oc, NULL); /* Flush any buffered data */
>
> +        if (hls->flags & HLS_TEMP_FILE) {
> +            size_t len = strlen(oc->filename);
> +            char final_filename[sizeof(oc->filename)];
> +            av_strlcpy(final_filename, oc->filename, len);
> +            final_filename[len-4] = '\0';
> +            ff_rename(oc->filename, final_filename, s);
> +            oc->filename[len-4] = '\0';
> +        }
> +
>          new_start_pos = avio_tell(hls->avf->pb);
>          hls->size = new_start_pos - hls->start_pos;
>          ret = hls_append_segment(s, hls, hls->duration, hls->start_pos,
> hls->size);
> @@ -1406,6 +1420,7 @@ static const AVOption options[] = {
>      {"hls_subtitle_path",     "set path of hls subtitles",
> OFFSET(subtitle_filename), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
>      {"hls_flags",     "set flags affecting HLS playlist and media file
> generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E,
> "flags"},
>      {"single_file",   "generate a single media file indexed with byte
> ranges", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE }, 0, UINT_MAX,   E,
> "flags"},
> +    {"temp_file", "write segment to temporary file and rename when
> complete", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_TEMP_FILE }, 0, UINT_MAX,   E,
> "flags"},
>      {"delete_segments", "delete segment files that are no longer part of
> the playlist", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_DELETE_SEGMENTS }, 0,
> UINT_MAX,   E, "flags"},
>      {"round_durations", "round durations in m3u8 to whole numbers", 0,
> AV_OPT_TYPE_CONST, {.i64 = HLS_ROUND_DURATIONS }, 0, UINT_MAX,   E,
> "flags"},
>      {"discont_start", "start the playlist with a discontinuity tag", 0,
> AV_OPT_TYPE_CONST, {.i64 = HLS_DISCONT_START }, 0, UINT_MAX,   E, "flags"},
> --
> 2.10.1 (Apple Git-78)
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


Why use temp filename?
There have no info in m3u8 list before the segment complete.
Bodecs Bela Feb. 1, 2017, 2:03 p.m. UTC | #2
2017.01.31. 21:29 keltezéssel, Aman Gupta írta:
> From: Aman Gupta <aman@tmm1.net>
>
> Adds a `-hls_flags +temp_file` which will write segment data to
> filename.tmp, and then rename to filename when the segment is complete
> and before the file is added to the m3u8 playlist.
>
> This patch is similar in spirit to one used in Plex's ffmpeg fork, and
> allows a transcoding webserver to ensure incomplete segment files are
> never served up accidentally.
> ---
>   libavformat/hlsenc.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index bd1e684..23b9011 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -76,6 +76,7 @@ typedef enum HLSFlags {
>       HLS_SECOND_LEVEL_SEGMENT_INDEX = (1 << 8), // include segment index in segment filenames when use_localtime  e.g.: %%03d
>       HLS_SECOND_LEVEL_SEGMENT_DURATION = (1 << 9), // include segment duration (microsec) in segment filenames when use_localtime  e.g.: %%09t
>       HLS_SECOND_LEVEL_SEGMENT_SIZE = (1 << 10), // include segment size (bytes) in segment filenames when use_localtime  e.g.: %%014s
> +    HLS_TEMP_FILE = (1 << 11),
>   } HLSFlags;
>   
>   typedef enum {
> @@ -915,6 +916,10 @@ static int hls_start(AVFormatContext *s)
>   
>       set_http_options(&options, c);
>   
> +    if (c->flags & HLS_TEMP_FILE) {
> +        av_strlcat(oc->filename, ".tmp", sizeof(oc->filename));
> +    }
> +
>       if (c->key_info_file) {
>           if ((err = hls_encryption_start(s)) < 0)
>               goto fail;
> @@ -1276,6 +1281,15 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>   
>           av_write_frame(oc, NULL); /* Flush any buffered data */
>   
> +        if (hls->flags & HLS_TEMP_FILE) {
> +            size_t len = strlen(oc->filename);
> +            char final_filename[sizeof(oc->filename)];
> +            av_strlcpy(final_filename, oc->filename, len);
> +            final_filename[len-4] = '\0';
> +            ff_rename(oc->filename, final_filename, s);
> +            oc->filename[len-4] = '\0';
> +        }
> +
>           new_start_pos = avio_tell(hls->avf->pb);
>           hls->size = new_start_pos - hls->start_pos;
>           ret = hls_append_segment(s, hls, hls->duration, hls->start_pos, hls->size);
> @@ -1406,6 +1420,7 @@ static const AVOption options[] = {
>       {"hls_subtitle_path",     "set path of hls subtitles", OFFSET(subtitle_filename), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
>       {"hls_flags",     "set flags affecting HLS playlist and media file generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E, "flags"},
>       {"single_file",   "generate a single media file indexed with byte ranges", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE }, 0, UINT_MAX,   E, "flags"},
> +    {"temp_file", "write segment to temporary file and rename when complete", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_TEMP_FILE }, 0, UINT_MAX,   E, "flags"},
>       {"delete_segments", "delete segment files that are no longer part of the playlist", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_DELETE_SEGMENTS }, 0, UINT_MAX,   E, "flags"},
>       {"round_durations", "round durations in m3u8 to whole numbers", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_ROUND_DURATIONS }, 0, UINT_MAX,   E, "flags"},
>       {"discont_start", "start the playlist with a discontinuity tag", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_DISCONT_START }, 0, UINT_MAX,   E, "flags"},
I think the phrase  in your email subject "to create atomically" 
misleading because another renaming may occur afterwards your temp 
renaming if second_level_segment_size or second_level_segment_duration 
flag is on.
But the idea to write segment content into a temp file is good by me. To 
ensure the amoticity the two distinct renamings may be merged.

I have never thought of it but after your idea I concluded  that the 
segment file names are predictable.
In live streaming enviroment somebody predicting the segment name may 
see the program some seconds earlier than any other viewer by 
downloading the unpublished segment. (with one segment duration seconds 
earlier)
If the temp file name would be random name, then your idea may be seen 
as a security feature.
What is your opinion about this possibility?

bb
Aman Karmani Feb. 1, 2017, 11:48 p.m. UTC | #3
On Tue, Jan 31, 2017 at 12:29 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:

> From: Aman Gupta <aman@tmm1.net>
>
> Adds a `-hls_flags +temp_file` which will write segment data to
> filename.tmp, and then rename to filename when the segment is complete
> and before the file is added to the m3u8 playlist.
>
> This patch is similar in spirit to one used in Plex's ffmpeg fork, and
> allows a transcoding webserver to ensure incomplete segment files are
> never served up accidentally.
> ---
>  libavformat/hlsenc.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index bd1e684..23b9011 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -76,6 +76,7 @@ typedef enum HLSFlags {
>      HLS_SECOND_LEVEL_SEGMENT_INDEX = (1 << 8), // include segment index
> in segment filenames when use_localtime  e.g.: %%03d
>      HLS_SECOND_LEVEL_SEGMENT_DURATION = (1 << 9), // include segment
> duration (microsec) in segment filenames when use_localtime  e.g.: %%09t
>      HLS_SECOND_LEVEL_SEGMENT_SIZE = (1 << 10), // include segment size
> (bytes) in segment filenames when use_localtime  e.g.: %%014s
> +    HLS_TEMP_FILE = (1 << 11),
>  } HLSFlags;
>
>  typedef enum {
> @@ -915,6 +916,10 @@ static int hls_start(AVFormatContext *s)
>
>      set_http_options(&options, c);
>
> +    if (c->flags & HLS_TEMP_FILE) {
> +        av_strlcat(oc->filename, ".tmp", sizeof(oc->filename));
> +    }
> +
>      if (c->key_info_file) {
>          if ((err = hls_encryption_start(s)) < 0)
>              goto fail;
> @@ -1276,6 +1281,15 @@ static int hls_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>
>          av_write_frame(oc, NULL); /* Flush any buffered data */
>
> +        if (hls->flags & HLS_TEMP_FILE) {
> +            size_t len = strlen(oc->filename);
> +            char final_filename[sizeof(oc->filename)];
> +            av_strlcpy(final_filename, oc->filename, len);
> +            final_filename[len-4] = '\0';
> +            ff_rename(oc->filename, final_filename, s);
>

This doesn't work on windows, since you cannot rename the file while it is
still open. Strangely ff_rename is not logging any errors in this case.


> +            oc->filename[len-4] = '\0';
> +        }
> +
>          new_start_pos = avio_tell(hls->avf->pb);
>          hls->size = new_start_pos - hls->start_pos;
>          ret = hls_append_segment(s, hls, hls->duration, hls->start_pos,
> hls->size);
> @@ -1406,6 +1420,7 @@ static const AVOption options[] = {
>      {"hls_subtitle_path",     "set path of hls subtitles",
> OFFSET(subtitle_filename), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
>      {"hls_flags",     "set flags affecting HLS playlist and media file
> generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E,
> "flags"},
>      {"single_file",   "generate a single media file indexed with byte
> ranges", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE }, 0, UINT_MAX,   E,
> "flags"},
> +    {"temp_file", "write segment to temporary file and rename when
> complete", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_TEMP_FILE }, 0, UINT_MAX,   E,
> "flags"},
>      {"delete_segments", "delete segment files that are no longer part of
> the playlist", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_DELETE_SEGMENTS }, 0,
> UINT_MAX,   E, "flags"},
>      {"round_durations", "round durations in m3u8 to whole numbers", 0,
> AV_OPT_TYPE_CONST, {.i64 = HLS_ROUND_DURATIONS }, 0, UINT_MAX,   E,
> "flags"},
>      {"discont_start", "start the playlist with a discontinuity tag", 0,
> AV_OPT_TYPE_CONST, {.i64 = HLS_DISCONT_START }, 0, UINT_MAX,   E, "flags"},
> --
> 2.10.1 (Apple Git-78)
>
>
Aman Karmani Feb. 1, 2017, 11:49 p.m. UTC | #4
On Wed, Feb 1, 2017 at 1:23 AM, Steven Liu <lingjiujianke@gmail.com> wrote:

>
>
> 2017-02-01 4:29 GMT+08:00 Aman Gupta <ffmpeg@tmm1.net>:
>
>> From: Aman Gupta <aman@tmm1.net>
>>
>> Adds a `-hls_flags +temp_file` which will write segment data to
>> filename.tmp, and then rename to filename when the segment is complete
>> and before the file is added to the m3u8 playlist.
>>
>> This patch is similar in spirit to one used in Plex's ffmpeg fork, and
>> allows a transcoding webserver to ensure incomplete segment files are
>> never served up accidentally.
>> ---
>>  libavformat/hlsenc.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index bd1e684..23b9011 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -76,6 +76,7 @@ typedef enum HLSFlags {
>>      HLS_SECOND_LEVEL_SEGMENT_INDEX = (1 << 8), // include segment index
>> in segment filenames when use_localtime  e.g.: %%03d
>>      HLS_SECOND_LEVEL_SEGMENT_DURATION = (1 << 9), // include segment
>> duration (microsec) in segment filenames when use_localtime  e.g.: %%09t
>>      HLS_SECOND_LEVEL_SEGMENT_SIZE = (1 << 10), // include segment size
>> (bytes) in segment filenames when use_localtime  e.g.: %%014s
>> +    HLS_TEMP_FILE = (1 << 11),
>>  } HLSFlags;
>>
>>  typedef enum {
>> @@ -915,6 +916,10 @@ static int hls_start(AVFormatContext *s)
>>
>>      set_http_options(&options, c);
>>
>> +    if (c->flags & HLS_TEMP_FILE) {
>> +        av_strlcat(oc->filename, ".tmp", sizeof(oc->filename));
>> +    }
>> +
>>      if (c->key_info_file) {
>>          if ((err = hls_encryption_start(s)) < 0)
>>              goto fail;
>> @@ -1276,6 +1281,15 @@ static int hls_write_packet(AVFormatContext *s,
>> AVPacket *pkt)
>>
>>          av_write_frame(oc, NULL); /* Flush any buffered data */
>>
>> +        if (hls->flags & HLS_TEMP_FILE) {
>> +            size_t len = strlen(oc->filename);
>> +            char final_filename[sizeof(oc->filename)];
>> +            av_strlcpy(final_filename, oc->filename, len);
>> +            final_filename[len-4] = '\0';
>> +            ff_rename(oc->filename, final_filename, s);
>> +            oc->filename[len-4] = '\0';
>> +        }
>> +
>>          new_start_pos = avio_tell(hls->avf->pb);
>>          hls->size = new_start_pos - hls->start_pos;
>>          ret = hls_append_segment(s, hls, hls->duration, hls->start_pos,
>> hls->size);
>> @@ -1406,6 +1420,7 @@ static const AVOption options[] = {
>>      {"hls_subtitle_path",     "set path of hls subtitles",
>> OFFSET(subtitle_filename), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
>>      {"hls_flags",     "set flags affecting HLS playlist and media file
>> generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E,
>> "flags"},
>>      {"single_file",   "generate a single media file indexed with byte
>> ranges", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE }, 0, UINT_MAX,   E,
>> "flags"},
>> +    {"temp_file", "write segment to temporary file and rename when
>> complete", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_TEMP_FILE }, 0, UINT_MAX,   E,
>> "flags"},
>>      {"delete_segments", "delete segment files that are no longer part of
>> the playlist", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_DELETE_SEGMENTS }, 0,
>> UINT_MAX,   E, "flags"},
>>      {"round_durations", "round durations in m3u8 to whole numbers", 0,
>> AV_OPT_TYPE_CONST, {.i64 = HLS_ROUND_DURATIONS }, 0, UINT_MAX,   E,
>> "flags"},
>>      {"discont_start", "start the playlist with a discontinuity tag", 0,
>> AV_OPT_TYPE_CONST, {.i64 = HLS_DISCONT_START }, 0, UINT_MAX,   E, "flags"},
>> --
>> 2.10.1 (Apple Git-78)
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
>
> Why use temp filename?
> There have no info in m3u8 list before the segment complete.
>

Plex and other "on-the-fly" transcoding webservers will create a fake m3u8
with all the segments listed. When segment2.ts is requested but not yet
available, it will wait for the file to show up on the disk and then serve
it to the client.
Aman Karmani Feb. 2, 2017, 11:27 p.m. UTC | #5
On Wed, Feb 1, 2017 at 6:03 AM, Bodecs Bela <bodecsb@vivanet.hu> wrote:

>
>
> 2017.01.31. 21:29 keltezéssel, Aman Gupta írta:
>
>> From: Aman Gupta <aman@tmm1.net>
>>
>> Adds a `-hls_flags +temp_file` which will write segment data to
>> filename.tmp, and then rename to filename when the segment is complete
>> and before the file is added to the m3u8 playlist.
>>
>> This patch is similar in spirit to one used in Plex's ffmpeg fork, and
>> allows a transcoding webserver to ensure incomplete segment files are
>> never served up accidentally.
>> ---
>>   libavformat/hlsenc.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index bd1e684..23b9011 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -76,6 +76,7 @@ typedef enum HLSFlags {
>>       HLS_SECOND_LEVEL_SEGMENT_INDEX = (1 << 8), // include segment index
>> in segment filenames when use_localtime  e.g.: %%03d
>>       HLS_SECOND_LEVEL_SEGMENT_DURATION = (1 << 9), // include segment
>> duration (microsec) in segment filenames when use_localtime  e.g.: %%09t
>>       HLS_SECOND_LEVEL_SEGMENT_SIZE = (1 << 10), // include segment size
>> (bytes) in segment filenames when use_localtime  e.g.: %%014s
>> +    HLS_TEMP_FILE = (1 << 11),
>>   } HLSFlags;
>>     typedef enum {
>> @@ -915,6 +916,10 @@ static int hls_start(AVFormatContext *s)
>>         set_http_options(&options, c);
>>   +    if (c->flags & HLS_TEMP_FILE) {
>> +        av_strlcat(oc->filename, ".tmp", sizeof(oc->filename));
>> +    }
>> +
>>       if (c->key_info_file) {
>>           if ((err = hls_encryption_start(s)) < 0)
>>               goto fail;
>> @@ -1276,6 +1281,15 @@ static int hls_write_packet(AVFormatContext *s,
>> AVPacket *pkt)
>>             av_write_frame(oc, NULL); /* Flush any buffered data */
>>   +        if (hls->flags & HLS_TEMP_FILE) {
>> +            size_t len = strlen(oc->filename);
>> +            char final_filename[sizeof(oc->filename)];
>> +            av_strlcpy(final_filename, oc->filename, len);
>> +            final_filename[len-4] = '\0';
>> +            ff_rename(oc->filename, final_filename, s);
>> +            oc->filename[len-4] = '\0';
>> +        }
>> +
>>           new_start_pos = avio_tell(hls->avf->pb);
>>           hls->size = new_start_pos - hls->start_pos;
>>           ret = hls_append_segment(s, hls, hls->duration, hls->start_pos,
>> hls->size);
>> @@ -1406,6 +1420,7 @@ static const AVOption options[] = {
>>       {"hls_subtitle_path",     "set path of hls subtitles",
>> OFFSET(subtitle_filename), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
>>       {"hls_flags",     "set flags affecting HLS playlist and media file
>> generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E,
>> "flags"},
>>       {"single_file",   "generate a single media file indexed with byte
>> ranges", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE }, 0, UINT_MAX,   E,
>> "flags"},
>> +    {"temp_file", "write segment to temporary file and rename when
>> complete", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_TEMP_FILE }, 0, UINT_MAX,   E,
>> "flags"},
>>       {"delete_segments", "delete segment files that are no longer part
>> of the playlist", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_DELETE_SEGMENTS }, 0,
>> UINT_MAX,   E, "flags"},
>>       {"round_durations", "round durations in m3u8 to whole numbers", 0,
>> AV_OPT_TYPE_CONST, {.i64 = HLS_ROUND_DURATIONS }, 0, UINT_MAX,   E,
>> "flags"},
>>       {"discont_start", "start the playlist with a discontinuity tag", 0,
>> AV_OPT_TYPE_CONST, {.i64 = HLS_DISCONT_START }, 0, UINT_MAX,   E, "flags"},
>>
> I think the phrase  in your email subject "to create atomically"
> misleading because another renaming may occur afterwards your temp renaming
> if second_level_segment_size or second_level_segment_duration flag is on.
> But the idea to write segment content into a temp file is good by me. To
> ensure the amoticity the two distinct renamings may be merged.
>

I agree my use of "atomic" is confusing. I removed this from the commit
message in v2 of the patch.


>
> I have never thought of it but after your idea I concluded  that the
> segment file names are predictable.
> In live streaming enviroment somebody predicting the segment name may see
> the program some seconds earlier than any other viewer by downloading the
> unpublished segment. (with one segment duration seconds earlier)
> If the temp file name would be random name, then your idea may be seen as
> a security feature.
> What is your opinion about this possibility?


I don't have a strong opinion on this. Presumably if someone is concerned
about this, they can still use my temp_file patch and configure their web
server to reject any requests for *.tmp


>
>
> bb
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Bodecs Bela Feb. 3, 2017, 7:37 a.m. UTC | #6
2017.02.03. 0:27 keltezéssel, Aman Gupta írta:
> On Wed, Feb 1, 2017 at 6:03 AM, Bodecs Bela <bodecsb@vivanet.hu> wrote:
>
>>
>> 2017.01.31. 21:29 keltezéssel, Aman Gupta írta:
>>
>>> From: Aman Gupta <aman@tmm1.net>
>>>
>>> Adds a `-hls_flags +temp_file` which will write segment data to
>>> filename.tmp, and then rename to filename when the segment is complete
>>> and before the file is added to the m3u8 playlist.
>>>
>>> This patch is similar in spirit to one used in Plex's ffmpeg fork, and
>>> allows a transcoding webserver to ensure incomplete segment files are
>>> never served up accidentally.
>>> ---
>>>    libavformat/hlsenc.c | 15 +++++++++++++++
>>>    1 file changed, 15 insertions(+)
>>>
>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>> index bd1e684..23b9011 100644
>>> --- a/libavformat/hlsenc.c
>>> +++ b/libavformat/hlsenc.c
>>> @@ -76,6 +76,7 @@ typedef enum HLSFlags {
>>>        HLS_SECOND_LEVEL_SEGMENT_INDEX = (1 << 8), // include segment index
>>> in segment filenames when use_localtime  e.g.: %%03d
>>>        HLS_SECOND_LEVEL_SEGMENT_DURATION = (1 << 9), // include segment
>>> duration (microsec) in segment filenames when use_localtime  e.g.: %%09t
>>>        HLS_SECOND_LEVEL_SEGMENT_SIZE = (1 << 10), // include segment size
>>> (bytes) in segment filenames when use_localtime  e.g.: %%014s
>>> +    HLS_TEMP_FILE = (1 << 11),
>>>    } HLSFlags;
>>>      typedef enum {
>>> @@ -915,6 +916,10 @@ static int hls_start(AVFormatContext *s)
>>>          set_http_options(&options, c);
>>>    +    if (c->flags & HLS_TEMP_FILE) {
>>> +        av_strlcat(oc->filename, ".tmp", sizeof(oc->filename));
>>> +    }
>>> +
>>>        if (c->key_info_file) {
>>>            if ((err = hls_encryption_start(s)) < 0)
>>>                goto fail;
>>> @@ -1276,6 +1281,15 @@ static int hls_write_packet(AVFormatContext *s,
>>> AVPacket *pkt)
>>>              av_write_frame(oc, NULL); /* Flush any buffered data */
>>>    +        if (hls->flags & HLS_TEMP_FILE) {
>>> +            size_t len = strlen(oc->filename);
>>> +            char final_filename[sizeof(oc->filename)];
>>> +            av_strlcpy(final_filename, oc->filename, len);
>>> +            final_filename[len-4] = '\0';
>>> +            ff_rename(oc->filename, final_filename, s);
>>> +            oc->filename[len-4] = '\0';
>>> +        }
>>> +
>>>            new_start_pos = avio_tell(hls->avf->pb);
>>>            hls->size = new_start_pos - hls->start_pos;
>>>            ret = hls_append_segment(s, hls, hls->duration, hls->start_pos,
>>> hls->size);
>>> @@ -1406,6 +1420,7 @@ static const AVOption options[] = {
>>>        {"hls_subtitle_path",     "set path of hls subtitles",
>>> OFFSET(subtitle_filename), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
>>>        {"hls_flags",     "set flags affecting HLS playlist and media file
>>> generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E,
>>> "flags"},
>>>        {"single_file",   "generate a single media file indexed with byte
>>> ranges", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE }, 0, UINT_MAX,   E,
>>> "flags"},
>>> +    {"temp_file", "write segment to temporary file and rename when
>>> complete", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_TEMP_FILE }, 0, UINT_MAX,   E,
>>> "flags"},
>>>        {"delete_segments", "delete segment files that are no longer part
>>> of the playlist", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_DELETE_SEGMENTS }, 0,
>>> UINT_MAX,   E, "flags"},
>>>        {"round_durations", "round durations in m3u8 to whole numbers", 0,
>>> AV_OPT_TYPE_CONST, {.i64 = HLS_ROUND_DURATIONS }, 0, UINT_MAX,   E,
>>> "flags"},
>>>        {"discont_start", "start the playlist with a discontinuity tag", 0,
>>> AV_OPT_TYPE_CONST, {.i64 = HLS_DISCONT_START }, 0, UINT_MAX,   E, "flags"},
>>>
>> I think the phrase  in your email subject "to create atomically"
>> misleading because another renaming may occur afterwards your temp renaming
>> if second_level_segment_size or second_level_segment_duration flag is on.
>> But the idea to write segment content into a temp file is good by me. To
>> ensure the amoticity the two distinct renamings may be merged.
>>
> I agree my use of "atomic" is confusing. I removed this from the commit
> message in v2 of the patch.
>
>
>> I have never thought of it but after your idea I concluded  that the
>> segment file names are predictable.
>> In live streaming enviroment somebody predicting the segment name may see
>> the program some seconds earlier than any other viewer by downloading the
>> unpublished segment. (with one segment duration seconds earlier)
>> If the temp file name would be random name, then your idea may be seen as
>> a security feature.
>> What is your opinion about this possibility?
>
> I don't have a strong opinion on this. Presumably if someone is concerned
> about this, they can still use my temp_file patch and configure their web
> server to reject any requests for *.tmp
>
Yes, I agree with you. Maybe You can put a short notice about this into 
the documentation.
>>
>> bb
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index bd1e684..23b9011 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -76,6 +76,7 @@  typedef enum HLSFlags {
     HLS_SECOND_LEVEL_SEGMENT_INDEX = (1 << 8), // include segment index in segment filenames when use_localtime  e.g.: %%03d
     HLS_SECOND_LEVEL_SEGMENT_DURATION = (1 << 9), // include segment duration (microsec) in segment filenames when use_localtime  e.g.: %%09t
     HLS_SECOND_LEVEL_SEGMENT_SIZE = (1 << 10), // include segment size (bytes) in segment filenames when use_localtime  e.g.: %%014s
+    HLS_TEMP_FILE = (1 << 11),
 } HLSFlags;
 
 typedef enum {
@@ -915,6 +916,10 @@  static int hls_start(AVFormatContext *s)
 
     set_http_options(&options, c);
 
+    if (c->flags & HLS_TEMP_FILE) {
+        av_strlcat(oc->filename, ".tmp", sizeof(oc->filename));
+    }
+
     if (c->key_info_file) {
         if ((err = hls_encryption_start(s)) < 0)
             goto fail;
@@ -1276,6 +1281,15 @@  static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
 
         av_write_frame(oc, NULL); /* Flush any buffered data */
 
+        if (hls->flags & HLS_TEMP_FILE) {
+            size_t len = strlen(oc->filename);
+            char final_filename[sizeof(oc->filename)];
+            av_strlcpy(final_filename, oc->filename, len);
+            final_filename[len-4] = '\0';
+            ff_rename(oc->filename, final_filename, s);
+            oc->filename[len-4] = '\0';
+        }
+
         new_start_pos = avio_tell(hls->avf->pb);
         hls->size = new_start_pos - hls->start_pos;
         ret = hls_append_segment(s, hls, hls->duration, hls->start_pos, hls->size);
@@ -1406,6 +1420,7 @@  static const AVOption options[] = {
     {"hls_subtitle_path",     "set path of hls subtitles", OFFSET(subtitle_filename), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
     {"hls_flags",     "set flags affecting HLS playlist and media file generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E, "flags"},
     {"single_file",   "generate a single media file indexed with byte ranges", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE }, 0, UINT_MAX,   E, "flags"},
+    {"temp_file", "write segment to temporary file and rename when complete", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_TEMP_FILE }, 0, UINT_MAX,   E, "flags"},
     {"delete_segments", "delete segment files that are no longer part of the playlist", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_DELETE_SEGMENTS }, 0, UINT_MAX,   E, "flags"},
     {"round_durations", "round durations in m3u8 to whole numbers", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_ROUND_DURATIONS }, 0, UINT_MAX,   E, "flags"},
     {"discont_start", "start the playlist with a discontinuity tag", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_DISCONT_START }, 0, UINT_MAX,   E, "flags"},