Sets sending status for active RtpRtcp modules.
When a simulcast stream is enabled or disabled, we want this state
change to be reflected properly in the RtpRtcp modules. Each video send
stream can contain multiple rtp_rtcp_modules pertaining to different
simulcast streams. These modules are currently all turned on/off when
the send stream is started and stopped. This change allows for
individual modules to be turned on/off. This means if a module stops
sending it will send a bye message, so the receiving side will not
expect more frames to be sent when the stream is inactive and the
encoder is no longer encoding/sending images.
Bug: webrtc:8653
Change-Id: Ib6d00240f627b4ff1714646e847026f24c7c3aa4
Reviewed-on: https://webrtc-review.googlesource.com/42841
Commit-Queue: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21880}diff --git a/video/payload_router.cc b/video/payload_router.cc
index 1d16fbd..4ecb620 100644
--- a/video/payload_router.cc
+++ b/video/payload_router.cc
@@ -151,11 +151,22 @@
rtc::CritScope lock(&crit_);
if (active_ == active)
return;
- active_ = active;
+ const std::vector<bool> active_modules(rtp_modules_.size(), active);
+ SetActiveModules(active_modules);
+}
- for (auto& module : rtp_modules_) {
- module->SetSendingStatus(active_);
- module->SetSendingMediaStatus(active_);
+void PayloadRouter::SetActiveModules(const std::vector<bool> active_modules) {
+ rtc::CritScope lock(&crit_);
+ RTC_DCHECK_EQ(rtp_modules_.size(), active_modules.size());
+ active_ = false;
+ for (size_t i = 0; i < active_modules.size(); ++i) {
+ if (active_modules[i]) {
+ active_ = true;
+ }
+ // Sends a kRtcpByeCode when going from true to false.
+ rtp_modules_[i]->SetSendingStatus(active_modules[i]);
+ // If set to false this module won't send media.
+ rtp_modules_[i]->SetSendingMediaStatus(active_modules[i]);
}
}
@@ -217,6 +228,10 @@
params_[stream_index].Set(&rtp_video_header);
}
uint32_t frame_id;
+ if (!rtp_modules_[stream_index]->Sending()) {
+ // The payload router could be active but this module isn't sending.
+ return Result(Result::ERROR_SEND_FAILED);
+ }
bool send_result = rtp_modules_[stream_index]->SendOutgoingData(
encoded_image._frameType, payload_type_, encoded_image._timeStamp,
encoded_image.capture_time_ms_, encoded_image._buffer,
diff --git a/video/payload_router.h b/video/payload_router.h
index 13b6cae..68779ef 100644
--- a/video/payload_router.h
+++ b/video/payload_router.h
@@ -40,6 +40,9 @@
// PayloadRouter will only route packets if being active, all packets will be
// dropped otherwise.
void SetActive(bool active);
+ // Sets the sending status of the rtp modules and appropriately sets the
+ // payload router to active if any rtp modules are active.
+ void SetActiveModules(const std::vector<bool> active_modules);
bool IsActive();
std::map<uint32_t, RtpPayloadState> GetRtpPayloadStates() const;
diff --git a/video/payload_router_unittest.cc b/video/payload_router_unittest.cc
index a512582..b193da8 100644
--- a/video/payload_router_unittest.cc
+++ b/video/payload_router_unittest.cc
@@ -70,6 +70,7 @@
encoded_image._length, nullptr, _, _))
.Times(1)
.WillOnce(Return(true));
+ EXPECT_CALL(rtp, Sending()).WillOnce(Return(true));
EXPECT_EQ(
EncodedImageCallback::Result::OK,
payload_router.OnEncodedImage(encoded_image, nullptr, nullptr).error);
@@ -91,12 +92,13 @@
encoded_image._length, nullptr, _, _))
.Times(1)
.WillOnce(Return(true));
+ EXPECT_CALL(rtp, Sending()).WillOnce(Return(true));
EXPECT_EQ(
EncodedImageCallback::Result::OK,
payload_router.OnEncodedImage(encoded_image, nullptr, nullptr).error);
}
-TEST(PayloadRouterTest, SendSimulcast) {
+TEST(PayloadRouterTest, SendSimulcastSetActive) {
NiceMock<MockRtpRtcp> rtp_1;
NiceMock<MockRtpRtcp> rtp_2;
std::vector<RtpRtcp*> modules = {&rtp_1, &rtp_2};
@@ -117,6 +119,7 @@
codec_info_1.codecSpecific.VP8.simulcastIdx = 0;
payload_router.SetActive(true);
+ EXPECT_CALL(rtp_1, Sending()).WillOnce(Return(true));
EXPECT_CALL(rtp_1, SendOutgoingData(encoded_image._frameType, kPayloadType,
encoded_image._timeStamp,
encoded_image.capture_time_ms_, &payload,
@@ -133,6 +136,7 @@
codec_info_2.codecType = kVideoCodecVP8;
codec_info_2.codecSpecific.VP8.simulcastIdx = 1;
+ EXPECT_CALL(rtp_2, Sending()).WillOnce(Return(true));
EXPECT_CALL(rtp_2, SendOutgoingData(encoded_image._frameType, kPayloadType,
encoded_image._timeStamp,
encoded_image.capture_time_ms_, &payload,
@@ -159,6 +163,65 @@
.error);
}
+// Tests how setting individual rtp modules to active affects the overall
+// behavior of the payload router. First sets one module to active and checks
+// that outgoing data can be sent on this module, and checks that no data can be
+// sent if both modules are inactive.
+TEST(PayloadRouterTest, SendSimulcastSetActiveModules) {
+ NiceMock<MockRtpRtcp> rtp_1;
+ NiceMock<MockRtpRtcp> rtp_2;
+ std::vector<RtpRtcp*> modules = {&rtp_1, &rtp_2};
+
+ uint8_t payload = 'a';
+ EncodedImage encoded_image;
+ encoded_image._timeStamp = 1;
+ encoded_image.capture_time_ms_ = 2;
+ encoded_image._frameType = kVideoFrameKey;
+ encoded_image._buffer = &payload;
+ encoded_image._length = 1;
+ PayloadRouter payload_router(modules, {kSsrc1, kSsrc2}, kPayloadType, {});
+ CodecSpecificInfo codec_info_1;
+ memset(&codec_info_1, 0, sizeof(CodecSpecificInfo));
+ codec_info_1.codecType = kVideoCodecVP8;
+ codec_info_1.codecSpecific.VP8.simulcastIdx = 0;
+ CodecSpecificInfo codec_info_2;
+ memset(&codec_info_2, 0, sizeof(CodecSpecificInfo));
+ codec_info_2.codecType = kVideoCodecVP8;
+ codec_info_2.codecSpecific.VP8.simulcastIdx = 1;
+
+ // Only setting one stream to active will still set the payload router to
+ // active and allow sending data on the active stream.
+ std::vector<bool> active_modules({true, false});
+ payload_router.SetActiveModules(active_modules);
+
+ EXPECT_CALL(rtp_1, Sending()).WillOnce(Return(true));
+ EXPECT_CALL(rtp_1, SendOutgoingData(encoded_image._frameType, kPayloadType,
+ encoded_image._timeStamp,
+ encoded_image.capture_time_ms_, &payload,
+ encoded_image._length, nullptr, _, _))
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_EQ(EncodedImageCallback::Result::OK,
+ payload_router.OnEncodedImage(encoded_image, &codec_info_1, nullptr)
+ .error);
+
+ // Setting both streams to inactive will turn the payload router to inactive.
+ active_modules = {false, false};
+ payload_router.SetActiveModules(active_modules);
+ // An incoming encoded image will not ask the module to send outgoing data
+ // because the payload router is inactive.
+ EXPECT_CALL(rtp_1, SendOutgoingData(_, _, _, _, _, _, _, _, _)).Times(0);
+ EXPECT_CALL(rtp_1, Sending()).Times(0);
+ EXPECT_CALL(rtp_2, SendOutgoingData(_, _, _, _, _, _, _, _, _)).Times(0);
+ EXPECT_CALL(rtp_2, Sending()).Times(0);
+ EXPECT_NE(EncodedImageCallback::Result::OK,
+ payload_router.OnEncodedImage(encoded_image, &codec_info_1, nullptr)
+ .error);
+ EXPECT_NE(EncodedImageCallback::Result::OK,
+ payload_router.OnEncodedImage(encoded_image, &codec_info_2, nullptr)
+ .error);
+}
+
TEST(PayloadRouterTest, SimulcastTargetBitrate) {
NiceMock<MockRtpRtcp> rtp_1;
NiceMock<MockRtpRtcp> rtp_2;
@@ -262,6 +325,7 @@
codec_info.codecSpecific.VP8.layerSync = true;
codec_info.codecSpecific.VP8.nonReference = true;
+ EXPECT_CALL(rtp2, Sending()).WillOnce(Return(true));
EXPECT_CALL(rtp2, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _))
.WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused,
Unused, const RTPVideoHeader* header, Unused) {
@@ -296,6 +360,7 @@
codec_info.codecSpecific.H264.packetization_mode =
H264PacketizationMode::SingleNalUnit;
+ EXPECT_CALL(rtp1, Sending()).WillOnce(Return(true));
EXPECT_CALL(rtp1, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _))
.WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused,
Unused, const RTPVideoHeader* header, Unused) {
@@ -389,6 +454,7 @@
EXPECT_EQ(kPictureId, header->codecHeader.VP8.pictureId);
return true;
}));
+ EXPECT_CALL(rtp, Sending()).WillOnce(Return(true));
EXPECT_EQ(EncodedImageCallback::Result::OK,
router.OnEncodedImage(image_, &codec_info_, nullptr).error);
@@ -408,6 +474,7 @@
PayloadRouter router(modules, {kSsrc1, kSsrc2}, kPayloadType, states);
router.SetActive(true);
+ // Modules are sending for this test.
// OnEncodedImage, simulcastIdx: 0.
codec_info_.codecType = kVideoCodecVP8;
codec_info_.codecSpecific.VP8.pictureId = kPictureId;
@@ -420,6 +487,7 @@
EXPECT_EQ(kInitialPictureId1, header->codecHeader.VP8.pictureId);
return true;
}));
+ EXPECT_CALL(rtp1, Sending()).WillOnce(Return(true));
EXPECT_EQ(EncodedImageCallback::Result::OK,
router.OnEncodedImage(image_, &codec_info_, nullptr).error);
@@ -435,6 +503,7 @@
EXPECT_EQ(kInitialPictureId2, header->codecHeader.VP8.pictureId);
return true;
}));
+ EXPECT_CALL(rtp2, Sending()).WillOnce(Return(true));
EXPECT_EQ(EncodedImageCallback::Result::OK,
router.OnEncodedImage(image_, &codec_info_, nullptr).error);
@@ -465,6 +534,7 @@
EXPECT_EQ(kMaxTwoBytePictureId, header->codecHeader.VP8.pictureId);
return true;
}));
+ EXPECT_CALL(rtp, Sending()).WillOnce(Return(true));
EXPECT_EQ(EncodedImageCallback::Result::OK,
router.OnEncodedImage(image_, &codec_info_, nullptr).error);
@@ -491,6 +561,7 @@
EXPECT_EQ(kNoPictureId, header->codecHeader.VP8.pictureId);
return true;
}));
+ EXPECT_CALL(rtp, Sending()).WillOnce(Return(true));
EXPECT_EQ(EncodedImageCallback::Result::OK,
router.OnEncodedImage(image_, &codec_info_, nullptr).error);
@@ -512,6 +583,7 @@
EXPECT_EQ(kPictureId, header->codecHeader.VP9.picture_id);
return true;
}));
+ EXPECT_CALL(rtp, Sending()).WillOnce(Return(true));
EXPECT_EQ(EncodedImageCallback::Result::OK,
router.OnEncodedImage(image_, &codec_info_, nullptr).error);
diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index 800cca2..d5785d8 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -280,6 +280,7 @@
void SignalNetworkState(NetworkState state);
bool DeliverRtcp(const uint8_t* packet, size_t length);
+ void UpdateActiveSimulcastLayers(const std::vector<bool> active_layers);
void Start();
void Stop();
@@ -332,6 +333,12 @@
// Implements VideoBitrateAllocationObserver.
void OnBitrateAllocationUpdated(const BitrateAllocation& allocation) override;
+ // Starts monitoring and sends a keyframe.
+ void StartupVideoSendStream();
+ // Removes the bitrate observer, stops monitoring and notifies the video
+ // encoder of the bitrate update.
+ void StopVideoSendStream();
+
void ConfigureProtection();
void ConfigureSsrcs();
void SignalEncoderTimedOut();
@@ -622,6 +629,19 @@
RTC_DCHECK(!send_stream_);
}
+void VideoSendStream::UpdateActiveSimulcastLayers(
+ const std::vector<bool> active_layers) {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
+ RTC_LOG(LS_INFO) << "VideoSendStream::UpdateActiveSimulcastLayers";
+ VideoSendStreamImpl* send_stream = send_stream_.get();
+ worker_queue_->PostTask([this, send_stream, active_layers] {
+ send_stream->UpdateActiveSimulcastLayers(active_layers);
+ thread_sync_event_.Set();
+ });
+
+ thread_sync_event_.Wait(rtc::Event::kForever);
+}
+
void VideoSendStream::Start() {
RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_LOG(LS_INFO) << "VideoSendStream::Start";
@@ -912,6 +932,22 @@
return true;
}
+void VideoSendStreamImpl::UpdateActiveSimulcastLayers(
+ const std::vector<bool> active_layers) {
+ RTC_DCHECK_RUN_ON(worker_queue_);
+ RTC_DCHECK_EQ(rtp_rtcp_modules_.size(), active_layers.size());
+ RTC_LOG(LS_INFO) << "VideoSendStream::UpdateActiveSimulcastLayers";
+ bool previously_active = payload_router_.IsActive();
+ payload_router_.SetActiveModules(active_layers);
+ if (!payload_router_.IsActive() && previously_active) {
+ // Payload router switched from active to inactive.
+ StopVideoSendStream();
+ } else if (payload_router_.IsActive() && !previously_active) {
+ // Payload router switched from inactive to active.
+ StartupVideoSendStream();
+ }
+}
+
void VideoSendStreamImpl::Start() {
RTC_DCHECK_RUN_ON(worker_queue_);
RTC_LOG(LS_INFO) << "VideoSendStream::Start";
@@ -919,12 +955,15 @@
return;
TRACE_EVENT_INSTANT0("webrtc", "VideoSendStream::Start");
payload_router_.SetActive(true);
+ StartupVideoSendStream();
+}
+void VideoSendStreamImpl::StartupVideoSendStream() {
+ RTC_DCHECK_RUN_ON(worker_queue_);
bitrate_allocator_->AddObserver(
this, encoder_min_bitrate_bps_, encoder_max_bitrate_bps_,
max_padding_bitrate_, !config_->suspend_below_min_bitrate,
config_->track_id, encoder_bitrate_priority_);
-
// Start monitoring encoder activity.
{
rtc::CritScope lock(&encoder_activity_crit_sect_);
@@ -945,6 +984,10 @@
return;
TRACE_EVENT_INSTANT0("webrtc", "VideoSendStream::Stop");
payload_router_.SetActive(false);
+ StopVideoSendStream();
+}
+
+void VideoSendStreamImpl::StopVideoSendStream() {
bitrate_allocator_->RemoveObserver(this);
{
rtc::CritScope lock(&encoder_activity_crit_sect_);
diff --git a/video/video_send_stream.h b/video/video_send_stream.h
index d62c302..9212370 100644
--- a/video/video_send_stream.h
+++ b/video/video_send_stream.h
@@ -72,6 +72,8 @@
bool DeliverRtcp(const uint8_t* packet, size_t length);
// webrtc::VideoSendStream implementation.
+ void UpdateActiveSimulcastLayers(
+ const std::vector<bool> active_layers) override;
void Start() override;
void Stop() override;
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index 3512b85..26ac918 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -1971,62 +1971,62 @@
DestroyStreams();
}
+class StartStopBitrateObserver : public test::FakeEncoder {
+ public:
+ StartStopBitrateObserver()
+ : FakeEncoder(Clock::GetRealTimeClock()),
+ encoder_init_(false, false),
+ bitrate_changed_(false, false) {}
+ int32_t InitEncode(const VideoCodec* config,
+ int32_t number_of_cores,
+ size_t max_payload_size) override {
+ rtc::CritScope lock(&crit_);
+ encoder_init_.Set();
+ return FakeEncoder::InitEncode(config, number_of_cores, max_payload_size);
+ }
+
+ int32_t SetRateAllocation(const BitrateAllocation& bitrate,
+ uint32_t framerate) override {
+ rtc::CritScope lock(&crit_);
+ bitrate_kbps_ = bitrate.get_sum_kbps();
+ bitrate_changed_.Set();
+ return FakeEncoder::SetRateAllocation(bitrate, framerate);
+ }
+
+ bool WaitForEncoderInit() {
+ return encoder_init_.Wait(VideoSendStreamTest::kDefaultTimeoutMs);
+ }
+
+ bool WaitBitrateChanged(bool non_zero) {
+ do {
+ rtc::Optional<int> bitrate_kbps;
+ {
+ rtc::CritScope lock(&crit_);
+ bitrate_kbps = bitrate_kbps_;
+ }
+ if (!bitrate_kbps)
+ continue;
+
+ if ((non_zero && *bitrate_kbps > 0) ||
+ (!non_zero && *bitrate_kbps == 0)) {
+ return true;
+ }
+ } while (bitrate_changed_.Wait(VideoSendStreamTest::kDefaultTimeoutMs));
+ return false;
+ }
+
+ private:
+ rtc::CriticalSection crit_;
+ rtc::Event encoder_init_;
+ rtc::Event bitrate_changed_;
+ rtc::Optional<int> bitrate_kbps_ RTC_GUARDED_BY(crit_);
+};
+
// This test that if the encoder use an internal source, VideoEncoder::SetRates
// will be called with zero bitrate during initialization and that
// VideoSendStream::Stop also triggers VideoEncoder::SetRates Start to be called
// with zero bitrate.
TEST_F(VideoSendStreamTest, VideoSendStreamStopSetEncoderRateToZero) {
- class StartStopBitrateObserver : public test::FakeEncoder {
- public:
- StartStopBitrateObserver()
- : FakeEncoder(Clock::GetRealTimeClock()),
- encoder_init_(false, false),
- bitrate_changed_(false, false) {}
- int32_t InitEncode(const VideoCodec* config,
- int32_t number_of_cores,
- size_t max_payload_size) override {
- rtc::CritScope lock(&crit_);
- encoder_init_.Set();
- return FakeEncoder::InitEncode(config, number_of_cores, max_payload_size);
- }
-
- int32_t SetRateAllocation(const BitrateAllocation& bitrate,
- uint32_t framerate) override {
- rtc::CritScope lock(&crit_);
- bitrate_kbps_ = bitrate.get_sum_kbps();
- bitrate_changed_.Set();
- return FakeEncoder::SetRateAllocation(bitrate, framerate);
- }
-
- bool WaitForEncoderInit() {
- return encoder_init_.Wait(VideoSendStreamTest::kDefaultTimeoutMs);
- }
-
- bool WaitBitrateChanged(bool non_zero) {
- do {
- rtc::Optional<int> bitrate_kbps;
- {
- rtc::CritScope lock(&crit_);
- bitrate_kbps = bitrate_kbps_;
- }
- if (!bitrate_kbps)
- continue;
-
- if ((non_zero && *bitrate_kbps > 0) ||
- (!non_zero && *bitrate_kbps == 0)) {
- return true;
- }
- } while (bitrate_changed_.Wait(VideoSendStreamTest::kDefaultTimeoutMs));
- return false;
- }
-
- private:
- rtc::CriticalSection crit_;
- rtc::Event encoder_init_;
- rtc::Event bitrate_changed_;
- rtc::Optional<int> bitrate_kbps_ RTC_GUARDED_BY(crit_);
- };
-
test::NullTransport transport;
StartStopBitrateObserver encoder;
@@ -2065,6 +2065,64 @@
});
}
+// Tests that when the encoder uses an internal source, the VideoEncoder will
+// be updated with a new bitrate when turning the VideoSendStream on/off with
+// VideoSendStream::UpdateActiveSimulcastLayers, and when the VideoStreamEncoder
+// is reconfigured with new active layers.
+TEST_F(VideoSendStreamTest, VideoSendStreamUpdateActiveSimulcastLayers) {
+ test::NullTransport transport;
+ StartStopBitrateObserver encoder;
+
+ task_queue_.SendTask([this, &transport, &encoder]() {
+ CreateSenderCall(Call::Config(event_log_.get()));
+ // Create two simulcast streams.
+ CreateSendConfig(2, 0, 0, &transport);
+
+ sender_call_->SignalChannelNetworkState(MediaType::VIDEO, kNetworkUp);
+
+ video_send_config_.encoder_settings.encoder = &encoder;
+ video_send_config_.encoder_settings.internal_source = true;
+ video_send_config_.encoder_settings.payload_name = "VP8";
+
+ CreateVideoStreams();
+ });
+
+ EXPECT_TRUE(encoder.WaitForEncoderInit());
+
+ // When we turn on the simulcast layers it will update the BitrateAllocator,
+ // which in turn updates the VideoEncoder's bitrate.
+ task_queue_.SendTask([this]() {
+ video_send_stream_->UpdateActiveSimulcastLayers({true, true});
+ });
+ EXPECT_TRUE(encoder.WaitBitrateChanged(true));
+
+ video_encoder_config_.simulcast_layers[0].active = true;
+ video_encoder_config_.simulcast_layers[1].active = false;
+ task_queue_.SendTask([this]() {
+ video_send_stream_->ReconfigureVideoEncoder(video_encoder_config_.Copy());
+ });
+ // TODO(bugs.webrtc.org/8807): Currently we require a hard reconfiguration to
+ // update the VideoBitrateAllocator and BitrateAllocator of which layers are
+ // active. Once the change is made for a "soft" reconfiguration we can remove
+ // the expecation for an encoder init. We can also test that bitrate changes
+ // when just updating individual active layers, which should change the
+ // bitrate set to the video encoder.
+ EXPECT_TRUE(encoder.WaitForEncoderInit());
+ EXPECT_TRUE(encoder.WaitBitrateChanged(true));
+
+ // Turning off both simulcast layers should trigger a bitrate change of 0.
+ video_encoder_config_.simulcast_layers[0].active = false;
+ video_encoder_config_.simulcast_layers[1].active = false;
+ task_queue_.SendTask([this]() {
+ video_send_stream_->UpdateActiveSimulcastLayers({false, false});
+ });
+ EXPECT_TRUE(encoder.WaitBitrateChanged(false));
+
+ task_queue_.SendTask([this]() {
+ DestroyStreams();
+ DestroyCalls();
+ });
+}
TEST_F(VideoSendStreamTest, CapturesTextureAndVideoFrames) {
class FrameObserver : public rtc::VideoSinkInterface<VideoFrame> {
public:
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 5203885..778f740 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -564,6 +564,10 @@
}
}
+// TODO(bugs.webrtc.org/8807): Currently this always does a hard
+// reconfiguration, but this isn't always necessary. Add in logic to only update
+// the VideoBitrateAllocator and call OnEncoderConfigurationChanged with a
+// "soft" reconfiguration.
void VideoStreamEncoder::ReconfigureEncoder() {
RTC_DCHECK_RUN_ON(&encoder_queue_);
RTC_DCHECK(pending_encoder_reconfiguration_);
