Removed extended jitter report from RtcpSender.

This was never used (value always 0, when sent)

BUG=2450
R=asapersson@webrtc.org

Review URL: https://codereview.webrtc.org/1208843003 .

Cr-Commit-Position: refs/heads/master@{#9631}

webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h

@@ -487,13 +487,6 @@ class RtpRtcp : public Module {
     virtual void SetREMBData(uint32_t bitrate,
                              const std::vector<uint32_t>& ssrcs) = 0;
 
-    /*
-    *   (IJ) Extended jitter report.
-    */
-    virtual bool IJ() const = 0;
-
-    virtual void SetIJStatus(bool enable) = 0;
-
     /*
     *   (TMMBR) Temporary Max Media Bit Rate
     */

webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h

@@ -185,9 +185,6 @@ class MockRtpRtcp : public RtpRtcp {
   MOCK_METHOD2(SetREMBData,
                void(const uint32_t bitrate,
                     const std::vector<uint32_t>& ssrcs));
-  MOCK_CONST_METHOD0(IJ,
-      bool());
-  MOCK_METHOD1(SetIJStatus, void(const bool));
   MOCK_CONST_METHOD0(TMMBR,
       bool());
   MOCK_METHOD1(SetTMMBRStatus, void(const bool enable));

webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc

@@ -295,7 +295,7 @@ RTCPReceiver::IncomingRTCPPacket(RTCPPacketInformation& rtcpPacketInformation,
             HandleSenderReceiverReport(*rtcpParser, rtcpPacketInformation);
             break;
           case RTCPPacketTypes::kSdes:
-            HandleSDES(*rtcpParser);
+            HandleSDES(*rtcpParser, rtcpPacketInformation);
             break;
           case RTCPPacketTypes::kXrHeader:
             HandleXrHeader(*rtcpParser, rtcpPacketInformation);
@@ -754,12 +754,14 @@ int32_t RTCPReceiver::BoundingSet(bool &tmmbrOwner, TMMBRSet* boundingSetRec) {
 }
 
 // no need for critsect we have _criticalSectionRTCPReceiver
-void RTCPReceiver::HandleSDES(RTCPUtility::RTCPParserV2& rtcpParser) {
+void RTCPReceiver::HandleSDES(RTCPUtility::RTCPParserV2& rtcpParser,
+                              RTCPPacketInformation& rtcpPacketInformation) {
   RTCPUtility::RTCPPacketTypes pktType = rtcpParser.Iterate();
   while (pktType == RTCPPacketTypes::kSdesChunk) {
     HandleSDESChunk(rtcpParser);
     pktType = rtcpParser.Iterate();
   }
+  rtcpPacketInformation.rtcpPacketTypeFlags |= kRtcpSdes;
 }
 
 // no need for critsect we have _criticalSectionRTCPReceiver

webrtc/modules/rtp_rtcp/source/rtcp_receiver.h

@@ -131,7 +131,8 @@ protected:
         RTCPHelp::RTCPPacketInformation& rtcpPacketInformation,
         uint32_t remoteSSRC);
 
-    void HandleSDES(RTCPUtility::RTCPParserV2& rtcpParser);
+    void HandleSDES(RTCPUtility::RTCPParserV2& rtcpParser,
+                    RTCPHelp::RTCPPacketInformation& rtcpPacketInformation);
 
     void HandleSDESChunk(RTCPUtility::RTCPParserV2& rtcpParser);
 

webrtc/modules/rtp_rtcp/source/rtcp_sender.cc

@@ -90,7 +90,6 @@ struct RTCPSender::RtcpContext {
         buffer_size(buffer_size),
         ntp_sec(0),
         ntp_frac(0),
-        jitter_transmission_offset(0),
         position(0) {}
 
   uint8_t* AllocateData(uint32_t bytes) {
@@ -109,7 +108,6 @@ struct RTCPSender::RtcpContext {
   uint32_t buffer_size;
   uint32_t ntp_sec;
   uint32_t ntp_frac;
-  uint32_t jitter_transmission_offset;
   uint32_t position;
 };
 
@@ -146,7 +144,6 @@ RTCPSender::RTCPSender(
       using_nack_(false),
       sending_(false),
       remb_enabled_(false),
-      extended_jitter_report_enabled_(false),
       next_time_to_send_rtcp_(0),
       start_timestamp_(0),
       last_rtp_timestamp_(0),
@@ -176,8 +173,6 @@ RTCPSender::RTCPSender(
   builders_[kRtcpSr] = &RTCPSender::BuildSR;
   builders_[kRtcpRr] = &RTCPSender::BuildRR;
   builders_[kRtcpSdes] = &RTCPSender::BuildSDES;
-  builders_[kRtcpTransmissionTimeOffset] =
-      &RTCPSender::BuildExtendedJitterReport;
   builders_[kRtcpPli] = &RTCPSender::BuildPLI;
   builders_[kRtcpFir] = &RTCPSender::BuildFIR;
   builders_[kRtcpSli] = &RTCPSender::BuildSLI;
@@ -280,16 +275,6 @@ void RTCPSender::SetTMMBRStatus(bool enable) {
   }
 }
 
-bool RTCPSender::IJ() const {
-  CriticalSectionScoped lock(critical_section_rtcp_sender_.get());
-  return extended_jitter_report_enabled_;
-}
-
-void RTCPSender::SetIJStatus(bool enable) {
-  CriticalSectionScoped lock(critical_section_rtcp_sender_.get());
-  extended_jitter_report_enabled_ = enable;
-}
-
 void RTCPSender::SetStartTimestamp(uint32_t start_timestamp) {
   CriticalSectionScoped lock(critical_section_rtcp_sender_.get());
   start_timestamp_ = start_timestamp;
@@ -563,45 +548,6 @@ RTCPSender::BuildResult RTCPSender::BuildRR(RtcpContext* ctx) {
   return BuildResult::kSuccess;
 }
 
-// From RFC 5450: Transmission Time Offsets in RTP Streams.
-//        0                   1                   2                   3
-//        0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
-//       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-//   hdr |V=2|P|    RC   |   PT=IJ=195   |             length            |
-//       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-//       |                      inter-arrival jitter                     |
-//       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-//       .                                                               .
-//       .                                                               .
-//       .                                                               .
-//       |                      inter-arrival jitter                     |
-//       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-//
-//  If present, this RTCP packet must be placed after a receiver report
-//  (inside a compound RTCP packet), and MUST have the same value for RC
-//  (reception report count) as the receiver report.
-
-RTCPSender::BuildResult RTCPSender::BuildExtendedJitterReport(
-    RtcpContext* ctx) {
-  // sanity
-  if (ctx->position + 8 >= IP_PACKET_SIZE)
-    return BuildResult::kTruncated;
-
-  // add picture loss indicator
-  uint8_t RC = 1;
-  *ctx->AllocateData(1) = 0x80 + RC;
-  *ctx->AllocateData(1) = 195;
-
-  // Used fixed length of 2
-  *ctx->AllocateData(1) = 0;
-  *ctx->AllocateData(1) = 1;
-
-  // Add inter-arrival jitter
-  ByteWriter<uint32_t>::WriteBigEndian(ctx->AllocateData(4),
-                                       ctx->jitter_transmission_offset);
-  return BuildResult::kSuccess;
-}
-
 RTCPSender::BuildResult RTCPSender::BuildPLI(RtcpContext* ctx) {
   // sanity
   if (ctx->position + 12 >= IP_PACKET_SIZE)
@@ -1386,8 +1332,6 @@ int RTCPSender::PrepareRTCP(const FeedbackState& feedback_state,
           AddReportBlock(report_block);
         }
       }
-      if (extended_jitter_report_enabled_)
-        SetFlag(kRtcpTransmissionTimeOffset, true);
     }
   }
 

webrtc/modules/rtp_rtcp/source/rtcp_sender.h

@@ -134,11 +134,6 @@ public:
 
  int32_t SetTMMBN(const TMMBRSet* boundingSet, uint32_t maxBitrateKbit);
 
- // Extended jitter report
- bool IJ() const;
-
- void SetIJStatus(bool enable);
-
  int32_t SetApplicationSpecificData(uint8_t subType,
                                     uint32_t name,
                                     const uint8_t* data,
@@ -198,8 +193,6 @@ private:
      EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
  BuildResult BuildRR(RtcpContext* context)
      EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
- BuildResult BuildExtendedJitterReport(RtcpContext* context)
-     EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
  BuildResult BuildSDES(RtcpContext* context)
      EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
  BuildResult BuildPLI(RtcpContext* context)
@@ -242,7 +235,6 @@ private:
  bool using_nack_ GUARDED_BY(critical_section_rtcp_sender_);
  bool sending_ GUARDED_BY(critical_section_rtcp_sender_);
  bool remb_enabled_ GUARDED_BY(critical_section_rtcp_sender_);
- bool extended_jitter_report_enabled_ GUARDED_BY(critical_section_rtcp_sender_);
 
  int64_t next_time_to_send_rtcp_ GUARDED_BY(critical_section_rtcp_sender_);
 

webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc

@@ -272,10 +272,14 @@ class TestTransport : public Transport,
   RTCPHelp::RTCPPacketInformation rtcp_packet_info_;
 };
 
+namespace {
+  static const uint32_t kRemoteBitrateEstimatorMinBitrateBps = 30000;
+  static const int kMaxPacketLength = 1500;
+  static const uint32_t kMainSsrc = 0x11111111;
+}
+
 class RtcpSenderTest : public ::testing::Test {
  protected:
-  static const uint32_t kRemoteBitrateEstimatorMinBitrateBps = 30000;
-
   RtcpSenderTest()
       : over_use_detector_options_(),
         clock_(1335900000),
@@ -298,15 +302,25 @@ class RtcpSenderTest : public ::testing::Test {
 
     rtp_rtcp_impl_ = new ModuleRtpRtcpImpl(configuration);
     rtp_receiver_.reset(RtpReceiver::CreateVideoReceiver(
-        0, &clock_, test_transport_, NULL, rtp_payload_registry_.get()));
+        configuration.id, &clock_, test_transport_, NULL,
+        rtp_payload_registry_.get()));
     rtcp_sender_ =
-        new RTCPSender(0, false, &clock_, receive_statistics_.get(), NULL);
+        new RTCPSender(configuration.id, false, &clock_,
+            receive_statistics_.get(), NULL);
+    rtcp_sender_->SetSSRC(kMainSsrc);
     rtcp_receiver_ =
-        new RTCPReceiver(0, &clock_, false, NULL, NULL, NULL, rtp_rtcp_impl_);
+        new RTCPReceiver(configuration.id, &clock_, false, NULL, NULL, NULL,
+            rtp_rtcp_impl_);
+    rtcp_receiver_->SetRemoteSSRC(kMainSsrc);
+
+    std::set<uint32_t> registered_ssrcs;
+    registered_ssrcs.insert(kMainSsrc);
+    rtcp_receiver_->SetSsrcs(kMainSsrc, registered_ssrcs);
     test_transport_->SetRTCPReceiver(rtcp_receiver_);
     // Initialize
     EXPECT_EQ(0, rtcp_sender_->RegisterSendTransport(test_transport_));
   }
+
   ~RtcpSenderTest() {
     delete rtcp_sender_;
     delete rtcp_receiver_;
@@ -332,7 +346,6 @@ class RtcpSenderTest : public ::testing::Test {
   rtc::scoped_ptr<RemoteBitrateEstimator> remote_bitrate_estimator_;
   rtc::scoped_ptr<ReceiveStatistics> receive_statistics_;
 
-  enum {kMaxPacketLength = 1500};
   uint8_t packet_[kMaxPacketLength];
 };
 
@@ -342,21 +355,14 @@ TEST_F(RtcpSenderTest, RtcpOff) {
   EXPECT_EQ(-1, rtcp_sender_->SendRTCP(feedback_state, kRtcpSr));
 }
 
-TEST_F(RtcpSenderTest, IJStatus) {
-  ASSERT_FALSE(rtcp_sender_->IJ());
-  rtcp_sender_->SetIJStatus(true);
-  EXPECT_TRUE(rtcp_sender_->IJ());
-}
-
 TEST_F(RtcpSenderTest, TestCompound) {
   const bool marker_bit = false;
   const uint8_t payload_type = 100;
   const uint16_t seq_num = 11111;
   const uint32_t timestamp = 1234567;
-  const uint32_t ssrc = 0x11111111;
   size_t packet_length = 0;
-  CreateRtpPacket(marker_bit, payload_type, seq_num, timestamp, ssrc, packet_,
+  CreateRtpPacket(marker_bit, payload_type, seq_num, timestamp, kMainSsrc,
-      &packet_length);
+                  packet_, &packet_length);
   EXPECT_EQ(25u, packet_length);
 
   VideoCodec codec_inst;
@@ -380,25 +386,27 @@ TEST_F(RtcpSenderTest, TestCompound) {
   EXPECT_TRUE(rtp_receiver_->IncomingRtpPacket(header, packet_, packet_length,
                                                payload_specific, true));
 
-  rtcp_sender_->SetIJStatus(true);
+  rtcp_sender_->SetCNAME("Foo");
   rtcp_sender_->SetRTCPStatus(kRtcpCompound);
   RTCPSender::FeedbackState feedback_state = rtp_rtcp_impl_->GetFeedbackState();
   EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpRr));
 
-  // Transmission time offset packet should be received.
+  // Sdes packet should be received, along with report blocks.
   ASSERT_TRUE(test_transport_->rtcp_packet_info_.rtcpPacketTypeFlags &
-      kRtcpTransmissionTimeOffset);
+              kRtcpSdes);
+  EXPECT_GT(test_transport_->rtcp_packet_info_.report_blocks.size(), 0u);
 }
 
 TEST_F(RtcpSenderTest, TestCompound_NoRtpReceived) {
-  rtcp_sender_->SetIJStatus(true);
+  rtcp_sender_->SetCNAME("Foo");
   rtcp_sender_->SetRTCPStatus(kRtcpCompound);
   RTCPSender::FeedbackState feedback_state = rtp_rtcp_impl_->GetFeedbackState();
   EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpRr));
 
-  // Transmission time offset packet should not be received.
+  // Sdes should be received, but no report blocks.
-  ASSERT_FALSE(test_transport_->rtcp_packet_info_.rtcpPacketTypeFlags &
+  ASSERT_TRUE(test_transport_->rtcp_packet_info_.rtcpPacketTypeFlags &
-      kRtcpTransmissionTimeOffset);
+              kRtcpSdes);
+  EXPECT_EQ(0u, test_transport_->rtcp_packet_info_.report_blocks.size());
 }
 
 TEST_F(RtcpSenderTest, TestXrReceiverReferenceTime) {
@@ -477,7 +485,7 @@ TEST_F(RtcpSenderTest, SendsTmmbnIfSetAndEmpty) {
   EXPECT_EQ(0, rtcp_sender_->SetTMMBN(&bounding_set, 3));
   ASSERT_EQ(0U, test_transport_->rtcp_packet_info_.rtcpPacketTypeFlags);
   RTCPSender::FeedbackState feedback_state = rtp_rtcp_impl_->GetFeedbackState();
-  EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state,kRtcpSr));
+  EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpSr));
   // We now expect the packet to show up in the rtcp_packet_info_ of
   // test_transport_.
   ASSERT_NE(0U, test_transport_->rtcp_packet_info_.rtcpPacketTypeFlags);
