Always include the MID RTP header extension on every packet when configured
This removes the optimization that would stop sending the MID RTP
header extension when an RTCP report block is received. The old
implementation was not flexible enough for the API, and making
those changes is too involved at this time as we need this to work
now to unblock other work.
Bug: webrtc:4050
Change-Id: I099f8e9047a40993d93bcda9164eb82fdf810387
Reviewed-on: https://webrtc-review.googlesource.com/67192
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22776}diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn
index b817e12..5ef9e14 100644
--- a/modules/rtp_rtcp/BUILD.gn
+++ b/modules/rtp_rtcp/BUILD.gn
@@ -123,8 +123,6 @@
"source/forward_error_correction.h",
"source/forward_error_correction_internal.cc",
"source/forward_error_correction_internal.h",
- "source/mid_oracle.cc",
- "source/mid_oracle.h",
"source/packet_loss_stats.cc",
"source/packet_loss_stats.h",
"source/playout_delay_oracle.cc",
@@ -337,7 +335,6 @@
"source/flexfec_header_reader_writer_unittest.cc",
"source/flexfec_receiver_unittest.cc",
"source/flexfec_sender_unittest.cc",
- "source/mid_oracle_unittest.cc",
"source/nack_rtx_unittest.cc",
"source/packet_loss_stats_unittest.cc",
"source/playout_delay_oracle_unittest.cc",
diff --git a/modules/rtp_rtcp/source/mid_oracle.cc b/modules/rtp_rtcp/source/mid_oracle.cc
deleted file mode 100644
index 6d91ad5..0000000
--- a/modules/rtp_rtcp/source/mid_oracle.cc
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Copyright (c) 2018 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.
- */
-
-#include "modules/rtp_rtcp/source/mid_oracle.h"
-
-namespace webrtc {
-
-MidOracle::MidOracle(const std::string& mid) : mid_(mid) {}
-
-MidOracle::~MidOracle() = default;
-
-void MidOracle::OnReceivedRtcpReportBlocks(
- const ReportBlockList& report_blocks) {
- if (!send_mid_) {
- return;
- }
- for (const RTCPReportBlock& report_block : report_blocks) {
- if (report_block.source_ssrc == ssrc_) {
- send_mid_ = false;
- break;
- }
- }
-}
-
-} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/mid_oracle.h b/modules/rtp_rtcp/source/mid_oracle.h
deleted file mode 100644
index 541ee27..0000000
--- a/modules/rtp_rtcp/source/mid_oracle.h
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- * Copyright (c) 2018 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.
- */
-
-#ifndef MODULES_RTP_RTCP_SOURCE_MID_ORACLE_H_
-#define MODULES_RTP_RTCP_SOURCE_MID_ORACLE_H_
-
-#include <stdint.h>
-#include <string>
-
-#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
-#include "rtc_base/constructormagic.h"
-
-namespace webrtc {
-
-// The MidOracle instructs an RTPSender to send the MID header extension on a
-// new SSRC stream until it receives an RTCP report block for that stream (which
-// implies that the remote side is able to demultiplex it and can remember the
-// MID --> SSRC mapping).
-class MidOracle {
- public:
- explicit MidOracle(const std::string& mid);
- ~MidOracle();
-
- // MID value to put into the header extension.
- const std::string& mid() const { return mid_; }
-
- // True if the MID header extension should be included on the next outgoing
- // packet.
- bool send_mid() const { return send_mid_; }
-
- // Change the RTP stream SSRC. This will cause MIDs to be included until an
- // RTCP report block lists this SSRC as received.
- void SetSsrc(uint32_t ssrc) {
- ssrc_ = ssrc;
- send_mid_ = true;
- }
-
- // Feedback to decide when to stop sending the MID header extension.
- void OnReceivedRtcpReportBlocks(const ReportBlockList& report_blocks);
-
- private:
- const std::string mid_;
- bool send_mid_ = false;
- uint32_t ssrc_ = 0;
-
- RTC_DISALLOW_COPY_AND_ASSIGN(MidOracle);
-};
-
-} // namespace webrtc
-
-#endif // MODULES_RTP_RTCP_SOURCE_MID_ORACLE_H_
diff --git a/modules/rtp_rtcp/source/mid_oracle_unittest.cc b/modules/rtp_rtcp/source/mid_oracle_unittest.cc
deleted file mode 100644
index 06e7930..0000000
--- a/modules/rtp_rtcp/source/mid_oracle_unittest.cc
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * Copyright (c) 2018 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.
- */
-
-#include "modules/rtp_rtcp/source/mid_oracle.h"
-
-#include "rtc_base/logging.h"
-#include "test/gtest.h"
-
-namespace {
-
-using ::webrtc::RTCPReportBlock;
-using ::webrtc::MidOracle;
-
-RTCPReportBlock ReportBlockWithSourceSsrc(uint32_t ssrc) {
- RTCPReportBlock report_block;
- report_block.source_ssrc = ssrc;
- return report_block;
-}
-
-TEST(MidOracleTest, DoNotSendMidInitially) {
- MidOracle mid_oracle("mid");
- EXPECT_FALSE(mid_oracle.send_mid());
-}
-
-TEST(MidOracleTest, SendMidOnceSsrcSet) {
- MidOracle mid_oracle("mid");
- mid_oracle.SetSsrc(52);
- EXPECT_TRUE(mid_oracle.send_mid());
-}
-
-TEST(MidOracleTest, IgnoreReportBlockWithUnknownSourceSsrc) {
- MidOracle mid_oracle("mid");
- mid_oracle.SetSsrc(52);
- mid_oracle.OnReceivedRtcpReportBlocks({ReportBlockWithSourceSsrc(63)});
- EXPECT_TRUE(mid_oracle.send_mid());
-}
-
-TEST(MidOracleTest, StopSendingMidAfterReceivingRtcpReportWithKnownSourceSsrc) {
- constexpr uint32_t kSsrc = 52;
-
- MidOracle mid_oracle("mid");
- mid_oracle.SetSsrc(kSsrc);
- mid_oracle.OnReceivedRtcpReportBlocks({ReportBlockWithSourceSsrc(kSsrc)});
-
- EXPECT_FALSE(mid_oracle.send_mid());
-}
-
-TEST(MidOracleTest, RestartSendingMidWhenSsrcChanges) {
- constexpr uint32_t kInitialSsrc = 52;
- constexpr uint32_t kChangedSsrc = 63;
-
- MidOracle mid_oracle("mid");
- mid_oracle.SetSsrc(kInitialSsrc);
- mid_oracle.OnReceivedRtcpReportBlocks(
- {ReportBlockWithSourceSsrc(kInitialSsrc)});
- mid_oracle.SetSsrc(kChangedSsrc);
-
- EXPECT_TRUE(mid_oracle.send_mid());
-}
-
-} // namespace
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index aa133e8..8db6791 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -329,9 +329,6 @@
void RTPSender::SetRtxSsrc(uint32_t ssrc) {
rtc::CritScope lock(&send_critsect_);
ssrc_rtx_.emplace(ssrc);
- if (mid_oracle_rtx_) {
- mid_oracle_rtx_->SetSsrc(ssrc);
- }
}
uint32_t RTPSender::RtxSsrc() const {
@@ -731,16 +728,6 @@
void RTPSender::OnReceivedRtcpReportBlocks(
const ReportBlockList& report_blocks) {
playout_delay_oracle_.OnReceivedRtcpReportBlocks(report_blocks);
-
- {
- rtc::CritScope lock(&send_critsect_);
- if (mid_oracle_) {
- mid_oracle_->OnReceivedRtcpReportBlocks(report_blocks);
- }
- if (mid_oracle_rtx_) {
- mid_oracle_rtx_->OnReceivedRtcpReportBlocks(report_blocks);
- }
- }
}
// Called from pacer when we can send the packet.
@@ -1087,9 +1074,9 @@
packet->SetExtension<PlayoutDelayLimits>(
playout_delay_oracle_.playout_delay());
}
- if (mid_oracle_ && mid_oracle_->send_mid()) {
+ if (!mid_.empty()) {
// This is a no-op if the MID header extension is not registered.
- packet->SetExtension<RtpMid>(mid_oracle_->mid());
+ packet->SetExtension<RtpMid>(mid_);
}
return packet;
}
@@ -1163,9 +1150,6 @@
if (!sequence_number_forced_) {
sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber);
}
- if (mid_oracle_) {
- mid_oracle_->SetSsrc(ssrc);
- }
}
uint32_t RTPSender::SSRC() const {
@@ -1177,28 +1161,7 @@
void RTPSender::SetMid(const std::string& mid) {
// This is configured via the API.
rtc::CritScope lock(&send_critsect_);
-
- // Cannot change MID once sending.
- RTC_DCHECK(!sending_media_);
-
- // Cannot change the MID if it is already set.
- if (mid_oracle_) {
- RTC_DCHECK_EQ(mid_oracle_->mid(), mid);
- return;
- }
- if (mid_oracle_rtx_) {
- RTC_DCHECK_EQ(mid_oracle_rtx_->mid(), mid);
- return;
- }
-
- mid_oracle_ = rtc::MakeUnique<MidOracle>(mid);
- if (ssrc_) {
- mid_oracle_->SetSsrc(*ssrc_);
- }
- mid_oracle_rtx_ = rtc::MakeUnique<MidOracle>(mid);
- if (ssrc_rtx_) {
- mid_oracle_rtx_->SetSsrc(*ssrc_rtx_);
- }
+ mid_ = mid;
}
rtc::Optional<uint32_t> RTPSender::FlexfecSsrc() const {
@@ -1286,9 +1249,9 @@
rtx_packet->SetSsrc(*ssrc_rtx_);
// Possibly include the MID header extension.
- if (mid_oracle_rtx_ && mid_oracle_rtx_->send_mid()) {
+ if (!mid_.empty()) {
// This is a no-op if the MID header extension is not registered.
- rtx_packet->SetExtension<RtpMid>(mid_oracle_rtx_->mid());
+ rtx_packet->SetExtension<RtpMid>(mid_);
}
}
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index c0af340..4b4b9c6 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -24,7 +24,6 @@
#include "modules/rtp_rtcp/include/flexfec_sender.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
-#include "modules/rtp_rtcp/source/mid_oracle.h"
#include "modules/rtp_rtcp/source/playout_delay_oracle.h"
#include "modules/rtp_rtcp/source/rtp_packet_history.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
@@ -321,7 +320,8 @@
// Must be explicitly set by the application, use of rtc::Optional
// only to keep track of correct use.
rtc::Optional<uint32_t> ssrc_ RTC_GUARDED_BY(send_critsect_);
- std::unique_ptr<MidOracle> mid_oracle_ RTC_GUARDED_BY(send_critsect_);
+ // MID value to send in the MID header extension.
+ std::string mid_ RTC_GUARDED_BY(send_critsect_);
uint32_t last_rtp_timestamp_ RTC_GUARDED_BY(send_critsect_);
int64_t capture_time_ms_ RTC_GUARDED_BY(send_critsect_);
int64_t last_timestamp_time_ms_ RTC_GUARDED_BY(send_critsect_);
@@ -330,7 +330,6 @@
std::vector<uint32_t> csrcs_ RTC_GUARDED_BY(send_critsect_);
int rtx_ RTC_GUARDED_BY(send_critsect_);
rtc::Optional<uint32_t> ssrc_rtx_ RTC_GUARDED_BY(send_critsect_);
- std::unique_ptr<MidOracle> mid_oracle_rtx_ RTC_GUARDED_BY(send_critsect_);
// Mapping rtx_payload_type_map_[associated] = rtx.
std::map<int8_t, int8_t> rtx_payload_type_map_ RTC_GUARDED_BY(send_critsect_);
size_t rtp_overhead_bytes_per_packet_ RTC_GUARDED_BY(send_critsect_);
