diff --git a/pc/audio_rtp_receiver.cc b/pc/audio_rtp_receiver.cc index 3918c64f33..4234a655f4 100644 --- a/pc/audio_rtp_receiver.cc +++ b/pc/audio_rtp_receiver.cc @@ -167,11 +167,6 @@ void AudioRtpReceiver::Stop() { track_->internal()->set_ended(); } -void AudioRtpReceiver::SetSourceEnded() { - RTC_DCHECK_RUN_ON(&signaling_thread_checker_); - source_->SetState(MediaSourceInterface::kEnded); -} - // RTC_RUN_ON(&signaling_thread_checker_) void AudioRtpReceiver::RestartMediaChannel(absl::optional ssrc) { bool enabled = track_->internal()->enabled(); diff --git a/pc/audio_rtp_receiver.h b/pc/audio_rtp_receiver.h index 6f70243c0f..281ba69708 100644 --- a/pc/audio_rtp_receiver.h +++ b/pc/audio_rtp_receiver.h @@ -98,7 +98,6 @@ class AudioRtpReceiver : public ObserverInterface, // RtpReceiverInternal implementation. void Stop() override; - void SetSourceEnded() override; void SetupMediaChannel(uint32_t ssrc) override; void SetupUnsignaledMediaChannel() override; uint32_t ssrc() const override; diff --git a/pc/peer_connection_jsep_unittest.cc b/pc/peer_connection_jsep_unittest.cc index 270062909d..0fa36c9549 100644 --- a/pc/peer_connection_jsep_unittest.cc +++ b/pc/peer_connection_jsep_unittest.cc @@ -1927,10 +1927,17 @@ TEST_F(PeerConnectionJsepTest, RollbackRemovesTransceiver) { caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); auto callee = CreatePeerConnection(); EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u); + ASSERT_EQ(callee->pc()->GetTransceivers().size(), 1u); + auto transceiver = callee->pc()->GetTransceivers()[0]; EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback())); EXPECT_EQ(callee->pc()->GetTransceivers().size(), 0u); EXPECT_EQ(callee->observer()->remove_track_events_.size(), 1u); + // The removed transceiver should be stopped and its receiver track should be + // ended. + EXPECT_TRUE(transceiver->stopping()); + EXPECT_TRUE(transceiver->stopping()); + EXPECT_EQ(transceiver->receiver()->track()->state(), + MediaStreamTrackInterface::kEnded); } TEST_F(PeerConnectionJsepTest, RollbackKeepsTransceiverAndClearsMid) { @@ -1949,6 +1956,13 @@ TEST_F(PeerConnectionJsepTest, RollbackKeepsTransceiverAndClearsMid) { EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u); EXPECT_EQ(callee->observer()->remove_track_events_.size(), 1u); + // Because the transceiver is reusable, it must not be stopped and its + // receiver track must still be live. + auto transceiver = callee->pc()->GetTransceivers()[0]; + EXPECT_FALSE(transceiver->stopping()); + EXPECT_FALSE(transceiver->stopping()); + EXPECT_EQ(transceiver->receiver()->track()->state(), + MediaStreamTrackInterface::kLive); } TEST_F(PeerConnectionJsepTest, diff --git a/pc/rtp_receiver.h b/pc/rtp_receiver.h index 7d124dfd69..d591c464af 100644 --- a/pc/rtp_receiver.h +++ b/pc/rtp_receiver.h @@ -47,15 +47,6 @@ class RtpReceiverInternal : public RtpReceiverInterface { // state set to `kEnded`, a final state that cannot be reversed. virtual void Stop() = 0; - // Call on the signaling thread to set the source's state to `ended` before - // clearing the media channel (`SetMediaChannel(nullptr)`) on the worker - // thread. - // The difference between `Stop()` and `SetSourceEnded()` is that the latter - // does not change the state of the associated track. - // NOTE: Calling this function should be followed with a call to - // `SetMediaChannel(nullptr)` on the worker thread, to complete the operation. - virtual void SetSourceEnded() = 0; - // Sets the underlying MediaEngine channel associated with this RtpSender. // A VoiceMediaChannel should be used for audio RtpSenders and // a VideoMediaChannel should be used for video RtpSenders. diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index 5e3a084d2d..29bf947eee 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -216,12 +216,6 @@ void RtpTransceiver::SetChannel( RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(1); - if (!channel_) { - for (const auto& receiver : receivers_) - receiver->internal()->SetSourceEnded(); - RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(1); // There should not be an invoke. - } - if (channel_to_delete || !senders_.empty() || !receivers_.empty()) { channel_manager_->worker_thread()->Invoke(RTC_FROM_HERE, [&]() { auto* media_channel = channel_ ? channel_->media_channel() : nullptr; diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index fcec8eb9e5..0e5ca0e2be 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -2933,6 +2933,7 @@ RTCError SdpOfferAnswerHandler::Rollback(SdpType desc_type) { if (transceiver->internal()->reused_for_addtrack()) { transceiver->internal()->set_created_by_addtrack(true); } else { + transceiver->internal()->StopTransceiverProcedure(); transceivers()->Remove(transceiver); } } @@ -3395,9 +3396,9 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiversAndDataChannels( AssociateTransceiver(source, new_session.GetType(), i, new_content, old_local_content, old_remote_content); if (!transceiver_or_error.ok()) { - // In the case where a transceiver is rejected locally, we don't - // expect to find a transceiver, but might find it in the case - // where state is still "stopping", not "stopped". + // In the case where a transceiver is rejected locally prior to being + // associated, we don't expect to find a transceiver, but might find it + // in the case where state is still "stopping", not "stopped". if (new_content.rejected) { continue; } @@ -3406,6 +3407,36 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiversAndDataChannels( auto transceiver = transceiver_or_error.MoveValue(); RTCError error = UpdateTransceiverChannel(transceiver, new_content, bundle_group); + // Handle locally rejected content. This code path is only needed for apps + // that SDP munge. Remote rejected content is handled in + // ApplyRemoteDescriptionUpdateTransceiverState(). + if (source == cricket::ContentSource::CS_LOCAL && new_content.rejected) { + // Local offer. + if (new_session.GetType() == SdpType::kOffer) { + // If the RtpTransceiver API was used, it would already have made the + // transceiver stopping. But if the rejection was caused by SDP + // munging then we need to ensure the transceiver is stopping here. + if (!transceiver->internal()->stopping()) { + transceiver->internal()->StopStandard(); + } + RTC_DCHECK(transceiver->internal()->stopping()); + } else { + // Local answer. + RTC_DCHECK(new_session.GetType() == SdpType::kAnswer || + new_session.GetType() == SdpType::kPrAnswer); + // When RtpTransceiver API is used, rejection happens in the offer and + // the transceiver will already be stopped at local answer time + // (calling stop between SRD(offer) and SLD(answer) would not reject + // the content in the answer - instead this would trigger a follow-up + // O/A exchange). So if the content was rejected but the transceiver + // is not already stopped, SDP munging has happened and we need to + // ensure the transceiver is stopped. + if (!transceiver->internal()->stopped()) { + transceiver->internal()->StopTransceiverProcedure(); + } + RTC_DCHECK(transceiver->internal()->stopped()); + } + } if (!error.ok()) { return error; } diff --git a/pc/test/mock_rtp_receiver_internal.h b/pc/test/mock_rtp_receiver_internal.h index c222a04e2f..779dcdcf08 100644 --- a/pc/test/mock_rtp_receiver_internal.h +++ b/pc/test/mock_rtp_receiver_internal.h @@ -57,7 +57,6 @@ class MockRtpReceiverInternal : public RtpReceiverInternal { // RtpReceiverInternal methods. MOCK_METHOD(void, Stop, (), (override)); - MOCK_METHOD(void, SetSourceEnded, (), (override)); MOCK_METHOD(void, SetMediaChannel, (cricket::MediaChannel*), (override)); MOCK_METHOD(void, SetupMediaChannel, (uint32_t), (override)); MOCK_METHOD(void, SetupUnsignaledMediaChannel, (), (override)); diff --git a/pc/video_rtp_receiver.cc b/pc/video_rtp_receiver.cc index 8e11625347..3cc568f12a 100644 --- a/pc/video_rtp_receiver.cc +++ b/pc/video_rtp_receiver.cc @@ -114,11 +114,6 @@ void VideoRtpReceiver::Stop() { track_->internal()->set_ended(); } -void VideoRtpReceiver::SetSourceEnded() { - RTC_DCHECK_RUN_ON(&signaling_thread_checker_); - source_->SetState(MediaSourceInterface::kEnded); -} - // RTC_RUN_ON(&signaling_thread_checker_) void VideoRtpReceiver::RestartMediaChannel(absl::optional ssrc) { MediaSourceInterface::SourceState state = source_->state(); diff --git a/pc/video_rtp_receiver.h b/pc/video_rtp_receiver.h index 4261e417d2..05532a2eb1 100644 --- a/pc/video_rtp_receiver.h +++ b/pc/video_rtp_receiver.h @@ -88,7 +88,6 @@ class VideoRtpReceiver : public RtpReceiverInternal { // RtpReceiverInternal implementation. void Stop() override; - void SetSourceEnded() override; void SetupMediaChannel(uint32_t ssrc) override; void SetupUnsignaledMediaChannel() override; uint32_t ssrc() const override;