Hooking up EncoderStateFeedback to handle intra requests instead of trigger

ViEEncoder directly. This is one step towards adding send- and receive only
channels and getting rid of the default module.

BUG=769

Review URL: https://webrtc-codereview.appspot.com/824004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@2812 4adac7df-926f-26a2-2b94-8c16560cd09d

src/modules/rtp_rtcp/interface/rtp_rtcp_defines.h

@@ -225,6 +225,8 @@ class RtcpIntraFrameObserver {
   virtual void OnReceivedRPSI(uint32_t ssrc,
                               uint64_t picture_id) = 0;
 
+  virtual void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc) = 0;
+
   virtual ~RtcpIntraFrameObserver() {}
 };
 

src/modules/rtp_rtcp/source/rtcp_receiver.cc

@@ -133,9 +133,19 @@ void RTCPReceiver::RegisterRtcpObservers(
 }
 
 
-void RTCPReceiver::SetSSRC( const WebRtc_UWord32 ssrc) {
+void RTCPReceiver::SetSSRC(const WebRtc_UWord32 ssrc) {
+  WebRtc_UWord32 old_ssrc = 0;
+  {
     CriticalSectionScoped lock(_criticalSectionRTCPReceiver);
+    old_ssrc = _SSRC;
     _SSRC = ssrc;
+  }
+  {
+    CriticalSectionScoped lock(_criticalSectionFeedbacks);
+    if (_cbRtcpIntraFrameObserver && old_ssrc != ssrc) {
+      _cbRtcpIntraFrameObserver->OnLocalSsrcChanged(old_ssrc, ssrc);
+    }
+  }
 }
 
 WebRtc_Word32 RTCPReceiver::ResetRTT(const WebRtc_UWord32 remoteSSRC) {
@@ -1196,6 +1206,12 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket(
     // Might trigger a OnReceivedBandwidthEstimateUpdate.
     UpdateTMMBR();
   }
+  unsigned int local_ssrc = 0;
+  {
+    // We don't want to hold this critsect when triggering the callbacks below.
+    CriticalSectionScoped lock(_criticalSectionRTCPReceiver);
+    local_ssrc = _SSRC;
+  }
   if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpSrReq) {
     _rtpRtcp.OnRequestSendReport();
   }
@@ -1228,18 +1244,15 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket(
                        "SIG [RTCP] Incoming FIR from SSRC:0x%x",
                        rtcpPacketInformation.remoteSSRC);
         }
-        _cbRtcpIntraFrameObserver->OnReceivedIntraFrameRequest(
+        _cbRtcpIntraFrameObserver->OnReceivedIntraFrameRequest(local_ssrc);
-            rtcpPacketInformation.remoteSSRC);
       }
       if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpSli) {
         _cbRtcpIntraFrameObserver->OnReceivedSLI(
-            rtcpPacketInformation.remoteSSRC,
+            local_ssrc, rtcpPacketInformation.sliPictureId);
-            rtcpPacketInformation.sliPictureId);
       }
       if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpRpsi) {
         _cbRtcpIntraFrameObserver->OnReceivedRPSI(
-            rtcpPacketInformation.remoteSSRC,
+            local_ssrc, rtcpPacketInformation.rpsiPictureId);
-            rtcpPacketInformation.rpsiPictureId);
       }
     }
     if (_cbRtcpBandwidthObserver) {

src/modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -106,6 +106,7 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration)
   // make sure that RTCP objects are aware of our SSRC
   WebRtc_UWord32 SSRC = _rtpSender.SSRC();
   _rtcpSender.SetSSRC(SSRC);
+  _rtcpReceiver.SetSSRC(SSRC);
 
   WEBRTC_TRACE(kTraceMemory, kTraceRtpRtcp, _id, "%s created", __FUNCTION__);
 }

src/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc

@@ -71,6 +71,7 @@ class RtcpCallback : public RtcpFeedback, public RtcpIntraFrameObserver {
                               uint64_t pictureId) {
     EXPECT_EQ(kTestPictureId, pictureId);
   };
+  virtual void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc) {};
  private:
   RtpRtcp* _rtpRtcpModule;
 };

