zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Address failing wpt test cases for the rollback feature

Browse Source 
Also fix https://crbug.com/1025542.

Bug: chromium:1025557, chromium:1025542
Change-Id: I614ca6282f1f1d4d1e2cd507c0efd6bc6a898408
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159932
Commit-Queue: Eldar Rello <elrello@microsoft.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29909}
This commit is contained in:
Eldar Rello
2019-11-25 18:49:44 +02:00
committed by Commit Bot
parent 6620506591
commit 353a718dfd
6 changed files with 174 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
pc/jsep_transport_controller.cc
View File

@ -473,14 +473,19 @@ void JsepTransportController::SetMediaTransportSettings(
use_datagram_transport_for_data_channels_receive_only;
}
void JsepTransportController::RollbackTransportForMid(const std::string& mid) {
void JsepTransportController::RollbackTransportForMids(
const std::vector<std::string>& mids) {
if (!network_thread_->IsCurrent()) {
network_thread_->Invoke<void>(RTC_FROM_HERE,
[=] { RollbackTransportForMid(mid); });
[=] { RollbackTransportForMids(mids); });
return;
}
RemoveTransportForMid(mid);
MaybeDestroyJsepTransport(mid);
for (auto&& mid : mids) {
RemoveTransportForMid(mid);
}
for (auto&& mid : mids) {
MaybeDestroyJsepTransport(mid);
}
}
rtc::scoped_refptr<webrtc::IceTransportInterface>

6
pc/jsep_transport_controller.h
View File

@ -241,9 +241,9 @@ class JsepTransportController : public sigslot::has_slots<> {
bool use_datagram_transport_for_data_channels,
bool use_datagram_transport_for_data_channels_receive_only);
// TODO(elrello): For now the rollback only removes mid to transport mapping
// and deletes unused transport, but doesn't consider anything more complex.
void RollbackTransportForMid(const std::string& mid);
// TODO(elrello): For now the rollback only removes mid to transport mappings
// and deletes unused transports, but doesn't consider anything more complex.
void RollbackTransportForMids(const std::vector<std::string>& mids);
// If media transport is present enabled and supported,
// when this method is called, it creates a media transport and generates its

89
pc/peer_connection.cc
View File

