[PATCH] avformat/dashenc: use AVCodecContext timebase when computing missing bitrate
--- 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; if (bitrate >= 0) os->bit_rate = bitrate; } -- 2.25.2
On 5/27/20 8:37 PM, Przemysław Sobala wrote:
--- 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; if (bitrate >= 0) os->bit_rate = bitrate; } LGTM. Thanks.
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 -- Anton Khirnov
On Mon, Jun 1, 2020 at 10:06 AM Anton Khirnov <anton@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. -- Przemysław Sobala
On 6/1/20 5:24 PM, Przemysław Sobala wrote:
On Mon, Jun 1, 2020 at 10:06 AM Anton Khirnov <anton@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
-- Przemysław Sobala _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://urldefense.proofpoint.com/v2/url?u=https-3A__ffmpeg.org_mailman_listinfo_ffmpeg-2Ddevel&d=DwIGaQ&c=96ZbZZcaMF4w0F4jpN6LZg&r=xOoesbz-6ff1GPXp5Lg4jf1ZG99yp4a1qhxVn_YOwRU&m=d5_uQoLoKY0i_-ZFJQGxnQcD0feA0He0ST4ojg4---4&s=dstVnsDMFsrakFvnEaO5zT4dmLntnO4RBfr7YTHwI_4&e=
To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Mon, Jun 1, 2020 at 3:30 PM Jeyapal, Karthick <kjeyapal@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@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. -- Regards Przemysław Sobala
On Tue, Jun 2, 2020 at 10:19 AM Przemysław Sobala < przemyslaw.sobala@gmail.com> wrote:
On Mon, Jun 1, 2020 at 3:30 PM Jeyapal, Karthick <kjeyapal@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@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? -- pozdrawiam Przemysław Sobala
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@gmail.com> wrote:
On Mon, Jun 1, 2020 at 3:30 PM Jeyapal, Karthick <kjeyapal@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@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 :)
-- pozdrawiam Przemysław Sobala _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://urldefense.proofpoint.com/v2/url?u=https-3A__ffmpeg.org_mailman_listinfo_ffmpeg-2Ddevel&d=DwIGaQ&c=96ZbZZcaMF4w0F4jpN6LZg&r=xOoesbz-6ff1GPXp5Lg4jf1ZG99yp4a1qhxVn_YOwRU&m=q7hq8J8ltTngyTvXBudpQq8HN4Ure-eQJZiCG85-TgI&s=pM0LIY83Tgww_6eGlIuJe7lzdcfZcKoqyH2rMDz396E&e=
To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Thu, Jun 4, 2020 at 11:24 AM Jeyapal, Karthick <kjeyapal@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@gmail.com> wrote:
On Mon, Jun 1, 2020 at 3:30 PM Jeyapal, Karthick <kjeyapal@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@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
On Thu, Jun 4, 2020 at 3:13 PM Przemysław Sobala < przemyslaw.sobala@gmail.com> wrote:
On Thu, Jun 4, 2020 at 11:24 AM Jeyapal, Karthick <kjeyapal@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@gmail.com> wrote:
On Mon, Jun 1, 2020 at 3:30 PM Jeyapal, Karthick <kjeyapal@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@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.
Again, any comments? -- regards Przemysław Sobala
participants (3)
-
Anton Khirnov -
Jeyapal, Karthick -
Przemysław Sobala