Callback for send bitrate estimates - new roll

Issue https://webrtc-codereview.appspot.com/4459004/ was commited as
r5259, after which flakiness was detected and a rollback was performed
at r5261.

Patch Set 1 of this issue is the code submitted in r5259. Subsequent
patch sets fixes a race condition which caused the seen problems.

The root cause was a dead lock between a thread sending rtp packets and
and a timed module processing thread:

webrtc::RTPSender::BitrateUpdated()  // Get RTPSender stats lock
webrtc::Bitrate::Process()  // Get Bitrate lock
webrtc::RTPSender::ProcessBitrate()
webrtc::ModuleRtpRtcpImpl::Process()
...

webrtc::Bitrate::Update()  // Get Bitrate lock
webrtc::RTPSender::UpdateRtpStats()  // Get RTPSender stats lock
webrtc::RTPSender::SendToNetwork()
...

This is fixed in Bitrate::Process() by releasing the lock before
calling the callback.

BUG=2235
R=mflodman@webrtc.org

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

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

3 files changed, 46 insertions, 6 deletions

diff --git a/video_engine/vie_channel.cc b/video_engine/vie_channel.cc
index 810f66bd..44b90f41 100644
--- a/video_engine/vie_channel.cc
+++ b/video_engine/vie_channel.cc
@@ -359,6 +359,7 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec,
       rtp_rtcp->RegisterSendFrameCountObserver(NULL);
       rtp_rtcp->RegisterSendChannelRtcpStatisticsCallback(NULL);
       rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(NULL);
+      rtp_rtcp->RegisterVideoBitrateObserver(NULL);
       simulcast_rtp_rtcp_.pop_back();
       removed_rtp_rtcp_.push_front(rtp_rtcp);
     }
@@ -419,6 +420,8 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec,
           rtp_rtcp_->GetSendChannelRtcpStatisticsCallback());
       rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(
           rtp_rtcp_->GetSendChannelRtpStatisticsCallback());
+      rtp_rtcp->RegisterVideoBitrateObserver(
+          rtp_rtcp_->GetVideoBitrateObserver());
     }
     // |RegisterSimulcastRtpRtcpModules| resets all old weak pointers and old
     // modules can be deleted after this step.
@@ -432,6 +435,7 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec,
       rtp_rtcp->RegisterSendFrameCountObserver(NULL);
       rtp_rtcp->RegisterSendChannelRtcpStatisticsCallback(NULL);
       rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(NULL);
+      rtp_rtcp->RegisterVideoBitrateObserver(NULL);
       simulcast_rtp_rtcp_.pop_back();
       removed_rtp_rtcp_.push_front(rtp_rtcp);
     }
@@ -1427,6 +1431,17 @@ bool ViEChannel::GetSendSideDelay(int* avg_send_delay,
   return valid_estimate;
 }
 
+void ViEChannel::RegisterSendBitrateObserver(
+    BitrateStatisticsObserver* observer) {
+  rtp_rtcp_->RegisterVideoBitrateObserver(observer);
+  CriticalSectionScoped cs(rtp_rtcp_cs_.get());
+  for (std::list<RtpRtcp*>::const_iterator it = simulcast_rtp_rtcp_.begin();
+       it != simulcast_rtp_rtcp_.end();
+       it++) {
+    (*it)->RegisterVideoBitrateObserver(observer);
+  }
+}
+
 void ViEChannel::GetEstimatedReceiveBandwidth(
     uint32_t* estimated_bandwidth) const {
   vie_receiver_.EstimatedReceiveBandwidth(estimated_bandwidth);
diff --git a/video_engine/vie_channel.h b/video_engine/vie_channel.h
index 29b14649..de167312 100644
--- a/video_engine/vie_channel.h
+++ b/video_engine/vie_channel.h
@@ -202,6 +202,9 @@ class ViEChannel
   bool GetSendSideDelay(int* avg_send_delay, int* max_send_delay) const;
   void GetEstimatedReceiveBandwidth(uint32_t* estimated_bandwidth) const;
 
+  // Called on any new send bitrate estimate.
+  void RegisterSendBitrateObserver(BitrateStatisticsObserver* observer);
+
   int32_t StartRTPDump(const char file_nameUTF8[1024],
                        RTPDirections direction);
   int32_t StopRTPDump(RTPDirections direction);
diff --git a/video_engine/vie_rtp_rtcp_impl.cc b/video_engine/vie_rtp_rtcp_impl.cc
index ce2f4344..e07ab6c1 100644
--- a/video_engine/vie_rtp_rtcp_impl.cc
+++ b/video_engine/vie_rtp_rtcp_impl.cc
@@ -1206,16 +1206,38 @@ int ViERTP_RTCPImpl::DeregisterReceiveChannelRtpStatisticsCallback(
   // TODO(sprang): Implement
   return -1;
 }
+
+// Called whenever the send bitrate is updated.
 int ViERTP_RTCPImpl::RegisterSendBitrateObserver(
-    int channel, BitrateStatisticsObserver* callback) {
-  // TODO(sprang): Implement
-  return -1;
+    const int video_channel,
+    BitrateStatisticsObserver* observer) {
+  WEBRTC_TRACE(kTraceApiCall,
+               kTraceVideo,
+               ViEId(shared_data_->instance_id(), video_channel),
+               "%s(channel: %d)",
+               __FUNCTION__,
+               video_channel);
+  ViEChannelManagerScoped cs(*(shared_data_->channel_manager()));
+  ViEChannel* vie_channel = cs.Channel(video_channel);
+  assert(vie_channel != NULL);
+  vie_channel->RegisterSendBitrateObserver(observer);
+  return 0;
 }
 
 int ViERTP_RTCPImpl::DeregisterSendBitrateObserver(
-    int channel, BitrateStatisticsObserver* callback) {
-  // TODO(sprang): Implement
-  return -1;
+    const int video_channel,
+    BitrateStatisticsObserver* observer) {
+  WEBRTC_TRACE(kTraceApiCall,
+               kTraceVideo,
+               ViEId(shared_data_->instance_id(), video_channel),
+               "%s(channel: %d)",
+               __FUNCTION__,
+               video_channel);
+  ViEChannelManagerScoped cs(*(shared_data_->channel_manager()));
+  ViEChannel* vie_channel = cs.Channel(video_channel);
+  assert(vie_channel != NULL);
+  vie_channel->RegisterSendBitrateObserver(NULL);
+  return 0;
 }
 
 int ViERTP_RTCPImpl::RegisterSendFrameCountObserver(