@ -985,6 +985,28 @@ bool PeerConnectionInterface::RTCConfiguration::operator!=(
return !(*this == o);
}
void PeerConnection::TransceiverStableState::set_newly_created() {
RTC_DCHECK(!has_m_section_);
newly_created_ = true;
}
void PeerConnection::TransceiverStableState::SetMSectionIfUnset(
absl::optional<std::string> mid,
absl::optional<size_t> mline_index) {
if (!has_m_section_) {
mid_ = mid;
mline_index_ = mline_index;
has_m_section_ = true;
}
}
void PeerConnection::TransceiverStableState::SetRemoteStreamIdsIfUnset(
const std::vector<std::string>& ids) {
if (!remote_stream_ids_.has_value()) {
remote_stream_ids_ = ids;
}
}
// Generate a RTCP CNAME when a PeerConnection is created.
std::string GenerateRtcpCname() {
std::string cname;
@ -1619,6 +1641,7 @@ PeerConnection::AddTrackUnifiedPlan(
}
transceiver->sender()->SetTrack(track);
transceiver->internal()->sender_internal()->set_stream_ids(stream_ids);
transceiver->internal()->set_reused_for_addtrack(true);
} else {
cricket::MediaType media_type =
(track->kind() == MediaStreamTrackInterface::kAudioKind
@ -3242,6 +3265,9 @@ RTCError PeerConnection::ApplyRemoteDescription(
// The remote description has signaled the stream IDs.
stream_ids = media_desc->streams()[0].stream_ids();
}
transceiver_stable_states_by_transceivers_[transceiver]
.SetRemoteStreamIdsIfUnset(transceiver->receiver()->stream_ids());
RTC_LOG(LS_INFO) << "Processing the MSIDs for MID=" << content->name
<< " (" << GetStreamIdsString(stream_ids) << ").";
SetAssociatedRemoteStreams(transceiver->internal()->receiver_internal(),
@ -3778,9 +3804,8 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source,
transceiver->internal()->set_direction(
RtpTransceiverDirection::kRecvOnly);
if (type == SdpType::kOffer) {
transceiver_stable_states_by_transceivers_[transceiver] =
TransceiverStableState(RtpTransceiverDirection::kRecvOnly,
absl::nullopt, absl::nullopt, true);
transceiver_stable_states_by_transceivers_[transceiver]
.set_newly_created();
}
}
// Check if the offer indicated simulcast but the answer rejected it.
@ -3816,19 +3841,14 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source,
}
}
if (type == SdpType::kOffer) {
// Make sure we don't overwrite existing stable states and that the
// state is really going to change when adding new record to the map.
auto it = transceiver_stable_states_by_transceivers_.find(transceiver);
if (it == transceiver_stable_states_by_transceivers_.end() &&
(transceiver->internal()->mid() != content.name ||
transceiver->internal()->mline_index() != mline_index)) {
transceiver_stable_states_by_transceivers_[transceiver] =
TransceiverStableState(transceiver->internal()->direction(),
transceiver->internal()->mid(),
transceiver->internal()->mline_index(), false);
bool state_changes = transceiver->internal()->mid() != content.name ||
transceiver->internal()->mline_index() != mline_index;
if (state_changes) {
transceiver_stable_states_by_transceivers_[transceiver]
.SetMSectionIfUnset(transceiver->internal()->mid(),
transceiver->internal()->mline_index());
}
}
// Associate the found or created RtpTransceiver with the m= section by
// setting the value of the RtpTransceiver's mid property to the MID of the m=
// section, and establish a mapping between the transceiver and the index of
@ -8017,23 +8037,43 @@ RTCError PeerConnection::Rollback(SdpType sdp_type) {
}
RTC_DCHECK_RUN_ON(signaling_thread());
RTC_DCHECK(IsUnifiedPlan());
std::vector<std::string> mids;
std::vector<rtc::scoped_refptr<MediaStreamInterface>> all_added_streams;
std::vector<rtc::scoped_refptr<MediaStreamInterface>> all_removed_streams;
std::vector<rtc::scoped_refptr<RtpReceiverInterface>> removed_receivers;
for (auto&& transceivers_stable_state_pair :
transceiver_stable_states_by_transceivers_) {
auto transceiver = transceivers_stable_state_pair.first;
auto state = transceivers_stable_state_pair.second;
if (state.remote_stream_ids()) {
std::vector<rtc::scoped_refptr<MediaStreamInterface>> added_streams;
std::vector<rtc::scoped_refptr<MediaStreamInterface>> removed_streams;
SetAssociatedRemoteStreams(transceiver->internal()->receiver_internal(),
state.remote_stream_ids().value(),
&added_streams, &removed_streams);
all_added_streams.insert(all_added_streams.end(), added_streams.begin(),
added_streams.end());
all_removed_streams.insert(all_removed_streams.end(),
removed_streams.begin(),
removed_streams.end());
if (!state.has_m_section() && !state.newly_created()) {
continue;
}
}
RTC_DCHECK(transceiver->internal()->mid().has_value());
std::string mid = transceiver->internal()->mid().value();
transport_controller_->RollbackTransportForMid(mid);
mids.push_back(mid);
DestroyTransceiverChannel(transceiver);
if (signaling_state() == PeerConnectionInterface::kHaveRemoteOffer &&
transceiver->receiver()) {
Observer()->OnRemoveTrack(transceiver->receiver());
removed_receivers.push_back(transceiver->receiver());
}
if (state.newly_created()) {
// Remove added transceivers with no added track.
if (transceiver->internal()->sender()->track()) {
if (transceiver->internal()->reused_for_addtrack()) {
transceiver->internal()->set_created_by_addtrack(true);
} else {
int remaining_transceiver_count = 0;
@ -8047,15 +8087,26 @@ RTCError PeerConnection::Rollback(SdpType sdp_type) {
}
transceiver->internal()->sender_internal()->set_transport(nullptr);
transceiver->internal()->receiver_internal()->set_transport(nullptr);
transceiver->internal()->set_direction(state.direction());
transceiver->internal()->set_mid(state.mid());
transceiver->internal()->set_mline_index(state.mline_index());
}
transport_controller_->RollbackTransportForMids(mids);
transceiver_stable_states_by_transceivers_.clear();
pending_local_description_.reset();
pending_remote_description_.reset();
ChangeSignalingState(PeerConnectionInterface::kStable);
// Once all processing has finished, fire off callbacks.
for (const auto& receiver : removed_receivers) {
Observer()->OnRemoveTrack(receiver);
}
for (const auto& stream : all_added_streams) {
Observer()->OnAddStream(stream);
}
for (const auto& stream : all_removed_streams) {
Observer()->OnRemoveStream(stream);
}
// The assumption is that in case of implicit rollback UpdateNegotiationNeeded
// gets called in SetRemoteDescription.
if (sdp_type == SdpType::kRollback) {

27
pc/peer_connection.h
View File

@ -403,23 +403,26 @@ class PeerConnection : public PeerConnectionInternal,
class TransceiverStableState {
public:
TransceiverStableState() {}
TransceiverStableState(RtpTransceiverDirection direction,
absl::optional<std::string> mid,
absl::optional<size_t> mline_index,
bool newly_created)
: direction_(direction),
mid_(mid),
mline_index_(mline_index),
newly_created_(newly_created) {}
RtpTransceiverDirection direction() const { return direction_; }
void set_newly_created();
void SetMSectionIfUnset(absl::optional<std::string> mid,
absl::optional<size_t> mline_index);
void SetRemoteStreamIdsIfUnset(const std::vector<std::string>& ids);
absl::optional<std::string> mid() const { return mid_; }
absl::optional<size_t> mline_index() const { return mline_index_; }
absl::optional<std::vector<std::string>> remote_stream_ids() const {
return remote_stream_ids_;
}
bool has_m_section() const { return has_m_section_; }
bool newly_created() const { return newly_created_; }
private:
RtpTransceiverDirection direction_ = RtpTransceiverDirection::kRecvOnly;
absl::optional<std::string> mid_;
absl::optional<size_t> mline_index_;
absl::optional<std::vector<std::string>> remote_stream_ids_;
// Indicates that mid value from stable state has been captured and
// that rollback has to restore the transceiver. Also protects against
// subsequent overwrites.
bool has_m_section_ = false;
// Indicates that the transceiver was created as part of applying a
// description to track potential need for removing transceiver during
// rollback.
@ -1365,6 +1368,10 @@ class PeerConnection : public PeerConnectionInternal,
std::map<rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>,
TransceiverStableState>
transceiver_stable_states_by_transceivers_;
// Holds remote stream ids for transceivers from stable state.
std::map<rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>,
std::vector<std::string>>
remote_stream_ids_by_transceivers_;
std::vector<
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>>
transceivers_; // TODO(bugs.webrtc.org/9987): Accessed on both signaling

66
pc/peer_connection_jsep_unittest.cc
View File

@ -1879,6 +1879,27 @@ TEST_F(PeerConnectionJsepTest, RollbackKeepsTransceiverAndClearsMid) {
EXPECT_EQ(callee->observer()->remove_track_events_.size(), 1u);
}
TEST_F(PeerConnectionJsepTest,
RollbackKeepsTransceiverAfterAddTrackEvenWhenTrackIsNulled) {
auto caller = CreatePeerConnection();
caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
auto callee = CreatePeerConnection();
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
callee->AddAudioTrack("a");
callee->pc()->GetTransceivers()[0]->sender()->SetTrack(nullptr);
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->sender()->track(), nullptr);
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
// Transceiver can't be removed as track was added to it.
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
// Mid got cleared to make it reusable.
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
// Transceiver should be counted as addTrack-created after rollback.
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
EXPECT_EQ(callee->observer()->remove_track_events_.size(), 1u);
}
TEST_F(PeerConnectionJsepTest, RollbackRestoresMid) {
auto caller = CreatePeerConnection();
caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
@ -1986,6 +2007,30 @@ TEST_F(PeerConnectionJsepTest, RollbackToNegotiatedStableState) {
audio_transport); // Audio transport is still the same.
}
TEST_F(PeerConnectionJsepTest, RollbackHasToDestroyTransport) {
RTCConfiguration config;
config.sdp_semantics = SdpSemantics::kUnifiedPlan;
config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle;
auto pc = CreatePeerConnection(config);
pc->AddAudioTrack("a");
pc->AddVideoTrack("b");
EXPECT_TRUE(pc->CreateOfferAndSetAsLocal());
auto offer = pc->CreateOffer();
EXPECT_EQ(pc->pc()->GetTransceivers().size(), 2u);
auto audio_transport =
pc->pc()->GetTransceivers()[0]->sender()->dtls_transport();
EXPECT_EQ(pc->pc()->GetTransceivers()[0]->sender()->dtls_transport(),
pc->pc()->GetTransceivers()[1]->sender()->dtls_transport());
EXPECT_NE(pc->pc()->GetTransceivers()[1]->sender()->dtls_transport(),
nullptr);
EXPECT_TRUE(pc->SetRemoteDescription(pc->CreateRollback()));
EXPECT_TRUE(pc->SetLocalDescription(std::move(offer)));
EXPECT_NE(pc->pc()->GetTransceivers()[0]->sender()->dtls_transport(),
nullptr);
EXPECT_NE(pc->pc()->GetTransceivers()[0]->sender()->dtls_transport(),
audio_transport);
}
TEST_F(PeerConnectionJsepTest, RollbackLocalDirectionChange) {
auto caller = CreatePeerConnection();
caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
@ -2063,4 +2108,25 @@ TEST_F(PeerConnectionJsepTest, NoRollbackNeeded) {
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
}
TEST_F(PeerConnectionJsepTest, RollbackMultipleStreamChanges) {
auto callee = CreatePeerConnection();
auto caller = CreatePeerConnection();
caller->AddAudioTrack("a_1", {"id_1"});
caller->AddVideoTrack("v_0", {"id_0"}); // Provide an extra stream id.
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
caller->pc()->GetTransceivers()[0]->sender()->SetStreams({"id_2"});
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
caller->pc()->GetTransceivers()[0]->sender()->SetStreams({"id_3"});
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->receiver()->stream_ids()[0],
"id_3");
EXPECT_TRUE(callee->SetRemoteDescription(callee->CreateRollback()));
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->receiver()->stream_ids().size(),
1u);
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->receiver()->stream_ids()[0],
"id_1");
}
} // namespace webrtc

9
pc/rtp_transceiver.h
View File

@ -154,8 +154,16 @@ class RtpTransceiver final
void set_created_by_addtrack(bool created_by_addtrack) {
created_by_addtrack_ = created_by_addtrack;
}
// If AddTrack has been called then transceiver can't be removed during
// rollback.
void set_reused_for_addtrack(bool reused_for_addtrack) {
reused_for_addtrack_ = reused_for_addtrack;
}
bool created_by_addtrack() const { return created_by_addtrack_; }
bool reused_for_addtrack() const { return reused_for_addtrack_; }
// Returns true if this transceiver has ever had the current direction set to
// sendonly or sendrecv.
bool has_ever_been_used_to_send() const {
@ -201,6 +209,7 @@ class RtpTransceiver final
absl::optional<std::string> mid_;
absl::optional<size_t> mline_index_;
bool created_by_addtrack_ = false;
bool reused_for_addtrack_ = false;
bool has_ever_been_used_to_send_ = false;
cricket::ChannelInterface* channel_ = nullptr;