Thread annotate RTCPSender.

Also fixes data races in RTCPSender::SetCSRCStatus() and
RTCPSender::SetStartTimestamp().

BUG=
R=tommi@webrtc.org

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

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

5 files changed, 105 insertions, 148 deletions

diff --git a/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc b/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc
index 88463e47..0514277f 100644
--- a/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc
@@ -97,7 +97,6 @@ void RtcpFormatRembTest::SetUp() {
   rtcp_receiver_ = new RTCPReceiver(0, system_clock_, dummy_rtp_rtcp_impl_);
   test_transport_ = new TestTransport(rtcp_receiver_);
 
-  EXPECT_EQ(0, rtcp_sender_->Init());
   EXPECT_EQ(0, rtcp_sender_->RegisterSendTransport(test_transport_));
 }
 
diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc
index fa129abe..2cf7e1cb 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -188,61 +188,6 @@ RTCPSender::~RTCPSender() {
 }
 
 int32_t
-RTCPSender::Init()
-{
-    CriticalSectionScoped lock(_criticalSectionRTCPSender);
-
-    _method = kRtcpOff;
-    _cbTransport = NULL;
-    _usingNack = false;
-    _sending = false;
-    _sendTMMBN = false;
-    _TMMBR = false;
-    _IJ = false;
-    _REMB = false;
-    _sendREMB = false;
-    last_rtp_timestamp_ = 0;
-    last_frame_capture_time_ms_ = -1;
-    start_timestamp_ = -1;
-    _SSRC = 0;
-    _remoteSSRC = 0;
-    _cameraDelayMS = 0;
-    _sequenceNumberFIR = 0;
-    _tmmbr_Send = 0;
-    _packetOH_Send = 0;
-    _nextTimeToSendRTCP = 0;
-    _CSRCs = 0;
-    _appSend = false;
-    _appSubType = 0;
-
-    if(_appData)
-    {
-        delete [] _appData;
-        _appData = NULL;
-    }
-    _appLength = 0;
-
-    xrSendReceiverReferenceTimeEnabled_ = false;
-
-    _xrSendVoIPMetric = false;
-
-    memset(&_xrVoIPMetric, 0, sizeof(_xrVoIPMetric));
-    memset(_CNAME, 0, sizeof(_CNAME));
-    memset(_lastSendReport, 0, sizeof(_lastSendReport));
-    memset(_lastRTCPTime, 0, sizeof(_lastRTCPTime));
-    last_xr_rr_.clear();
-
-    memset(&packet_type_counter_, 0, sizeof(packet_type_counter_));
-    return 0;
-}
-
-void
-RTCPSender::ChangeUniqueId(const int32_t id)
-{
-    _id = id;
-}
-
-int32_t
 RTCPSender::RegisterSendTransport(Transport* outgoingTransport)
 {
     CriticalSectionScoped lock(_criticalSectionTransport);
@@ -381,6 +326,7 @@ RTCPSender::SetIJStatus(const bool enable)
 }
 
 void RTCPSender::SetStartTimestamp(uint32_t start_timestamp) {
+  CriticalSectionScoped lock(_criticalSectionRTCPSender);
   start_timestamp_ = start_timestamp;
 }
 
