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}

webrtc/video/call.cc

@@ -180,8 +180,9 @@ Call::Call(const Call::Config& config)
 
   // TODO(pbos): Remove base channel when CreateReceiveChannel no longer
   // requires one.
-  CHECK(channel_group_->CreateSendChannel(
+  CHECK(channel_group_->CreateSendChannel(base_channel_id_, 0,
-      base_channel_id_, 0, &transport_adapter_, num_cpu_cores_, 1, true));
+                                          &transport_adapter_, num_cpu_cores_,
+                                          std::vector<uint32_t>(), true));
 
   if (config.overuse_callback) {
     overuse_observer_proxy_.reset(

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 @@ VideoSendStream::VideoSendStream(
       stats_proxy_(Clock::GetRealTimeClock(), config) {
   DCHECK(!config_.rtp.ssrcs.empty());
   CHECK(channel_group->CreateSendChannel(channel_id_, 0, &transport_adapter_,
-                                         num_cpu_cores,
+                                         num_cpu_cores, config_.rtp.ssrcs,
-                                         config_.rtp.ssrcs.size(), true));
+                                         true));
   vie_channel_ = channel_group_->GetChannel(channel_id_);
   vie_encoder_ = channel_group_->GetEncoder(channel_id_);
 
@@ -495,9 +494,6 @@ bool VideoSendStream::SetSendCodec(VideoCodec video_codec) {
 
   // 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(),

webrtc/video_engine/encoder_state_feedback.cc

@@ -54,34 +54,16 @@ EncoderStateFeedback::~EncoderStateFeedback() {
   assert(encoders_.empty());
 }
 
-void EncoderStateFeedback::UpdateSsrcs(const std::vector<uint32_t>& ssrcs,
+void EncoderStateFeedback::AddEncoder(const std::vector<uint32_t>& ssrcs,
-                                       ViEEncoder* encoder) {
+                                      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();

webrtc/video_engine/encoder_state_feedback.h

@@ -35,11 +35,8 @@ class EncoderStateFeedback {
   EncoderStateFeedback();
   ~EncoderStateFeedback();
 
-  // Update SSRCs for an encoder.
+  // Adds an encoder to receive feedback for a set of SSRCs.
-  void UpdateSsrcs(const std::vector<uint32_t>& ssrc, ViEEncoder* encoder);
+  void AddEncoder(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);
 
   // Removes a registered ViEEncoder.
   void RemoveEncoder(const ViEEncoder* encoder);

webrtc/video_engine/encoder_state_feedback_unittest.cc

@@ -68,7 +68,7 @@ class VieKeyRequestTest : public ::testing::Test {
 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 @@ TEST_F(VieKeyRequestTest, MultipleEncoders) {
   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));
+  encoder_state_feedback_->AddEncoder(std::vector<uint32_t>(1, ssrc_1),
-  EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc_2, &encoder_2));
+                                      &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 @@ TEST_F(VieKeyRequestTest, MultipleEncoders) {
   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

webrtc/video_engine/vie_channel_group.cc

@@ -193,9 +193,11 @@ bool ChannelGroup::CreateSendChannel(int channel_id,
                                      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 @@ bool ChannelGroup::CreateSendChannel(int channel_id,
   }
   ViEEncoder* encoder = vie_encoder.get();
   if (!CreateChannel(channel_id, engine_id, transport, number_of_cores,
-                     vie_encoder.release(), max_rtp_streams, true,
+                     vie_encoder.release(), ssrcs.empty() ? 1 : ssrcs.size(),
-                     disable_default_encoder)) {
+                     true, disable_default_encoder)) {
     return false;
   }
   ViEChannel* channel = channel_map_[channel_id];
@@ -214,14 +216,11 @@ bool ChannelGroup::CreateSendChannel(int channel_id,
   encoder->StartThreadsAndSetSharedMembers(channel->send_payload_router(),
                                            channel->vcm_protection_callback());
 
-  // Register the ViEEncoder to get key frame requests for this channel.
+  if (!ssrcs.empty()) {
-  unsigned int ssrc = 0;
+    encoder_state_feedback_->AddEncoder(ssrcs, encoder);
-  int stream_idx = 0;
+    std::vector<uint32_t> first_ssrc(1, ssrcs[0]);
-  channel->GetLocalSSRC(stream_idx, &ssrc);
+    encoder->SetSsrcs(first_ssrc);
-  encoder_state_feedback_->AddEncoder(ssrc, encoder);
+  }
-  std::vector<uint32_t> ssrcs;
-  ssrcs.push_back(ssrc);
-  encoder->SetSsrcs(ssrcs);
   return true;
 }
 

webrtc/video_engine/vie_channel_group.h

@@ -46,7 +46,7 @@ class ChannelGroup : public BitrateObserver {
                          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,