src/video_engine/encoder_state_feedback.cc

@@ -37,6 +37,10 @@ class EncoderStateFeedbackObserver : public  RtcpIntraFrameObserver {
     owner_->OnReceivedRPSI(ssrc, picture_id);
   }
 
+  virtual void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc) {
+    owner_->OnLocalSsrcChanged(old_ssrc, new_ssrc);
+  }
+
  private:
   EncoderStateFeedback* owner_;
 };
@@ -51,8 +55,10 @@ EncoderStateFeedback::~EncoderStateFeedback() {
 
 bool EncoderStateFeedback::AddEncoder(uint32_t ssrc, ViEEncoder* encoder)  {
   CriticalSectionScoped lock(crit_.get());
-  if (encoders_.find(ssrc) != encoders_.end())
+  if (encoders_.find(ssrc) != encoders_.end()) {
+    // Two encoders must not have the same ssrc.
     return false;
+  }
 
   encoders_[ssrc] = encoder;
   return true;
@@ -98,4 +104,16 @@ void EncoderStateFeedback::OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id) {
   it->second->OnReceivedRPSI(ssrc, picture_id);
 }
 
+void EncoderStateFeedback::OnLocalSsrcChanged(uint32_t old_ssrc,
+                                              uint32_t new_ssrc) {
+  CriticalSectionScoped lock(crit_.get());
+  SsrcEncoderMap::iterator it = encoders_.find(old_ssrc);
+  if (it == encoders_.end() || encoders_.find(new_ssrc) != encoders_.end()) {
+    return;
+  }
+
+  encoders_[new_ssrc] = it->second;
+  encoders_.erase(it);
+}
+
 }  // namespace webrtc

src/video_engine/encoder_state_feedback.h

@@ -49,6 +49,7 @@ class EncoderStateFeedback {
   void OnReceivedIntraFrameRequest(uint32_t ssrc);
   void OnReceivedSLI(uint32_t ssrc, uint8_t picture_id);
   void OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id);
+  void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc);
 
  private:
   typedef std::map<uint32_t,  ViEEncoder*> SsrcEncoderMap;

src/video_engine/encoder_state_feedback_unittest.cc

@@ -45,6 +45,8 @@ class MockVieEncoder : public ViEEncoder {
                void(uint32_t ssrc, uint8_t picture_id));
   MOCK_METHOD2(OnReceivedRPSI,
                void(uint32_t ssrc, uint64_t picture_id));