@@ -686,13 +632,9 @@ int32_t RTCPSender::BuildSR(const FeedbackState& feedback_state,
     // the frame being captured at this moment. We are calculating that
     // timestamp as the last frame's timestamp + the time since the last frame
     // was captured.
-    {
-      // Needs protection since this method is called on the process thread.
-      CriticalSectionScoped lock(_criticalSectionRTCPSender);
-      RTPtime = start_timestamp_ + last_rtp_timestamp_ + (
-          _clock->TimeInMilliseconds() - last_frame_capture_time_ms_) *
-          (feedback_state.frequency_hz / 1000);
-    }
+    RTPtime = start_timestamp_ + last_rtp_timestamp_ +
+              (_clock->TimeInMilliseconds() - last_frame_capture_time_ms_) *
+                  (feedback_state.frequency_hz / 1000);
 
     // Add sender data
     // Save  for our length field
@@ -2102,6 +2044,7 @@ RTCPSender::SendToNetwork(const uint8_t* dataBuffer,
 int32_t
 RTCPSender::SetCSRCStatus(const bool include)
 {
+    CriticalSectionScoped lock(_criticalSectionRTCPSender);
     _includeCSRCs = include;
     return 0;
 }
diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h
index 71d92c8e..fad3b5e3 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.h
+++ b/modules/rtp_rtcp/source/rtcp_sender.h
@@ -23,6 +23,7 @@
 #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h"
 #include "webrtc/modules/rtp_rtcp/source/tmmbr_help.h"
 #include "webrtc/system_wrappers/interface/scoped_ptr.h"
+#include "webrtc/system_wrappers/interface/thread_annotations.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
@@ -74,10 +75,6 @@ public:
                ReceiveStatistics* receive_statistics);
     virtual ~RTCPSender();
 
-    void ChangeUniqueId(const int32_t id);
-
-    int32_t Init();
-
     int32_t RegisterSendTransport(Transport* outgoingTransport);
 
     RTCPMethod Status() const;
@@ -184,13 +181,12 @@ public:
 private:
     int32_t SendToNetwork(const uint8_t* dataBuffer, const uint16_t length);
 
-    void UpdatePacketRate();
-
     int32_t WriteAllReportBlocksToBuffer(uint8_t* rtcpbuffer,
                             int pos,
                             uint8_t& numberOfReportBlocks,
                             const uint32_t NTPsec,
-                            const uint32_t NTPfrac);
+                            const uint32_t NTPfrac)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
 
     int32_t WriteReportBlocksToBuffer(
         uint8_t* rtcpbuffer,
@@ -211,12 +207,14 @@ private:
                     uint8_t* rtcpbuffer,
                     int& pos,
                     uint32_t NTPsec,
-                    uint32_t NTPfrac);
+                    uint32_t NTPfrac)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
 
     int32_t BuildRR(uint8_t* rtcpbuffer,
                     int& pos,
                     const uint32_t NTPsec,
-                    const uint32_t NTPfrac);
+                    const uint32_t NTPfrac)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
 
     int PrepareRTCP(
         const FeedbackState& feedback_state,
@@ -233,117 +231,136 @@ private:
     int32_t BuildExtendedJitterReport(
         uint8_t* rtcpbuffer,
         int& pos,
-        const uint32_t jitterTransmissionTimeOffset);
-
-    int32_t BuildSDEC(uint8_t* rtcpbuffer, int& pos);
-    int32_t BuildPLI(uint8_t* rtcpbuffer, int& pos);
-    int32_t BuildREMB(uint8_t* rtcpbuffer, int& pos);
-    int32_t BuildTMMBR(ModuleRtpRtcpImpl* module,
-                       uint8_t* rtcpbuffer,
-                       int& pos);
-    int32_t BuildTMMBN(uint8_t* rtcpbuffer, int& pos);
-    int32_t BuildAPP(uint8_t* rtcpbuffer, int& pos);
-    int32_t BuildVoIPMetric(uint8_t* rtcpbuffer, int& pos);
-    int32_t BuildBYE(uint8_t* rtcpbuffer, int& pos);
-    int32_t BuildFIR(uint8_t* rtcpbuffer, int& pos, bool repeat);
-    int32_t BuildSLI(uint8_t* rtcpbuffer,
-                     int& pos,
-                     const uint8_t pictureID);
+        const uint32_t jitterTransmissionTimeOffset)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+
+    int32_t BuildSDEC(uint8_t* rtcpbuffer, int& pos)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+    int32_t BuildPLI(uint8_t* rtcpbuffer, int& pos)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+    int32_t BuildREMB(uint8_t* rtcpbuffer, int& pos)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+    int32_t BuildTMMBR(ModuleRtpRtcpImpl* module, uint8_t* rtcpbuffer, int& pos)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+    int32_t BuildTMMBN(uint8_t* rtcpbuffer, int& pos)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+    int32_t BuildAPP(uint8_t* rtcpbuffer, int& pos)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+    int32_t BuildVoIPMetric(uint8_t* rtcpbuffer, int& pos)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+    int32_t BuildBYE(uint8_t* rtcpbuffer, int& pos)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+    int32_t BuildFIR(uint8_t* rtcpbuffer, int& pos, bool repeat)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
+    int32_t BuildSLI(uint8_t* rtcpbuffer, int& pos, const uint8_t pictureID)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
     int32_t BuildRPSI(uint8_t* rtcpbuffer,
                       int& pos,
                       const uint64_t pictureID,
-                      const uint8_t payloadType);
+                      const uint8_t payloadType)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
 
     int32_t BuildNACK(uint8_t* rtcpbuffer,
                       int& pos,
                       const int32_t nackSize,
                       const uint16_t* nackList,
-                          std::string* nackString);
-
+                      std::string* nackString)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
     int32_t BuildReceiverReferenceTime(uint8_t* buffer,
                                        int& pos,
                                        uint32_t ntp_sec,
-                                       uint32_t ntp_frac);
+                                       uint32_t ntp_frac)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
     int32_t BuildDlrr(uint8_t* buffer,
                       int& pos,
-                      const RtcpReceiveTimeInfo& info);
+                      const RtcpReceiveTimeInfo& info)
+        EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPSender);
 
 private:
