Thanks for your feedback. Will soon make these changes and re-submit.
-Amit
On Fri, Sep 28, 2018 at 7:02 PM Jeyapal, Karthick <kjeya...@akamai.com>
wrote:
> Please find my comments inlined below.
> On 9/28/18 11:36 AM, Amit Kale wrote:
> > If this flag is set, AVERAGE-BANDWIDTH value will be added to a master
> playlist entry. This flag implies peak_segment_bw.
> Better to add a code like below to set peak segment bw flag
> explicitly(with comments) in hls_init function.
> if (hls->flags & HLS_AVG_BW)
> hls->flags |= HLS_PEAK_SEGMENT_BW
> >
> > Signed-off-by: Amit Kale<am...@hotstar.com>
> > ---
> > doc/muxers.texi | 4 ++++
> > libavformat/dashenc.c | 2 +-
> > libavformat/hlsenc.c | 20 ++++++++++++++++----
> > libavformat/hlsplaylist.c | 6 ++++--
> > libavformat/hlsplaylist.h | 4 ++--
> > 5 files changed, 27 insertions(+), 9 deletions(-)
> >
> > Index: ffmpeg/doc/muxers.texi
> > ===================================================================
> > --- ffmpeg.orig/doc/muxers.texi
> > +++ ffmpeg/doc/muxers.texi
> > @@ -793,6 +793,10 @@ Possible values:
> > If this flag is set, BANDWIDTH value in a master playlist entry will be
> > set to the peak segment bandwidth.
> >
> > +@item avg_bw
> Rename this to average_bandwidth
> > +If this flag is set, AVERAGE-BANDWIDTH value will be added to a master
> > +playlist entry. This flag implies peak_segment_bw.
> > +
> > @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/dashenc.c
> > ===================================================================
> > --- ffmpeg.orig/libavformat/dashenc.c
> > +++ ffmpeg/libavformat/dashenc.c
> > @@ -919,7 +919,7 @@ static int write_manifest(AVFormatContex
> > av_strlcat(codec_str, audio_codec_str,
> sizeof(codec_str));
> > }
> > get_hls_playlist_name(playlist_file, sizeof(playlist_file),
> NULL, i);
> > - ff_hls_write_stream_info(st, out, stream_bitrate,
> playlist_file, agroup,
> > + ff_hls_write_stream_info(st, out, stream_bitrate, 0,
> playlist_file, agroup,
> > codec_str, NULL);
> > }
> > avio_close(out);
> > Index: ffmpeg/libavformat/hlsenc.c
> > ===================================================================
> > --- ffmpeg.orig/libavformat/hlsenc.c
> > +++ ffmpeg/libavformat/hlsenc.c
> > @@ -100,6 +100,7 @@ typedef enum HLSFlags {
> > HLS_PERIODIC_REKEY = (1 << 12),
> > HLS_INDEPENDENT_SEGMENTS = (1 << 13),
> > HLS_PEAK_SEGMENT_BW = (1 << 14),
> > + HLS_AVG_BW = (1 << 15),
> > } HLSFlags;
> >
> > typedef enum {
> > @@ -115,6 +116,8 @@ typedef struct VariantStream {
> > AVOutputFormat *vtt_oformat;
> > AVIOContext *out;
> > int packets_written;
> > + int64_t bytes_written;
> > + double total_duration;
> These can be local variables. It can be calculated during peak bandwidth
> calculation loop.
> > int init_range_length;
> >
> > AVFormatContext *avf;
> > @@ -1183,7 +1186,7 @@ static int create_master_playlist(AVForm
> > AVStream *vid_st, *aud_st;
> > AVDictionary *options = NULL;
> > unsigned int i, j;
> > - int m3u8_name_size, ret, bandwidth;
> > + int m3u8_name_size, ret, bandwidth, avgbw;
> Could rename it to average_bandwidth for the sake of consistency.
> > char *m3u8_rel_name, *ccgroup;
> > ClosedCaptionsStream *ccs;
> >
> > @@ -1303,7 +1306,9 @@ static int create_master_playlist(AVForm
> > }
> >
> > bandwidth = 0;
> > - if (last && hls->flags & HLS_PEAK_SEGMENT_BW) {
> > + avgbw = 0;
> > + if (last && (hls->flags & HLS_PEAK_SEGMENT_BW ||
> > + hls->flags & HLS_AVG_BW)) {
> '|| hls->flags & HLS_AVG_BW' is not required if HLS_PEAK_SEGMENT_BW flag
> is set in hls_init as mentioned earlier.
> > HLSSegment *hs = vs->segments;
> > while (hs) {
> > int64_t segment_bandwidth = hs->size * 8 / hs->duration;
> vs->bytes_written and vs->total_duration can be made local variables and
> its value could be calculated here.
> Anyways we are reading hs->size and hs->duration here.
> > @@ -1311,6 +1316,8 @@ static int create_master_playlist(AVForm
> > bandwidth = segment_bandwidth;
> > hs = hs->next;
> > }
> > + if (hls->flags & HLS_AVG_BW)
> > + avgbw = (vs->bytes_written * 8) / vs->total_duration;
> > } else {
> > if (vid_st)
> > bandwidth += get_stream_bit_rate(vid_st);
> > @@ -1334,8 +1341,8 @@ static int create_master_playlist(AVForm
> > vs->ccgroup);
> > }
> >
> > - ff_hls_write_stream_info(vid_st, hls->m3u8_out, bandwidth,
> m3u8_rel_name,
> > - aud_st ? vs->agroup : NULL, vs->codec_attr, ccgroup);
> > + ff_hls_write_stream_info(vid_st, hls->m3u8_out, bandwidth,
> avgbw,
> > + m3u8_rel_name, aud_st ? vs->agroup : NULL, vs->codec_attr,
> ccgroup);
> >
> > av_freep(&m3u8_rel_name);
> > }
> > @@ -2211,6 +2218,8 @@ static int hls_write_packet(AVFormatCont
> > new_start_pos = avio_tell(vs->avf->pb);
> > if (hls->segment_type != SEGMENT_TYPE_FMP4) {
> > vs->size = new_start_pos - vs->start_pos;
> > + vs->bytes_written += vs->size;
> > + vs->total_duration += vs->duration;
> Remove from here (after made local variable)
> > } else {
> > vs->size = new_start_pos;
> > }
> > @@ -2397,6 +2406,8 @@ failed:
> > } else {
> > vs->size = avio_tell(vs->avf->pb);
> > }
> > + vs->bytes_written += vs->size;
> > + vs->total_duration += vs->duration;
> Remove from here (after made local variable)
> > if (hls->segment_type != SEGMENT_TYPE_FMP4)
> > ff_format_io_close(s, &oc->pb);
> >
> > @@ -2837,6 +2848,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"},
> > + {"avg_bandwidth", "sets AVERAGE-BANDWIDTH in master play list,
> implies peak_segment_bw flag", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_AVG_BW },
> 0, UINT_MAX, E, "flags"},
> Rename the option to average_bandwidth
> > {"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"},
> > Index: ffmpeg/libavformat/hlsplaylist.c
> > ===================================================================
> > --- ffmpeg.orig/libavformat/hlsplaylist.c
> > +++ ffmpeg/libavformat/hlsplaylist.c
> > @@ -46,8 +46,8 @@ void ff_hls_write_audio_rendition(AVIOCo
> > }
> >
> > void ff_hls_write_stream_info(AVStream *st, AVIOContext *out,
> > - int bandwidth, char *filename, char
> *agroup,
> > - char *codecs, char *ccgroup) {
> > + int bandwidth, int avgbw, char *filename,
> Could rename it to average_bandwidth for the sake of consistency.
> > + char *agroup, char *codecs, char
> *ccgroup) {
> >
> > if (!out || !filename)
> > return;
> > @@ -59,6 +59,8 @@ void ff_hls_write_stream_info(AVStream *
> > }
> >
> > avio_printf(out, "#EXT-X-STREAM-INF:BANDWIDTH=%d", bandwidth);
> > + if (avgbw != 0)
> > + avio_printf(out, ",AVERAGE-BANDWIDTH=%d", avgbw);
> > if (st && st->codecpar->width > 0 && st->codecpar->height > 0)
> > avio_printf(out, ",RESOLUTION=%dx%d", st->codecpar->width,
> > st->codecpar->height);
> > Index: ffmpeg/libavformat/hlsplaylist.h
> > ===================================================================
> > --- ffmpeg.orig/libavformat/hlsplaylist.h
> > +++ ffmpeg/libavformat/hlsplaylist.h
> > @@ -40,8 +40,8 @@ void ff_hls_write_playlist_version(AVIOC
> > void ff_hls_write_audio_rendition(AVIOContext *out, char *agroup,
> > char *filename, int name_id, int
> is_default);
> > void ff_hls_write_stream_info(AVStream *st, AVIOContext *out,
> > - int bandwidth, char *filename, char
> *agroup,
> > - char *codecs, char *ccgroup);
> > + int bandwidth, int avgbw, char *filename,
> Could rename it to average_bandwidth for the sake of consistency.
> > + char *agroup, char *codecs, char
> *ccgroup);
> > void ff_hls_write_playlist_header(AVIOContext *out, int version, int
> allowcache,
> > int target_duration, int64_t sequence,
> > uint32_t playlist_type);
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> Regards,
> Karthick
>
> _______________________________________________
> 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
