diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 1721e41d2a..4ab336c19f 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -20,6 +20,7 @@ #include "absl/algorithm/container.h" #include "absl/memory/memory.h" +#include "absl/strings/match.h" #include "absl/strings/string_view.h" #include "api/array_view.h" #include "api/crypto/crypto_options.h" @@ -194,7 +195,9 @@ std::string GetSetDescriptionErrorMessage(cricket::ContentSource source, const RTCError& error) { rtc::StringBuilder oss; oss << "Failed to set " << (source == cricket::CS_LOCAL ? "local" : "remote") - << " " << SdpTypeToString(type) << " sdp: " << error.message(); + << " " << SdpTypeToString(type) << " sdp: "; + RTC_DCHECK(!absl::StartsWith(error.message(), oss.str())) << error.message(); + oss << error.message(); return oss.Release(); } @@ -728,8 +731,10 @@ class SdpOfferAnswerHandler::RemoteDescriptionOperation { : handler_(handler), desc_(std::move(desc)), observer_(std::move(observer)), - operations_chain_callback_(std::move(operations_chain_callback)) { + operations_chain_callback_(std::move(operations_chain_callback)), + unified_plan_(handler_->IsUnifiedPlan()) { if (!desc_) { + type_ = static_cast(-1); InvalidParam("SessionDescription is NULL."); } else { type_ = desc_->GetType(); @@ -747,20 +752,27 @@ class SdpOfferAnswerHandler::RemoteDescriptionOperation { // Notifies the observer that the operation is complete and releases the // reference to the observer. void SignalCompletion() { - if (observer_) { - observer_->OnSetRemoteDescriptionComplete(error_); - observer_ = nullptr; // Only fire the notification once. + if (!observer_) + return; + + if (!error_.ok() && type_ != static_cast(-1)) { + std::string error_message = + GetSetDescriptionErrorMessage(cricket::CS_REMOTE, type_, error_); + RTC_LOG(LS_ERROR) << error_message; + error_.set_message(std::move(error_message)); } + + observer_->OnSetRemoteDescriptionComplete(error_); + observer_ = nullptr; // Only fire the notification once. } // If a session error has occurred the PeerConnection is in a possibly // inconsistent state so fail right away. bool HaveSessionError() { RTC_DCHECK(ok()); - if (handler_->session_error() == SessionError::kNone) - return false; - SetError(RTCErrorType::INTERNAL_ERROR, handler_->GetSessionErrorMsg()); - return true; + if (handler_->session_error() != SessionError::kNone) + InternalError(handler_->GetSessionErrorMsg()); + return !ok(); } // Returns true if the operation was a rollback operation. If this function @@ -771,7 +783,7 @@ class SdpOfferAnswerHandler::RemoteDescriptionOperation { RTC_DCHECK(ok()); if (type_ != SdpType::kRollback) { // Check if we can do an implicit rollback. - if (type_ == SdpType::kOffer && handler_->IsUnifiedPlan() && + if (type_ == SdpType::kOffer && unified_plan_ && handler_->pc_->configuration()->enable_implicit_rollback && handler_->signaling_state() == PeerConnectionInterface::kHaveLocalOffer) { @@ -780,7 +792,7 @@ class SdpOfferAnswerHandler::RemoteDescriptionOperation { return false; } - if (handler_->IsUnifiedPlan()) { + if (unified_plan_) { error_ = handler_->Rollback(type_); } else if (type_ == SdpType::kRollback) { Unsupported("Rollback not supported in Plan B"); @@ -808,51 +820,99 @@ class SdpOfferAnswerHandler::RemoteDescriptionOperation { bundle_groups_by_mid_ = GetBundleGroupsByMid(description()); error_ = handler_->ValidateSessionDescription( desc_.get(), cricket::CS_REMOTE, bundle_groups_by_mid_); - if (!error_.ok()) { - std::string error_message = - GetSetDescriptionErrorMessage(cricket::CS_REMOTE, type_, error_); - RTC_LOG(LS_ERROR) << error_message; - error_.set_message(std::move(error_message)); - return false; - } - - return true; + return ok(); } - // TODO(tommi): Instead of calling `handler_->ApplyRemoteDescription` here, - // pass the ownership of `this` to that method and break down the steps - // currently implemented in that function into smaller methods implemented - // by this class. Once we have all of this broken down, we can start avoiding - // the embedded calls to Invoke() and apply the description asynchronously. - bool ApplyRemoteDescription() { + // Transfers ownership of the session description object over to `handler_`. + bool ReplaceRemoteDescriptionAndCheckEror() { RTC_DCHECK_RUN_ON(handler_->signaling_thread()); RTC_DCHECK(ok()); - // TODO(tommi): It's not ideal to move desc_ ownership. - error_ = handler_->ApplyRemoteDescription(std::move(desc_), - bundle_groups_by_mid()); - // `desc` may be destroyed at this point. + RTC_DCHECK(desc_); + RTC_DCHECK(!replaced_remote_description_); +#if RTC_DCHECK_IS_ON + const auto* existing_remote_description = handler_->remote_description(); +#endif - if (!error_.ok()) { - // If ApplyRemoteDescription fails, the PeerConnection could be in an - // inconsistent state, so act conservatively here and set the session - // error so that future calls to SetLocalDescription/SetRemoteDescription - // fail. - handler_->SetSessionError(SessionError::kContent, error_.message()); - std::string error_message = - GetSetDescriptionErrorMessage(cricket::CS_REMOTE, type_, error_); - RTC_LOG(LS_ERROR) << error_message; - error_.set_message(std::move(error_message)); - return false; + error_ = handler_->ReplaceRemoteDescription(std::move(desc_), type_, + &replaced_remote_description_); + + if (ok()) { +#if RTC_DCHECK_IS_ON + // Sanity check that our `old_remote_description()` method always returns + // the same value as `remote_description()` did before the call to + // ReplaceRemoteDescription. + RTC_DCHECK_EQ(existing_remote_description, old_remote_description()); +#endif + } else { + SetAsSessionError(); } - return true; + return ok(); + } + + bool UpdateChannels() { + RTC_DCHECK(ok()); + RTC_DCHECK(!desc_) << "ReplaceRemoteDescription hasn't been called"; + + const auto* remote_description = handler_->remote_description(); + + const cricket::SessionDescription* session_desc = + remote_description->description(); + + // Transport and Media channels will be created only when offer is set. + if (unified_plan_) { + error_ = handler_->UpdateTransceiversAndDataChannels( + cricket::CS_REMOTE, *remote_description, + handler_->local_description(), old_remote_description(), + bundle_groups_by_mid_); + } else { + // Media channels will be created only when offer is set. These may use + // new transports just created by PushdownTransportDescription. + if (type_ == SdpType::kOffer) { + // TODO(mallinath) - Handle CreateChannel failure, as new local + // description is applied. Restore back to old description. + error_ = handler_->CreateChannels(*session_desc); + } + // Remove unused channels if MediaContentDescription is rejected. + handler_->RemoveUnusedChannels(session_desc); + } + + return ok(); + } + + bool UpdateSessionState() { + RTC_DCHECK(ok()); + error_ = handler_->UpdateSessionState( + type_, cricket::CS_REMOTE, + handler_->remote_description()->description(), bundle_groups_by_mid_); + if (!ok()) + SetAsSessionError(); + return ok(); + } + + bool UseCandidatesInRemoteDescription() { + RTC_DCHECK(ok()); + if (handler_->local_description() && + !handler_->UseCandidatesInRemoteDescription()) { + InvalidParam(kInvalidCandidates); + } + return ok(); } // Convenience getter for desc_->GetType(). SdpType type() const { return type_; } - + bool unified_plan() const { return unified_plan_; } cricket::SessionDescription* description() { return desc_->description(); } + const SessionDescriptionInterface* old_remote_description() const { + RTC_DCHECK(!desc_) << "Called before replacing the remote description"; + if (type_ == SdpType::kAnswer) + return replaced_remote_description_.get(); + return replaced_remote_description_ + ? replaced_remote_description_.get() + : handler_->current_remote_description(); + } + // Returns a reference to a cached map of bundle groups ordered by mid. // Note that this will only be valid after a successful call to // `IsDescriptionValid`. @@ -872,18 +932,35 @@ class SdpOfferAnswerHandler::RemoteDescriptionOperation { SetError(RTCErrorType::INVALID_PARAMETER, std::move(message)); } + void InternalError(std::string message) { + SetError(RTCErrorType::INTERNAL_ERROR, std::move(message)); + } + void SetError(RTCErrorType type, std::string message) { RTC_DCHECK(ok()) << "Overwriting an existing error?"; error_ = RTCError(type, std::move(message)); } + // Called when the PeerConnection could be in an inconsistent state and we set + // the session error so that future calls to + // SetLocalDescription/SetRemoteDescription fail. + void SetAsSessionError() { + RTC_DCHECK(!ok()); + handler_->SetSessionError(SessionError::kContent, error_.message()); + } + SdpOfferAnswerHandler* const handler_; std::unique_ptr desc_; + // Keeps the replaced session description object alive while the operation + // is taking place since methods that depend on `old_remote_description()` + // for updating the state, need it. + std::unique_ptr replaced_remote_description_; rtc::scoped_refptr observer_; std::function operations_chain_callback_; RTCError error_ = RTCError::OK(); std::map bundle_groups_by_mid_; SdpType type_; + const bool unified_plan_; }; // Used by parameterless SetLocalDescription() to create an offer or answer. // Upon completion of creating the session description, SetLocalDescription() is @@ -1446,10 +1523,12 @@ RTCError SdpOfferAnswerHandler::ApplyLocalDescription( } if (IsUnifiedPlan()) { - RTCError error = UpdateTransceiversAndDataChannels( + error = UpdateTransceiversAndDataChannels( cricket::CS_LOCAL, *local_description(), old_local_description, remote_description(), bundle_groups_by_mid); if (!error.ok()) { + RTC_LOG(LS_ERROR) << error.message() << " (" << SdpTypeToString(type) + << ")"; return error; } std::vector> remove_list; @@ -1510,6 +1589,8 @@ RTCError SdpOfferAnswerHandler::ApplyLocalDescription( // description is applied. Restore back to old description. RTCError error = CreateChannels(*local_description()->description()); if (!error.ok()) { + RTC_LOG(LS_ERROR) << error.message() << " (" << SdpTypeToString(type) + << ")"; return error; } } @@ -1521,13 +1602,13 @@ RTCError SdpOfferAnswerHandler::ApplyLocalDescription( local_description()->description(), bundle_groups_by_mid); if (!error.ok()) { + RTC_LOG(LS_ERROR) << error.message() << " (" << SdpTypeToString(type) + << ")"; return error; } - if (remote_description()) { - // Now that we have a local description, we can push down remote candidates. - UseCandidatesInSessionDescription(remote_description()); - } + // Now that we have a local description, we can push down remote candidates. + UseCandidatesInRemoteDescription(); pending_ice_restarts_.clear(); if (session_error() != SessionError::kNone) { @@ -1680,102 +1761,72 @@ void SdpOfferAnswerHandler::SetRemoteDescription( }); } -RTCError SdpOfferAnswerHandler::ApplyRemoteDescription( +RTCError SdpOfferAnswerHandler::ReplaceRemoteDescription( std::unique_ptr desc, - const std::map& - bundle_groups_by_mid) { + SdpType sdp_type, + std::unique_ptr* replaced_description) { + RTC_DCHECK(replaced_description); + if (sdp_type == SdpType::kAnswer) { + *replaced_description = pending_remote_description_ + ? std::move(pending_remote_description_) + : std::move(current_remote_description_); + current_remote_description_ = std::move(desc); + pending_remote_description_ = nullptr; + current_local_description_ = std::move(pending_local_description_); + } else { + *replaced_description = std::move(pending_remote_description_); + pending_remote_description_ = std::move(desc); + } + + // The session description to apply now must be accessed by + // `remote_description()`. + const cricket::SessionDescription* session_desc = + remote_description()->description(); + + // Report statistics about any use of simulcast. + ReportSimulcastApiVersion(kSimulcastVersionApplyRemoteDescription, + *session_desc); + + // NOTE: This will perform an Invoke() to the network thread. + return transport_controller()->SetRemoteDescription(sdp_type, session_desc); +} + +void SdpOfferAnswerHandler::ApplyRemoteDescription( + std::unique_ptr operation) { TRACE_EVENT0("webrtc", "SdpOfferAnswerHandler::ApplyRemoteDescription"); RTC_DCHECK_RUN_ON(signaling_thread()); - RTC_DCHECK(desc); + RTC_DCHECK(operation->description()); // Invalidate the [legacy] stats cache to make sure that it gets updated next // time getStats() gets called, as updating the session description affects // the stats. pc_->stats()->InvalidateCache(); - // Take a reference to the old remote description since it's used below to - // compare against the new remote description. When setting the new remote - // description, grab ownership of the replaced session description in case it - // is the same as `old_remote_description`, to keep it alive for the duration - // of the method. - const SessionDescriptionInterface* old_remote_description = - remote_description(); - std::unique_ptr replaced_remote_description; - SdpType type = desc->GetType(); - if (type == SdpType::kAnswer) { - replaced_remote_description = pending_remote_description_ - ? std::move(pending_remote_description_) - : std::move(current_remote_description_); - current_remote_description_ = std::move(desc); - pending_remote_description_ = nullptr; - current_local_description_ = std::move(pending_local_description_); - } else { - replaced_remote_description = std::move(pending_remote_description_); - pending_remote_description_ = std::move(desc); - } - // The session description to apply now must be accessed by - // `remote_description()`. - RTC_DCHECK(remote_description()); + if (!operation->ReplaceRemoteDescriptionAndCheckEror()) + return; - // Report statistics about any use of simulcast. - ReportSimulcastApiVersion(kSimulcastVersionApplyRemoteDescription, - *remote_description()->description()); - - RTCError error = PushdownTransportDescription(cricket::CS_REMOTE, type); - if (!error.ok()) { - return error; - } - - const bool is_unified_plan = IsUnifiedPlan(); - - // Transport and Media channels will be created only when offer is set. - if (is_unified_plan) { - RTCError error = UpdateTransceiversAndDataChannels( - cricket::CS_REMOTE, *remote_description(), local_description(), - old_remote_description, bundle_groups_by_mid); - if (!error.ok()) { - return error; - } - } else { - // Media channels will be created only when offer is set. These may use new - // transports just created by PushdownTransportDescription. - if (type == SdpType::kOffer) { - // TODO(mallinath) - Handle CreateChannel failure, as new local - // description is applied. Restore back to old description. - RTCError error = CreateChannels(*remote_description()->description()); - if (!error.ok()) { - return error; - } - } - // Remove unused channels if MediaContentDescription is rejected. - RemoveUnusedChannels(remote_description()->description()); - } + if (!operation->UpdateChannels()) + return; // NOTE: Candidates allocation will be initiated only when // SetLocalDescription is called. - error = UpdateSessionState(type, cricket::CS_REMOTE, - remote_description()->description(), - bundle_groups_by_mid); - if (!error.ok()) { - return error; - } + if (!operation->UpdateSessionState()) + return; - if (local_description() && - !UseCandidatesInSessionDescription(remote_description())) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, kInvalidCandidates); - } + if (!operation->UseCandidatesInRemoteDescription()) + return; - if (old_remote_description) { + if (operation->old_remote_description()) { for (const cricket::ContentInfo& content : - old_remote_description->description()->contents()) { + operation->old_remote_description()->description()->contents()) { // Check if this new SessionDescription contains new ICE ufrag and // password that indicates the remote peer requests an ICE restart. // TODO(deadbeef): When we start storing both the current and pending // remote description, this should reset pending_ice_restarts and compare // against the current description. - if (CheckForRemoteIceRestart(old_remote_description, remote_description(), - content.name)) { - if (type == SdpType::kOffer) { + if (CheckForRemoteIceRestart(operation->old_remote_description(), + remote_description(), content.name)) { + if (operation->type() == SdpType::kOffer) { pending_ice_restarts_.insert(content.name); } } else { @@ -1787,14 +1838,14 @@ RTCError SdpOfferAnswerHandler::ApplyRemoteDescription( // description plus any candidates added since then. We should remove // this once we're sure it won't break anything. WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription( - old_remote_description, content.name, mutable_remote_description()); + operation->old_remote_description(), content.name, + mutable_remote_description()); } } } - if (session_error() != SessionError::kNone) { - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, GetSessionErrorMsg()); - } + if (operation->HaveSessionError()) + return; // Set the the ICE connection state to connecting since the connection may // become writable with peer reflexive candidates before any remote candidate @@ -1818,8 +1869,8 @@ RTCError SdpOfferAnswerHandler::ApplyRemoteDescription( data_channel_controller()->AllocateSctpSids(role); } - if (is_unified_plan) { - ApplyRemoteDescriptionUpdateTransceiverState(type); + if (operation->unified_plan()) { + ApplyRemoteDescriptionUpdateTransceiverState(operation->type()); } const cricket::AudioContentDescription* audio_desc = @@ -1835,13 +1886,13 @@ RTCError SdpOfferAnswerHandler::ApplyRemoteDescription( remote_peer_supports_msid_ = true; } - if (!is_unified_plan) { + if (!operation->unified_plan()) { PlanBUpdateSendersAndReceivers( GetFirstAudioContent(remote_description()->description()), audio_desc, GetFirstVideoContent(remote_description()->description()), video_desc); } - if (type == SdpType::kAnswer) { + if (operation->type() == SdpType::kAnswer) { if (local_ice_credentials_to_replace_->SatisfiesIceRestart( *current_local_description_)) { local_ice_credentials_to_replace_->ClearIceCredentials(); @@ -1850,7 +1901,10 @@ RTCError SdpOfferAnswerHandler::ApplyRemoteDescription( RemoveStoppedTransceivers(); } - return RTCError::OK(); + // Consider the operation complete at this point. + operation->SignalCompletion(); + + SetRemoteDescriptionPostProcess(operation->type() == SdpType::kAnswer); } void SdpOfferAnswerHandler::ApplyRemoteDescriptionUpdateTransceiverState( @@ -2294,13 +2348,7 @@ void SdpOfferAnswerHandler::DoSetRemoteDescription( if (!operation->IsDescriptionValid()) return; - if (!operation->ApplyRemoteDescription()) - return; - - // Consider the operation complete at this point. - operation->SignalCompletion(); - - SetRemoteDescriptionPostProcess(operation->type() == SdpType::kAnswer); + ApplyRemoteDescription(std::move(operation)); } // Called after a DoSetRemoteDescription operation completes. @@ -3138,17 +3186,16 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription( RTC_DCHECK_EQ(SessionError::kNone, session_error()); if (!sdesc || !sdesc->description()) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, kInvalidSdp); + return RTCError(RTCErrorType::INVALID_PARAMETER, kInvalidSdp); } SdpType type = sdesc->GetType(); if ((source == cricket::CS_LOCAL && !ExpectSetLocalDescription(type)) || (source == cricket::CS_REMOTE && !ExpectSetRemoteDescription(type))) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_STATE, - (rtc::StringBuilder("Called in wrong state: ") - << PeerConnectionInterface::AsString(signaling_state())) - .Release()); + return RTCError(RTCErrorType::INVALID_STATE, + (rtc::StringBuilder("Called in wrong state: ") + << PeerConnectionInterface::AsString(signaling_state())) + .Release()); } RTCError error = ValidateMids(*sdesc->description()); @@ -3169,14 +3216,12 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription( // Verify ice-ufrag and ice-pwd. if (!VerifyIceUfragPwdPresent(sdesc->description(), bundle_groups_by_mid)) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - kSdpWithoutIceUfragPwd); + return RTCError(RTCErrorType::INVALID_PARAMETER, kSdpWithoutIceUfragPwd); } if (!pc_->ValidateBundleSettings(sdesc->description(), bundle_groups_by_mid)) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - kBundleWithoutRtcpMux); + return RTCError(RTCErrorType::INVALID_PARAMETER, kBundleWithoutRtcpMux); } // TODO(skvlad): When the local rtcp-mux policy is Require, reject any @@ -3192,8 +3237,7 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription( if (!MediaSectionsHaveSameCount(*offer_desc, *sdesc->description()) || !MediaSectionsInSameOrder(*offer_desc, nullptr, *sdesc->description(), type)) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - kMlineMismatchInAnswer); + return RTCError(RTCErrorType::INVALID_PARAMETER, kMlineMismatchInAnswer); } } else { // The re-offers should respect the order of m= sections in current @@ -3217,8 +3261,8 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription( if (current_desc && !MediaSectionsInSameOrder(*current_desc, secondary_current_desc, *sdesc->description(), type)) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - kMlineMismatchInSubsequentOffer); + return RTCError(RTCErrorType::INVALID_PARAMETER, + kMlineMismatchInSubsequentOffer); } } @@ -3233,10 +3277,10 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription( if ((desc.type() == cricket::MEDIA_TYPE_AUDIO || desc.type() == cricket::MEDIA_TYPE_VIDEO) && desc.streams().size() > 1u) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - "Media section has more than one track specified " - "with a=ssrc lines which is not supported with " - "Unified Plan."); + return RTCError( + RTCErrorType::INVALID_PARAMETER, + "Media section has more than one track specified with a=ssrc lines " + "which is not supported with Unified Plan."); } } } @@ -3264,9 +3308,9 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiversAndDataChannels( if (pc_->configuration()->bundle_policy == PeerConnectionInterface::kBundlePolicyMaxBundle && bundle_groups_by_mid.empty()) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - "max-bundle configured but session description " - "has no BUNDLE group"); + return RTCError( + RTCErrorType::INVALID_PARAMETER, + "max-bundle configured but session description has no BUNDLE group"); } } @@ -3324,8 +3368,7 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiversAndDataChannels( } else if (media_type == cricket::MEDIA_TYPE_UNSUPPORTED) { RTC_LOG(LS_INFO) << "Ignoring unsupported media type"; } else { - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, - "Unknown section type."); + return RTCError(RTCErrorType::INTERNAL_ERROR, "Unknown section type."); } } @@ -3370,8 +3413,8 @@ SdpOfferAnswerHandler::AssociateTransceiver( } if (!transceiver) { // This may happen normally when media sections are rejected. - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - "Transceiver not found based on m-line index"); + return RTCError(RTCErrorType::INVALID_PARAMETER, + "Transceiver not found based on m-line index"); } } else { RTC_DCHECK_EQ(source, cricket::CS_REMOTE); @@ -3431,9 +3474,8 @@ SdpOfferAnswerHandler::AssociateTransceiver( } if (transceiver->media_type() != media_desc->type()) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_PARAMETER, - "Transceiver type does not match media description type."); + return RTCError(RTCErrorType::INVALID_PARAMETER, + "Transceiver type does not match media description type."); } if (media_desc->HasSimulcast()) { @@ -3492,9 +3534,8 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiverChannel( channel = CreateVideoChannel(content.name); } if (!channel) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INTERNAL_ERROR, - "Failed to create channel for mid=" + content.name); + return RTCError(RTCErrorType::INTERNAL_ERROR, + "Failed to create channel for mid=" + content.name); } transceiver->internal()->SetChannel(channel); } @@ -3519,8 +3560,8 @@ RTCError SdpOfferAnswerHandler::UpdateDataChannel( if (!data_channel_controller()->data_channel_transport()) { RTC_LOG(LS_INFO) << "Creating data channel, mid=" << content.mid(); if (!CreateDataChannel(content.name)) { - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, - "Failed to create data channel."); + return RTCError(RTCErrorType::INTERNAL_ERROR, + "Failed to create data channel."); } } } @@ -4330,12 +4371,14 @@ RTCError SdpOfferAnswerHandler::PushdownMediaDescription( RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK(sdesc); + // Note: This will perform an Invoke over to the worker thread, which we'll + // also do in a loop below. if (!UpdatePayloadTypeDemuxingState(source, bundle_groups_by_mid)) { // Note that this is never expected to fail, since RtpDemuxer doesn't return // an error when changing payload type demux criteria, which is all this // does. - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, - "Failed to update payload type demuxing state."); + return RTCError(RTCErrorType::INTERNAL_ERROR, + "Failed to update payload type demuxing state."); } // Push down the new SDP media section for each audio/video transceiver. @@ -4370,20 +4413,14 @@ RTCError SdpOfferAnswerHandler::PushdownMediaDescription( // - crbug.com/1157227 // - crbug.com/1187289 for (const auto& entry : channels) { - RTCError error = - pc_->worker_thread()->Invoke(RTC_FROM_HERE, [&]() { - std::string error; - bool success = - (source == cricket::CS_LOCAL) - ? entry.first->SetLocalContent(entry.second, type, error) - : entry.first->SetRemoteContent(entry.second, type, error); - if (!success) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, error); - } - return RTCError::OK(); - }); - if (!error.ok()) { - return error; + std::string error; + bool success = pc_->worker_thread()->Invoke(RTC_FROM_HERE, [&]() { + return (source == cricket::CS_LOCAL) + ? entry.first->SetLocalContent(entry.second, type, error) + : entry.first->SetRemoteContent(entry.second, type, error); + }); + if (!success) { + return RTCError(RTCErrorType::INVALID_PARAMETER, error); } } @@ -4543,9 +4580,9 @@ void SdpOfferAnswerHandler::UpdateEndedRemoteMediaStreams() { } } -bool SdpOfferAnswerHandler::UseCandidatesInSessionDescription( - const SessionDescriptionInterface* remote_desc) { +bool SdpOfferAnswerHandler::UseCandidatesInRemoteDescription() { RTC_DCHECK_RUN_ON(signaling_thread()); + auto* remote_desc = remote_description(); if (!remote_desc) { return true; } @@ -4559,7 +4596,7 @@ bool SdpOfferAnswerHandler::UseCandidatesInSessionDescription( if (!ReadyToUseRemoteCandidate(candidate, remote_desc, &valid)) { if (valid) { RTC_LOG(LS_INFO) - << "UseCandidatesInSessionDescription: Not ready to use " + << "UseCandidatesInRemoteDescription: Not ready to use " "candidate."; } continue; @@ -4674,8 +4711,8 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) { !rtp_manager()->GetAudioTransceiver()->internal()->channel()) { cricket::VoiceChannel* voice_channel = CreateVoiceChannel(voice->name); if (!voice_channel) { - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, - "Failed to create voice channel."); + return RTCError(RTCErrorType::INTERNAL_ERROR, + "Failed to create voice channel."); } rtp_manager()->GetAudioTransceiver()->internal()->SetChannel(voice_channel); } @@ -4685,8 +4722,8 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) { !rtp_manager()->GetVideoTransceiver()->internal()->channel()) { cricket::VideoChannel* video_channel = CreateVideoChannel(video->name); if (!video_channel) { - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, - "Failed to create video channel."); + return RTCError(RTCErrorType::INTERNAL_ERROR, + "Failed to create video channel."); } rtp_manager()->GetVideoTransceiver()->internal()->SetChannel(video_channel); } @@ -4695,8 +4732,8 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) { if (data && !data->rejected && !data_channel_controller()->data_channel_transport()) { if (!CreateDataChannel(data->name)) { - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, - "Failed to create data channel."); + return RTCError(RTCErrorType::INTERNAL_ERROR, + "Failed to create data channel."); } } diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h index 3318a4fe50..7349999807 100644 --- a/pc/sdp_offer_answer.h +++ b/pc/sdp_offer_answer.h @@ -232,10 +232,14 @@ class SdpOfferAnswerHandler : public SdpStateProvider, std::unique_ptr desc, const std::map& bundle_groups_by_mid); - RTCError ApplyRemoteDescription( + void ApplyRemoteDescription( + std::unique_ptr operation); + + RTCError ReplaceRemoteDescription( std::unique_ptr desc, - const std::map& - bundle_groups_by_mid); + SdpType sdp_type, + std::unique_ptr* replaced_description) + RTC_RUN_ON(signaling_thread()); // Part of ApplyRemoteDescription steps specific to Unified Plan. void ApplyRemoteDescriptionUpdateTransceiverState(SdpType sdp_type); @@ -496,9 +500,11 @@ class SdpOfferAnswerHandler : public SdpStateProvider, // exist. void UpdateEndedRemoteMediaStreams(); - // Uses all remote candidates in `remote_desc` in this session. - bool UseCandidatesInSessionDescription( - const SessionDescriptionInterface* remote_desc); + // Uses all remote candidates in the currently set remote_description(). + // If no remote description is currently set (nullptr), the return value will + // be true. If `UseCandidate()` fails for any candidate in the remote + // description, the return value will be false. + bool UseCandidatesInRemoteDescription(); // Uses `candidate` in this session. bool UseCandidate(const IceCandidateInterface* candidate); // Returns true if we are ready to push down the remote candidate.