Remove UpdateSsrcs from EncoderStateFeedback.
Removes ability to modify set SSRCs from EncoderStateFeedback after
construction.
BUG=webrtc:1695
R=sprang@webrtc.org
TBR=stefan@webrtc.org
Review URL: https://codereview.webrtc.org/1241123002
Cr-Commit-Position: refs/heads/master@{#9603}
diff --git a/webrtc/video/call.cc b/webrtc/video/call.cc
index a739325..adfaa53 100644
--- a/webrtc/video/call.cc
+++ b/webrtc/video/call.cc
@@ -180,8 +180,9 @@
// TODO(pbos): Remove base channel when CreateReceiveChannel no longer
// requires one.
- CHECK(channel_group_->CreateSendChannel(
- base_channel_id_, 0, &transport_adapter_, num_cpu_cores_, 1, true));
+ CHECK(channel_group_->CreateSendChannel(base_channel_id_, 0,
+ &transport_adapter_, num_cpu_cores_,
+ std::vector<uint32_t>(), true));
if (config.overuse_callback) {
overuse_observer_proxy_.reset(
diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc
index c0d7349..7cd1f9e 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -20,7 +20,6 @@
#include "webrtc/system_wrappers/interface/logging.h"
#include "webrtc/system_wrappers/interface/trace_event.h"
#include "webrtc/video/video_capture_input.h"
-#include "webrtc/video_engine/encoder_state_feedback.h"
#include "webrtc/video_engine/vie_channel.h"
#include "webrtc/video_engine/vie_channel_group.h"
#include "webrtc/video_engine/vie_defines.h"
@@ -121,8 +120,8 @@
stats_proxy_(Clock::GetRealTimeClock(), config) {
DCHECK(!config_.rtp.ssrcs.empty());
CHECK(channel_group->CreateSendChannel(channel_id_, 0, &transport_adapter_,
- num_cpu_cores,
- config_.rtp.ssrcs.size(), true));
+ num_cpu_cores, config_.rtp.ssrcs,
+ true));
vie_channel_ = channel_group_->GetChannel(channel_id_);
vie_encoder_ = channel_group_->GetEncoder(channel_id_);
@@ -495,9 +494,6 @@
// Update used SSRCs.
vie_encoder_->SetSsrcs(used_ssrcs);
- EncoderStateFeedback* encoder_state_feedback =
- channel_group_->GetEncoderStateFeedback();
- encoder_state_feedback->UpdateSsrcs(used_ssrcs, vie_encoder_);
// Update the protection mode, we might be switching NACK/FEC.
vie_encoder_->UpdateProtectionMethod(vie_encoder_->nack_enabled(),
diff --git a/webrtc/video_engine/encoder_state_feedback.cc b/webrtc/video_engine/encoder_state_feedback.cc
index 4d9db58..55a0c43 100644
--- a/webrtc/video_engine/encoder_state_feedback.cc
+++ b/webrtc/video_engine/encoder_state_feedback.cc
@@ -54,34 +54,16 @@
assert(encoders_.empty());
}
-void EncoderStateFeedback::UpdateSsrcs(const std::vector<uint32_t>& ssrcs,
- ViEEncoder* encoder) {
+void EncoderStateFeedback::AddEncoder(const std::vector<uint32_t>& ssrcs,
+ ViEEncoder* encoder) {
+ DCHECK(!ssrcs.empty());
CriticalSectionScoped lock(crit_.get());
- SsrcEncoderMap::iterator it = encoders_.begin();
- while (it != encoders_.end()) {
- if (it->second == encoder) {
- encoders_.erase(it++);
- } else {
- ++it;
- }
- }
for (uint32_t ssrc : ssrcs) {
DCHECK(encoders_.find(ssrc) == encoders_.end());
encoders_[ssrc] = encoder;
}
}
-bool EncoderStateFeedback::AddEncoder(uint32_t ssrc, ViEEncoder* encoder) {
- CriticalSectionScoped lock(crit_.get());
- if (encoders_.find(ssrc) != encoders_.end()) {
- // Two encoders must not have the same ssrc.
- return false;
- }
-
- encoders_[ssrc] = encoder;
- return true;
-}
-
void EncoderStateFeedback::RemoveEncoder(const ViEEncoder* encoder) {
CriticalSectionScoped lock(crit_.get());
SsrcEncoderMap::iterator it = encoders_.begin();
diff --git a/webrtc/video_engine/encoder_state_feedback.h b/webrtc/video_engine/encoder_state_feedback.h
index d74c6c2..40fbf94 100644
--- a/webrtc/video_engine/encoder_state_feedback.h
+++ b/webrtc/video_engine/encoder_state_feedback.h
@@ -35,11 +35,8 @@
EncoderStateFeedback();
~EncoderStateFeedback();
- // Update SSRCs for an encoder.
- void UpdateSsrcs(const std::vector<uint32_t>& ssrc, ViEEncoder* encoder);
-
- // Adds an encoder to receive feedback for a unique ssrc.
- bool AddEncoder(uint32_t ssrc, ViEEncoder* encoder);
+ // Adds an encoder to receive feedback for a set of SSRCs.
+ void AddEncoder(const std::vector<uint32_t>& ssrc, ViEEncoder* encoder);
// Removes a registered ViEEncoder.
void RemoveEncoder(const ViEEncoder* encoder);
diff --git a/webrtc/video_engine/encoder_state_feedback_unittest.cc b/webrtc/video_engine/encoder_state_feedback_unittest.cc
index eab33d4..07e1990 100644
--- a/webrtc/video_engine/encoder_state_feedback_unittest.cc
+++ b/webrtc/video_engine/encoder_state_feedback_unittest.cc
@@ -68,7 +68,7 @@
TEST_F(VieKeyRequestTest, CreateAndTriggerRequests) {
const int ssrc = 1234;
MockVieEncoder encoder(process_thread_.get(), &pacer_);
- EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc, &encoder));
+ encoder_state_feedback_->AddEncoder(std::vector<uint32_t>(1, ssrc), &encoder);
EXPECT_CALL(encoder, OnReceivedIntraFrameRequest(ssrc))
.Times(1);
@@ -97,8 +97,10 @@
const int ssrc_2 = 5678;
MockVieEncoder encoder_1(process_thread_.get(), &pacer_);
MockVieEncoder encoder_2(process_thread_.get(), &pacer_);
- EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc_1, &encoder_1));
- EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc_2, &encoder_2));
+ encoder_state_feedback_->AddEncoder(std::vector<uint32_t>(1, ssrc_1),
+ &encoder_1);
+ encoder_state_feedback_->AddEncoder(std::vector<uint32_t>(1, ssrc_2),
+ &encoder_2);
EXPECT_CALL(encoder_1, OnReceivedIntraFrameRequest(ssrc_1))
.Times(1);
@@ -139,12 +141,4 @@
encoder_state_feedback_->RemoveEncoder(&encoder_2);
}
-TEST_F(VieKeyRequestTest, AddTwiceError) {
- const int ssrc = 1234;
- MockVieEncoder encoder(process_thread_.get(), &pacer_);
- EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc, &encoder));
- EXPECT_FALSE(encoder_state_feedback_->AddEncoder(ssrc, &encoder));
- encoder_state_feedback_->RemoveEncoder(&encoder);
-}
-
} // namespace webrtc
diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc
index 055e1c3..cb041bd 100644
--- a/webrtc/video_engine/vie_channel_group.cc
+++ b/webrtc/video_engine/vie_channel_group.cc
@@ -193,9 +193,11 @@
int engine_id,
Transport* transport,
int number_of_cores,
- size_t max_rtp_streams,
+ const std::vector<uint32_t>& ssrcs,
bool disable_default_encoder) {
- DCHECK_GT(max_rtp_streams, 0u);
+ // TODO(pbos): Remove checks for empty ssrcs and add this check when there's
+ // no base channel.
+ // DCHECK(!ssrcs.empty());
rtc::scoped_ptr<ViEEncoder> vie_encoder(
new ViEEncoder(channel_id, number_of_cores, *config_.get(),
*process_thread_, pacer_.get(), bitrate_allocator_.get(),
@@ -205,8 +207,8 @@
}
ViEEncoder* encoder = vie_encoder.get();
if (!CreateChannel(channel_id, engine_id, transport, number_of_cores,
- vie_encoder.release(), max_rtp_streams, true,
- disable_default_encoder)) {
+ vie_encoder.release(), ssrcs.empty() ? 1 : ssrcs.size(),
+ true, disable_default_encoder)) {
return false;
}
ViEChannel* channel = channel_map_[channel_id];
@@ -214,14 +216,11 @@
encoder->StartThreadsAndSetSharedMembers(channel->send_payload_router(),
channel->vcm_protection_callback());
- // Register the ViEEncoder to get key frame requests for this channel.
- unsigned int ssrc = 0;
- int stream_idx = 0;
- channel->GetLocalSSRC(stream_idx, &ssrc);
- encoder_state_feedback_->AddEncoder(ssrc, encoder);
- std::vector<uint32_t> ssrcs;
- ssrcs.push_back(ssrc);
- encoder->SetSsrcs(ssrcs);
+ if (!ssrcs.empty()) {
+ encoder_state_feedback_->AddEncoder(ssrcs, encoder);
+ std::vector<uint32_t> first_ssrc(1, ssrcs[0]);
+ encoder->SetSsrcs(first_ssrc);
+ }
return true;
}
diff --git a/webrtc/video_engine/vie_channel_group.h b/webrtc/video_engine/vie_channel_group.h
index 37064c8..ff001f9 100644
--- a/webrtc/video_engine/vie_channel_group.h
+++ b/webrtc/video_engine/vie_channel_group.h
@@ -46,7 +46,7 @@
int engine_id,
Transport* transport,
int number_of_cores,
- size_t max_rtp_streams,
+ const std::vector<uint32_t>& ssrcs,
bool disable_default_encoder);
bool CreateReceiveChannel(int channel_id,
int engine_id,
