[FFmpeg-devel] avformat/hlsenc: add hls_fmp4_init_refresh option

add option for refresh init file after m3u8 refresh everytime.

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 doc/muxers.texi      |  3 +++
 libavformat/hlsenc.c | 44 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 41 insertions(+), 6 deletions(-)

Steven Liu:
> add option for refresh init file after m3u8 refresh everytime.
> 
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  doc/muxers.texi      |  3 +++
>  libavformat/hlsenc.c | 44 ++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index d304181671..fc38f40bc9 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -836,6 +836,9 @@ fmp4 files may be used in HLS version 7 and above.
>  @item hls_fmp4_init_filename @var{filename}
>  Set filename to the fragment files header file, default filename is @file{init.mp4}.
>  
> +@item hls_fmp4_init_refresh @var{filename}
> +Refresh init file after m3u8 file refresh every time, default is @var{0}.

The first "refresh" means "resend", doesn't it? Using "refresh" suggests
that the init file will be resent with slightly different content each
time it is sent; yet this doesn't seem to be the case.

> +
>  When @code{var_stream_map} is set with two or more variant streams, the
>  @var{filename} pattern must contain the string "%v", this string specifies
>  the position of variant stream index in the generated init file names.
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index a3c59432c5..6bc77ec17e 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -119,6 +119,7 @@ typedef struct VariantStream {
>      int packets_written;
>      int init_range_length;
>      uint8_t *temp_buffer;
> +    uint8_t *init_buffer;
>  
>      AVFormatContext *avf;
>      AVFormatContext *vtt_avf;
> @@ -192,6 +193,7 @@ typedef struct HLSContext {
>      char *segment_filename;
>      char *fmp4_init_filename;
>      int segment_type;
> +    int refresh_init_file;  ///< refresh init file into disk after refresh m3u8
>  
>      int use_localtime;      ///< flag to expand filename with localtime
>      int use_localtime_mkdir;///< flag to mkdir dirname in timebased filename
> @@ -982,7 +984,8 @@ static int sls_flag_check_duration_size(HLSContext *hls, VariantStream *vs)
>      return ret;
>  }
>  
> -static void sls_flag_file_rename(HLSContext *hls, VariantStream *vs, char *old_filename) {
> +static void sls_flag_file_rename(HLSContext *hls, VariantStream *vs, char *old_filename)
> +{

Stray change.

>      if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE | HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
>          strlen(vs->current_segment_final_filename_fmt)) {
>          ff_rename(old_filename, vs->avf->url, hls);
> @@ -2234,6 +2237,25 @@ static int hls_write_header(AVFormatContext *s)
>      return ret;
>  }
>  
> +static int hls_init_file_refresh(AVFormatContext *s, VariantStream *vs)
> +{
> +    HLSContext *hls = s->priv_data;
> +    AVDictionary *options = NULL;
> +    int ret = 0;
> +
> +    set_http_options(s, &options, hls);
> +    ret = hlsenc_io_open(s, &vs->out, hls->fmp4_init_filename, &options);

You should free options here unconditionally before checking for errors.

> +    if (ret < 0) {
> +        av_dict_free(&options);
> +        return ret;
> +    }
> +    avio_write(vs->out, vs->init_buffer, vs->init_range_length);
> +    hlsenc_io_close(s, &vs->out, hls->fmp4_init_filename);
> +    av_dict_free(&options);
> +
> +    return ret;
> +}
> +
>  static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>  {
>      HLSContext *hls = s->priv_data;
> @@ -2246,7 +2268,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>      int range_length = 0;
>      const char *proto = NULL;
>      int use_temp_file = 0;
> -    uint8_t *buffer = NULL;
>      VariantStream *vs = NULL;
>      char *old_filename = NULL;
>  
> @@ -2332,9 +2353,10 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>          avio_flush(oc->pb);
>          if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>              if (!vs->init_range_length) {
> -                range_length = avio_close_dyn_buf(oc->pb, &buffer);
> -                avio_write(vs->out, buffer, range_length);
> -                av_freep(&buffer);
> +                range_length = avio_close_dyn_buf(oc->pb, &vs->init_buffer);

You are relying on this code to be executed only once to not produce
memleaks in case refresh_init_file is set, aren't you? If so, is it
actually certain that range_length can't be zero in some weird corner case?

> +                avio_write(vs->out, vs->init_buffer, range_length);
> +                if (!hls->refresh_init_file)
> +                    av_freep(&vs->init_buffer);
>                  vs->init_range_length = range_length;
>                  avio_open_dyn_buf(&oc->pb);
>                  vs->packets_written = 0;
> @@ -2446,6 +2468,14 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>              }
>          }
>  
> +        if (hls->refresh_init_file && hls->segment_type == SEGMENT_TYPE_FMP4) {
> +            ret = hls_init_file_refresh(s, vs);
> +            if (ret < 0) {
> +                av_freep(&old_filename);
> +                return ret;
> +            }
> +        }
> +
>          if (hls->flags & HLS_SINGLE_FILE) {
>              vs->number++;
>              vs->start_pos += vs->size;
> @@ -2510,7 +2540,8 @@ static void hls_free_variant_streams(struct HLSContext *hls)
>          }
>  
>          avformat_free_context(vs->avf);
> -
> +        if (!hls->refresh_init_file)

This check makes no sense to me at all: If refresh_init_file is set, the
init_buffer will never be freed at all. This check needs to be reversed.
(Or actually dropped altogether: It is safer to simply free it
unconditionally.)

(Did you test this code with Valgrind or asan? It can find errors like
this one.)

> +            av_freep(&vs->init_buffer);
>          hls_free_segments(vs->segments);
>          hls_free_segments(vs->old_segments);
>          av_freep(&vs->m3u8_name);
> @@ -3010,6 +3041,7 @@ static const AVOption options[] = {
>      {"mpegts",   "make segment file to mpegts files in m3u8", 0, AV_OPT_TYPE_CONST, {.i64 = SEGMENT_TYPE_MPEGTS }, 0, UINT_MAX,   E, "segment_type"},
>      {"fmp4",   "make segment file to fragment mp4 files in m3u8", 0, AV_OPT_TYPE_CONST, {.i64 = SEGMENT_TYPE_FMP4 }, 0, UINT_MAX,   E, "segment_type"},
>      {"hls_fmp4_init_filename", "set fragment mp4 file init filename", OFFSET(fmp4_init_filename),   AV_OPT_TYPE_STRING, {.str = "init.mp4"},            0,       0,         E},
> +    {"hls_fmp4_init_refresh", "refresh fragment mp4 init file after refresh m3u8 every time", OFFSET(refresh_init_file), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, 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 and playlist to temporary file and rename when complete", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_TEMP_FILE }, 0, UINT_MAX,   E, "flags"},
>

> 2020年3月31日 上午10:06，Andreas Rheinhardt <andreas.rheinhardt@gmail.com> 写道：
> 
> Steven Liu:
>> add option for refresh init file after m3u8 refresh everytime.
>> 
>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>> ---
>> doc/muxers.texi      |  3 +++
>> libavformat/hlsenc.c | 44 ++++++++++++++++++++++++++++++++++++++------
>> 2 files changed, 41 insertions(+), 6 deletions(-)
>> 
>> diff --git a/doc/muxers.texi b/doc/muxers.texi
>> index d304181671..fc38f40bc9 100644
>> --- a/doc/muxers.texi
>> +++ b/doc/muxers.texi
>> @@ -836,6 +836,9 @@ fmp4 files may be used in HLS version 7 and above.
>> @item hls_fmp4_init_filename @var{filename}
>> Set filename to the fragment files header file, default filename is @file{init.mp4}.
>> 
>> +@item hls_fmp4_init_refresh @var{filename}
>> +Refresh init file after m3u8 file refresh every time, default is @var{0}.
> 
> The first "refresh" means "resend", doesn't it? Using "refresh" suggests
> that the init file will be resent with slightly different content each
> time it is sent; yet this doesn't seem to be the case.
I cannot sure resend or rewrite, but your suggestion maybe right.
> 
>> +
>> When @code{var_stream_map} is set with two or more variant streams, the
>> @var{filename} pattern must contain the string "%v", this string specifies
>> the position of variant stream index in the generated init file names.
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index a3c59432c5..6bc77ec17e 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -119,6 +119,7 @@ typedef struct VariantStream {
>>     int packets_written;
>>     int init_range_length;
>>     uint8_t *temp_buffer;
>> +    uint8_t *init_buffer;
>> 
>>     AVFormatContext *avf;
>>     AVFormatContext *vtt_avf;
>> @@ -192,6 +193,7 @@ typedef struct HLSContext {
>>     char *segment_filename;
>>     char *fmp4_init_filename;
>>     int segment_type;
>> +    int refresh_init_file;  ///< refresh init file into disk after refresh m3u8
>> 
>>     int use_localtime;      ///< flag to expand filename with localtime
>>     int use_localtime_mkdir;///< flag to mkdir dirname in timebased filename
>> @@ -982,7 +984,8 @@ static int sls_flag_check_duration_size(HLSContext *hls, VariantStream *vs)
>>     return ret;
>> }
>> 
>> -static void sls_flag_file_rename(HLSContext *hls, VariantStream *vs, char *old_filename) {
>> +static void sls_flag_file_rename(HLSContext *hls, VariantStream *vs, char *old_filename)
>> +{
> 
> Stray change.
> 
>>     if ((hls->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE | HLS_SECOND_LEVEL_SEGMENT_DURATION)) &&
>>         strlen(vs->current_segment_final_filename_fmt)) {
>>         ff_rename(old_filename, vs->avf->url, hls);
>> @@ -2234,6 +2237,25 @@ static int hls_write_header(AVFormatContext *s)
>>     return ret;
>> }
>> 
>> +static int hls_init_file_refresh(AVFormatContext *s, VariantStream *vs)
>> +{
>> +    HLSContext *hls = s->priv_data;
>> +    AVDictionary *options = NULL;
>> +    int ret = 0;
>> +
>> +    set_http_options(s, &options, hls);
>> +    ret = hlsenc_io_open(s, &vs->out, hls->fmp4_init_filename, &options);
> 
> You should free options here unconditionally before checking for errors.
Ok,
> 
>> +    if (ret < 0) {
>> +        av_dict_free(&options);
>> +        return ret;
>> +    }
>> +    avio_write(vs->out, vs->init_buffer, vs->init_range_length);
>> +    hlsenc_io_close(s, &vs->out, hls->fmp4_init_filename);
>> +    av_dict_free(&options);
>> +
>> +    return ret;
>> +}
>> +
>> static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>> {
>>     HLSContext *hls = s->priv_data;
>> @@ -2246,7 +2268,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>>     int range_length = 0;
>>     const char *proto = NULL;
>>     int use_temp_file = 0;
>> -    uint8_t *buffer = NULL;
>>     VariantStream *vs = NULL;
>>     char *old_filename = NULL;
>> 
>> @@ -2332,9 +2353,10 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>>         avio_flush(oc->pb);
>>         if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>>             if (!vs->init_range_length) {
>> -                range_length = avio_close_dyn_buf(oc->pb, &buffer);
>> -                avio_write(vs->out, buffer, range_length);
>> -                av_freep(&buffer);
>> +                range_length = avio_close_dyn_buf(oc->pb, &vs->init_buffer);
> 
> You are relying on this code to be executed only once to not produce
> memleaks in case refresh_init_file is set, aren't you?
Yes,

> If so, is it
> actually certain that range_length can't be zero in some weird corner case?
Can you give me one example to describe this comment? The oc->pb is not null here.
> 
>> +                avio_write(vs->out, vs->init_buffer, range_length);
>> +                if (!hls->refresh_init_file)
>> +                    av_freep(&vs->init_buffer);
>>                 vs->init_range_length = range_length;
>>                 avio_open_dyn_buf(&oc->pb);
>>                 vs->packets_written = 0;
>> @@ -2446,6 +2468,14 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>>             }
>>         }
>> 
>> +        if (hls->refresh_init_file && hls->segment_type == SEGMENT_TYPE_FMP4) {
>> +            ret = hls_init_file_refresh(s, vs);
>> +            if (ret < 0) {
>> +                av_freep(&old_filename);
>> +                return ret;
>> +            }
>> +        }
>> +
>>         if (hls->flags & HLS_SINGLE_FILE) {
>>             vs->number++;
>>             vs->start_pos += vs->size;
>> @@ -2510,7 +2540,8 @@ static void hls_free_variant_streams(struct HLSContext *hls)
>>         }
>> 
>>         avformat_free_context(vs->avf);
>> -
>> +        if (!hls->refresh_init_file)
> 
> This check makes no sense to me at all: If refresh_init_file is set, the
> init_buffer will never be freed at all. This check needs to be reversed.
> (Or actually dropped altogether: It is safer to simply free it
> unconditionally.)
Ok,
> 
> (Did you test this code with Valgrind or asan? It can find errors like
> this one.)

> 
>> +            av_freep(&vs->init_buffer);
>>         hls_free_segments(vs->segments);
>>         hls_free_segments(vs->old_segments);
>>         av_freep(&vs->m3u8_name);
>> @@ -3010,6 +3041,7 @@ static const AVOption options[] = {
>>     {"mpegts",   "make segment file to mpegts files in m3u8", 0, AV_OPT_TYPE_CONST, {.i64 = SEGMENT_TYPE_MPEGTS }, 0, UINT_MAX,   E, "segment_type"},
>>     {"fmp4",   "make segment file to fragment mp4 files in m3u8", 0, AV_OPT_TYPE_CONST, {.i64 = SEGMENT_TYPE_FMP4 }, 0, UINT_MAX,   E, "segment_type"},
>>     {"hls_fmp4_init_filename", "set fragment mp4 file init filename", OFFSET(fmp4_init_filename),   AV_OPT_TYPE_STRING, {.str = "init.mp4"},          0,       0,         E},
>> +    {"hls_fmp4_init_refresh", "refresh fragment mp4 init file after refresh m3u8 every time", OFFSET(refresh_init_file), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, 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 and playlist to temporary file and rename when complete", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_TEMP_FILE }, 0, UINT_MAX,   E, "flags"},
>> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks

Steven Liu