-    int32_t            _id;
+    const int32_t            _id;
     const bool               _audio;
-    Clock*                   _clock;
-    RTCPMethod               _method;
+    Clock* const             _clock;
+    RTCPMethod               _method GUARDED_BY(_criticalSectionRTCPSender);
 
     CriticalSectionWrapper* _criticalSectionTransport;
-    Transport*              _cbTransport;
+    Transport*              _cbTransport GUARDED_BY(_criticalSectionTransport);
 
     CriticalSectionWrapper* _criticalSectionRTCPSender;
-    bool                    _usingNack;
-    bool                    _sending;
-    bool                    _sendTMMBN;
-    bool                    _REMB;
-    bool                    _sendREMB;
-    bool                    _TMMBR;
-    bool                    _IJ;
-
-    int64_t        _nextTimeToSendRTCP;
-
-    uint32_t start_timestamp_;
-    uint32_t last_rtp_timestamp_;
-    int64_t last_frame_capture_time_ms_;
-    uint32_t _SSRC;
-    uint32_t _remoteSSRC;  // SSRC that we receive on our RTP channel
-    char _CNAME[RTCP_CNAME_SIZE];
-
-
-    ReceiveStatistics* receive_statistics_;
-    std::map<uint32_t, RTCPReportBlock*> internal_report_blocks_;
-    std::map<uint32_t, RTCPReportBlock*> external_report_blocks_;
-    std::map<uint32_t, RTCPUtility::RTCPCnameInformation*> _csrcCNAMEs;
-
-    int32_t         _cameraDelayMS;
+    bool                    _usingNack GUARDED_BY(_criticalSectionRTCPSender);
+    bool                    _sending GUARDED_BY(_criticalSectionRTCPSender);
+    bool                    _sendTMMBN GUARDED_BY(_criticalSectionRTCPSender);
+    bool                    _REMB GUARDED_BY(_criticalSectionRTCPSender);
+    bool                    _sendREMB GUARDED_BY(_criticalSectionRTCPSender);
+    bool                    _TMMBR GUARDED_BY(_criticalSectionRTCPSender);
+    bool                    _IJ GUARDED_BY(_criticalSectionRTCPSender);
+
+    int64_t        _nextTimeToSendRTCP GUARDED_BY(_criticalSectionRTCPSender);
+
+    uint32_t start_timestamp_ GUARDED_BY(_criticalSectionRTCPSender);
+    uint32_t last_rtp_timestamp_ GUARDED_BY(_criticalSectionRTCPSender);
+    int64_t last_frame_capture_time_ms_ GUARDED_BY(_criticalSectionRTCPSender);
+    uint32_t _SSRC GUARDED_BY(_criticalSectionRTCPSender);
+    // SSRC that we receive on our RTP channel
+    uint32_t _remoteSSRC GUARDED_BY(_criticalSectionRTCPSender);
+    char _CNAME[RTCP_CNAME_SIZE] GUARDED_BY(_criticalSectionRTCPSender);
+
+    ReceiveStatistics* receive_statistics_
+        GUARDED_BY(_criticalSectionRTCPSender);
+    std::map<uint32_t, RTCPReportBlock*> internal_report_blocks_
+        GUARDED_BY(_criticalSectionRTCPSender);
+    std::map<uint32_t, RTCPReportBlock*> external_report_blocks_
+        GUARDED_BY(_criticalSectionRTCPSender);
+    std::map<uint32_t, RTCPUtility::RTCPCnameInformation*> _csrcCNAMEs
+        GUARDED_BY(_criticalSectionRTCPSender);
+
+    int32_t         _cameraDelayMS GUARDED_BY(_criticalSectionRTCPSender);
 
     // Sent
-    uint32_t        _lastSendReport[RTCP_NUMBER_OF_SR];  // allow packet loss and RTT above 1 sec
-    uint32_t        _lastRTCPTime[RTCP_NUMBER_OF_SR];
+    uint32_t _lastSendReport[RTCP_NUMBER_OF_SR] GUARDED_BY(
+        _criticalSectionRTCPSender);  // allow packet loss and RTT above 1 sec
+    uint32_t _lastRTCPTime[RTCP_NUMBER_OF_SR] GUARDED_BY(
+        _criticalSectionRTCPSender);
 
     // Sent XR receiver reference time report.
     // <mid ntp (mid 32 bits of the 64 bits NTP timestamp), send time in ms>.
