[FFmpeg-devel] [PATCH 2/2] avformat/dashenc: Calculate average bitrate for adaptation sets in static manifest

[Jeyapal, Karthick]

Thanks for sending the revised patch. One more comment inline.

On 6/17/20 4:23 PM, Przemysław Sobala wrote:
> If stream's bitrate is not specified:
> - for static manifest: an average bitrate will be calculated and used,
> - for dynamic manifest: first segment's bitrate will be calculated and used, as before,
> for bandwidth setting in adaptation sets.
> ---
>  libavformat/dashenc.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index 05deb5c1b5..20edc9779c 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -115,6 +115,7 @@ typedef struct OutputStream {
>      int64_t last_dts, last_pts;
>      int last_flags;
>      int bit_rate;
> +    int first_segment_bit_rate;
>      SegmentType segment_type;  /* segment type selected for this particular stream */
>      const char *format_name;
>      const char *extension_name;
> @@ -840,8 +841,12 @@ static int write_adaptation_set(AVFormatContext *s, AVIOContext *out, int as_ind
>              continue;
>  
>          if (os->bit_rate > 0)
> -            snprintf(bandwidth_str, sizeof(bandwidth_str), " bandwidth=\"%d\"",
> -                     os->bit_rate);
> +            snprintf(bandwidth_str, sizeof(bandwidth_str), " bandwidth=\"%d\"", os->bit_rate);
> +        else if (final) {
> +            int average_bit_rate = os->pos * 8 * AV_TIME_BASE / c->total_duration;
> +            snprintf(bandwidth_str, sizeof(bandwidth_str), " bandwidth=\"%d\"", average_bit_rate);
> +        } else if (os->first_segment_bit_rate > 0)
> +            snprintf(bandwidth_str, sizeof(bandwidth_str), " bandwidth=\"%d\"", os->first_segment_bit_rate);
>  
>          if (as->media_type == AVMEDIA_TYPE_VIDEO) {
>              avio_printf(out, "\t\t\t<Representation id=\"%d\" mimeType=\"video/%s\" codecs=\"%s\"%s width=\"%d\" height=\"%d\"",
> @@ -1305,7 +1310,13 @@ static int write_manifest(AVFormatContext *s, int final)
>              OutputStream *os = &c->streams[i];
>              char *agroup = NULL;
>              char *codec_str_ptr = NULL;
> -            int stream_bitrate = st->codecpar->bit_rate + os->muxer_overhead;
> +            int stream_bitrate = os->muxer_overhead;
> +            if (os->bit_rate > 0)
> +                stream_bitrate += os->bit_rate;
> +            else if (final)
> +                stream_bitrate += os->pos * 8 * AV_TIME_BASE / c->total_duration;
> +            else if (os->first_segment_bit_rate > 0)
> +                stream_bitrate += os->first_segment_bit_rate;
>              if (st->codecpar->codec_type != AVMEDIA_TYPE_VIDEO)
>                  continue;
>              if (os->segment_type != SEGMENT_TYPE_MP4)
> @@ -1959,10 +1970,10 @@ static int dash_flush(AVFormatContext *s, int final, int stream)
>          os->total_pkt_duration = 0;
>  
>          if (!os->bit_rate) {
> -            // calculate average bitrate of first segment
> -            int64_t bitrate = (int64_t) range_length * 8 * AV_TIME_BASE / duration;
> -            if (bitrate >= 0)
> -                os->bit_rate = bitrate;
> +            // calculate average bitrate
> +            int64_t segment_bitrate = (int64_t) range_length * 8 * AV_TIME_BASE / duration;
> +            if (!os->first_segment_bit_rate && segment_bitrate)
> +                os->first_segment_bit_rate = segment_bitrate;
Can this be merged into one if condition like below.
if (!os->bit_rate && !os->first_segment_bit_rate ) 
	os->first_segment_bit_rate = (int64_t) range_length * 8 * AV_TIME_BASE / duration;
It will make the code simpler.
>          }
>          add_segment(os, os->filename, os->start_pts, os->max_pts - os->start_pts, os->pos, range_length, index_length, next_exp_index);
>          av_log(s, AV_LOG_VERBOSE, "Representation %d media segment %d written to: %s\n", i, os->segment_index, os->full_path);