Avoid rescheduling the next RTCP packet if the RTCP sender status doesn't change.

webrtc/modules/rtp_rtcp/source/rtcp_sender.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 189 matching lines @@
 
 RTCPSender::~RTCPSender() {}
 
 RtcpMode RTCPSender::Status() const {
   rtc::CritScope lock(&critical_section_rtcp_sender_);
   return method_;
 }
 
 void RTCPSender::SetRTCPStatus(RtcpMode method) {
   rtc::CritScope lock(&critical_section_rtcp_sender_);
+  bool was_off = (method_ == RtcpMode::kOff);

[pbos-webrtc, 2016/03/29 17:48:46]

remove ()s

[skvlad, 2016/03/29 18:09:25]

Done.

   method_ = method;
 
   if (method == RtcpMode::kOff)
     return;
-  next_time_to_send_rtcp_ =
-      clock_->TimeInMilliseconds() +
-      (audio_ ? RTCP_INTERVAL_AUDIO_MS / 2 : RTCP_INTERVAL_VIDEO_MS / 2);
+
+  if (was_off) {
+    next_time_to_send_rtcp_ =
+        clock_->TimeInMilliseconds() +
+        (audio_ ? RTCP_INTERVAL_AUDIO_MS / 2 : RTCP_INTERVAL_VIDEO_MS / 2);
+  }

[pthatcher1, 2016/03/25 23:03:40]

Might be more clear as:

if (method == method_)
  return;

method_ = method;

if (method == RtcpMode::kOff)
  return;

next_time_to_send_rtcp_ = ...;


Only the first early return needs to be added.

[skvlad, 2016/03/29 18:09:25]

There are three possible values for RtcpMode: kOff, kCompound, and kReducedSize.
My understanding is that only switching from kOff to one of the active modes
should reschedule the packet.

Rewriting the condition as you described does indeed make it clearer, but it
makes the RTCP packet be rescheduled when the mode is changed from kCompound to
kReducedSize which is probably not necessary.

[pthatcher, 2016/03/29 18:53:09]

Ah, true.


Well, how about this?

if (method == method_)
  return;

if (method_ == RtcpMode::kOff)
  // When switching off => on, reschedule.
  next_time_to_send_rtcp_ = ...;

method_ = method;

[skvlad, 2016/03/29 19:51:31]

Good idea; i've replaced it with a slightly modified version of this. Please
take a look.

 }
 
 bool RTCPSender::Sending() const {
   rtc::CritScope lock(&critical_section_rtcp_sender_);
   return sending_;
 }
 
 int32_t RTCPSender::SetSendingStatus(const FeedbackState& feedback_state,
                                      bool sending) {
   bool sendRTCPBye = false;
@@ skipping 802 matching lines @@
     // RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(Sender);
     // but we can't because of an incorrect warning (C4822) in MVS 2013.
   } sender(transport_, event_log_);
 
   uint8_t buffer[IP_PACKET_SIZE];
   return packet.BuildExternalBuffer(buffer, IP_PACKET_SIZE, &sender) &&
          !sender.send_failure_;
 }
 
 }  // namespace webrtc