-    std::map<uint32_t, int64_t> last_xr_rr_;
+    std::map<uint32_t, int64_t> last_xr_rr_
+        GUARDED_BY(_criticalSectionRTCPSender);
 
     // send CSRCs
-    uint8_t         _CSRCs;
-    uint32_t        _CSRC[kRtpCsrcSize];
-    bool                _includeCSRCs;
+    uint8_t         _CSRCs GUARDED_BY(_criticalSectionRTCPSender);
+    uint32_t        _CSRC[kRtpCsrcSize] GUARDED_BY(_criticalSectionRTCPSender);
+    bool                _includeCSRCs GUARDED_BY(_criticalSectionRTCPSender);
 
     // Full intra request
-    uint8_t         _sequenceNumberFIR;
+    uint8_t         _sequenceNumberFIR GUARDED_BY(_criticalSectionRTCPSender);
 
     // REMB
-    uint8_t       _lengthRembSSRC;
-    uint8_t       _sizeRembSSRC;
-    uint32_t*     _rembSSRC;
-    uint32_t      _rembBitrate;
+    uint8_t       _lengthRembSSRC GUARDED_BY(_criticalSectionRTCPSender);
+    uint8_t       _sizeRembSSRC GUARDED_BY(_criticalSectionRTCPSender);
+    uint32_t*     _rembSSRC GUARDED_BY(_criticalSectionRTCPSender);
+    uint32_t      _rembBitrate GUARDED_BY(_criticalSectionRTCPSender);
 
-    TMMBRHelp           _tmmbrHelp;
-    uint32_t      _tmmbr_Send;
-    uint32_t      _packetOH_Send;
+    TMMBRHelp           _tmmbrHelp GUARDED_BY(_criticalSectionRTCPSender);
+    uint32_t      _tmmbr_Send GUARDED_BY(_criticalSectionRTCPSender);
+    uint32_t      _packetOH_Send GUARDED_BY(_criticalSectionRTCPSender);
 
     // APP
-    bool                 _appSend;
-    uint8_t        _appSubType;
-    uint32_t       _appName;
-    uint8_t*       _appData;
-    uint16_t       _appLength;
+    bool                 _appSend GUARDED_BY(_criticalSectionRTCPSender);
+    uint8_t        _appSubType GUARDED_BY(_criticalSectionRTCPSender);
+    uint32_t       _appName GUARDED_BY(_criticalSectionRTCPSender);
+    uint8_t*       _appData GUARDED_BY(_criticalSectionRTCPSender);
+    uint16_t       _appLength GUARDED_BY(_criticalSectionRTCPSender);
 
     // True if sending of XR Receiver reference time report is enabled.
-    bool xrSendReceiverReferenceTimeEnabled_;
+    bool xrSendReceiverReferenceTimeEnabled_
+        GUARDED_BY(_criticalSectionRTCPSender);
 
     // XR VoIP metric
-    bool                _xrSendVoIPMetric;
-    RTCPVoIPMetric      _xrVoIPMetric;
+    bool _xrSendVoIPMetric GUARDED_BY(_criticalSectionRTCPSender);
+    RTCPVoIPMetric _xrVoIPMetric GUARDED_BY(_criticalSectionRTCPSender);
 
-    RtcpPacketTypeCounter packet_type_counter_;
+    RtcpPacketTypeCounter packet_type_counter_
+        GUARDED_BY(_criticalSectionRTCPSender);
 };
 }  // namespace webrtc
 
diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
index dfb655c5..cba1c346 100644
--- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
@@ -304,7 +304,6 @@ class RtcpSenderTest : public ::testing::Test {
     rtcp_receiver_ = new RTCPReceiver(0, &clock_, rtp_rtcp_impl_);
     test_transport_->SetRTCPReceiver(rtcp_receiver_);
     // Initialize
-    EXPECT_EQ(0, rtcp_sender_->Init());
     EXPECT_EQ(0, rtcp_sender_->RegisterSendTransport(test_transport_));
   }
   ~RtcpSenderTest() {
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 0c771c51..34a08a8e 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -47,13 +47,12 @@ RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) {
   if (configuration.clock) {
     return new ModuleRtpRtcpImpl(configuration);
   } else {
+    // No clock implementation provided, use default clock.
     RtpRtcp::Configuration configuration_copy;
     memcpy(&configuration_copy, &configuration,
            sizeof(RtpRtcp::Configuration));
     configuration_copy.clock = Clock::GetRealTimeClock();
-    ModuleRtpRtcpImpl* rtp_rtcp_instance =
-        new ModuleRtpRtcpImpl(configuration_copy);
-    return rtp_rtcp_instance;
+    return new ModuleRtpRtcpImpl(configuration_copy);
   }
 }