diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h index 3dccee8a46..cddaa212dc 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/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& ssrcs) = 0; - /* - * (IJ) Extended jitter report. - */ - virtual bool IJ() const = 0; - - virtual void SetIJStatus(bool enable) = 0; - /* * (TMMBR) Temporary Max Media Bit Rate */ diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 56291ef188..13837d3ccd 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/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& ssrcs)); - MOCK_CONST_METHOD0(IJ, - bool()); - MOCK_METHOD1(SetIJStatus, void(const bool)); MOCK_CONST_METHOD0(TMMBR, bool()); MOCK_METHOD1(SetTMMBRStatus, void(const bool enable)); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index 55974bf74b..732772c9f7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/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 diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index 569af90130..6ae47a6423 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/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); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index d15de162d9..55ca9a72f9 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/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::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); } } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h index afe2eaeaf1..67d58aa8b8 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h +++ b/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_); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index a0a79aa020..eb76da67a6 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/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 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 remote_bitrate_estimator_; rtc::scoped_ptr 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_, - &packet_length); + CreateRtpPacket(marker_bit, payload_type, seq_num, timestamp, kMainSsrc, + 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. - ASSERT_FALSE(test_transport_->rtcp_packet_info_.rtcpPacketTypeFlags & - kRtcpTransmissionTimeOffset); + // Sdes should be received, but no report blocks. + ASSERT_TRUE(test_transport_->rtcp_packet_info_.rtcpPacketTypeFlags & + 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 diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index b92ac5920f..c8b4b33840 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/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) { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index f4bed46e68..5548e540d0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/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& 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;