Periodically update channel parameters and send TargetBitrate message.
Currently, parameters are periodically updated, but the TargetBitrate
message is only sent if a new bitrate is set. It should be sent
periodically to indicate the signaled bitrate is valid and to prevent
stale values due to loss of RTCP packets.
BUG=webrtc:6897
Review-Url: https://codereview.webrtc.org/2616393003
Cr-Commit-Position: refs/heads/master@{#16096}
diff --git a/webrtc/modules/video_coding/video_sender.cc b/webrtc/modules/video_coding/video_sender.cc
index 33d8439..0b54d13 100644
--- a/webrtc/modules/video_coding/video_sender.cc
+++ b/webrtc/modules/video_coding/video_sender.cc
@@ -63,12 +63,6 @@
send_stats_callback_->SendStatistics(bitRate, frameRate);
}
}
- {
- rtc::CritScope cs(¶ms_crit_);
- // Force an encoder parameters update, so that incoming frame rate is
- // updated even if bandwidth hasn't changed.
- encoder_params_.input_frame_rate = _mediaOpt.InputFrameRate();
- }
}
int64_t VideoSender::TimeUntilNextProcess() {
diff --git a/webrtc/modules/video_coding/video_sender_unittest.cc b/webrtc/modules/video_coding/video_sender_unittest.cc
index f0246ca..50e17bf 100644
--- a/webrtc/modules/video_coding/video_sender_unittest.cc
+++ b/webrtc/modules/video_coding/video_sender_unittest.cc
@@ -317,22 +317,6 @@
AddFrame();
clock_.AdvanceTimeMilliseconds(kFrameIntervalMs);
- // Add enough frames so that input frame rate will be updated.
- const int kFramesToSend =
- (VCMProcessTimer::kDefaultProcessIntervalMs / kFrameIntervalMs) + 1;
- for (int i = 0; i < kFramesToSend; ++i) {
- AddFrame();
- clock_.AdvanceTimeMilliseconds(kFrameIntervalMs);
- }
-
- EXPECT_CALL(encoder_,
- SetRateAllocation(new_rate_allocation, kActualFrameRate))
- .Times(1)
- .WillOnce(Return(0));
-
- sender_->Process();
- AddFrame();
-
// Expect no call to encoder_.SetRates if the new bitrate is zero.
EXPECT_CALL(encoder_, SetRateAllocation(_, _)).Times(0);
sender_->SetChannelParameters(0, 0, 200, rate_allocator_.get(), nullptr);
@@ -376,23 +360,6 @@
rate_allocator_.get(), nullptr);
}
-TEST_F(TestVideoSenderWithMockEncoder, EncoderFramerateUpdatedViaProcess) {
- sender_->SetChannelParameters(settings_.startBitrate * 1000, 0, 200,
- rate_allocator_.get(), nullptr);
- const int64_t kRateStatsWindowMs = 2000;
- const uint32_t kInputFps = 20;
- int64_t start_time = clock_.TimeInMilliseconds();
- while (clock_.TimeInMilliseconds() < start_time + kRateStatsWindowMs) {
- AddFrame();
- clock_.AdvanceTimeMilliseconds(1000 / kInputFps);
- }
- EXPECT_CALL(encoder_, SetRateAllocation(_, kInputFps))
- .Times(1)
- .WillOnce(Return(0));
- sender_->Process();
- AddFrame();
-}
-
TEST_F(TestVideoSenderWithMockEncoder,
NoRedundantSetChannelParameterOrSetRatesCalls) {
const uint8_t kLossRate = 4;
@@ -412,13 +379,7 @@
AddFrame();
clock_.AdvanceTimeMilliseconds(1000 / kInputFps);
}
- // After process, input framerate should be updated but not ChannelParameters
- // as they are the same as before.
- EXPECT_CALL(encoder_, SetRateAllocation(_, kInputFps))
- .Times(1)
- .WillOnce(Return(0));
- sender_->Process();
- AddFrame();
+
// Call to SetChannelParameters with changed bitrate should call encoder
// SetRates but not encoder SetChannelParameters (that are unchanged).
uint32_t new_bitrate_bps = 2 * settings_.startBitrate * 1000;
diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc
index bead39d..eaf693a 100644
--- a/webrtc/video/vie_encoder.cc
+++ b/webrtc/video/vie_encoder.cc
@@ -545,9 +545,16 @@
<< ", texture=" << last_frame_info_->is_texture;
}
+ int64_t now_ms = clock_->TimeInMilliseconds();
if (pending_encoder_reconfiguration_) {
ReconfigureEncoder();
+ } else if (!last_parameters_update_ms_ ||
+ now_ms - *last_parameters_update_ms_ >=
+ vcm::VCMProcessTimer::kDefaultProcessIntervalMs) {
+ video_sender_.UpdateChannelParemeters(rate_allocator_.get(),
+ bitrate_observer_);
}
+ last_parameters_update_ms_.emplace(now_ms);
if (EncoderPaused()) {
TraceFrameDropStart();
diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h
index ec8ee38..4a2ca1f 100644
--- a/webrtc/video/vie_encoder.h
+++ b/webrtc/video/vie_encoder.h
@@ -235,6 +235,7 @@
int dropped_frame_count_ ACCESS_ON(&encoder_queue_);
VideoBitrateAllocationObserver* bitrate_observer_ ACCESS_ON(&encoder_queue_);
+ rtc::Optional<int64_t> last_parameters_update_ms_ ACCESS_ON(&encoder_queue_);
// All public methods are proxied to |encoder_queue_|. It must must be
// destroyed first to make sure no tasks are run that use other members.
diff --git a/webrtc/video/vie_encoder_unittest.cc b/webrtc/video/vie_encoder_unittest.cc
index a50f7aa..dae172d 100644
--- a/webrtc/video/vie_encoder_unittest.cc
+++ b/webrtc/video/vie_encoder_unittest.cc
@@ -13,10 +13,13 @@
#include "webrtc/api/video/i420_buffer.h"
#include "webrtc/base/logging.h"
+#include "webrtc/modules/video_coding/utility/default_video_bitrate_allocator.h"
#include "webrtc/system_wrappers/include/metrics_default.h"
+#include "webrtc/system_wrappers/include/sleep.h"
#include "webrtc/test/encoder_settings.h"
#include "webrtc/test/fake_encoder.h"
#include "webrtc/test/frame_generator.h"
+#include "webrtc/test/gmock.h"
#include "webrtc/test/gtest.h"
#include "webrtc/video/send_statistics_proxy.h"
#include "webrtc/video/vie_encoder.h"
@@ -35,6 +38,8 @@
using DegredationPreference = VideoSendStream::DegradationPreference;
using ScaleReason = ScalingObserverInterface::ScaleReason;
+using ::testing::_;
+using ::testing::Return;
namespace {
const size_t kMaxPayloadLength = 1440;
@@ -163,19 +168,20 @@
ConfigureEncoder(std::move(video_encoder_config), nack_enabled);
}
- VideoFrame CreateFrame(int64_t ntp_ts, rtc::Event* destruction_event) const {
+ VideoFrame CreateFrame(int64_t ntp_time_ms,
+ rtc::Event* destruction_event) const {
VideoFrame frame(new rtc::RefCountedObject<TestBuffer>(
destruction_event, codec_width_, codec_height_),
99, 99, kVideoRotation_0);
- frame.set_ntp_time_ms(ntp_ts);
+ frame.set_ntp_time_ms(ntp_time_ms);
return frame;
}
- VideoFrame CreateFrame(int64_t ntp_ts, int width, int height) const {
+ VideoFrame CreateFrame(int64_t ntp_time_ms, int width, int height) const {
VideoFrame frame(
new rtc::RefCountedObject<TestBuffer>(nullptr, width, height), 99, 99,
kVideoRotation_0);
- frame.set_ntp_time_ms(ntp_ts);
+ frame.set_ntp_time_ms(ntp_time_ms);
return frame;
}
@@ -979,4 +985,48 @@
1, metrics::NumEvents("WebRTC.Video.CpuLimitedResolutionInPercent", 50));
}
+TEST_F(ViEEncoderTest, CallsBitrateObserver) {
+ class MockBitrateObserver : public VideoBitrateAllocationObserver {
+ public:
+ MOCK_METHOD1(OnBitrateAllocationUpdated, void(const BitrateAllocation&));
+ } bitrate_observer;
+ vie_encoder_->SetBitrateObserver(&bitrate_observer);
+
+ const int kDefaultFps = 30;
+ const BitrateAllocation expected_bitrate =
+ DefaultVideoBitrateAllocator(fake_encoder_.codec_config())
+ .GetAllocation(kTargetBitrateBps, kDefaultFps);
+
+ // First called on bitrate updated, then again on first frame.
+ EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate))
+ .Times(2);
+ vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
+
+ const int64_t kStartTimeMs = 1;
+ video_source_.IncomingCapturedFrame(
+ CreateFrame(kStartTimeMs, codec_width_, codec_height_));
+ sink_.WaitForEncodedFrame(kStartTimeMs);
+
+ // Not called on second frame.
+ EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate))
+ .Times(0);
+ video_source_.IncomingCapturedFrame(
+ CreateFrame(kStartTimeMs + 1, codec_width_, codec_height_));
+ sink_.WaitForEncodedFrame(kStartTimeMs + 1);
+
+ // Called after a process interval.
+ const int64_t kProcessIntervalMs =
+ vcm::VCMProcessTimer::kDefaultProcessIntervalMs;
+ // TODO(sprang): ViEEncoder should die and/or get injectable clock.
+ // Sleep for one processing interval plus one frame to avoid flakiness.
+ SleepMs(kProcessIntervalMs + 1000 / kDefaultFps);
+ EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate))
+ .Times(1);
+ video_source_.IncomingCapturedFrame(CreateFrame(
+ kStartTimeMs + kProcessIntervalMs, codec_width_, codec_height_));
+ sink_.WaitForEncodedFrame(kStartTimeMs + kProcessIntervalMs);
+
+ vie_encoder_->Stop();
+}
+
} // namespace webrtc
