From bd1e5d5aa5a52bbf4c5554f28026acabc15a15d4 Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Tue, 17 Jan 2023 18:08:26 +0100 Subject: [PATCH] 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 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: I558a95f7b587302b5e95f6ec26d1eb1fedf3dbed Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291240 Reviewed-by: Harald Alvestrand Commit-Queue: Florent Castelli Cr-Commit-Position: refs/heads/main@{#39150} --- pc/peer_connection.cc | 5 +++++ pc/peer_connection_integrationtest.cc | 30 +++++++++++++++++++++++++++ pc/rtp_transmission_manager.cc | 22 +++++++++++--------- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 5633be352e..36ffd1122f 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1096,6 +1096,11 @@ 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 f46ef2ebdf..220415baf2 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -2834,6 +2834,14 @@ 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. @@ -3410,6 +3418,28 @@ 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 96b748b4b4..96b308842b 100644 --- a/pc/rtp_transmission_manager.cc +++ b/pc/rtp_transmission_manager.cc @@ -150,10 +150,11 @@ 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()); + auto new_sender = CreateSender( + media_type, track->id(), track, adjusted_stream_ids, + init_send_encodings + ? *init_send_encodings + : std::vector(1, RtpEncodingParameters{})); if (track->kind() == MediaStreamTrackInterface::kAudioKind) { new_sender->internal()->SetMediaChannel(voice_media_send_channel()); GetAudioTransceiver()->internal()->AddSender(new_sender); @@ -217,10 +218,11 @@ 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()); + auto sender = CreateSender( + media_type, sender_id, track, stream_ids, + init_send_encodings + ? *init_send_encodings + : std::vector(1, RtpEncodingParameters{})); auto receiver = CreateReceiver(media_type, rtc::CreateRandomUuid()); transceiver = CreateAndAddTransceiver(sender, receiver); transceiver->internal()->set_created_by_addtrack(true); @@ -411,7 +413,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, @@ -458,7 +460,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 =