[FFmpeg-devel,2/3] Adds a new hls_flag peak_segment_bw

If this flag is set, BANDWIDTH value in a master playlist entry will be set to the peak segment bandwidth. This patch also delays freeing of hls stream data, so that it's available for bandwidth calculation.

Signed-off-by: Amit Kale<amitk@hotstar.com>
---
  doc/muxers.texi      |  4 ++++
  libavformat/hlsenc.c | 29 +++++++++++++++++++++--------
  2 files changed, 25 insertions(+), 8 deletions(-)

On 9/28/18 11:33 AM, Amit Kale wrote:
> If this flag is set, BANDWIDTH value in a master playlist entry will be set to the peak segment bandwidth. This patch also delays freeing of hls stream data, so that it's available for bandwidth calculation.

This is a very useful feature. Thanks for contributing back to ffmpeg.
I think the patch and its logic is overall good. Comments for some minor issues inlined below.
>

> Signed-off-by: Amit Kale<amitk@hotstar.com>

> ---

>  doc/muxers.texi      |  4 ++++

>  libavformat/hlsenc.c | 29 +++++++++++++++++++++--------

>  2 files changed, 25 insertions(+), 8 deletions(-)

>

> Index: ffmpeg/doc/muxers.texi

> ===================================================================

> --- ffmpeg.orig/doc/muxers.texi

> +++ ffmpeg/doc/muxers.texi

> @@ -789,6 +789,10 @@ subdirectories.

>  Possible values:

>  

>  @table @samp

> +@item peak_segment_bw

I think abbreviations are not allowed/recommended in command line options of ffmpeg.
Maybe better to rename it to peak_segment_bandwidth or peak _bandwidth.
> +If this flag is set, BANDWIDTH value in a master playlist entry will be

> +set to the peak segment bandwidth.

> +

>  @item single_file

>  If this flag is set, the muxer will store all segments in a single MPEG-TS

>  file, and will use byte ranges in the playlist. HLS playlists generated with

> Index: ffmpeg/libavformat/hlsenc.c

> ===================================================================

> --- ffmpeg.orig/libavformat/hlsenc.c

> +++ ffmpeg/libavformat/hlsenc.c

> @@ -99,6 +99,7 @@ typedef enum HLSFlags {

>      HLS_TEMP_FILE = (1 << 11),

>      HLS_PERIODIC_REKEY = (1 << 12),

>      HLS_INDEPENDENT_SEGMENTS = (1 << 13),

> +    HLS_PEAK_SEGMENT_BW = (1 << 14),

Looks like you haven't used "git format-patch" to generate this patch. Due to this the spacing looks odd and it is not possible to apply this patch directly.
Could you please resend this patch generated from  "git format-patch"
>  } HLSFlags;

>  

>  typedef enum {

> @@ -1174,7 +1175,8 @@ static int64_t get_stream_bit_rate(AVStr

>  }

>  

>  static int create_master_playlist(AVFormatContext *s,

> -                                  VariantStream * const input_vs)

> +                                  VariantStream * const input_vs,

> +                                  int last)

>  {

>      HLSContext *hls = s->priv_data;

>      VariantStream *vs, *temp_vs;

> @@ -1191,13 +1193,16 @@ static int create_master_playlist(AVForm

>          for (i = 0; i < hls->nb_varstreams; i++)

>              if (!hls->var_streams[i].m3u8_created)

>                  return 0;

> -    } else {

> +    } else if (!last) {

>           /* Keep publishing the master playlist at the configured rate */

>          if (&hls->var_streams[0] != input_vs || !hls->master_publish_rate ||

>              input_vs->number % hls->master_publish_rate)

>              return 0;

>      }

>  

> +    if (hls->flags & HLS_PEAK_SEGMENT_BW && hls->version < 7)

> +        hls->version = 7;

> +

It is better to move this logic to "hls_window" function, where version setting logic is present.
Otherwise version setting logic gets cluttered.
>      set_http_options(s, &options, hls);

>  

>      ret = hlsenc_io_open(s, &hls->m3u8_out, hls->master_m3u8_url, &options);

> @@ -1298,11 +1303,21 @@ static int create_master_playlist(AVForm

>          }

>  

>          bandwidth = 0;

> -        if (vid_st)

> -            bandwidth += get_stream_bit_rate(vid_st);

> -        if (aud_st)

> -            bandwidth += get_stream_bit_rate(aud_st);

> -        bandwidth += bandwidth / 10;

> +        if (last && hls->flags & HLS_PEAK_SEGMENT_BW) {

> +            HLSSegment *hs = vs->segments;

> +            while (hs) {

> +               int64_t segment_bandwidth = hs->size * 8 / hs->duration;

> +                if (bandwidth < segment_bandwidth)

> +                    bandwidth = segment_bandwidth;

> +                hs = hs->next;

> +            }

> +        } else {

> +            if (vid_st)

> +                bandwidth += get_stream_bit_rate(vid_st);

> +            if (aud_st)

> +                bandwidth += get_stream_bit_rate(aud_st);

> +            bandwidth += bandwidth / 10;

> +        }

>  

>          ccgroup = NULL;

>          if (vid_st && vs->ccgroup) {

> @@ -1443,7 +1458,7 @@ fail:

>          ff_rename(temp_filename, vs->m3u8_name, s);

>  

>      if (ret >= 0 && hls->master_pl_name)

> -        if (create_master_playlist(s, vs) < 0)

> +        if (create_master_playlist(s, vs, last) < 0)

>              av_log(s, AV_LOG_WARNING, "Master playlist creation failed\n");

>  

>      return ret;

> @@ -2421,10 +2436,13 @@ failed:

>              av_freep(&vs->vtt_m3u8_name);

>              avformat_free_context(vtt_oc);

>          }

> +        av_free(old_filename);

>  

> +    }

> +    for (i = 0; i < hls->nb_varstreams; i++) {

> +        vs = &hls->var_streams[i];

>          hls_free_segments(vs->segments);

>          hls_free_segments(vs->old_segments);

> -        av_free(old_filename);

>          av_freep(&vs->m3u8_name);

>          av_freep(&vs->streams);

>          av_freep(&vs->agroup);

> @@ -2819,6 +2837,7 @@ static const AVOption options[] = {

>      {"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_flags",     "set flags affecting HLS playlist and media file generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E, "flags"},

> +    {"peak_segment_bw",   "sets bandwidth in master play list to peak segment bandwidth", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_PEAK_SEGMENT_BW }, 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"},

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel