Move Destroy/Create steps for DataChannelTransport to PeerConnection.

This moves steps from the sdp code for pc state over to the PC class
and slightly simplifies the contract between the two classes.
Moving forward it's easier to consolidate those steps in the PC
class with other grouped operations e.g. during teardown.

Also removing GetDataMid() method in favor of the sctp_mid() property.

Bug: none
Change-Id: I938f953099d327377abd94e6b2c9ece803d88e40
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324300
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40981}
This commit is contained in:
Tommi 2023-10-21 16:47:56 +02:00 committed by WebRTC LUCI CQ
parent 5e31e8148a
commit 840cf78600
7 changed files with 59 additions and 102 deletions

View File

@ -2115,20 +2115,31 @@ void PeerConnection::OnSelectedCandidatePairChanged(
Observer()->OnIceSelectedCandidatePairChanged(event);
}
absl::optional<std::string> PeerConnection::GetDataMid() const {
bool PeerConnection::CreateDataChannelTransport(absl::string_view mid) {
RTC_DCHECK_RUN_ON(signaling_thread());
return sctp_mid_s_;
}
RTC_DCHECK(!sctp_mid().has_value() || mid == sctp_mid().value());
RTC_LOG(LS_INFO) << "Creating data channel, mid=" << mid;
absl::optional<std::string> transport_name =
network_thread()->BlockingCall([&] {
RTC_DCHECK_RUN_ON(network_thread());
return SetupDataChannelTransport_n(mid);
});
if (!transport_name)
return false;
void PeerConnection::SetSctpDataInfo(absl::string_view mid,
absl::string_view transport_name) {
RTC_DCHECK_RUN_ON(signaling_thread());
sctp_mid_s_ = std::string(mid);
SetSctpTransportName(std::string(transport_name));
SetSctpTransportName(transport_name.value());
return true;
}
void PeerConnection::ResetSctpDataInfo() {
void PeerConnection::DestroyDataChannelTransport(RTCError error) {
RTC_DCHECK_RUN_ON(signaling_thread());
network_thread()->BlockingCall([&] {
RTC_DCHECK_RUN_ON(network_thread());
TeardownDataChannelTransport_n(error);
});
sctp_mid_s_.reset();
SetSctpTransportName("");
}

View File

@ -394,15 +394,8 @@ class PeerConnection : public PeerConnectionInternal,
const std::map<std::string, const cricket::ContentGroup*>&
bundle_groups_by_mid) override;
// Returns the MID for the data section associated with the
// SCTP data channel, if it has been set. If no data
// channels are configured this will return nullopt.
absl::optional<std::string> GetDataMid() const override;
void SetSctpDataInfo(absl::string_view mid,
absl::string_view transport_name) override;
void ResetSctpDataInfo() override;
bool CreateDataChannelTransport(absl::string_view mid) override;
void DestroyDataChannelTransport(RTCError error) override;
// Asynchronously calls SctpTransport::Start() on the network thread for
// `sctp_mid()` if set. Called as part of setting the local description.
@ -430,9 +423,9 @@ class PeerConnection : public PeerConnectionInternal,
// this session.
bool SrtpRequired() const override;
absl::optional<std::string> SetupDataChannelTransport_n(
absl::string_view mid) override RTC_RUN_ON(network_thread());
void TeardownDataChannelTransport_n(RTCError error) override
absl::optional<std::string> SetupDataChannelTransport_n(absl::string_view mid)
RTC_RUN_ON(network_thread());
void TeardownDataChannelTransport_n(RTCError error)
RTC_RUN_ON(network_thread());
const FieldTrialsView& trials() const override { return *trials_; }

View File

@ -95,7 +95,6 @@ class PeerConnectionSdpMethods {
const std::map<std::string, const cricket::ContentGroup*>&
bundle_groups_by_mid) = 0;
virtual absl::optional<std::string> GetDataMid() const = 0;
// Internal implementation for AddTransceiver family of methods. If
// `fire_callback` is set, fires OnRenegotiationNeeded callback if successful.
virtual RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>>
@ -117,17 +116,14 @@ class PeerConnectionSdpMethods {
// Returns true if SRTP (either using DTLS-SRTP or SDES) is required by
// this session.
virtual bool SrtpRequired() const = 0;
// Configures the data channel transport on the network thread.
// The return value will be unset if an error occurs. If the setup succeeded
// the return value will be set and contain the name of the transport
// (empty string if a name isn't available).
virtual absl::optional<std::string> SetupDataChannelTransport_n(
absl::string_view mid) = 0;
virtual void TeardownDataChannelTransport_n(RTCError error) = 0;
virtual void SetSctpDataInfo(absl::string_view mid,
absl::string_view transport_name) = 0;
virtual void ResetSctpDataInfo() = 0;
// Initializes the data channel transport for the peerconnection instance.
// This will have the effect that `sctp_mid()` and `sctp_transport_name()`
// will return a set value (even though it might be an empty string) and the
// dc transport will be initialized on the network thread.
virtual bool CreateDataChannelTransport(absl::string_view mid) = 0;
// Tears down the data channel transport state and clears the `sctp_mid()` and
// `sctp_transport_name()` properties.
virtual void DestroyDataChannelTransport(RTCError error) = 0;
virtual const FieldTrialsView& trials() const = 0;
virtual void ClearStatsCache() = 0;

View File

@ -3791,13 +3791,15 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiversAndDataChannels(
return error;
}
} else if (media_type == cricket::MEDIA_TYPE_DATA) {
if (pc_->GetDataMid() && new_content.name != *(pc_->GetDataMid())) {
const auto data_mid = pc_->sctp_mid();
if (data_mid && new_content.name != data_mid.value()) {
// Ignore all but the first data section.
RTC_LOG(LS_INFO) << "Ignoring data media section with MID="
<< new_content.name;
continue;
}
RTCError error = UpdateDataChannel(source, new_content, bundle_group);
RTCError error =
UpdateDataChannelTransport(source, new_content, bundle_group);
if (!error.ok()) {
return error;
}
@ -3979,7 +3981,7 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiverChannel(
return RTCError::OK();
}
RTCError SdpOfferAnswerHandler::UpdateDataChannel(
RTCError SdpOfferAnswerHandler::UpdateDataChannelTransport(
cricket::ContentSource source,
const cricket::ContentInfo& content,
const cricket::ContentGroup* bundle_group) {
@ -3991,8 +3993,8 @@ RTCError SdpOfferAnswerHandler::UpdateDataChannel(
sb << "Rejected data channel transport with mid=" << content.mid();
RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA, sb.Release());
error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE);
DestroyDataChannelTransport(error);
} else if (!CreateDataChannel(content.name)) {
pc_->DestroyDataChannelTransport(error);
} else if (!pc_->CreateDataChannelTransport(content.name)) {
LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR,
"Failed to create data channel.");
}
@ -4323,8 +4325,9 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanOffer(
session_options->media_description_options.push_back(
GetMediaDescriptionOptionsForRejectedData(mid));
} else {
RTC_CHECK(pc_->GetDataMid());
if (mid == *(pc_->GetDataMid())) {
const auto data_mid = pc_->sctp_mid();
RTC_CHECK(data_mid);
if (mid == data_mid.value()) {
session_options->media_description_options.push_back(
GetMediaDescriptionOptionsForActiveData(mid));
} else {
@ -4365,9 +4368,9 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanOffer(
}
// Lastly, add a m-section if we have requested local data channels and an
// m section does not already exist.
if (!pc_->GetDataMid() && data_channel_controller()->HasDataChannels()) {
if (!pc_->sctp_mid() && data_channel_controller()->HasDataChannels()) {
// Attempt to recycle a stopped m-line.
// TODO(crbug.com/1442604): GetDataMid() should return the mid if one was
// TODO(crbug.com/1442604): sctp_mid() should return the mid if one was
// ever created but rejected.
bool recycled = false;
for (size_t i = 0; i < session_options->media_description_options.size();
@ -4510,7 +4513,7 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanAnswer(
// Reject all data sections if data channels are disabled.
// Reject a data section if it has already been rejected.
// Reject all data sections except for the first one.
if (content.rejected || content.name != *(pc_->GetDataMid())) {
if (content.rejected || content.name != *(pc_->sctp_mid())) {
session_options->media_description_options.push_back(
GetMediaDescriptionOptionsForRejectedData(content.name));
} else {
@ -5003,14 +5006,14 @@ void SdpOfferAnswerHandler::RemoveUnusedChannels(
RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA,
"No data channel section in the description.");
error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE);
DestroyDataChannelTransport(error);
pc_->DestroyDataChannelTransport(error);
} else if (data_info->rejected) {
rtc::StringBuilder sb;
sb << "Rejected data channel with mid=" << data_info->name << ".";
RTCError error(RTCErrorType::OPERATION_ERROR_WITH_DATA, sb.Release());
error.set_error_detail(RTCErrorDetailType::DATA_CHANNEL_FAILURE);
DestroyDataChannelTransport(error);
pc_->DestroyDataChannelTransport(error);
}
}
@ -5193,7 +5196,7 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) {
}
const cricket::ContentInfo* data = cricket::GetFirstDataContent(&desc);
if (data && !data->rejected && !CreateDataChannel(data->name)) {
if (data && !data->rejected && !pc_->CreateDataChannelTransport(data->name)) {
LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR,
"Failed to create data channel.");
}
@ -5201,33 +5204,6 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) {
return RTCError::OK();
}
bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) {
RTC_DCHECK_RUN_ON(signaling_thread());
RTC_DCHECK(!pc_->sctp_mid().has_value() || mid == pc_->sctp_mid().value());
RTC_LOG(LS_INFO) << "Creating data channel, mid=" << mid;
absl::optional<std::string> transport_name =
context_->network_thread()->BlockingCall([&] {
RTC_DCHECK_RUN_ON(context_->network_thread());
return pc_->SetupDataChannelTransport_n(mid);
});
if (!transport_name)
return false;
pc_->SetSctpDataInfo(mid, *transport_name);
return true;
}
void SdpOfferAnswerHandler::DestroyDataChannelTransport(RTCError error) {
RTC_DCHECK_RUN_ON(signaling_thread());
context_->network_thread()->BlockingCall(
[&, data_channel_controller = data_channel_controller()] {
RTC_DCHECK_RUN_ON(context_->network_thread());
pc_->TeardownDataChannelTransport_n(error);
});
pc_->ResetSctpDataInfo();
}
void SdpOfferAnswerHandler::DestroyAllChannels() {
RTC_DCHECK_RUN_ON(signaling_thread());
if (!transceivers()) {
@ -5252,7 +5228,7 @@ void SdpOfferAnswerHandler::DestroyAllChannels() {
}
}
DestroyDataChannelTransport({});
pc_->DestroyDataChannelTransport({});
}
void SdpOfferAnswerHandler::GenerateMediaDescriptionOptions(

View File

@ -359,9 +359,9 @@ class SdpOfferAnswerHandler : public SdpStateProvider {
// Either creates or destroys the local data channel according to the given
// media section.
RTCError UpdateDataChannel(cricket::ContentSource source,
const cricket::ContentInfo& content,
const cricket::ContentGroup* bundle_group)
RTCError UpdateDataChannelTransport(cricket::ContentSource source,
const cricket::ContentInfo& content,
const cricket::ContentGroup* bundle_group)
RTC_RUN_ON(signaling_thread());
// Check if a call to SetLocalDescription is acceptable with a session
// description of the given type.
@ -526,12 +526,6 @@ class SdpOfferAnswerHandler : public SdpStateProvider {
// This method will also delete any existing media channels before creating.
RTCError CreateChannels(const cricket::SessionDescription& desc);
bool CreateDataChannel(const std::string& mid);
// Destroys the RTP data channel transport and/or the SCTP data channel
// transport and clears it.
void DestroyDataChannelTransport(RTCError error);
// Generates MediaDescriptionOptions for the `session_opts` based on existing
// local description or remote description.
void GenerateMediaDescriptionOptions(

View File

@ -339,9 +339,6 @@ class FakePeerConnectionBase : public PeerConnectionInternal {
return false;
}
absl::optional<std::string> GetDataMid() const override {
return absl::nullopt;
}
RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>> AddTransceiver(
cricket::MediaType media_type,
rtc::scoped_refptr<MediaStreamTrackInterface> track,
@ -358,14 +355,10 @@ class FakePeerConnectionBase : public PeerConnectionInternal {
Call* call_ptr() override { return nullptr; }
bool SrtpRequired() const override { return false; }
absl::optional<std::string> SetupDataChannelTransport_n(
absl::string_view mid) override {
return absl::nullopt;
bool CreateDataChannelTransport(absl::string_view mid) override {
return false;
}
void TeardownDataChannelTransport_n(RTCError error) override {}
void SetSctpDataInfo(absl::string_view mid,
absl::string_view transport_name) override {}
void ResetSctpDataInfo() override {}
void DestroyDataChannelTransport(RTCError error) override {}
const FieldTrialsView& trials() const override { return field_trials_; }

View File

@ -248,7 +248,6 @@ class MockPeerConnectionInternal : public PeerConnectionInternal {
(const cricket::SessionDescription*,
(const std::map<std::string, const cricket::ContentGroup*>&)),
(override));
MOCK_METHOD(absl::optional<std::string>, GetDataMid, (), (const, override));
MOCK_METHOD(RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>>,
AddTransceiver,
(cricket::MediaType,
@ -263,16 +262,11 @@ class MockPeerConnectionInternal : public PeerConnectionInternal {
(override));
MOCK_METHOD(Call*, call_ptr, (), (override));
MOCK_METHOD(bool, SrtpRequired, (), (const, override));
MOCK_METHOD(absl::optional<std::string>,
SetupDataChannelTransport_n,
(absl::string_view mid),
MOCK_METHOD(bool,
CreateDataChannelTransport,
(absl::string_view),
(override));
MOCK_METHOD(void, TeardownDataChannelTransport_n, (RTCError), (override));
MOCK_METHOD(void,
SetSctpDataInfo,
(absl::string_view, absl::string_view),
(override));
MOCK_METHOD(void, ResetSctpDataInfo, (), (override));
MOCK_METHOD(void, DestroyDataChannelTransport, (RTCError error), (override));
MOCK_METHOD(const FieldTrialsView&, trials, (), (const, override));
// PeerConnectionInternal