diff --git a/packager/hls/base/media_playlist.cc b/packager/hls/base/media_playlist.cc index 215baa6bf9..40a3983b5d 100644 --- a/packager/hls/base/media_playlist.cc +++ b/packager/hls/base/media_playlist.cc @@ -340,8 +340,7 @@ MediaPlaylist::MediaPlaylist(const HlsParams& hls_params, : hls_params_(hls_params), file_name_(file_name), name_(name), - group_id_(group_id), - bandwidth_estimator_(hls_params_.target_segment_duration) {} + group_id_(group_id) {} MediaPlaylist::~MediaPlaylist() {} diff --git a/packager/hls/public/hls_params.h b/packager/hls/public/hls_params.h index f35618db4b..65f31bffb3 100644 --- a/packager/hls/public/hls_params.h +++ b/packager/hls/public/hls_params.h @@ -52,11 +52,7 @@ struct HlsParams { /// i.e. subtitles or close-captions. std::string default_text_language; /// This is the target segment duration requested by the user. The actual - /// segment duration may be different to the target segment duration. - /// This parameter is included here to for bandwidth estimator to exclude the - /// segments with duration less than half of the target duration from - /// bandwidth estimation. See - /// https://github.com/google/shaka-packager/issues/498 for details. It will + /// segment duration may be different to the target segment duration. It will /// be populated from segment duration specified in ChunkingParams if not /// specified. double target_segment_duration = 0; diff --git a/packager/mpd/base/bandwidth_estimator.cc b/packager/mpd/base/bandwidth_estimator.cc index 85fcb653db..8ecb3e5ba0 100644 --- a/packager/mpd/base/bandwidth_estimator.cc +++ b/packager/mpd/base/bandwidth_estimator.cc @@ -8,13 +8,13 @@ #include #include +#include #include "packager/base/logging.h" namespace shaka { -BandwidthEstimator::BandwidthEstimator(double target_segment_duration) - : target_segment_duration_(target_segment_duration) {} +BandwidthEstimator::BandwidthEstimator() = default; BandwidthEstimator::~BandwidthEstimator() = default; @@ -28,12 +28,66 @@ void BandwidthEstimator::AddBlock(uint64_t size_in_bytes, double duration) { const int kBitsInByte = 8; const uint64_t size_in_bits = size_in_bytes * kBitsInByte; total_size_in_bits_ += size_in_bits; - total_duration_ += duration; - const uint64_t bitrate = static_cast(ceil(size_in_bits / duration)); + const size_t kTargetDurationThreshold = 10; + if (initial_blocks_.size() < kTargetDurationThreshold) { + initial_blocks_.push_back({size_in_bits, duration}); + return; + } - if (duration < 0.5 * target_segment_duration_) { + if (target_block_duration_ == 0) { + // Use the average duration as the target block duration. It will be used + // to filter small blocks from bandwidth calculation. + target_block_duration_ = GetAverageBlockDuration(); + for (const Block& block : initial_blocks_) { + max_bitrate_ = + std::max(max_bitrate_, GetBitrate(block, target_block_duration_)); + } + return; + } + max_bitrate_ = std::max(max_bitrate_, GetBitrate({size_in_bits, duration}, + target_block_duration_)); +} + +uint64_t BandwidthEstimator::Estimate() const { + if (total_duration_ == 0) + return 0; + return static_cast(ceil(total_size_in_bits_ / total_duration_)); +} + +uint64_t BandwidthEstimator::Max() const { + if (max_bitrate_ != 0) + return max_bitrate_; + + // We don't have the |target_block_duration_| yet. Calculate a target + // duration from the current available blocks. + DCHECK(target_block_duration_ == 0); + const double target_block_duration = GetAverageBlockDuration(); + + // Calculate maximum bitrate with the target duration calculated above. + uint64_t max_bitrate = 0; + for (const Block& block : initial_blocks_) { + max_bitrate = + std::max(max_bitrate, GetBitrate(block, target_block_duration)); + } + return max_bitrate; +} + +double BandwidthEstimator::GetAverageBlockDuration() const { + if (initial_blocks_.empty()) + return 0.0; + const double sum = + std::accumulate(initial_blocks_.begin(), initial_blocks_.end(), 0.0, + [](double duration, const Block& block) { + return duration + block.duration; + }); + return sum / initial_blocks_.size(); +} + +uint64_t BandwidthEstimator::GetBitrate(const Block& block, + double target_block_duration) const { + if (block.duration < 0.5 * target_block_duration) { // https://tools.ietf.org/html/rfc8216#section-4.1 // The peak segment bit rate of a Media Playlist is the largest bit rate of // any continuous set of segments whose total duration is between 0.5 @@ -44,29 +98,16 @@ void BandwidthEstimator::AddBlock(uint64_t size_in_bytes, double duration) { // segment duration, it will never be larger than the actual target // duration. // - // TODO(kqyang): Review if we can just stick to the user provided segment - // duration as our target duration. - // // We also apply the same exclusion to the bandwidth computation for DASH as // the bitrate for the short segment is not a good signal for peak // bandwidth. // See https://github.com/google/shaka-packager/issues/498 for details. - VLOG(1) << "Exclude short segment (duration " << duration - << ", target_duration " << target_segment_duration_ + VLOG(1) << "Exclude short segment (duration " << block.duration + << ", target_duration " << target_block_duration << ") in peak bandwidth computation."; - return; + return 0.0; } - max_bitrate_ = std::max(bitrate, max_bitrate_); -} - -uint64_t BandwidthEstimator::Estimate() const { - if (total_duration_ == 0) - return 0; - return static_cast(ceil(total_size_in_bits_ / total_duration_)); -} - -uint64_t BandwidthEstimator::Max() const { - return max_bitrate_; + return static_cast(ceil(block.size_in_bits / block.duration)); } } // namespace shaka diff --git a/packager/mpd/base/bandwidth_estimator.h b/packager/mpd/base/bandwidth_estimator.h index 5ba06dcbb9..8e1ddc3ef7 100644 --- a/packager/mpd/base/bandwidth_estimator.h +++ b/packager/mpd/base/bandwidth_estimator.h @@ -9,11 +9,13 @@ #include +#include + namespace shaka { class BandwidthEstimator { public: - explicit BandwidthEstimator(double target_segment_duration); + BandwidthEstimator(); ~BandwidthEstimator(); /// @param size is the size of the block in bytes. Should be positive. @@ -28,14 +30,29 @@ class BandwidthEstimator { /// @return The max bandwidth, in bits per second, of the number of blocks /// specified in the constructor. The value is rounded up to the - /// nearest integer. + /// nearest integer. Note that small blocks w.r.t. + /// |target_block_duration| are not counted. uint64_t Max() const; private: BandwidthEstimator(const BandwidthEstimator&) = delete; BandwidthEstimator& operator=(const BandwidthEstimator&) = delete; - const double target_segment_duration_ = 0; + struct Block { + uint64_t size_in_bits; + double duration; + }; + // Return the average block duration of the blocks in |initial_blocks_|. + double GetAverageBlockDuration() const; + // Return the bitrate of the block. Note that a bitrate of 0 is returned if + // the block duration is less than 50% of target block duration. + uint64_t GetBitrate(const Block& block, double target_block_duration) const; + + std::vector initial_blocks_; + // Target block duration will be estimated from the average duration of the + // initial blocks. + double target_block_duration_ = 0; + uint64_t total_size_in_bits_ = 0; double total_duration_ = 0; uint64_t max_bitrate_ = 0; diff --git a/packager/mpd/base/bandwidth_estimator_unittest.cc b/packager/mpd/base/bandwidth_estimator_unittest.cc index a56a6ebb8c..3b3f529d0a 100644 --- a/packager/mpd/base/bandwidth_estimator_unittest.cc +++ b/packager/mpd/base/bandwidth_estimator_unittest.cc @@ -16,7 +16,7 @@ const uint64_t kBitsInByte = 8; TEST(BandwidthEstimatorTest, AllBlocks) { const double kDuration = 1.0; - BandwidthEstimator be(kDuration); + BandwidthEstimator be; const uint64_t kNumBlocksToAdd = 100; uint64_t total_bytes = 0; for (uint64_t i = 1; i <= kNumBlocksToAdd; ++i) { @@ -31,9 +31,9 @@ TEST(BandwidthEstimatorTest, AllBlocks) { EXPECT_EQ(kMax, be.Max()); } -TEST(BandwidthEstimatorTest, ExcludeShortSegments) { +TEST(BandwidthEstimatorTest, ExcludeShortBlocks) { const double kDuration = 1.0; - BandwidthEstimator be(kDuration); + BandwidthEstimator be; // Add 4 blocks with duration 0.1, 0.8, 1.8 and 0.2 respectively. The first // and the last blocks are excluded as they are too short. @@ -46,4 +46,21 @@ TEST(BandwidthEstimatorTest, ExcludeShortSegments) { EXPECT_EQ(kExpectedMax, be.Max()); } +TEST(BandwidthEstimatorTest, ExcludeShortBlocksMore) { + const double kDuration = 1.0; + BandwidthEstimator be; + + for (int k = 0; k < 100; k++) { + // Add 4 blocks with duration 0.1, 0.8, 1.8 and 0.2 respectively. The first + // and the last blocks are excluded as they are too short. + be.AddBlock(1, 0.1 * kDuration); + be.AddBlock(1, 0.8 * kDuration); + be.AddBlock(1, 1.8 * kDuration); + be.AddBlock(1, 0.2 * kDuration); + } + + const uint64_t kExpectedMax = 1 / 0.8 * kBitsInByte; + EXPECT_EQ(kExpectedMax, be.Max()); +} + } // namespace shaka diff --git a/packager/mpd/base/representation.cc b/packager/mpd/base/representation.cc index 820ccea49e..ebaa4e05fb 100644 --- a/packager/mpd/base/representation.cc +++ b/packager/mpd/base/representation.cc @@ -88,7 +88,6 @@ Representation::Representation( std::unique_ptr state_change_listener) : media_info_(media_info), id_(id), - bandwidth_estimator_(mpd_options.mpd_params.target_segment_duration), mpd_options_(mpd_options), state_change_listener_(std::move(state_change_listener)), allow_approximate_segment_timeline_( diff --git a/packager/mpd/base/representation_unittest.cc b/packager/mpd/base/representation_unittest.cc index ae8a5552b6..c9a9f97684 100644 --- a/packager/mpd/base/representation_unittest.cc +++ b/packager/mpd/base/representation_unittest.cc @@ -443,9 +443,6 @@ std::string GetDefaultMediaInfo() { class SegmentTemplateTest : public RepresentationTest { public: - SegmentTemplateTest() - : bandwidth_estimator_(kTargetSegmentDurationInSeconds) {} - void SetUp() override { mpd_options_.mpd_type = MpdType::kDynamic; representation_ = diff --git a/packager/mpd/public/mpd_params.h b/packager/mpd/public/mpd_params.h index 2b249cc412..7a194f8d0a 100644 --- a/packager/mpd/public/mpd_params.h +++ b/packager/mpd/public/mpd_params.h @@ -77,10 +77,8 @@ struct MpdParams { /// This is the target segment duration requested by the user. The actual /// segment duration may be different to the target segment duration. /// This parameter is included here to calculate the approximate - /// SegmentTimeline if it is enabled. It is also used by the bandwidth - /// estimator to exclude the segments with duration less than half of the - /// target duration from bandwidth estimation. It will be populated from - /// segment duration specified in ChunkingParams if not specified. + /// SegmentTimeline if it is enabled. It will be populated from segment + /// duration specified in ChunkingParams if not specified. double target_segment_duration = 0; };