From 9f9671fe7fa63738d564adbf94e8863597b6ac0b Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Thu, 19 Jan 2023 13:56:21 +0000 Subject: [PATCH] Revert "Reland "Ensure RTCRtpSenders are always created with one encoding"" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit fc5d627cef71f906e921476c2e6b1e01d07732fe. Reason for revert: Breaks upstream WPT tests Original change's description: > Reland "Ensure RTCRtpSenders are always created with one encoding" > > This is a reland of commit b8023690d9f0e150cfe698cd68b477903ac66178 > > Original change's description: > > Ensure RTCRtpSenders are always created with one encoding > > > > It is possible to have AddTransceiver calls with an empty array > > of encodings or AddTrack calls. In both cases, before negotiation, > > the sender's encodings array would be empty and it was not possible > > to update any value. > > > > Now, a default entry should be created in those cases, allowing to > > update the configuration before negotiation. > > > > Bug: webrtc:10567 > > Change-Id: I1271e2965e1a97c1e472451e0ab8867fc24f6c2b > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290994 > > Auto-Submit: Florent Castelli > > Reviewed-by: Henrik Boström > > Commit-Queue: Florent Castelli > > Cr-Commit-Position: refs/heads/main@{#39126} > > Bug: webrtc:10567 > Change-Id: I2d52fa5b1d7cfdc9dce279fcf9faf1e0129c9008 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291140 > Reviewed-by: Henrik Boström > Reviewed-by: Harald Alvestrand > Commit-Queue: Florent Castelli > Cr-Commit-Position: refs/heads/main@{#39145} Bug: webrtc:10567 Change-Id: If9b5adb5debb7c87a15662a8d0f232405a0e8136 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291221 Auto-Submit: Evan Shrubsole Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Commit-Queue: Harald Alvestrand Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#39147} --- pc/peer_connection.cc | 5 ----- pc/peer_connection_integrationtest.cc | 30 --------------------------- pc/rtp_transmission_manager.cc | 22 +++++++++----------- 3 files changed, 10 insertions(+), 47 deletions(-) diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 36ffd1122f..5633be352e 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1096,11 +1096,6 @@ PeerConnection::AddTransceiver( } } - // If no encoding parameters were provided, a default entry is created. - if (parameters.encodings.empty()) { - parameters.encodings.push_back({}); - } - if (UnimplementedRtpParameterHasValue(parameters)) { LOG_AND_RETURN_ERROR( RTCErrorType::UNSUPPORTED_PARAMETER, diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 220415baf2..f46ef2ebdf 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -2834,14 +2834,6 @@ TEST_P(PeerConnectionIntegrationTest, UnsignaledSsrcGetParametersVideo) { EXPECT_TRUE(parameters.encodings[0].ssrc.has_value()); } -TEST_P(PeerConnectionIntegrationTest, - GetParametersHasEncodingsBeforeNegotiation) { - ASSERT_TRUE(CreatePeerConnectionWrappers()); - auto sender = caller()->AddTrack(caller()->CreateLocalVideoTrack()); - auto parameters = sender->GetParameters(); - EXPECT_EQ(parameters.encodings.size(), 1u); -} - // Test that if a track is removed and added again with a different stream ID, // the new stream ID is successfully communicated in SDP and media continues to // flow end-to-end. @@ -3418,28 +3410,6 @@ TEST_F(PeerConnectionIntegrationTestUnifiedPlan, } } -TEST_F(PeerConnectionIntegrationTestUnifiedPlan, - GetParametersHasEncodingsBeforeNegotiation) { - ASSERT_TRUE(CreatePeerConnectionWrappers()); - auto result = caller()->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO); - auto transceiver = result.MoveValue(); - auto parameters = transceiver->sender()->GetParameters(); - EXPECT_EQ(parameters.encodings.size(), 1u); -} - -TEST_F(PeerConnectionIntegrationTestUnifiedPlan, - GetParametersHasInitEncodingsBeforeNegotiation) { - ASSERT_TRUE(CreatePeerConnectionWrappers()); - RtpTransceiverInit init; - init.send_encodings.push_back({}); - init.send_encodings[0].max_bitrate_bps = 12345; - auto result = caller()->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); - auto transceiver = result.MoveValue(); - auto parameters = transceiver->sender()->GetParameters(); - ASSERT_EQ(parameters.encodings.size(), 1u); - EXPECT_EQ(parameters.encodings[0].max_bitrate_bps, 12345); -} - INSTANTIATE_TEST_SUITE_P( PeerConnectionIntegrationTest, PeerConnectionIntegrationTest, diff --git a/pc/rtp_transmission_manager.cc b/pc/rtp_transmission_manager.cc index 96b308842b..96b748b4b4 100644 --- a/pc/rtp_transmission_manager.cc +++ b/pc/rtp_transmission_manager.cc @@ -150,11 +150,10 @@ RtpTransmissionManager::AddTrackPlanB( (track->kind() == MediaStreamTrackInterface::kAudioKind ? cricket::MEDIA_TYPE_AUDIO : cricket::MEDIA_TYPE_VIDEO); - auto new_sender = CreateSender( - media_type, track->id(), track, adjusted_stream_ids, - init_send_encodings - ? *init_send_encodings - : std::vector(1, RtpEncodingParameters{})); + auto new_sender = + CreateSender(media_type, track->id(), track, adjusted_stream_ids, + init_send_encodings ? *init_send_encodings + : std::vector()); if (track->kind() == MediaStreamTrackInterface::kAudioKind) { new_sender->internal()->SetMediaChannel(voice_media_send_channel()); GetAudioTransceiver()->internal()->AddSender(new_sender); @@ -218,11 +217,10 @@ RtpTransmissionManager::AddTrackUnifiedPlan( if (FindSenderById(sender_id)) { sender_id = rtc::CreateRandomUuid(); } - auto sender = CreateSender( - media_type, sender_id, track, stream_ids, - init_send_encodings - ? *init_send_encodings - : std::vector(1, RtpEncodingParameters{})); + auto sender = CreateSender(media_type, sender_id, track, stream_ids, + init_send_encodings + ? *init_send_encodings + : std::vector()); auto receiver = CreateReceiver(media_type, rtc::CreateRandomUuid()); transceiver = CreateAndAddTransceiver(sender, receiver); transceiver->internal()->set_created_by_addtrack(true); @@ -413,7 +411,7 @@ void RtpTransmissionManager::AddAudioTrack(AudioTrackInterface* track, // Normal case; we've never seen this track before. auto new_sender = CreateSender(cricket::MEDIA_TYPE_AUDIO, track->id(), rtc::scoped_refptr(track), - {stream->id()}, {{}}); + {stream->id()}, {}); new_sender->internal()->SetMediaChannel(voice_media_send_channel()); GetAudioTransceiver()->internal()->AddSender(new_sender); // If the sender has already been configured in SDP, we call SetSsrc, @@ -460,7 +458,7 @@ void RtpTransmissionManager::AddVideoTrack(VideoTrackInterface* track, // Normal case; we've never seen this track before. auto new_sender = CreateSender(cricket::MEDIA_TYPE_VIDEO, track->id(), rtc::scoped_refptr(track), - {stream->id()}, {{}}); + {stream->id()}, {}); new_sender->internal()->SetMediaChannel(video_media_send_channel()); GetVideoTransceiver()->internal()->AddSender(new_sender); const RtpSenderInfo* sender_info =