@@ -512,4 +520,5 @@ TEST_F(RtcpSenderTest, SendsTmmbnIfSetAndValid) {
       &incoming_set));
   EXPECT_EQ(kSourceSsrc, incoming_set.Ssrc(0));
 }
+
 }  // namespace webrtc

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -650,15 +650,6 @@ void ModuleRtpRtcpImpl::SetREMBData(const uint32_t bitrate,
   rtcp_sender_.SetREMBData(bitrate, ssrcs);
 }
 
-// (IJ) Extended jitter report.
-bool ModuleRtpRtcpImpl::IJ() const {
-  return rtcp_sender_.IJ();
-}
-
-void ModuleRtpRtcpImpl::SetIJStatus(const bool enable) {
-  rtcp_sender_.SetIJStatus(enable);
-}
-
 int32_t ModuleRtpRtcpImpl::RegisterSendRtpHeaderExtension(
     const RTPExtensionType type,
     const uint8_t id) {

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -191,11 +191,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp {
   void SetREMBData(uint32_t bitrate,
                    const std::vector<uint32_t>& ssrcs) override;
 
-  // (IJ) Extended jitter report.
-  bool IJ() const override;
-
-  void SetIJStatus(bool enable) override;
-
   // (TMMBR) Temporary Max Media Bit Rate.
   bool TMMBR() const override;