[FFmpeg-devel] [PATCH] avformat/dashenc: use AVCodecContext timebase when computing missing bitrate

[Przemysław Sobala]

On Thu, Jun 4, 2020 at 11:24 AM Jeyapal, Karthick <kjeyapal at akamai.com>
wrote:

>
> On 6/4/20 1:29 PM, Przemysław Sobala wrote:
> > On Tue, Jun 2, 2020 at 10:19 AM Przemysław Sobala <
> > przemyslaw.sobala at gmail.com> wrote:
> >
> >> On Mon, Jun 1, 2020 at 3:30 PM Jeyapal, Karthick <kjeyapal at akamai.com>
> >> wrote:
> >>
> >>>
> >>> On 6/1/20 5:24 PM, Przemysław Sobala wrote:
> >>>> On Mon, Jun 1, 2020 at 10:06 AM Anton Khirnov <anton at khirnov.net>
> >>> wrote:
> >>>>
> >>>>> Quoting Przemysław Sobala (2020-05-27 17:07:22)
> >>>>>> ---
> >>>>>>  libavformat/dashenc.c | 2 +-
> >>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> >>>>>> index 0cf0df50ef..00a37b175d 100644
> >>>>>> --- a/libavformat/dashenc.c
> >>>>>> +++ b/libavformat/dashenc.c
> >>>>>> @@ -1959,7 +1959,7 @@ static int dash_flush(AVFormatContext *s, int
> >>>>> final, int stream)
> >>>>>>
> >>>>>>          if (!os->bit_rate) {
> >>>>>>              // calculate average bitrate of first segment
> >>>>>> -            int64_t bitrate = (int64_t) range_length * 8 *
> >>> AV_TIME_BASE
> >>>>> / duration;
> >>>>>> +            int64_t bitrate = (int64_t) range_length * 8 *
> >>>>> (c->use_timeline ? os->ctx->streams[0]->time_base.den :
> AV_TIME_BASE) /
> >>>>> duration;
> >>>>>
> >>>>> That does not look like an AVCodecContext
> >>>>>
> >>>>
> >>>> Of course not. time_base is AVStream's field. I don't know why I wrote
> >>>> AVCodecContext... Please amend that commit message if possible.
> >>> Amended and Pushed!
> >>>
> >>> Thanks,
> >>> Karthick
> >>>
> >>>
> >> Thanks.
> >> What do you think about computing an average bitrate for all segments,
> not
> >> only the first one (in case of a static - not dynamic - DASH manifest),
> if
> >> one would not want to specify bitrate while encoding using x264 CRF rate
> >> control? I could prepare such a patch that, if bitrate is not specified,
> >> it'd be computed at the end, for static manifest, for all segments.
> It'd be
> >> more accurate comparing to the first segment's bitrate.
> >>
> >
> > Any comments about that?
> Any patch that fixes/improves the current behavior is always welcome :)
>

I was thinking about something like (diff against release/4.2 branch):


$ git diff origin/release/4.2 -- libavformat/dashenc.c
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index f0e45da89ad..60250b6d33a 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -88,6 +88,7 @@ typedef struct OutputStream {
     int64_t first_pts, start_pts, max_pts;
     int64_t last_dts, last_pts;
     int bit_rate;
+    double average_bit_rate;
     SegmentType segment_type;  /* segment type selected for this
particular stream */
     const char *format_name;
     const char *extension_name;
@@ -775,6 +776,9 @@ static int write_adaptation_set(AVFormatContext *s,
AVIOContext *out, int as_ind
         if (os->bit_rate > 0)
             snprintf(bandwidth_str, sizeof(bandwidth_str), "
bandwidth=\"%d\"",
                      os->bit_rate);
+        else if (final)
+            snprintf(bandwidth_str, sizeof(bandwidth_str), "
bandwidth=\"%d\"",
+                     (int) os->average_bit_rate);

         if (as->media_type == AVMEDIA_TYPE_VIDEO) {
             AVStream *st = s->streams[i];
@@ -1618,12 +1622,10 @@ static int dash_flush(AVFormatContext *s, int
final, int stream)
         os->total_pkt_size = 0;

         if (!os->bit_rate) {
-            // calculate average bitrate of first segment
-            int64_t bitrate = (int64_t) range_length * 8 * AV_TIME_BASE /
av_rescale_q(os->max_pts - os->start_pts,
-
            st->time_base,
-
            AV_TIME_BASE_Q);
-            if (bitrate >= 0)
-                os->bit_rate = bitrate;
+            // calculate average bitrate
+            int64_t duration = av_rescale_q(os->max_pts - os->start_pts,
st->time_base, AV_TIME_BASE_Q);
+            int64_t segment_bitrate = (int64_t) range_length * 8 *
AV_TIME_BASE / duration;
+            os->average_bit_rate = (os->average_bit_rate *
(os->segment_index - 1) + segment_bitrate) / os->segment_index;
         }
         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);


This patch allows to compute the average bitrate for all segments and write
it inside a static manifest.
Unfortunately, for dynamic manifest, the bitrate, if not specified by
client, is not computed at all, and I have no idea how to handle that.
Previously, bitrate for dynamic manifest, if not specified by client, was
computed only for the first segment. I could introduce another field
"first_segment_bitrate" in struct OutputStream and fill the manifest with
that value, as long as it's not a static one, to preserve that behavior.

--
regards
Przemysław Sobala