Remove transport_name_ from Channel.

Because of this (seemingly simple) change, I had to change the return
type of transport_name from `const std::string&` to `absl::string_view`
to handle the case when there's no transport assigned.
That in turn caused an avalanche of required updates.

Bug: webrtc:12230, webrtc:11993
Change-Id: I16ec6c6a5fc2f5f7c7de572355a3c6ca924bb9d4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244084
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35617}
This commit is contained in:
Tomas Gunnarsson 2022-01-03 14:59:12 +00:00 committed by WebRTC LUCI CQ
parent 8a5ac16fbf
commit 94f0194d5a
15 changed files with 28 additions and 32 deletions

View File

@ -275,7 +275,7 @@ class RtpHelper : public Base {
} }
void OnPacketSent(const rtc::SentPacket& sent_packet) override {} void OnPacketSent(const rtc::SentPacket& sent_packet) override {}
void OnReadyToSend(bool ready) override { ready_to_send_ = ready; } void OnReadyToSend(bool ready) override { ready_to_send_ = ready; }
void OnNetworkRouteChanged(const std::string& transport_name, void OnNetworkRouteChanged(absl::string_view transport_name,
const rtc::NetworkRoute& network_route) override { const rtc::NetworkRoute& network_route) override {
last_network_route_ = network_route; last_network_route_ = network_route;
++num_network_route_changes_; ++num_network_route_changes_;

View File

@ -187,7 +187,7 @@ class MediaChannel {
virtual void OnReadyToSend(bool ready) = 0; virtual void OnReadyToSend(bool ready) = 0;
// Called when the network route used for sending packets changed. // Called when the network route used for sending packets changed.
virtual void OnNetworkRouteChanged( virtual void OnNetworkRouteChanged(
const std::string& transport_name, absl::string_view transport_name,
const rtc::NetworkRoute& network_route) = 0; const rtc::NetworkRoute& network_route) = 0;
// Creates a new outgoing media stream with SSRCs and CNAME as described // Creates a new outgoing media stream with SSRCs and CNAME as described
// by sp. // by sp.

View File

@ -1864,11 +1864,12 @@ void WebRtcVideoChannel::OnReadyToSend(bool ready) {
} }
void WebRtcVideoChannel::OnNetworkRouteChanged( void WebRtcVideoChannel::OnNetworkRouteChanged(
const std::string& transport_name, absl::string_view transport_name,
const rtc::NetworkRoute& network_route) { const rtc::NetworkRoute& network_route) {
RTC_DCHECK_RUN_ON(&network_thread_checker_); RTC_DCHECK_RUN_ON(&network_thread_checker_);
worker_thread_->PostTask(ToQueuedTask( worker_thread_->PostTask(ToQueuedTask(
task_safety_, [this, name = transport_name, route = network_route] { task_safety_,
[this, name = std::string(transport_name), route = network_route] {
RTC_DCHECK_RUN_ON(&thread_checker_); RTC_DCHECK_RUN_ON(&thread_checker_);
webrtc::RtpTransportControllerSendInterface* transport = webrtc::RtpTransportControllerSendInterface* transport =
call_->GetTransportControllerSend(); call_->GetTransportControllerSend();

View File

@ -167,7 +167,7 @@ class WebRtcVideoChannel : public VideoMediaChannel,
int64_t packet_time_us) override; int64_t packet_time_us) override;
void OnPacketSent(const rtc::SentPacket& sent_packet) override; void OnPacketSent(const rtc::SentPacket& sent_packet) override;
void OnReadyToSend(bool ready) override; void OnReadyToSend(bool ready) override;
void OnNetworkRouteChanged(const std::string& transport_name, void OnNetworkRouteChanged(absl::string_view transport_name,
const rtc::NetworkRoute& network_route) override; const rtc::NetworkRoute& network_route) override;
void SetInterface(NetworkInterface* iface) override; void SetInterface(NetworkInterface* iface) override;

View File

@ -2284,14 +2284,15 @@ void WebRtcVoiceMediaChannel::OnPacketSent(const rtc::SentPacket& sent_packet) {
} }
void WebRtcVoiceMediaChannel::OnNetworkRouteChanged( void WebRtcVoiceMediaChannel::OnNetworkRouteChanged(
const std::string& transport_name, absl::string_view transport_name,
const rtc::NetworkRoute& network_route) { const rtc::NetworkRoute& network_route) {
RTC_DCHECK_RUN_ON(&network_thread_checker_); RTC_DCHECK_RUN_ON(&network_thread_checker_);
call_->OnAudioTransportOverheadChanged(network_route.packet_overhead); call_->OnAudioTransportOverheadChanged(network_route.packet_overhead);
worker_thread_->PostTask(ToQueuedTask( worker_thread_->PostTask(ToQueuedTask(
task_safety_, [this, name = transport_name, route = network_route] { task_safety_,
[this, name = std::string(transport_name), route = network_route] {
RTC_DCHECK_RUN_ON(worker_thread_); RTC_DCHECK_RUN_ON(worker_thread_);
call_->GetTransportControllerSend()->OnNetworkRouteChanged(name, route); call_->GetTransportControllerSend()->OnNetworkRouteChanged(name, route);
})); }));

View File

@ -209,7 +209,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel,
void OnPacketReceived(rtc::CopyOnWriteBuffer packet, void OnPacketReceived(rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) override; int64_t packet_time_us) override;
void OnPacketSent(const rtc::SentPacket& sent_packet) override; void OnPacketSent(const rtc::SentPacket& sent_packet) override;
void OnNetworkRouteChanged(const std::string& transport_name, void OnNetworkRouteChanged(absl::string_view transport_name,
const rtc::NetworkRoute& network_route) override; const rtc::NetworkRoute& network_route) override;
void OnReadyToSend(bool ready) override; void OnReadyToSend(bool ready) override;
bool GetStats(VoiceMediaInfo* info, bool get_and_clear_legacy_stats) override; bool GetStats(VoiceMediaInfo* info, bool get_and_clear_legacy_stats) override;

View File

@ -232,8 +232,6 @@ bool BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) {
rtp_transport_ = rtp_transport; rtp_transport_ = rtp_transport;
if (rtp_transport_) { if (rtp_transport_) {
transport_name_ = rtp_transport_->transport_name();
if (!ConnectToRtpTransport()) { if (!ConnectToRtpTransport()) {
RTC_LOG(LS_ERROR) << "Failed to connect to the new RtpTransport for " RTC_LOG(LS_ERROR) << "Failed to connect to the new RtpTransport for "
<< ToString() << "."; << ToString() << ".";
@ -369,7 +367,7 @@ void BaseChannel::OnNetworkRouteChanged(
// use the same transport name and MediaChannel::OnNetworkRouteChanged cannot // use the same transport name and MediaChannel::OnNetworkRouteChanged cannot
// work correctly. Intentionally leave it broken to simplify the code and // work correctly. Intentionally leave it broken to simplify the code and
// encourage the users to stop using non-muxing RTCP. // encourage the users to stop using non-muxing RTCP.
media_channel_->OnNetworkRouteChanged(transport_name_, new_route); media_channel_->OnNetworkRouteChanged(transport_name(), new_route);
} }
void BaseChannel::SetFirstPacketReceivedCallback( void BaseChannel::SetFirstPacketReceivedCallback(

View File

@ -124,12 +124,11 @@ class BaseChannel : public ChannelInterface,
rtc::Thread* network_thread() const { return network_thread_; } rtc::Thread* network_thread() const { return network_thread_; }
const std::string& content_name() const override { return content_name_; } const std::string& content_name() const override { return content_name_; }
// TODO(deadbeef): This is redundant; remove this. // TODO(deadbeef): This is redundant; remove this.
const std::string& transport_name() const override { absl::string_view transport_name() const override {
RTC_DCHECK_RUN_ON(network_thread()); RTC_DCHECK_RUN_ON(network_thread());
if (rtp_transport_) if (rtp_transport_)
return rtp_transport_->transport_name(); return rtp_transport_->transport_name();
// TODO(tommi): Delete this variable. return "";
return transport_name_;
} }
// This function returns true if using SRTP (DTLS-based keying or SDES). // This function returns true if using SRTP (DTLS-based keying or SDES).
@ -304,14 +303,6 @@ class BaseChannel : public ChannelInterface,
std::function<void()> on_first_packet_received_ std::function<void()> on_first_packet_received_
RTC_GUARDED_BY(network_thread()); RTC_GUARDED_BY(network_thread());
// Won't be set when using raw packet transports. SDP-specific thing.
// TODO(bugs.webrtc.org/12230): Written on network thread, read on
// worker thread (at least).
// TODO(tommi): Remove this variable and instead use rtp_transport_ to
// return the transport name. This variable is currently required for
// "for_test" methods.
std::string transport_name_;
webrtc::RtpTransportInternal* rtp_transport_ webrtc::RtpTransportInternal* rtp_transport_
RTC_GUARDED_BY(network_thread()) = nullptr; RTC_GUARDED_BY(network_thread()) = nullptr;

View File

@ -14,6 +14,7 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "absl/strings/string_view.h"
#include "api/jsep.h" #include "api/jsep.h"
#include "api/media_types.h" #include "api/media_types.h"
#include "media/base/media_channel.h" #include "media/base/media_channel.h"
@ -32,8 +33,11 @@ class ChannelInterface {
virtual MediaChannel* media_channel() const = 0; virtual MediaChannel* media_channel() const = 0;
// Returns a string view for the transport name. Fetching the transport name
// must be done on the network thread only and note that the lifetime of
// the returned object should be assumed to only be the calling scope.
// TODO(deadbeef): This is redundant; remove this. // TODO(deadbeef): This is redundant; remove this.
virtual const std::string& transport_name() const = 0; virtual absl::string_view transport_name() const = 0;
virtual const std::string& content_name() const = 0; virtual const std::string& content_name() const = 0;

View File

@ -2666,8 +2666,8 @@ void PeerConnection::ReportTransportStats() {
media_types_by_transport_name; media_types_by_transport_name;
for (const auto& transceiver : rtp_manager()->transceivers()->UnsafeList()) { for (const auto& transceiver : rtp_manager()->transceivers()->UnsafeList()) {
if (transceiver->internal()->channel()) { if (transceiver->internal()->channel()) {
const std::string& transport_name = std::string transport_name(
transceiver->internal()->channel()->transport_name(); transceiver->internal()->channel()->transport_name());
media_types_by_transport_name[transport_name].insert( media_types_by_transport_name[transport_name].insert(
transceiver->media_type()); transceiver->media_type());
} }
@ -2827,7 +2827,8 @@ bool PeerConnection::OnTransportChanged(
if (dtls_transport) { if (dtls_transport) {
signaling_thread()->PostTask(ToQueuedTask( signaling_thread()->PostTask(ToQueuedTask(
signaling_thread_safety_.flag(), signaling_thread_safety_.flag(),
[this, name = dtls_transport->internal()->transport_name()] { [this,
name = std::string(dtls_transport->internal()->transport_name())] {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
sctp_transport_name_s_ = std::move(name); sctp_transport_name_s_ = std::move(name);
})); }));

View File

@ -2148,7 +2148,7 @@ void RTCStatsCollector::PrepareTransceiverStatsInfosAndCallStats_s_w_n() {
} }
stats.mid = channel->content_name(); stats.mid = channel->content_name();
stats.transport_name = channel->transport_name(); stats.transport_name = std::string(channel->transport_name());
if (media_type == cricket::MEDIA_TYPE_AUDIO) { if (media_type == cricket::MEDIA_TYPE_AUDIO) {
auto* voice_channel = static_cast<cricket::VoiceChannel*>(channel); auto* voice_channel = static_cast<cricket::VoiceChannel*>(channel);

View File

@ -887,7 +887,7 @@ StatsCollector::SessionStats StatsCollector::ExtractSessionInfo_n(
cricket::ChannelInterface* channel = transceiver->internal()->channel(); cricket::ChannelInterface* channel = transceiver->internal()->channel();
if (channel) { if (channel) {
stats.transport_names_by_mid[channel->content_name()] = stats.transport_names_by_mid[channel->content_name()] =
channel->transport_name(); std::string(channel->transport_name());
} }
} }

View File

@ -101,7 +101,7 @@ class VoiceChannelForTesting : public cricket::VoiceChannel {
test_transport_name_(std::move(transport_name)) {} test_transport_name_(std::move(transport_name)) {}
private: private:
const std::string& transport_name() const override { absl::string_view transport_name() const override {
return test_transport_name_; return test_transport_name_;
} }
@ -130,7 +130,7 @@ class VideoChannelForTesting : public cricket::VideoChannel {
test_transport_name_(std::move(transport_name)) {} test_transport_name_(std::move(transport_name)) {}
private: private:
const std::string& transport_name() const override { absl::string_view transport_name() const override {
return test_transport_name_; return test_transport_name_;
} }

View File

@ -26,7 +26,7 @@ class MockChannelInterface : public cricket::ChannelInterface {
public: public:
MOCK_METHOD(cricket::MediaType, media_type, (), (const, override)); MOCK_METHOD(cricket::MediaType, media_type, (), (const, override));
MOCK_METHOD(MediaChannel*, media_channel, (), (const, override)); MOCK_METHOD(MediaChannel*, media_channel, (), (const, override));
MOCK_METHOD(const std::string&, transport_name, (), (const, override)); MOCK_METHOD(absl::string_view, transport_name, (), (const, override));
MOCK_METHOD(const std::string&, content_name, (), (const, override)); MOCK_METHOD(const std::string&, content_name, (), (const, override));
MOCK_METHOD(void, Enable, (bool), (override)); MOCK_METHOD(void, Enable, (bool), (override));
MOCK_METHOD(void, MOCK_METHOD(void,

View File

@ -40,7 +40,7 @@ class MockVoiceMediaChannel : public VoiceMediaChannel {
MOCK_METHOD(void, OnReadyToSend, (bool ready), (override)); MOCK_METHOD(void, OnReadyToSend, (bool ready), (override));
MOCK_METHOD(void, MOCK_METHOD(void,
OnNetworkRouteChanged, OnNetworkRouteChanged,
(const std::string& transport_name, (absl::string_view transport_name,
const rtc::NetworkRoute& network_route), const rtc::NetworkRoute& network_route),
(override)); (override));
MOCK_METHOD(bool, AddSendStream, (const StreamParams& sp), (override)); MOCK_METHOD(bool, AddSendStream, (const StreamParams& sp), (override));