+  MOCK_METHOD2(OnLocalSsrcChanged,
+               void(uint32_t old_ssrc, uint32_t new_ssrc));
 };
 
 class VieKeyRequestTest : public ::testing::Test {

src/video_engine/vie_channel_group.cc

@@ -13,6 +13,7 @@
 #include "modules/bitrate_controller/include/bitrate_controller.h"
 #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
 #include "modules/rtp_rtcp/interface/rtp_rtcp.h"
+#include "video_engine/encoder_state_feedback.h"
 #include "video_engine/vie_channel.h"
 #include "video_engine/vie_encoder.h"
 #include "video_engine/vie_remb.h"
@@ -25,7 +26,8 @@ ChannelGroup::ChannelGroup(ProcessThread* process_thread,
     : remb_(new VieRemb(process_thread)),
       bitrate_controller_(BitrateController::CreateBitrateController()),
       remote_bitrate_estimator_(RemoteBitrateEstimator::Create(remb_.get(),
-                                                               options, mode)) {
+                                                               options, mode)),
+      encoder_state_feedback_(new EncoderStateFeedback()) {
 }
 
 ChannelGroup::~ChannelGroup() {
@@ -57,6 +59,10 @@ RemoteBitrateEstimator* ChannelGroup::GetRemoteBitrateEstimator() {
   return remote_bitrate_estimator_.get();
 }
 
+EncoderStateFeedback* ChannelGroup::GetEncoderStateFeedback() {
+  return encoder_state_feedback_.get();
+}
+
 bool ChannelGroup::SetChannelRembStatus(int channel_id,
                                         bool sender,
                                         bool receiver,

src/video_engine/vie_channel_group.h

@@ -19,6 +19,7 @@
 namespace webrtc {
 
 class BitrateController;
+class EncoderStateFeedback;
 struct OverUseDetectorOptions;
 class ProcessThread;
 class ViEChannel;
@@ -47,6 +48,7 @@ class ChannelGroup {
 
   BitrateController* GetBitrateController();
   RemoteBitrateEstimator* GetRemoteBitrateEstimator();
+  EncoderStateFeedback* GetEncoderStateFeedback();
 
  private:
   typedef std::set<int> ChannelSet;
@@ -54,6 +56,7 @@ class ChannelGroup {
   scoped_ptr<VieRemb> remb_;
   scoped_ptr<BitrateController> bitrate_controller_;
   scoped_ptr<RemoteBitrateEstimator> remote_bitrate_estimator_;
+  scoped_ptr<EncoderStateFeedback> encoder_state_feedback_;
   ChannelSet channels_;
 };
 

src/video_engine/vie_channel_manager.cc

@@ -16,6 +16,7 @@
 #include "system_wrappers/interface/critical_section_wrapper.h"
 #include "system_wrappers/interface/map_wrapper.h"
 #include "system_wrappers/interface/trace.h"
+#include "video_engine/encoder_state_feedback.h"
 #include "video_engine/vie_channel.h"
 #include "video_engine/vie_defines.h"
 #include "video_engine/vie_encoder.h"
@@ -103,10 +104,14 @@ int ViEChannelManager::CreateChannel(int* channel_id) {
       bitrate_controller->CreateRtcpBandwidthObserver();
   RemoteBitrateEstimator* remote_bitrate_estimator =
       group->GetRemoteBitrateEstimator();
+  EncoderStateFeedback* encoder_state_feedback =
+      group->GetEncoderStateFeedback();
 
   if (!(vie_encoder->Init() &&
         CreateChannelObject(new_channel_id, vie_encoder, bandwidth_observer,
-                            remote_bitrate_estimator, true))) {
+                            remote_bitrate_estimator,
+                            encoder_state_feedback->GetRtcpIntraFrameObserver(),
+                            true))) {
     delete vie_encoder;
     vie_encoder = NULL;
     ReturnChannelId(new_channel_id);
@@ -114,6 +119,11 @@ int ViEChannelManager::CreateChannel(int* channel_id) {
     return -1;
   }
 
+  // Add ViEEncoder to EncoderFeedBackObserver.
+  unsigned int ssrc = 0;
+  channel_map_[new_channel_id]->GetLocalSSRC(&ssrc);
+  encoder_state_feedback->AddEncoder(ssrc, vie_encoder);
+
   *channel_id = new_channel_id;
   group->AddChannel(*channel_id);
   channel_groups_.push_back(group);
@@ -141,6 +151,8 @@ int ViEChannelManager::CreateChannel(int* channel_id,
       bitrate_controller->CreateRtcpBandwidthObserver();
   RemoteBitrateEstimator* remote_bitrate_estimator =
       channel_group->GetRemoteBitrateEstimator();
+  EncoderStateFeedback* encoder_state_feedback =
+      channel_group->GetEncoderStateFeedback();
 
   ViEEncoder* vie_encoder = NULL;
   if (sender) {
@@ -149,18 +161,24 @@ int ViEChannelManager::CreateChannel(int* channel_id,
                                  *module_process_thread_,
                                  bitrate_controller);
     if (!(vie_encoder->Init() &&
-          CreateChannelObject(new_channel_id, vie_encoder,
+          CreateChannelObject(
-                              bandwidth_observer,
+              new_channel_id, vie_encoder, bandwidth_observer,
               remote_bitrate_estimator,
-                              sender))) {
+              encoder_state_feedback->GetRtcpIntraFrameObserver(), sender))) {
       delete vie_encoder;
       vie_encoder = NULL;
     }
+    // Register the ViEEncoder to get key frame requests for this channel.
+    unsigned int ssrc = 0;
+    channel_map_[new_channel_id]->GetLocalSSRC(&ssrc);
+    encoder_state_feedback->AddEncoder(ssrc, vie_encoder);
   } else {
     vie_encoder = ViEEncoderPtr(original_channel);
     assert(vie_encoder);
-    if (!CreateChannelObject(new_channel_id, vie_encoder, bandwidth_observer,
+    if (!CreateChannelObject(
-                             remote_bitrate_estimator, sender)) {
+        new_channel_id, vie_encoder, bandwidth_observer,
+        remote_bitrate_estimator,
+        encoder_state_feedback->GetRtcpIntraFrameObserver(), sender)) {
       vie_encoder = NULL;
     }
   }
@@ -206,9 +224,13 @@ int ViEChannelManager::DeleteChannel(int channel_id) {
     group = FindGroup(channel_id);
     group->SetChannelRembStatus(channel_id, false, false, vie_channel,
                                 vie_encoder);
-    unsigned int ssrc = 0;
+    unsigned int remote_ssrc = 0;
-    vie_channel->GetRemoteSSRC(&ssrc);
+    vie_channel->GetRemoteSSRC(&remote_ssrc);
-    group->RemoveChannel(channel_id, ssrc);
+    group->RemoveChannel(channel_id, remote_ssrc);
+
+    unsigned int local_ssrc = 0;
+    vie_channel->GetLocalSSRC(&local_ssrc);
+    group->GetEncoderStateFeedback()->RemoveEncoder(local_ssrc);
 
     // Check if other channels are using the same encoder.
     if (ChannelUsingViEEncoder(channel_id)) {
@@ -353,6 +375,7 @@ bool ViEChannelManager::CreateChannelObject(
     ViEEncoder* vie_encoder,
     RtcpBandwidthObserver* bandwidth_observer,
     RemoteBitrateEstimator* remote_bitrate_estimator,
+    RtcpIntraFrameObserver* intra_frame_observer,
     bool sender) {
   // Register the channel at the encoder.
   RtpRtcp* send_rtp_rtcp_module = vie_encoder->SendRtpRtcpModule();
@@ -360,7 +383,7 @@ bool ViEChannelManager::CreateChannelObject(
   ViEChannel* vie_channel = new ViEChannel(channel_id, engine_id_,
                                            number_of_cores_,
                                            *module_process_thread_,
-                                           vie_encoder,
+                                           intra_frame_observer,
                                            bandwidth_observer,
                                            remote_bitrate_estimator,
                                            send_rtp_rtcp_module,

src/video_engine/vie_channel_manager.h

@@ -85,6 +85,7 @@ class ViEChannelManager: private ViEManagerBase {
   bool CreateChannelObject(int channel_id, ViEEncoder* vie_encoder,
                            RtcpBandwidthObserver* bandwidth_observer,
                            RemoteBitrateEstimator* remote_bitrate_estimator,
+                           RtcpIntraFrameObserver* intra_frame_observer,
                            bool sender);
 
   // Used by ViEChannelScoped, forcing a manager user to use scoped.

src/video_engine/vie_encoder.cc

@@ -822,6 +822,9 @@ void ViEEncoder::OnReceivedIntraFrameRequest(uint32_t /*ssrc*/) {
   time_last_intra_request_ms_ = now;
 }
 
+void ViEEncoder::OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc) {
+}
+
 // Called from ViEBitrateObserver.
 void ViEEncoder::OnNetworkChanged(const uint32_t bitrate_bps,
                                   const uint8_t fraction_lost,

src/video_engine/vie_encoder.h

@@ -130,12 +130,9 @@ class ViEEncoder
 
   // Implements RtcpIntraFrameObserver.
   virtual void OnReceivedIntraFrameRequest(uint32_t ssrc);
-
+  virtual void OnReceivedSLI(uint32_t ssrc, uint8_t picture_id);
-  virtual void OnReceivedSLI(uint32_t ssrc,
+  virtual void OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id);
-                             uint8_t picture_id);
+  virtual void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc);
-
-  virtual void OnReceivedRPSI(uint32_t ssrc,
-                              uint64_t picture_id);
 
   // Effect filter.
   WebRtc_Word32 RegisterEffectFilter(ViEEffectFilter* effect_filter);