From 78323436cf916f02fbdb8e5aab9bc8ff82c4790d Mon Sep 17 00:00:00 2001 From: Emircan Uysaler Date: Fri, 8 Feb 2019 20:41:39 +0000 Subject: [PATCH] Revert "Enabling Simulcast use via AddTransceiver." This reverts commit ce470aab518f067a67aa03aaab1fc61a45afa0ec. Failing below Layout test. https://cs.chromium.org/chromium/src/third_party/blink/web_tests/external/wpt/webrtc/RTCRtpReceiver-getParameters-expected.txt?type=cs&sq=package:chromium&g=0 Original change's description: > Enabling Simulcast use via AddTransceiver. > > This change removes the restriction on multiple send encodings when > calling AddTransceiver. If RIDs are not provided in the > simulcast scenario, they are auto-generated by the platform. > > This effectively enables the use of spec-compliant simulcast. > Tests are also added to verify simulcast functionality. > > Bug: webrtc:10075 > Change-Id: I088feba70a26e85abcc7bfbe3ea1fe5103cd47d2 > Reviewed-on: https://webrtc-review.googlesource.com/c/121389 > Reviewed-by: Florent Castelli > Reviewed-by: Steve Anton > Commit-Queue: Amit Hilbuch > Cr-Commit-Position: refs/heads/master@{#26590} TBR=steveanton@webrtc.org,orphis@webrtc.org,amithi@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:10075 Change-Id: Idef5ca735eaef190f83eec8630cd54e23737d813 Reviewed-on: https://webrtc-review.googlesource.com/c/122040 Reviewed-by: Emircan Uysaler Commit-Queue: Emircan Uysaler Cr-Commit-Position: refs/heads/master@{#26618} --- pc/BUILD.gn | 1 - pc/peer_connection.cc | 56 +-- pc/peer_connection_rtp_unittest.cc | 22 +- pc/peer_connection_simulcast_unittest.cc | 417 ----------------------- pc/rtp_sender.cc | 9 +- pc/rtp_sender_receiver_unittest.cc | 35 +- 6 files changed, 57 insertions(+), 483 deletions(-) delete mode 100644 pc/peer_connection_simulcast_unittest.cc diff --git a/pc/BUILD.gn b/pc/BUILD.gn index a5f5f2ab88..5ea089ffbb 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -473,7 +473,6 @@ if (rtc_include_tests) { "peer_connection_media_unittest.cc", "peer_connection_rtp_unittest.cc", "peer_connection_signaling_unittest.cc", - "peer_connection_simulcast_unittest.cc", "peer_connection_wrapper.cc", "peer_connection_wrapper.h", "proxy_unittest.cc", diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index f28e7c02ea..759bf471f9 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1461,50 +1461,23 @@ PeerConnection::AddTransceiver( : cricket::MEDIA_TYPE_VIDEO)); } - size_t num_rids = absl::c_count_if(init.send_encodings, - [](const RtpEncodingParameters& encoding) { - return !encoding.rid.empty(); - }); - if (num_rids > 0 && num_rids != init.send_encodings.size()) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_PARAMETER, - "RIDs must be provided for either all or none of the send encodings."); - } - - if (absl::c_any_of(init.send_encodings, - [](const RtpEncodingParameters& encoding) { - return encoding.ssrc.has_value(); - })) { + // TODO(bugs.webrtc.org/7600): Verify init. + if (init.send_encodings.size() > 1) { LOG_AND_RETURN_ERROR( RTCErrorType::UNSUPPORTED_PARAMETER, - "Attempted to set an unimplemented parameter of RtpParameters."); + "Attempted to create an encoder with more than 1 encoding parameter."); + } + + for (const auto& encoding : init.send_encodings) { + if (encoding.ssrc.has_value()) { + LOG_AND_RETURN_ERROR( + RTCErrorType::UNSUPPORTED_PARAMETER, + "Attempted to set an unimplemented parameter of RtpParameters."); + } } RtpParameters parameters; parameters.encodings = init.send_encodings; - - // Encodings are dropped from the tail if too many are provided. - if (parameters.encodings.size() > kMaxSimulcastStreams) { - parameters.encodings.erase( - parameters.encodings.begin() + kMaxSimulcastStreams, - parameters.encodings.end()); - } - - // Single RID should be removed. - if (parameters.encodings.size() == 1 && - !parameters.encodings[0].rid.empty()) { - RTC_LOG(LS_INFO) << "Removing RID: " << parameters.encodings[0].rid << "."; - parameters.encodings[0].rid.clear(); - } - - // If RIDs were not provided, they are generated for simulcast scenario. - if (parameters.encodings.size() > 1 && num_rids == 0) { - rtc::UniqueStringGenerator rid_generator; - for (RtpEncodingParameters& encoding : parameters.encodings) { - encoding.rid = rid_generator(); - } - } - if (UnimplementedRtpParameterHasValue(parameters)) { LOG_AND_RETURN_ERROR( RTCErrorType::UNSUPPORTED_PARAMETER, @@ -1524,7 +1497,7 @@ PeerConnection::AddTransceiver( (track && !FindSenderById(track->id()) ? track->id() : rtc::CreateRandomUuid()); auto sender = CreateSender(media_type, sender_id, track, init.stream_ids, - parameters.encodings); + init.send_encodings); auto receiver = CreateReceiver(media_type, rtc::CreateRandomUuid()); auto transceiver = CreateAndAddTransceiver(sender, receiver); transceiver->internal()->set_direction(init.direction); @@ -3067,11 +3040,8 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, RTC_DCHECK_EQ(source, cricket::CS_REMOTE); // If the m= section is sendrecv or recvonly, and there are RtpTransceivers // of the same type... - // When simulcast is requested, a transceiver cannot be associated because - // AddTrack cannot be called to initialize it. if (!transceiver && - RtpTransceiverDirectionHasRecv(media_desc->direction()) && - !media_desc->HasSimulcast()) { + RtpTransceiverDirectionHasRecv(media_desc->direction())) { transceiver = FindAvailableTransceiverToReceive(media_desc->type()); } // If no RtpTransceiver was found in the previous step, create one with a diff --git a/pc/peer_connection_rtp_unittest.cc b/pc/peer_connection_rtp_unittest.cc index cf00138897..e41791fe30 100644 --- a/pc/peer_connection_rtp_unittest.cc +++ b/pc/peer_connection_rtp_unittest.cc @@ -1428,6 +1428,18 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, EXPECT_FALSE(caller->observer()->negotiation_needed()); } +// Test that AddTransceiver fails if trying to use simulcast using +// send_encodings as it isn't currently supported. +TEST_F(PeerConnectionRtpTestUnifiedPlan, CheckForUnsupportedSimulcast) { + auto caller = CreatePeerConnection(); + + RtpTransceiverInit init; + init.send_encodings.emplace_back(); + init.send_encodings.emplace_back(); + auto result = caller->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); + EXPECT_EQ(result.error().type(), RTCErrorType::UNSUPPORTED_PARAMETER); +} + // Test that AddTransceiver fails if trying to use unimplemented RTP encoding // parameters with the send_encodings parameters. TEST_F(PeerConnectionRtpTestUnifiedPlan, @@ -1440,7 +1452,7 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, auto default_send_encodings = init.send_encodings; // Unimplemented RtpParameters: ssrc, codec_payload_type, fec, rtx, dtx, - // ptime, scale_framerate_down_by, dependency_rids. + // ptime, scale_framerate_down_by, rid, dependency_rids. init.send_encodings[0].ssrc = 1; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, caller->pc() @@ -1489,6 +1501,14 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, .type()); init.send_encodings = default_send_encodings; + init.send_encodings[0].rid = "dummy_rid"; + EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, + caller->pc() + ->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init) + .error() + .type()); + init.send_encodings = default_send_encodings; + init.send_encodings[0].dependency_rids.push_back("dummy_rid"); EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, caller->pc() diff --git a/pc/peer_connection_simulcast_unittest.cc b/pc/peer_connection_simulcast_unittest.cc deleted file mode 100644 index f9e3d932e2..0000000000 --- a/pc/peer_connection_simulcast_unittest.cc +++ /dev/null @@ -1,417 +0,0 @@ -/* - * Copyright 2019 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 // no-presubmit-check TODO(webrtc:8982) - -#include "absl/algorithm/container.h" -#include "absl/memory/memory.h" -#include "api/audio_codecs/builtin_audio_decoder_factory.h" -#include "api/audio_codecs/builtin_audio_encoder_factory.h" -#include "api/create_peerconnection_factory.h" -#include "api/rtp_transceiver_interface.h" -#include "api/video_codecs/builtin_video_decoder_factory.h" -#include "api/video_codecs/builtin_video_encoder_factory.h" -#include "pc/peer_connection.h" -#include "pc/peer_connection_wrapper.h" -#include "pc/test/fake_audio_capture_module.h" -#include "pc/test/mock_peer_connection_observers.h" -#include "rtc_base/gunit.h" -#include "test/gmock.h" - -using ::testing::Contains; -using ::testing::Each; -using ::testing::ElementsAre; -using ::testing::ElementsAreArray; -using ::testing::Eq; -using ::testing::Field; -using ::testing::IsEmpty; -using ::testing::Ne; -using ::testing::Property; -using ::testing::SizeIs; - -using cricket::MediaContentDescription; -using cricket::RidDescription; -using cricket::SimulcastDescription; -using cricket::SimulcastLayer; -using cricket::StreamParams; - -namespace cricket { - -std::ostream& operator<<( // no-presubmit-check TODO(webrtc:8982) - std::ostream& os, // no-presubmit-check TODO(webrtc:8982) - const SimulcastLayer& layer) { - if (layer.is_paused) { - os << "~"; - } - return os << layer.rid; -} - -} // namespace cricket - -namespace { - -std::vector CreateLayers(const std::vector& rids, - const std::vector& active) { - RTC_DCHECK_EQ(rids.size(), active.size()); - std::vector result; - absl::c_transform(rids, active, std::back_inserter(result), - [](const std::string& rid, bool is_active) { - return SimulcastLayer(rid, !is_active); - }); - return result; -} -std::vector CreateLayers(const std::vector& rids, - bool active) { - return CreateLayers(rids, std::vector(rids.size(), active)); -} - -} // namespace -namespace webrtc { - -class PeerConnectionSimulcastTests : public testing::Test { - public: - PeerConnectionSimulcastTests() - : pc_factory_( - CreatePeerConnectionFactory(rtc::Thread::Current(), - rtc::Thread::Current(), - rtc::Thread::Current(), - FakeAudioCaptureModule::Create(), - CreateBuiltinAudioEncoderFactory(), - CreateBuiltinAudioDecoderFactory(), - CreateBuiltinVideoEncoderFactory(), - CreateBuiltinVideoDecoderFactory(), - nullptr, - nullptr)) {} - - rtc::scoped_refptr CreatePeerConnection( - MockPeerConnectionObserver* observer) { - PeerConnectionInterface::RTCConfiguration config; - config.sdp_semantics = SdpSemantics::kUnifiedPlan; - PeerConnectionDependencies pcd(observer); - auto pc = pc_factory_->CreatePeerConnection(config, std::move(pcd)); - EXPECT_TRUE(pc); - observer->SetPeerConnectionInterface(pc); - return pc; - } - - std::unique_ptr CreatePeerConnectionWrapper() { - auto observer = absl::make_unique(); - auto pc = CreatePeerConnection(observer.get()); - return absl::make_unique(pc_factory_, pc, - std::move(observer)); - } - - RtpTransceiverInit CreateTransceiverInit( - const std::vector& layers) { - RtpTransceiverInit init; - for (const SimulcastLayer& layer : layers) { - RtpEncodingParameters encoding; - encoding.rid = layer.rid; - encoding.active = !layer.is_paused; - init.send_encodings.push_back(encoding); - } - return init; - } - - rtc::scoped_refptr AddTransceiver( - PeerConnectionWrapper* pc, - const std::vector& layers) { - auto init = CreateTransceiverInit(layers); - return pc->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); - } - - SimulcastDescription RemoveSimulcast(SessionDescriptionInterface* sd) { - auto mcd = sd->description()->contents()[0].media_description(); - auto result = mcd->simulcast_description(); - mcd->set_simulcast_description(SimulcastDescription()); - return result; - } - - void AddRequestToReceiveSimulcast(const std::vector& layers, - SessionDescriptionInterface* sd) { - auto mcd = sd->description()->contents()[0].media_description(); - SimulcastDescription simulcast; - auto& receive_layers = simulcast.receive_layers(); - for (const SimulcastLayer& layer : layers) { - receive_layers.AddLayer(layer); - } - mcd->set_simulcast_description(simulcast); - } - - bool ValidateTransceiverParameters( - rtc::scoped_refptr transceiver, - const std::vector& layers) { - bool errors_before = ::testing::Test::HasFailure(); - auto parameters = transceiver->sender()->GetParameters(); - std::vector result_layers; - absl::c_transform(parameters.encodings, std::back_inserter(result_layers), - [](const RtpEncodingParameters& encoding) { - return SimulcastLayer(encoding.rid, !encoding.active); - }); - EXPECT_THAT(result_layers, ElementsAreArray(layers)); - // If there were errors before this code ran, we cannot tell if this - // validation succeeded or not. - return errors_before || !::testing::Test::HasFailure(); - } - - private: - rtc::scoped_refptr pc_factory_; -}; - -// Validates that RIDs are supported arguments when adding a transceiver. -TEST_F(PeerConnectionSimulcastTests, CanCreateTransceiverWithRid) { - auto pc = CreatePeerConnectionWrapper(); - auto layers = CreateLayers({"f"}, true); - auto transceiver = AddTransceiver(pc.get(), layers); - ASSERT_TRUE(transceiver); - auto parameters = transceiver->sender()->GetParameters(); - // Single RID should be removed. - EXPECT_THAT(parameters.encodings, - ElementsAre(Field(&RtpEncodingParameters::rid, Eq("")))); -} - -TEST_F(PeerConnectionSimulcastTests, CanCreateTransceiverWithSimulcast) { - auto pc = CreatePeerConnectionWrapper(); - auto layers = CreateLayers({"f", "h", "q"}, true); - auto transceiver = AddTransceiver(pc.get(), layers); - ASSERT_TRUE(transceiver); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); -} - -TEST_F(PeerConnectionSimulcastTests, RidsAreAutogeneratedIfNotProvided) { - auto pc = CreatePeerConnectionWrapper(); - auto init = CreateTransceiverInit(CreateLayers({"f", "h", "q"}, true)); - for (RtpEncodingParameters& parameters : init.send_encodings) { - parameters.rid = ""; - } - auto transceiver = pc->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); - auto parameters = transceiver->sender()->GetParameters(); - ASSERT_EQ(3u, parameters.encodings.size()); - EXPECT_THAT(parameters.encodings, - Each(Field(&RtpEncodingParameters::rid, Ne("")))); -} - -// Validates that an error is returned when there is a mix of supplied and not -// supplied RIDs in a call to AddTransceiver. -TEST_F(PeerConnectionSimulcastTests, MustSupplyAllOrNoRidsInSimulcast) { - auto pc_wrapper = CreatePeerConnectionWrapper(); - auto pc = pc_wrapper->pc(); - // Cannot create a layer with empty RID. Remove the RID after init is created. - auto layers = CreateLayers({"f", "h", "remove"}, true); - auto init = CreateTransceiverInit(layers); - init.send_encodings[2].rid = ""; - auto error = pc->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); - EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, error.error().type()); -} - -// Validates that a single RID does not get negotiated. -// This test is currently disabled because a single RID is not supported. -TEST_F(PeerConnectionSimulcastTests, SingleRidIsRemovedFromSessionDescription) { - auto pc = CreatePeerConnectionWrapper(); - auto transceiver = AddTransceiver(pc.get(), CreateLayers({"1"}, true)); - auto offer = pc->CreateOfferAndSetAsLocal(); - ASSERT_TRUE(offer); - auto contents = offer->description()->contents(); - ASSERT_EQ(1u, contents.size()); - EXPECT_THAT(contents[0].media_description()->streams(), - ElementsAre(Property(&StreamParams::has_rids, false))); -} - -TEST_F(PeerConnectionSimulcastTests, SimulcastLayersRemovedFromTail) { - static_assert( - kMaxSimulcastStreams < 8, - "Test assumes that the platform does not allow 8 simulcast layers"); - auto pc = CreatePeerConnectionWrapper(); - auto layers = CreateLayers({"1", "2", "3", "4", "5", "6", "7", "8"}, true); - std::vector expected_layers; - std::copy_n(layers.begin(), kMaxSimulcastStreams, - std::back_inserter(expected_layers)); - auto transceiver = AddTransceiver(pc.get(), layers); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, expected_layers)); -} - -// Checks that an offfer to send simulcast contains a SimulcastDescription. -TEST_F(PeerConnectionSimulcastTests, SimulcastAppearsInSessionDescription) { - auto pc = CreatePeerConnectionWrapper(); - std::vector rids({"f", "h", "q"}); - auto layers = CreateLayers(rids, true); - auto transceiver = AddTransceiver(pc.get(), layers); - auto offer = pc->CreateOffer(); - ASSERT_TRUE(offer); - auto contents = offer->description()->contents(); - ASSERT_EQ(1u, contents.size()); - auto content = contents[0]; - auto mcd = content.media_description(); - ASSERT_TRUE(mcd->HasSimulcast()); - auto simulcast = mcd->simulcast_description(); - EXPECT_THAT(simulcast.receive_layers(), IsEmpty()); - // The size is validated separately because GetAllLayers() flattens the list. - EXPECT_THAT(simulcast.send_layers(), SizeIs(3)); - std::vector result = simulcast.send_layers().GetAllLayers(); - EXPECT_THAT(result, ElementsAreArray(layers)); - auto streams = mcd->streams(); - ASSERT_EQ(1u, streams.size()); - auto stream = streams[0]; - EXPECT_FALSE(stream.has_ssrcs()); - EXPECT_TRUE(stream.has_rids()); - std::vector result_rids; - absl::c_transform(stream.rids(), std::back_inserter(result_rids), - [](const RidDescription& rid) { return rid.rid; }); - EXPECT_THAT(result_rids, ElementsAreArray(rids)); -} - -// Checks that Simulcast layers propagate to the sender parameters. -TEST_F(PeerConnectionSimulcastTests, SimulcastLayersAreSetInSender) { - auto local = CreatePeerConnectionWrapper(); - auto remote = CreatePeerConnectionWrapper(); - auto layers = CreateLayers({"f", "h", "q"}, true); - auto transceiver = AddTransceiver(local.get(), layers); - auto offer = local->CreateOfferAndSetAsLocal(); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); - - // Remove simulcast as the second peer connection won't support it. - auto simulcast = RemoveSimulcast(offer.get()); - std::string error; - EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; - auto answer = remote->CreateAnswerAndSetAsLocal(); - - // Setup an answer that mimics a server accepting simulcast. - auto mcd_answer = answer->description()->contents()[0].media_description(); - mcd_answer->mutable_streams().clear(); - auto simulcast_layers = simulcast.send_layers().GetAllLayers(); - auto& receive_layers = mcd_answer->simulcast_description().receive_layers(); - for (const auto& layer : simulcast_layers) { - receive_layers.AddLayer(layer); - } - EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); -} - -// Checks that paused Simulcast layers propagate to the sender parameters. -TEST_F(PeerConnectionSimulcastTests, PausedSimulcastLayersAreDisabledInSender) { - auto local = CreatePeerConnectionWrapper(); - auto remote = CreatePeerConnectionWrapper(); - auto layers = CreateLayers({"f", "h", "q"}, {true, false, true}); - auto server_layers = CreateLayers({"f", "h", "q"}, {true, false, false}); - RTC_DCHECK_EQ(layers.size(), server_layers.size()); - auto transceiver = AddTransceiver(local.get(), layers); - auto offer = local->CreateOfferAndSetAsLocal(); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); - - // Remove simulcast as the second peer connection won't support it. - RemoveSimulcast(offer.get()); - std::string error; - EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; - auto answer = remote->CreateAnswerAndSetAsLocal(); - - // Setup an answer that mimics a server accepting simulcast. - auto mcd_answer = answer->description()->contents()[0].media_description(); - mcd_answer->mutable_streams().clear(); - auto& receive_layers = mcd_answer->simulcast_description().receive_layers(); - for (const SimulcastLayer& layer : server_layers) { - receive_layers.AddLayer(layer); - } - EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, server_layers)); -} - -// Checks that when Simulcast is not supported by the remote party, then all -// the layers (except the first) are marked as disabled. -TEST_F(PeerConnectionSimulcastTests, SimulcastRejectedDisablesExtraLayers) { - auto local = CreatePeerConnectionWrapper(); - auto remote = CreatePeerConnectionWrapper(); - auto layers = CreateLayers({"1", "2", "3", "4"}, true); - // The number of layers does not change. - auto expected_layers = CreateLayers({"1", "2", "3", "4"}, false); - RTC_DCHECK_EQ(layers.size(), expected_layers.size()); - expected_layers[0].is_paused = false; - auto transceiver = AddTransceiver(local.get(), layers); - auto offer = local->CreateOfferAndSetAsLocal(); - // Remove simulcast as the second peer connection won't support it. - RemoveSimulcast(offer.get()); - std::string error; - EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; - auto answer = remote->CreateAnswerAndSetAsLocal(); - EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, expected_layers)); -} - -// Checks that if Simulcast is supported by remote party, but some layer is -// rejected, then only that layer is marked as disabled. -TEST_F(PeerConnectionSimulcastTests, RejectedSimulcastLayersAreDeactivated) { - auto local = CreatePeerConnectionWrapper(); - auto remote = CreatePeerConnectionWrapper(); - auto layers = CreateLayers({"1", "2", "3", "4"}, true); - auto transceiver = AddTransceiver(local.get(), layers); - auto offer = local->CreateOfferAndSetAsLocal(); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); - // Remove simulcast as the second peer connection won't support it. - auto removed_simulcast = RemoveSimulcast(offer.get()); - std::string error; - EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; - auto answer = remote->CreateAnswerAndSetAsLocal(); - auto mcd_answer = answer->description()->contents()[0].media_description(); - // Setup the answer to look like a server response. - // Remove one of the layers to reject it in the answer. - auto simulcast_layers = removed_simulcast.send_layers().GetAllLayers(); - simulcast_layers.erase(simulcast_layers.begin()); - auto& receive_layers = mcd_answer->simulcast_description().receive_layers(); - for (const auto& layer : simulcast_layers) { - receive_layers.AddLayer(layer); - } - ASSERT_TRUE(mcd_answer->HasSimulcast()); - EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; - layers[0].is_paused = true; - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); -} - -// Checks that simulcast is set up correctly when the server sends an offer -// requesting to receive simulcast. -TEST_F(PeerConnectionSimulcastTests, ServerSendsOfferToReceiveSimulcast) { - auto local = CreatePeerConnectionWrapper(); - auto remote = CreatePeerConnectionWrapper(); - auto layers = CreateLayers({"f", "h", "q"}, true); - AddTransceiver(local.get(), layers); - auto offer = local->CreateOfferAndSetAsLocal(); - // Remove simulcast as a sender and set it up as a receiver. - RemoveSimulcast(offer.get()); - AddRequestToReceiveSimulcast(layers, offer.get()); - std::string error; - EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; - auto transceiver = remote->pc()->GetTransceivers()[0]; - transceiver->SetDirection(RtpTransceiverDirection::kSendRecv); - EXPECT_TRUE(remote->CreateAnswerAndSetAsLocal()); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); -} - -// Checks that SetRemoteDescription doesn't attempt to associate a transceiver -// when simulcast is requested by the server. -TEST_F(PeerConnectionSimulcastTests, TransceiverIsNotRecycledWithSimulcast) { - auto local = CreatePeerConnectionWrapper(); - auto remote = CreatePeerConnectionWrapper(); - auto layers = CreateLayers({"f", "h", "q"}, true); - AddTransceiver(local.get(), layers); - auto offer = local->CreateOfferAndSetAsLocal(); - // Remove simulcast as a sender and set it up as a receiver. - RemoveSimulcast(offer.get()); - AddRequestToReceiveSimulcast(layers, offer.get()); - // Call AddTrack so that a transceiver is created. - remote->AddVideoTrack("fake_track"); - std::string error; - EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; - auto transceivers = remote->pc()->GetTransceivers(); - ASSERT_EQ(2u, transceivers.size()); - auto transceiver = transceivers[1]; - transceiver->SetDirection(RtpTransceiverDirection::kSendRecv); - EXPECT_TRUE(remote->CreateAnswerAndSetAsLocal()); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); -} - -} // namespace webrtc diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc index b9a269a208..d88b1fc5a0 100644 --- a/pc/rtp_sender.cc +++ b/pc/rtp_sender.cc @@ -41,6 +41,7 @@ bool UnimplementedRtpEncodingParameterHasValue( if (encoding_params.codec_payload_type.has_value() || encoding_params.fec.has_value() || encoding_params.rtx.has_value() || encoding_params.dtx.has_value() || encoding_params.ptime.has_value() || + !encoding_params.rid.empty() || encoding_params.scale_framerate_down_by.has_value() || !encoding_params.dependency_rids.empty()) { return true; @@ -250,7 +251,7 @@ RtpParameters AudioRtpSender::GetParameters() { if (stopped_) { return RtpParameters(); } - if (!media_channel_ || !ssrc_) { + if (!media_channel_) { RtpParameters result = init_parameters_; last_transaction_id_ = rtc::CreateRandomUuid(); result.transaction_id = last_transaction_id_.value(); @@ -287,7 +288,7 @@ RTCError AudioRtpSender::SetParameters(const RtpParameters& parameters) { RTCErrorType::UNSUPPORTED_PARAMETER, "Attempted to set an unimplemented parameter of RtpParameters."); } - if (!media_channel_ || !ssrc_) { + if (!media_channel_) { auto result = cricket::CheckRtpParametersInvalidModificationAndValues( init_parameters_, parameters); if (result.ok()) { @@ -505,7 +506,7 @@ RtpParameters VideoRtpSender::GetParameters() { if (stopped_) { return RtpParameters(); } - if (!media_channel_ || !ssrc_) { + if (!media_channel_) { RtpParameters result = init_parameters_; last_transaction_id_ = rtc::CreateRandomUuid(); result.transaction_id = last_transaction_id_.value(); @@ -542,7 +543,7 @@ RTCError VideoRtpSender::SetParameters(const RtpParameters& parameters) { RTCErrorType::UNSUPPORTED_PARAMETER, "Attempted to set an unimplemented parameter of RtpParameters."); } - if (!media_channel_ || !ssrc_) { + if (!media_channel_) { auto result = cricket::CheckRtpParametersInvalidModificationAndValues( init_parameters_, parameters); if (result.ok()) { diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index 974b77a28b..ba0e0b5e06 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -810,7 +810,7 @@ TEST_F(RtpSenderReceiverTest, EXPECT_EQ(1u, params.encodings.size()); // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime, - // scale_framerate_down_by, dependency_rids. + // scale_framerate_down_by, rid, dependency_rids. params.encodings[0].codec_payload_type = 1; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, audio_rtp_sender_->SetParameters(params).type()); @@ -836,6 +836,11 @@ TEST_F(RtpSenderReceiverTest, audio_rtp_sender_->SetParameters(params).type()); params = audio_rtp_sender_->GetParameters(); + params.encodings[0].rid = "dummy_rid"; + EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, + audio_rtp_sender_->SetParameters(params).type()); + params = audio_rtp_sender_->GetParameters(); + params.encodings[0].dependency_rids.push_back("dummy_rid"); EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, audio_rtp_sender_->SetParameters(params).type()); @@ -1077,7 +1082,7 @@ TEST_F(RtpSenderReceiverTest, EXPECT_EQ(1u, params.encodings.size()); // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime, - // scale_framerate_down_by, dependency_rids. + // scale_framerate_down_by, rid, dependency_rids. params.encodings[0].codec_payload_type = 1; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, video_rtp_sender_->SetParameters(params).type()); @@ -1103,6 +1108,11 @@ TEST_F(RtpSenderReceiverTest, video_rtp_sender_->SetParameters(params).type()); params = video_rtp_sender_->GetParameters(); + params.encodings[0].rid = "dummy_rid"; + EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, + video_rtp_sender_->SetParameters(params).type()); + params = video_rtp_sender_->GetParameters(); + params.encodings[0].dependency_rids.push_back("dummy_rid"); EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, video_rtp_sender_->SetParameters(params).type()); @@ -1110,20 +1120,6 @@ TEST_F(RtpSenderReceiverTest, DestroyVideoRtpSender(); } -TEST_F(RtpSenderReceiverTest, VideoSenderCanSetRid) { - CreateVideoRtpSender(); - RtpParameters params = video_rtp_sender_->GetParameters(); - EXPECT_EQ(1u, params.encodings.size()); - const std::string rid = "dummy_rid"; - params.encodings[0].rid = rid; - EXPECT_TRUE(video_rtp_sender_->SetParameters(params).ok()); - params = video_rtp_sender_->GetParameters(); - EXPECT_EQ(1u, params.encodings.size()); - EXPECT_EQ(rid, params.encodings[0].rid); - - DestroyVideoRtpSender(); -} - TEST_F(RtpSenderReceiverTest, VideoSenderCanSetScaleResolutionDownBy) { CreateVideoRtpSender(); @@ -1155,7 +1151,7 @@ TEST_F(RtpSenderReceiverTest, EXPECT_EQ(kVideoSimulcastLayerCount, params.encodings.size()); // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime, - // scale_framerate_down_by, dependency_rids. + // scale_framerate_down_by, rid, dependency_rids. for (size_t i = 0; i < params.encodings.size(); i++) { params.encodings[i].codec_payload_type = 1; EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, @@ -1182,6 +1178,11 @@ TEST_F(RtpSenderReceiverTest, video_rtp_sender_->SetParameters(params).type()); params = video_rtp_sender_->GetParameters(); + params.encodings[i].rid = "dummy_rid"; + EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, + video_rtp_sender_->SetParameters(params).type()); + params = video_rtp_sender_->GetParameters(); + params.encodings[i].dependency_rids.push_back("dummy_rid"); EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, video_rtp_sender_->SetParameters(params).type());