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

[Bodecs Bela]

2017.02.03. 0:27 keltezéssel, Aman Gupta írta:
> On Wed, Feb 1, 2017 at 6:03 AM, Bodecs Bela <bodecsb at vivanet.hu> wrote:
>
>>
>> 2017.01.31. 21:29 keltezéssel, Aman Gupta írta:
>>
>>> From: Aman Gupta <aman at 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 at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel