Configure RTX send status on new modules.

Fixes bug where newly-allocated modules wouldn't send payload-based
padding (or probably not send over RTX at all).

As the newly-added test exposed lock-inversions shown on tsan in
VideoReceiver, VideoReceiver was thread-annotated and locks taken less.
BUG=chromium:391085
R=mflodman@webrtc.org, stefan@webrtc.org

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

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

webrtc/modules/video_coding/main/source/video_coding_impl.h

@@ -25,6 +25,7 @@
 #include "webrtc/modules/video_coding/main/source/timing.h"
 #include "webrtc/system_wrappers/interface/clock.h"
 #include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
+#include "webrtc/system_wrappers/interface/thread_annotations.h"
 
 namespace webrtc {
 
@@ -199,21 +200,25 @@ class VideoReceiver {
                     // in any frame
   };
 
-  Clock* clock_;
+  Clock* const clock_;
   scoped_ptr<CriticalSectionWrapper> process_crit_sect_;
   CriticalSectionWrapper* _receiveCritSect;
-  bool _receiverInited;
+  bool _receiverInited GUARDED_BY(_receiveCritSect);
   VCMTiming _timing;
   VCMTiming _dualTiming;
   VCMReceiver _receiver;
   VCMReceiver _dualReceiver;
   VCMDecodedFrameCallback _decodedFrameCallback;
   VCMDecodedFrameCallback _dualDecodedFrameCallback;
-  VCMFrameTypeCallback* _frameTypeCallback;
-  VCMReceiveStatisticsCallback* _receiveStatsCallback;
-  VCMDecoderTimingCallback* _decoderTimingCallback;
-  VCMPacketRequestCallback* _packetRequestCallback;
-  VCMRenderBufferSizeCallback* render_buffer_callback_;
+  VCMFrameTypeCallback* _frameTypeCallback GUARDED_BY(process_crit_sect_);
+  VCMReceiveStatisticsCallback* _receiveStatsCallback
+      GUARDED_BY(process_crit_sect_);
+  VCMDecoderTimingCallback* _decoderTimingCallback
+      GUARDED_BY(process_crit_sect_);
+  VCMPacketRequestCallback* _packetRequestCallback
+      GUARDED_BY(process_crit_sect_);
+  VCMRenderBufferSizeCallback* render_buffer_callback_
+      GUARDED_BY(process_crit_sect_);
   VCMGenericDecoder* _decoder;
   VCMGenericDecoder* _dualDecoder;
 #ifdef DEBUG_DECODER_BIT_STREAM
@@ -221,9 +226,9 @@ class VideoReceiver {
 #endif
   VCMFrameBuffer _frameFromFile;
   VCMKeyRequestMode _keyRequestMode;
-  bool _scheduleKeyRequest;
-  size_t max_nack_list_size_;
-  EncodedImageCallback* pre_decode_image_callback_;
+  bool _scheduleKeyRequest GUARDED_BY(process_crit_sect_);
+  size_t max_nack_list_size_ GUARDED_BY(process_crit_sect_);
+  EncodedImageCallback* pre_decode_image_callback_ GUARDED_BY(_receiveCritSect);
 
   VCMCodecDataBase _codecDataBase;
   VCMProcessTimer _receiveStatsTimer;

webrtc/modules/video_coding/main/source/video_receiver.cc

@@ -271,8 +271,6 @@ int32_t VideoReceiver::SetVideoProtection(VCMVideoProtection videoProtection,
 
 // Initialize receiver, resets codec database etc
 int32_t VideoReceiver::InitializeReceiver() {
-  CriticalSectionScoped receive_cs(_receiveCritSect);
-  CriticalSectionScoped process_cs(process_crit_sect_.get());
   int32_t ret = _receiver.Initialize();
   if (ret < 0) {
     return ret;
@@ -285,15 +283,22 @@ int32_t VideoReceiver::InitializeReceiver() {
   _codecDataBase.ResetReceiver();
   _timing.Reset();
 
-  _decoder = NULL;
-  _decodedFrameCallback.SetUserReceiveCallback(NULL);
-  _receiverInited = true;
-  _frameTypeCallback = NULL;
-  _receiveStatsCallback = NULL;
-  _decoderTimingCallback = NULL;
-  _packetRequestCallback = NULL;
-  _keyRequestMode = kKeyOnError;
-  _scheduleKeyRequest = false;
+  {
+    CriticalSectionScoped receive_cs(_receiveCritSect);
+    _receiverInited = true;
+  }
+
+  {
+    CriticalSectionScoped process_cs(process_crit_sect_.get());
+    _decoder = NULL;
+    _decodedFrameCallback.SetUserReceiveCallback(NULL);
+    _frameTypeCallback = NULL;
+    _receiveStatsCallback = NULL;
+    _decoderTimingCallback = NULL;
+    _packetRequestCallback = NULL;
+    _keyRequestMode = kKeyOnError;
+    _scheduleKeyRequest = false;
+  }
 
   return VCM_OK;
 }
@@ -781,7 +786,6 @@ void VideoReceiver::SetNackSettings(size_t max_nack_list_size,
                                     int max_packet_age_to_nack,
                                     int max_incomplete_time_ms) {
   if (max_nack_list_size != 0) {
-    CriticalSectionScoped receive_cs(_receiveCritSect);
     CriticalSectionScoped process_cs(process_crit_sect_.get());
     max_nack_list_size_ = max_nack_list_size;
   }

webrtc/video/end_to_end_tests.cc

@@ -1543,4 +1543,80 @@ TEST_F(EndToEndTest, CanSwitchToUseAllSsrcs) {
   TestSendsSetSsrcs(kNumSsrcs, true);
 }
 
+TEST_F(EndToEndTest, RedundantPayloadsTransmittedOnAllSsrcs) {
+  // TODO(pbos): Use CallTest::kSendRtxSsrcs when they're an array (pending CL).
+  static const uint32_t kSendRtxSsrcs[kNumSsrcs] = {
+      0xBADCAFD, 0xBADCAFE, 0xBADCAFF};
+  class ObserveRedundantPayloads: public test::EndToEndTest {
+   public:
+    ObserveRedundantPayloads()
+        : EndToEndTest(kDefaultTimeoutMs), ssrcs_to_observe_(kNumSsrcs) {
+          for(size_t i = 0; i < kNumSsrcs; ++i) {
+            registered_rtx_ssrc_[kSendRtxSsrcs[i]] = true;
+          }
+        }
+
+   private:
+    virtual Action OnSendRtp(const uint8_t* packet, size_t length) OVERRIDE {
+      RTPHeader header;
+      EXPECT_TRUE(parser_->Parse(packet, static_cast<int>(length), &header));
+
+      if (!registered_rtx_ssrc_[header.ssrc])
+        return SEND_PACKET;
+
+      EXPECT_LE(static_cast<size_t>(header.headerLength + header.paddingLength),
+                length);
+      const bool packet_is_redundant_payload =
+          static_cast<size_t>(header.headerLength + header.paddingLength) <
+          length;
+
+      if (!packet_is_redundant_payload)
+        return SEND_PACKET;
+
+      if (!observed_redundant_retransmission_[header.ssrc]) {
+        observed_redundant_retransmission_[header.ssrc] = true;
+        if (--ssrcs_to_observe_ == 0)
+          observation_complete_->Set();
+      }
+
+      return SEND_PACKET;
+    }
+
+    virtual size_t GetNumStreams() const OVERRIDE { return kNumSsrcs; }
+
+    virtual void ModifyConfigs(
+        VideoSendStream::Config* send_config,
+        std::vector<VideoReceiveStream::Config>* receive_configs,
+        std::vector<VideoStream>* video_streams) OVERRIDE {
+      // Set low simulcast bitrates to not have to wait for bandwidth ramp-up.
+      for (size_t i = 0; i < video_streams->size(); ++i) {
+        (*video_streams)[i].min_bitrate_bps = 10000;
+        (*video_streams)[i].target_bitrate_bps = 15000;
+        (*video_streams)[i].max_bitrate_bps = 20000;
+      }
+      // Significantly higher than max bitrates for all video streams -> forcing
+      // padding to trigger redundant padding on all RTX SSRCs.
+      send_config->rtp.min_transmit_bitrate_bps = 100000;
+
+      send_config->rtp.rtx.payload_type = kSendRtxPayloadType;
+      send_config->rtp.rtx.pad_with_redundant_payloads = true;
+
+      for (size_t i = 0; i < kNumSsrcs; ++i)
+        send_config->rtp.rtx.ssrcs.push_back(kSendRtxSsrcs[i]);
+    }
+
+    virtual void PerformTest() OVERRIDE {
+      EXPECT_EQ(kEventSignaled, Wait())
+          << "Timed out while waiting for redundant payloads on all SSRCs.";
+    }
+
+   private:
+    size_t ssrcs_to_observe_;
+    std::map<uint32_t, bool> observed_redundant_retransmission_;
+    std::map<uint32_t, bool> registered_rtx_ssrc_;
+  } test;
+
+  RunBaseTest(&test);
+}
+
 }  // namespace webrtc

webrtc/video_engine/vie_channel.cc

@@ -278,6 +278,11 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec,
       rtp_rtcp->SetSendingStatus(rtp_rtcp_->Sending());
       rtp_rtcp->SetSendingMediaStatus(rtp_rtcp_->SendingMedia());
 
+      int mode;
+      uint32_t ssrc;
+      int payload_type;
+      rtp_rtcp_->RTXSendStatus(&mode, &ssrc, &payload_type);
+      rtp_rtcp->SetRTXSendStatus(mode);
       simulcast_rtp_rtcp_.push_back(rtp_rtcp);
 
       // Silently ignore error.