From 7a9a092708f1f3abc45f9aabda2db205132cc4ac Mon Sep 17 00:00:00 2001 From: Bjorn A Mellem Date: Tue, 26 Nov 2019 09:19:40 -0800 Subject: [PATCH] Delete media transport integration. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MediaTransport is deprecated and the code is unused. No-Try: True Bug: webrtc:9719 Change-Id: I5b864c1e74bf04df16c15f51b8fac3d407331dcd Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160620 Commit-Queue: Bjorn Mellem Reviewed-by: Niels Moller Reviewed-by: Gustaf Ullberg Reviewed-by: Åsa Persson Reviewed-by: Steve Anton Cr-Commit-Position: refs/heads/master@{#29923} --- api/peer_connection_interface.h | 15 +- api/test/fake_media_transport.h | 135 +---- api/test/loopback_media_transport.cc | 390 +------------ api/test/loopback_media_transport.h | 131 +---- api/test/loopback_media_transport_unittest.cc | 138 +---- api/transport/media/media_transport_config.cc | 13 +- api/transport/media/media_transport_config.h | 8 - audio/BUILD.gn | 4 - audio/audio_receive_stream.cc | 26 +- audio/audio_receive_stream_unittest.cc | 3 +- audio/audio_send_stream.cc | 19 +- audio/audio_send_stream_unittest.cc | 7 +- audio/channel_receive.cc | 77 +-- audio/channel_receive.h | 3 - audio/channel_send.cc | 199 +------ audio/channel_send.h | 3 - audio/test/media_transport_test.cc | 160 ------ call/BUILD.gn | 4 - call/audio_receive_stream.h | 3 - call/audio_send_stream.cc | 9 +- call/audio_send_stream.h | 6 - call/call_perf_tests.cc | 3 +- call/call_unittest.cc | 11 +- call/video_receive_stream.cc | 6 +- call/video_receive_stream.h | 10 - call/video_send_stream.cc | 9 +- call/video_send_stream.h | 4 - media/BUILD.gn | 1 - media/base/media_channel.h | 4 - media/base/rtp_data_engine_unittest.cc | 1 - media/engine/webrtc_video_engine.cc | 25 +- media/engine/webrtc_video_engine_unittest.cc | 77 --- media/engine/webrtc_voice_engine.cc | 15 +- media/engine/webrtc_voice_engine_unittest.cc | 1 - p2p/BUILD.gn | 2 - p2p/base/no_op_dtls_transport.cc | 162 ------ p2p/base/no_op_dtls_transport.h | 112 ---- pc/BUILD.gn | 1 - pc/channel.cc | 38 +- pc/channel.h | 16 +- pc/channel_manager_unittest.cc | 22 - pc/datagram_rtp_transport.h | 1 + pc/jsep_transport.cc | 24 - pc/jsep_transport.h | 47 +- pc/jsep_transport_controller.cc | 242 +------- pc/jsep_transport_controller.h | 82 +-- pc/jsep_transport_controller_unittest.cc | 536 ------------------ pc/jsep_transport_unittest.cc | 6 - pc/media_session.cc | 6 - pc/media_session.h | 5 - pc/peer_connection.cc | 93 +-- pc/peer_connection.h | 10 - pc/peer_connection_data_channel_unittest.cc | 53 +- pc/peer_connection_integrationtest.cc | 325 ----------- pc/peer_connection_interface_unittest.cc | 8 +- pc/peer_connection_media_unittest.cc | 127 ----- pc/session_description.h | 30 - pc/webrtc_sdp.cc | 63 -- pc/webrtc_sdp_unittest.cc | 115 ---- test/call_test.cc | 5 +- test/peer_scenario/peer_scenario_client.cc | 1 - test/scenario/audio_stream.cc | 3 +- test/scenario/stats_collection.h | 1 + video/encoder_rtcp_feedback.cc | 10 - video/encoder_rtcp_feedback.h | 11 +- video/encoder_rtcp_feedback_unittest.cc | 5 - video/video_quality_test.cc | 4 +- video/video_receive_stream.cc | 81 +-- video/video_receive_stream.h | 13 +- video/video_send_stream.cc | 2 +- video/video_send_stream_impl.cc | 45 +- video/video_send_stream_impl.h | 7 +- video/video_send_stream_impl_unittest.cc | 3 +- 73 files changed, 151 insertions(+), 3686 deletions(-) delete mode 100644 audio/test/media_transport_test.cc delete mode 100644 p2p/base/no_op_dtls_transport.cc delete mode 100644 p2p/base/no_op_dtls_transport.h diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 5047eefea7..d118e52270 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -619,19 +619,12 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { // correctly. This flag will be deprecated soon. Do not rely on it. bool active_reset_srtp_params = false; - // If MediaTransportFactory is provided in PeerConnectionFactory, this flag - // informs PeerConnection that it should use the MediaTransportInterface for - // media (audio/video). It's invalid to set it to |true| if the - // MediaTransportFactory wasn't provided. + // DEPRECATED. Do not use. This option is ignored by peer connection. + // TODO(webrtc:9719): Delete this option. bool use_media_transport = false; - // If MediaTransportFactory is provided in PeerConnectionFactory, this flag - // informs PeerConnection that it should use the MediaTransportInterface for - // data channels. It's invalid to set it to |true| if the - // MediaTransportFactory wasn't provided. Data channels over media - // transport are not compatible with RTP or SCTP data channels. Setting - // both |use_media_transport_for_data_channels| and - // |enable_rtp_data_channel| is invalid. + // DEPRECATED. Do not use. This option is ignored by peer connection. + // TODO(webrtc:9719): Delete this option. bool use_media_transport_for_data_channels = false; // If MediaTransportFactory is provided in PeerConnectionFactory, this flag diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h index 593135df0c..ce2d88ce62 100644 --- a/api/test/fake_media_transport.h +++ b/api/test/fake_media_transport.h @@ -22,131 +22,6 @@ namespace webrtc { -// TODO(sukhanov): For now fake media transport does nothing and is used only -// in jsepcontroller unittests. In the future we should implement fake media -// transport, which forwards frames to another fake media transport, so we -// could unit test audio / video integration. -class FakeMediaTransport : public MediaTransportInterface { - public: - explicit FakeMediaTransport( - const MediaTransportSettings& settings, - const absl::optional& transport_offer = "", - const absl::optional& remote_transport_parameters = "") - : settings_(settings), - transport_offer_(transport_offer), - remote_transport_parameters_(remote_transport_parameters) {} - ~FakeMediaTransport() = default; - - RTCError SendAudioFrame(uint64_t channel_id, - MediaTransportEncodedAudioFrame frame) override { - return RTCError::OK(); - } - - RTCError SendVideoFrame( - uint64_t channel_id, - const MediaTransportEncodedVideoFrame& frame) override { - return RTCError::OK(); - } - - RTCError RequestKeyFrame(uint64_t channel_id) override { - return RTCError::OK(); - } - - void SetReceiveAudioSink(MediaTransportAudioSinkInterface* sink) override {} - void SetReceiveVideoSink(MediaTransportVideoSinkInterface* sink) override {} - - // Returns true if fake media transport was created as a caller. - bool is_caller() const { return settings_.is_caller; } - absl::optional pre_shared_key() const { - return settings_.pre_shared_key; - } - - RTCError OpenChannel(int channel_id) override { return RTCError::OK(); } - - RTCError SendData(int channel_id, - const SendDataParams& params, - const rtc::CopyOnWriteBuffer& buffer) override { - return RTCError::OK(); - } - - RTCError CloseChannel(int channel_id) override { return RTCError::OK(); } - - void SetDataSink(DataChannelSink* sink) override {} - - bool IsReadyToSend() const override { return false; } - - void SetMediaTransportStateCallback( - MediaTransportStateCallback* callback) override { - state_callback_ = callback; - } - - void SetState(webrtc::MediaTransportState state) { - if (state_callback_) { - state_callback_->OnStateChanged(state); - } - } - - void AddTargetTransferRateObserver( - webrtc::TargetTransferRateObserver* observer) override { - RTC_CHECK(!absl::c_linear_search(target_rate_observers_, observer)); - target_rate_observers_.push_back(observer); - } - - void RemoveTargetTransferRateObserver( - webrtc::TargetTransferRateObserver* observer) override { - auto it = absl::c_find(target_rate_observers_, observer); - if (it != target_rate_observers_.end()) { - target_rate_observers_.erase(it); - } - } - - void SetAllocatedBitrateLimits( - const MediaTransportAllocatedBitrateLimits& limits) override {} - - void SetTargetBitrateLimits(const MediaTransportTargetRateConstraints& - target_rate_constraints) override { - target_rate_constraints_in_order_.push_back(target_rate_constraints); - } - - const std::vector& - target_rate_constraints_in_order() { - return target_rate_constraints_in_order_; - } - - int target_rate_observers_size() { return target_rate_observers_.size(); } - - // Settings that were passed down to fake media transport. - const MediaTransportSettings& settings() { return settings_; } - - absl::optional GetTransportParametersOffer() const override { - // At least right now, we intend to use GetTransportParametersOffer before - // the transport is connected. This may change in the future. - RTC_CHECK(!is_connected_); - return transport_offer_; - } - - const absl::optional& remote_transport_parameters() { - return remote_transport_parameters_; - } - - void Connect(rtc::PacketTransportInternal* packet_transport) { - RTC_CHECK(!is_connected_) << "::Connect was called twice"; - is_connected_ = true; - } - - bool is_connected() { return is_connected_; } - - private: - const MediaTransportSettings settings_; - MediaTransportStateCallback* state_callback_ = nullptr; - std::vector target_rate_observers_; - const absl::optional transport_offer_; - const absl::optional remote_transport_parameters_; - bool is_connected_ = false; - std::vector - target_rate_constraints_in_order_; -}; - // Fake media transport factory creates fake media transport. // Also creates fake datagram transport, since both media and datagram // transports are created by |MediaTransportFactory|. @@ -163,19 +38,13 @@ class FakeMediaTransportFactory : public MediaTransportFactory { rtc::PacketTransportInternal* packet_transport, rtc::Thread* network_thread, const MediaTransportSettings& settings) override { - std::unique_ptr media_transport = - std::make_unique(settings, transport_offer_); - media_transport->Connect(packet_transport); - return std::move(media_transport); + return RTCError(RTCErrorType::UNSUPPORTED_OPERATION); } RTCErrorOr> CreateMediaTransport( rtc::Thread* network_thread, const MediaTransportSettings& settings) override { - std::unique_ptr media_transport = - std::make_unique( - settings, transport_offer_, settings.remote_transport_parameters); - return std::move(media_transport); + return RTCError(RTCErrorType::UNSUPPORTED_OPERATION); } RTCErrorOr> diff --git a/api/test/loopback_media_transport.cc b/api/test/loopback_media_transport.cc index f1bce1c937..14b28acf4b 100644 --- a/api/test/loopback_media_transport.cc +++ b/api/test/loopback_media_transport.cc @@ -21,87 +21,6 @@ namespace { constexpr size_t kLoopbackMaxDatagramSize = 1200; -// Wrapper used to hand out unique_ptrs to loopback media transports without -// ownership changes. -class WrapperMediaTransport : public MediaTransportInterface { - public: - explicit WrapperMediaTransport(MediaTransportInterface* wrapped) - : wrapped_(wrapped) {} - - RTCError SendAudioFrame(uint64_t channel_id, - MediaTransportEncodedAudioFrame frame) override { - return wrapped_->SendAudioFrame(channel_id, std::move(frame)); - } - - RTCError SendVideoFrame( - uint64_t channel_id, - const MediaTransportEncodedVideoFrame& frame) override { - return wrapped_->SendVideoFrame(channel_id, frame); - } - - void SetKeyFrameRequestCallback( - MediaTransportKeyFrameRequestCallback* callback) override { - wrapped_->SetKeyFrameRequestCallback(callback); - } - - RTCError RequestKeyFrame(uint64_t channel_id) override { - return wrapped_->RequestKeyFrame(channel_id); - } - - void SetReceiveAudioSink(MediaTransportAudioSinkInterface* sink) override { - wrapped_->SetReceiveAudioSink(sink); - } - - void SetReceiveVideoSink(MediaTransportVideoSinkInterface* sink) override { - wrapped_->SetReceiveVideoSink(sink); - } - - void AddTargetTransferRateObserver( - TargetTransferRateObserver* observer) override { - wrapped_->AddTargetTransferRateObserver(observer); - } - - void RemoveTargetTransferRateObserver( - TargetTransferRateObserver* observer) override { - wrapped_->RemoveTargetTransferRateObserver(observer); - } - - void SetMediaTransportStateCallback( - MediaTransportStateCallback* callback) override { - wrapped_->SetMediaTransportStateCallback(callback); - } - - RTCError OpenChannel(int channel_id) override { - return wrapped_->OpenChannel(channel_id); - } - - RTCError SendData(int channel_id, - const SendDataParams& params, - const rtc::CopyOnWriteBuffer& buffer) override { - return wrapped_->SendData(channel_id, params, buffer); - } - - RTCError CloseChannel(int channel_id) override { - return wrapped_->CloseChannel(channel_id); - } - - void SetDataSink(DataChannelSink* sink) override { - wrapped_->SetDataSink(sink); - } - - bool IsReadyToSend() const override { return wrapped_->IsReadyToSend(); } - - void SetAllocatedBitrateLimits( - const MediaTransportAllocatedBitrateLimits& limits) override {} - - absl::optional GetTransportParametersOffer() const override { - return wrapped_->GetTransportParametersOffer(); - } - - private: - MediaTransportInterface* wrapped_; -}; - class WrapperDatagramTransport : public DatagramTransportInterface { public: explicit WrapperDatagramTransport(DatagramTransportInterface* wrapped) @@ -166,10 +85,8 @@ class WrapperDatagramTransport : public DatagramTransportInterface { } // namespace WrapperMediaTransportFactory::WrapperMediaTransportFactory( - MediaTransportInterface* wrapped_media_transport, DatagramTransportInterface* wrapped_datagram_transport) - : wrapped_media_transport_(wrapped_media_transport), - wrapped_datagram_transport_(wrapped_datagram_transport) {} + : wrapped_datagram_transport_(wrapped_datagram_transport) {} WrapperMediaTransportFactory::WrapperMediaTransportFactory( MediaTransportFactory* wrapped) @@ -180,12 +97,7 @@ WrapperMediaTransportFactory::CreateMediaTransport( rtc::PacketTransportInternal* packet_transport, rtc::Thread* network_thread, const MediaTransportSettings& settings) { - created_transport_count_++; - if (wrapped_factory_) { - return wrapped_factory_->CreateMediaTransport(packet_transport, - network_thread, settings); - } - return {std::make_unique(wrapped_media_transport_)}; + return RTCError(RTCErrorType::UNSUPPORTED_OPERATION); } RTCErrorOr> @@ -215,22 +127,14 @@ RTCErrorOr> WrapperMediaTransportFactory::CreateMediaTransport( rtc::Thread* network_thread, const MediaTransportSettings& settings) { - created_transport_count_++; - if (wrapped_factory_) { - return wrapped_factory_->CreateMediaTransport(network_thread, settings); - } - return {std::make_unique(wrapped_media_transport_)}; + return RTCError(RTCErrorType::UNSUPPORTED_OPERATION); } MediaTransportPair::MediaTransportPair(rtc::Thread* thread) - : first_(thread), - second_(thread), - first_datagram_transport_(thread), + : first_datagram_transport_(thread), second_datagram_transport_(thread), - first_factory_(&first_, &first_datagram_transport_), - second_factory_(&second_, &second_datagram_transport_) { - first_.Connect(&second_); - second_.Connect(&first_); + first_factory_(&first_datagram_transport_), + second_factory_(&second_datagram_transport_) { first_datagram_transport_.Connect(&second_datagram_transport_); second_datagram_transport_.Connect(&first_datagram_transport_); } @@ -251,205 +155,12 @@ void MediaTransportPair::LoopbackDataChannelTransport::Connect( other_ = other; } -MediaTransportPair::LoopbackMediaTransport::LoopbackMediaTransport( - rtc::Thread* thread) - : dc_transport_(thread), thread_(thread), other_(nullptr) { - RTC_LOG(LS_INFO) << "LoopbackMediaTransport"; -} - -MediaTransportPair::LoopbackMediaTransport::~LoopbackMediaTransport() { - RTC_LOG(LS_INFO) << "~LoopbackMediaTransport"; - rtc::CritScope lock(&sink_lock_); - RTC_CHECK(audio_sink_ == nullptr); - RTC_CHECK(video_sink_ == nullptr); - RTC_CHECK(target_transfer_rate_observers_.empty()); - RTC_CHECK(rtt_observers_.empty()); -} - -void MediaTransportPair::LoopbackMediaTransport::Connect( - LoopbackMediaTransport* other) { - other_ = other; - dc_transport_.Connect(&other->dc_transport_); -} - -void MediaTransportPair::LoopbackMediaTransport::Connect( - rtc::PacketTransportInternal* packet_transport) { - if (state_after_connect_) { - SetState(*state_after_connect_); - } -} - -absl::optional -MediaTransportPair::LoopbackMediaTransport::GetTransportParametersOffer() - const { - return "loopback-media-transport-parameters"; -} - -RTCError MediaTransportPair::LoopbackMediaTransport::SendAudioFrame( - uint64_t channel_id, - MediaTransportEncodedAudioFrame frame) { - { - rtc::CritScope lock(&stats_lock_); - ++stats_.sent_audio_frames; - } - invoker_.AsyncInvoke(RTC_FROM_HERE, thread_, [this, channel_id, frame] { - other_->OnData(channel_id, frame); - }); - return RTCError::OK(); -} - -RTCError MediaTransportPair::LoopbackMediaTransport::SendVideoFrame( - uint64_t channel_id, - const MediaTransportEncodedVideoFrame& frame) { - { - rtc::CritScope lock(&stats_lock_); - ++stats_.sent_video_frames; - } - // Ensure that we own the referenced data. - MediaTransportEncodedVideoFrame frame_copy = frame; - frame_copy.Retain(); - invoker_.AsyncInvoke( - RTC_FROM_HERE, thread_, [this, channel_id, frame_copy]() mutable { - other_->OnData(channel_id, std::move(frame_copy)); - }); - return RTCError::OK(); -} - -void MediaTransportPair::LoopbackMediaTransport::SetKeyFrameRequestCallback( - MediaTransportKeyFrameRequestCallback* callback) { - rtc::CritScope lock(&sink_lock_); - if (callback) { - RTC_CHECK(key_frame_callback_ == nullptr); - } - key_frame_callback_ = callback; -} - -RTCError MediaTransportPair::LoopbackMediaTransport::RequestKeyFrame( - uint64_t channel_id) { - invoker_.AsyncInvoke(RTC_FROM_HERE, thread_, [this, channel_id] { - other_->OnKeyFrameRequested(channel_id); - }); - return RTCError::OK(); -} - -void MediaTransportPair::LoopbackMediaTransport::SetReceiveAudioSink( - MediaTransportAudioSinkInterface* sink) { - rtc::CritScope lock(&sink_lock_); - if (sink) { - RTC_CHECK(audio_sink_ == nullptr); - } - audio_sink_ = sink; -} - -void MediaTransportPair::LoopbackMediaTransport::SetReceiveVideoSink( - MediaTransportVideoSinkInterface* sink) { - rtc::CritScope lock(&sink_lock_); - if (sink) { - RTC_CHECK(video_sink_ == nullptr); - } - video_sink_ = sink; -} - -void MediaTransportPair::LoopbackMediaTransport::AddTargetTransferRateObserver( - TargetTransferRateObserver* observer) { - RTC_CHECK(observer); - { - rtc::CritScope cs(&sink_lock_); - RTC_CHECK( - !absl::c_linear_search(target_transfer_rate_observers_, observer)); - target_transfer_rate_observers_.push_back(observer); - } - invoker_.AsyncInvoke(RTC_FROM_HERE, thread_, [this] { - RTC_DCHECK_RUN_ON(thread_); - const DataRate kBitrate = DataRate::kbps(300); - const Timestamp now = Timestamp::us(rtc::TimeMicros()); - - TargetTransferRate transfer_rate; - transfer_rate.at_time = now; - transfer_rate.target_rate = kBitrate; - transfer_rate.network_estimate.at_time = now; - transfer_rate.network_estimate.round_trip_time = TimeDelta::ms(20); - transfer_rate.network_estimate.bwe_period = TimeDelta::seconds(3); - transfer_rate.network_estimate.bandwidth = kBitrate; - - rtc::CritScope cs(&sink_lock_); - - for (auto* o : target_transfer_rate_observers_) { - o->OnTargetTransferRate(transfer_rate); - } - }); -} - -void MediaTransportPair::LoopbackMediaTransport:: - RemoveTargetTransferRateObserver(TargetTransferRateObserver* observer) { - rtc::CritScope cs(&sink_lock_); - auto it = absl::c_find(target_transfer_rate_observers_, observer); - if (it == target_transfer_rate_observers_.end()) { - RTC_LOG(LS_WARNING) - << "Attempt to remove an unknown TargetTransferRate observer"; - return; - } - target_transfer_rate_observers_.erase(it); -} - -void MediaTransportPair::LoopbackMediaTransport::AddRttObserver( - MediaTransportRttObserver* observer) { - RTC_CHECK(observer); - { - rtc::CritScope cs(&sink_lock_); - RTC_CHECK(!absl::c_linear_search(rtt_observers_, observer)); - rtt_observers_.push_back(observer); - } - invoker_.AsyncInvoke(RTC_FROM_HERE, thread_, [this] { - RTC_DCHECK_RUN_ON(thread_); - - rtc::CritScope cs(&sink_lock_); - for (auto* o : rtt_observers_) { - o->OnRttUpdated(20); - } - }); -} - -void MediaTransportPair::LoopbackMediaTransport::RemoveRttObserver( - MediaTransportRttObserver* observer) { - rtc::CritScope cs(&sink_lock_); - auto it = absl::c_find(rtt_observers_, observer); - if (it == rtt_observers_.end()) { - RTC_LOG(LS_WARNING) << "Attempt to remove an unknown RTT observer"; - return; - } - rtt_observers_.erase(it); -} - -void MediaTransportPair::LoopbackMediaTransport::SetMediaTransportStateCallback( - MediaTransportStateCallback* callback) { - rtc::CritScope lock(&sink_lock_); - state_callback_ = callback; - invoker_.AsyncInvoke(RTC_FROM_HERE, thread_, [this] { - RTC_DCHECK_RUN_ON(thread_); - OnStateChanged(); - }); -} - -RTCError MediaTransportPair::LoopbackMediaTransport::OpenChannel( - int channel_id) { - // No-op. No need to open channels for the loopback. - return dc_transport_.OpenChannel(channel_id); -} - RTCError MediaTransportPair::LoopbackDataChannelTransport::OpenChannel( int channel_id) { // No-op. No need to open channels for the loopback. return RTCError::OK(); } -RTCError MediaTransportPair::LoopbackMediaTransport::SendData( - int channel_id, - const SendDataParams& params, - const rtc::CopyOnWriteBuffer& buffer) { - return dc_transport_.SendData(channel_id, params, buffer); -} - RTCError MediaTransportPair::LoopbackDataChannelTransport::SendData( int channel_id, const SendDataParams& params, @@ -461,11 +172,6 @@ RTCError MediaTransportPair::LoopbackDataChannelTransport::SendData( return RTCError::OK(); } -RTCError MediaTransportPair::LoopbackMediaTransport::CloseChannel( - int channel_id) { - return dc_transport_.CloseChannel(channel_id); -} - RTCError MediaTransportPair::LoopbackDataChannelTransport::CloseChannel( int channel_id) { invoker_.AsyncInvoke(RTC_FROM_HERE, thread_, [this, channel_id] { @@ -478,15 +184,6 @@ RTCError MediaTransportPair::LoopbackDataChannelTransport::CloseChannel( return RTCError::OK(); } -void MediaTransportPair::LoopbackMediaTransport::SetDataSink( - DataChannelSink* sink) { - dc_transport_.SetDataSink(sink); -} - -bool MediaTransportPair::LoopbackMediaTransport::IsReadyToSend() const { - return dc_transport_.IsReadyToSend(); -} - void MediaTransportPair::LoopbackDataChannelTransport::SetDataSink( DataChannelSink* sink) { rtc::CritScope lock(&sink_lock_); @@ -501,65 +198,10 @@ bool MediaTransportPair::LoopbackDataChannelTransport::IsReadyToSend() const { return ready_to_send_; } -void MediaTransportPair::LoopbackMediaTransport::SetState( - MediaTransportState state) { - invoker_.AsyncInvoke(RTC_FROM_HERE, thread_, [this, state] { - RTC_DCHECK_RUN_ON(thread_); - state_ = state; - OnStateChanged(); - }); -} - -void MediaTransportPair::LoopbackMediaTransport::SetStateAfterConnect( - MediaTransportState state) { - state_after_connect_ = state; -} - -void MediaTransportPair::LoopbackMediaTransport::FlushAsyncInvokes() { - invoker_.Flush(thread_); - dc_transport_.FlushAsyncInvokes(); -} - void MediaTransportPair::LoopbackDataChannelTransport::FlushAsyncInvokes() { invoker_.Flush(thread_); } -MediaTransportPair::Stats -MediaTransportPair::LoopbackMediaTransport::GetStats() { - rtc::CritScope lock(&stats_lock_); - return stats_; -} - -void MediaTransportPair::LoopbackMediaTransport::OnData( - uint64_t channel_id, - MediaTransportEncodedAudioFrame frame) { - { - rtc::CritScope lock(&sink_lock_); - if (audio_sink_) { - audio_sink_->OnData(channel_id, frame); - } - } - { - rtc::CritScope lock(&stats_lock_); - ++stats_.received_audio_frames; - } -} - -void MediaTransportPair::LoopbackMediaTransport::OnData( - uint64_t channel_id, - MediaTransportEncodedVideoFrame frame) { - { - rtc::CritScope lock(&sink_lock_); - if (video_sink_) { - video_sink_->OnData(channel_id, frame); - } - } - { - rtc::CritScope lock(&stats_lock_); - ++stats_.received_video_frames; - } -} - void MediaTransportPair::LoopbackDataChannelTransport::OnData( int channel_id, DataMessageType type, @@ -570,14 +212,6 @@ void MediaTransportPair::LoopbackDataChannelTransport::OnData( } } -void MediaTransportPair::LoopbackMediaTransport::OnKeyFrameRequested( - int channel_id) { - rtc::CritScope lock(&sink_lock_); - if (key_frame_callback_) { - key_frame_callback_->OnKeyFrameRequested(channel_id); - } -} - void MediaTransportPair::LoopbackDataChannelTransport::OnRemoteCloseChannel( int channel_id) { rtc::CritScope lock(&sink_lock_); @@ -587,15 +221,6 @@ void MediaTransportPair::LoopbackDataChannelTransport::OnRemoteCloseChannel( } } -void MediaTransportPair::LoopbackMediaTransport::OnStateChanged() { - rtc::CritScope lock(&sink_lock_); - if (state_callback_) { - state_callback_->OnStateChanged(state_); - } - - dc_transport_.OnReadyToSend(state_ == MediaTransportState::kWritable); -} - void MediaTransportPair::LoopbackDataChannelTransport::OnReadyToSend( bool ready_to_send) { invoker_.AsyncInvoke(RTC_FROM_HERE, thread_, [this, ready_to_send] { @@ -608,9 +233,6 @@ void MediaTransportPair::LoopbackDataChannelTransport::OnReadyToSend( }); } -void MediaTransportPair::LoopbackMediaTransport::SetAllocatedBitrateLimits( - const MediaTransportAllocatedBitrateLimits& limits) {} - MediaTransportPair::LoopbackDatagramTransport::LoopbackDatagramTransport( rtc::Thread* thread) : thread_(thread), dc_transport_(thread) {} diff --git a/api/test/loopback_media_transport.h b/api/test/loopback_media_transport.h index 475c58665d..f2aed3e8e7 100644 --- a/api/test/loopback_media_transport.h +++ b/api/test/loopback_media_transport.h @@ -42,8 +42,7 @@ namespace webrtc { // CreateMediaTransport(); class WrapperMediaTransportFactory : public MediaTransportFactory { public: - WrapperMediaTransportFactory( - MediaTransportInterface* wrapped_media_transport, + explicit WrapperMediaTransportFactory( DatagramTransportInterface* wrapped_datagram_transport); explicit WrapperMediaTransportFactory(MediaTransportFactory* wrapped); @@ -65,7 +64,6 @@ class WrapperMediaTransportFactory : public MediaTransportFactory { int created_transport_count() const; private: - MediaTransportInterface* wrapped_media_transport_ = nullptr; DatagramTransportInterface* wrapped_datagram_transport_ = nullptr; MediaTransportFactory* wrapped_factory_ = nullptr; int created_transport_count_ = 0; @@ -85,10 +83,6 @@ class MediaTransportPair { explicit MediaTransportPair(rtc::Thread* thread); ~MediaTransportPair(); - // Ownership stays with MediaTransportPair - MediaTransportInterface* first() { return &first_; } - MediaTransportInterface* second() { return &second_; } - DatagramTransportInterface* first_datagram_transport() { return &first_datagram_transport_; } @@ -105,19 +99,15 @@ class MediaTransportPair { } void SetState(MediaTransportState state) { - first_.SetState(state); - second_.SetState(state); first_datagram_transport_.SetState(state); second_datagram_transport_.SetState(state); } void SetFirstState(MediaTransportState state) { - first_.SetState(state); first_datagram_transport_.SetState(state); } void SetSecondStateAfterConnect(MediaTransportState state) { - second_.SetState(state); second_datagram_transport_.SetState(state); } @@ -126,13 +116,10 @@ class MediaTransportPair { } void FlushAsyncInvokes() { - first_.FlushAsyncInvokes(); - second_.FlushAsyncInvokes(); + first_datagram_transport_.FlushAsyncInvokes(); + second_datagram_transport_.FlushAsyncInvokes(); } - Stats FirstStats() { return first_.GetStats(); } - Stats SecondStats() { return second_.GetStats(); } - int first_factory_transport_count() const { return first_factory_.created_transport_count(); } @@ -183,116 +170,6 @@ class MediaTransportPair { rtc::AsyncInvoker invoker_; }; - class LoopbackMediaTransport : public MediaTransportInterface { - public: - explicit LoopbackMediaTransport(rtc::Thread* thread); - - ~LoopbackMediaTransport() override; - - // Connects this loopback transport to another loopback transport. - void Connect(LoopbackMediaTransport* other); - - void Connect(rtc::PacketTransportInternal* transport) override; - - RTCError SendAudioFrame(uint64_t channel_id, - MediaTransportEncodedAudioFrame frame) override; - - RTCError SendVideoFrame( - uint64_t channel_id, - const MediaTransportEncodedVideoFrame& frame) override; - - void SetKeyFrameRequestCallback( - MediaTransportKeyFrameRequestCallback* callback) override; - - RTCError RequestKeyFrame(uint64_t channel_id) override; - - void SetReceiveAudioSink(MediaTransportAudioSinkInterface* sink) override; - - void SetReceiveVideoSink(MediaTransportVideoSinkInterface* sink) override; - - void AddTargetTransferRateObserver( - TargetTransferRateObserver* observer) override; - - void RemoveTargetTransferRateObserver( - TargetTransferRateObserver* observer) override; - - void AddRttObserver(MediaTransportRttObserver* observer) override; - void RemoveRttObserver(MediaTransportRttObserver* observer) override; - - void SetMediaTransportStateCallback( - MediaTransportStateCallback* callback) override; - - void SetState(MediaTransportState state); - - // When Connect() is called, the media transport will enter this state. - // This is useful for mimicking zero-RTT connectivity, for example. - void SetStateAfterConnect(MediaTransportState state); - - RTCError OpenChannel(int channel_id) override; - - RTCError SendData(int channel_id, - const SendDataParams& params, - const rtc::CopyOnWriteBuffer& buffer) override; - - RTCError CloseChannel(int channel_id) override; - - void SetDataSink(DataChannelSink* sink) override; - - bool IsReadyToSend() const override; - - void FlushAsyncInvokes(); - - Stats GetStats(); - - void SetAllocatedBitrateLimits( - const MediaTransportAllocatedBitrateLimits& limits) override; - - absl::optional GetTransportParametersOffer() const override; - - private: - void OnData(uint64_t channel_id, MediaTransportEncodedAudioFrame frame); - - void OnData(uint64_t channel_id, MediaTransportEncodedVideoFrame frame); - - void OnKeyFrameRequested(int channel_id); - - void OnStateChanged() RTC_RUN_ON(thread_); - - // Implementation of the data channel transport. - LoopbackDataChannelTransport dc_transport_; - - rtc::Thread* const thread_; - rtc::CriticalSection sink_lock_; - rtc::CriticalSection stats_lock_; - - MediaTransportAudioSinkInterface* audio_sink_ RTC_GUARDED_BY(sink_lock_) = - nullptr; - MediaTransportVideoSinkInterface* video_sink_ RTC_GUARDED_BY(sink_lock_) = - nullptr; - - MediaTransportKeyFrameRequestCallback* key_frame_callback_ - RTC_GUARDED_BY(sink_lock_) = nullptr; - - MediaTransportStateCallback* state_callback_ RTC_GUARDED_BY(sink_lock_) = - nullptr; - - std::vector target_transfer_rate_observers_ - RTC_GUARDED_BY(sink_lock_); - std::vector rtt_observers_ - RTC_GUARDED_BY(sink_lock_); - - MediaTransportState state_ RTC_GUARDED_BY(thread_) = - MediaTransportState::kPending; - - absl::optional state_after_connect_; - - LoopbackMediaTransport* other_; - - Stats stats_ RTC_GUARDED_BY(stats_lock_); - - rtc::AsyncInvoker invoker_; - }; - class LoopbackDatagramTransport : public DatagramTransportInterface { public: explicit LoopbackDatagramTransport(rtc::Thread* thread); @@ -351,8 +228,6 @@ class MediaTransportPair { rtc::AsyncInvoker invoker_; }; - LoopbackMediaTransport first_; - LoopbackMediaTransport second_; LoopbackDatagramTransport first_datagram_transport_; LoopbackDatagramTransport second_datagram_transport_; WrapperMediaTransportFactory first_factory_; diff --git a/api/test/loopback_media_transport_unittest.cc b/api/test/loopback_media_transport_unittest.cc index 346ac5faeb..f036de3eae 100644 --- a/api/test/loopback_media_transport_unittest.cc +++ b/api/test/loopback_media_transport_unittest.cc @@ -52,109 +52,15 @@ class MockStateCallback : public MediaTransportStateCallback { MOCK_METHOD1(OnStateChanged, void(MediaTransportState)); }; -// Test only uses the sequence number. -MediaTransportEncodedAudioFrame CreateAudioFrame(int sequence_number) { - static constexpr int kSamplingRateHz = 48000; - static constexpr int kStartingSampleIndex = 0; - static constexpr int kSamplesPerChannel = 480; - static constexpr int kPayloadType = 17; - - return MediaTransportEncodedAudioFrame( - kSamplingRateHz, kStartingSampleIndex, kSamplesPerChannel, - sequence_number, MediaTransportEncodedAudioFrame::FrameType::kSpeech, - kPayloadType, std::vector(kSamplesPerChannel)); -} - -MediaTransportEncodedVideoFrame CreateVideoFrame( - int frame_id, - const webrtc::EncodedImage& encoded_image) { - static constexpr int kPayloadType = 18; - return MediaTransportEncodedVideoFrame(frame_id, /*referenced_frame_ids=*/{}, - kPayloadType, encoded_image); -} - } // namespace -TEST(LoopbackMediaTransport, AudioWithNoSinkSilentlyIgnored) { - std::unique_ptr thread = rtc::Thread::Create(); - thread->Start(); - MediaTransportPair transport_pair(thread.get()); - transport_pair.first()->SendAudioFrame(1, CreateAudioFrame(0)); - transport_pair.second()->SendAudioFrame(2, CreateAudioFrame(0)); - transport_pair.FlushAsyncInvokes(); -} - -TEST(LoopbackMediaTransport, AudioDeliveredToSink) { - std::unique_ptr thread = rtc::Thread::Create(); - thread->Start(); - MediaTransportPair transport_pair(thread.get()); - ::testing::StrictMock sink; - EXPECT_CALL(sink, - OnData(1, ::testing::Property( - &MediaTransportEncodedAudioFrame::sequence_number, - ::testing::Eq(10)))); - transport_pair.second()->SetReceiveAudioSink(&sink); - transport_pair.first()->SendAudioFrame(1, CreateAudioFrame(10)); - - transport_pair.FlushAsyncInvokes(); - transport_pair.second()->SetReceiveAudioSink(nullptr); -} - -TEST(LoopbackMediaTransport, VideoDeliveredToSink) { - std::unique_ptr thread = rtc::Thread::Create(); - thread->Start(); - MediaTransportPair transport_pair(thread.get()); - ::testing::StrictMock sink; - constexpr uint8_t encoded_data[] = {1, 2, 3}; - EncodedImage encoded_image; - encoded_image.SetEncodedData( - EncodedImageBuffer::Create(encoded_data, sizeof(encoded_data))); - - EXPECT_CALL(sink, OnData(1, ::testing::Property( - &MediaTransportEncodedVideoFrame::frame_id, - ::testing::Eq(10)))) - .WillOnce(::testing::Invoke( - [&encoded_image](int frame_id, - const MediaTransportEncodedVideoFrame& frame) { - EXPECT_EQ(frame.encoded_image().data(), encoded_image.data()); - EXPECT_EQ(frame.encoded_image().size(), encoded_image.size()); - })); - - transport_pair.second()->SetReceiveVideoSink(&sink); - transport_pair.first()->SendVideoFrame(1, - CreateVideoFrame(10, encoded_image)); - - transport_pair.FlushAsyncInvokes(); - transport_pair.second()->SetReceiveVideoSink(nullptr); -} - -TEST(LoopbackMediaTransport, VideoKeyFrameRequestDeliveredToCallback) { - std::unique_ptr thread = rtc::Thread::Create(); - thread->Start(); - MediaTransportPair transport_pair(thread.get()); - ::testing::StrictMock callback1; - ::testing::StrictMock callback2; - const uint64_t kFirstChannelId = 1111; - const uint64_t kSecondChannelId = 2222; - - EXPECT_CALL(callback1, OnKeyFrameRequested(kSecondChannelId)); - EXPECT_CALL(callback2, OnKeyFrameRequested(kFirstChannelId)); - transport_pair.first()->SetKeyFrameRequestCallback(&callback1); - transport_pair.second()->SetKeyFrameRequestCallback(&callback2); - - transport_pair.first()->RequestKeyFrame(kFirstChannelId); - transport_pair.second()->RequestKeyFrame(kSecondChannelId); - - transport_pair.FlushAsyncInvokes(); -} - TEST(LoopbackMediaTransport, DataDeliveredToSink) { std::unique_ptr thread = rtc::Thread::Create(); thread->Start(); MediaTransportPair transport_pair(thread.get()); MockDataChannelSink sink; - transport_pair.first()->SetDataSink(&sink); + transport_pair.first_datagram_transport()->SetDataSink(&sink); const int channel_id = 1; EXPECT_CALL( @@ -166,10 +72,11 @@ TEST(LoopbackMediaTransport, DataDeliveredToSink) { SendDataParams params; params.type = DataMessageType::kText; rtc::CopyOnWriteBuffer buffer("foo"); - transport_pair.second()->SendData(channel_id, params, buffer); + transport_pair.second_datagram_transport()->SendData(channel_id, params, + buffer); transport_pair.FlushAsyncInvokes(); - transport_pair.first()->SetDataSink(nullptr); + transport_pair.first_datagram_transport()->SetDataSink(nullptr); } TEST(LoopbackMediaTransport, CloseDeliveredToSink) { @@ -178,10 +85,10 @@ TEST(LoopbackMediaTransport, CloseDeliveredToSink) { MediaTransportPair transport_pair(thread.get()); MockDataChannelSink first_sink; - transport_pair.first()->SetDataSink(&first_sink); + transport_pair.first_datagram_transport()->SetDataSink(&first_sink); MockDataChannelSink second_sink; - transport_pair.second()->SetDataSink(&second_sink); + transport_pair.second_datagram_transport()->SetDataSink(&second_sink); const int channel_id = 1; { @@ -191,11 +98,11 @@ TEST(LoopbackMediaTransport, CloseDeliveredToSink) { EXPECT_CALL(first_sink, OnChannelClosed(channel_id)); } - transport_pair.first()->CloseChannel(channel_id); + transport_pair.first_datagram_transport()->CloseChannel(channel_id); transport_pair.FlushAsyncInvokes(); - transport_pair.first()->SetDataSink(nullptr); - transport_pair.second()->SetDataSink(nullptr); + transport_pair.first_datagram_transport()->SetDataSink(nullptr); + transport_pair.second_datagram_transport()->SetDataSink(nullptr); } TEST(LoopbackMediaTransport, InitialStateDeliveredWhenCallbackSet) { @@ -206,7 +113,10 @@ TEST(LoopbackMediaTransport, InitialStateDeliveredWhenCallbackSet) { MockStateCallback state_callback; EXPECT_CALL(state_callback, OnStateChanged(MediaTransportState::kPending)); - transport_pair.first()->SetMediaTransportStateCallback(&state_callback); + thread->Invoke(RTC_FROM_HERE, [&transport_pair, &state_callback] { + transport_pair.first_datagram_transport()->SetTransportStateCallback( + &state_callback); + }); transport_pair.FlushAsyncInvokes(); } @@ -221,7 +131,10 @@ TEST(LoopbackMediaTransport, ChangedStateDeliveredWhenCallbackSet) { MockStateCallback state_callback; EXPECT_CALL(state_callback, OnStateChanged(MediaTransportState::kWritable)); - transport_pair.first()->SetMediaTransportStateCallback(&state_callback); + thread->Invoke(RTC_FROM_HERE, [&transport_pair, &state_callback] { + transport_pair.first_datagram_transport()->SetTransportStateCallback( + &state_callback); + }); transport_pair.FlushAsyncInvokes(); } @@ -234,7 +147,10 @@ TEST(LoopbackMediaTransport, StateChangeDeliveredToCallback) { EXPECT_CALL(state_callback, OnStateChanged(MediaTransportState::kPending)); EXPECT_CALL(state_callback, OnStateChanged(MediaTransportState::kWritable)); - transport_pair.first()->SetMediaTransportStateCallback(&state_callback); + thread->Invoke(RTC_FROM_HERE, [&transport_pair, &state_callback] { + transport_pair.first_datagram_transport()->SetTransportStateCallback( + &state_callback); + }); transport_pair.SetState(MediaTransportState::kWritable); transport_pair.FlushAsyncInvokes(); } @@ -247,9 +163,9 @@ TEST(LoopbackMediaTransport, NotReadyToSendWhenDataSinkSet) { MockDataChannelSink data_channel_sink; EXPECT_CALL(data_channel_sink, OnReadyToSend()).Times(0); - transport_pair.first()->SetDataSink(&data_channel_sink); + transport_pair.first_datagram_transport()->SetDataSink(&data_channel_sink); transport_pair.FlushAsyncInvokes(); - transport_pair.first()->SetDataSink(nullptr); + transport_pair.first_datagram_transport()->SetDataSink(nullptr); } TEST(LoopbackMediaTransport, ReadyToSendWhenDataSinkSet) { @@ -263,9 +179,9 @@ TEST(LoopbackMediaTransport, ReadyToSendWhenDataSinkSet) { MockDataChannelSink data_channel_sink; EXPECT_CALL(data_channel_sink, OnReadyToSend()); - transport_pair.first()->SetDataSink(&data_channel_sink); + transport_pair.first_datagram_transport()->SetDataSink(&data_channel_sink); transport_pair.FlushAsyncInvokes(); - transport_pair.first()->SetDataSink(nullptr); + transport_pair.first_datagram_transport()->SetDataSink(nullptr); } TEST(LoopbackMediaTransport, StateChangeDeliveredToDataSink) { @@ -276,10 +192,10 @@ TEST(LoopbackMediaTransport, StateChangeDeliveredToDataSink) { MockDataChannelSink data_channel_sink; EXPECT_CALL(data_channel_sink, OnReadyToSend()); - transport_pair.first()->SetDataSink(&data_channel_sink); + transport_pair.first_datagram_transport()->SetDataSink(&data_channel_sink); transport_pair.SetState(MediaTransportState::kWritable); transport_pair.FlushAsyncInvokes(); - transport_pair.first()->SetDataSink(nullptr); + transport_pair.first_datagram_transport()->SetDataSink(nullptr); } } // namespace webrtc diff --git a/api/transport/media/media_transport_config.cc b/api/transport/media/media_transport_config.cc index cea3f163c4..b9b19cb6f0 100644 --- a/api/transport/media/media_transport_config.cc +++ b/api/transport/media/media_transport_config.cc @@ -15,23 +15,14 @@ namespace webrtc { -MediaTransportConfig::MediaTransportConfig( - MediaTransportInterface* media_transport) - : media_transport(media_transport) { - RTC_DCHECK(media_transport != nullptr); -} - MediaTransportConfig::MediaTransportConfig(size_t rtp_max_packet_size) : rtp_max_packet_size(rtp_max_packet_size) { RTC_DCHECK_GT(rtp_max_packet_size, 0); } -std::string MediaTransportConfig::DebugString() - const { // TODO(sukhanov): Add rtp_max_packet_size (requires fixing - // audio_send/receive_stream_unittest.cc). +std::string MediaTransportConfig::DebugString() const { rtc::StringBuilder result; - result << "{media_transport: " - << (media_transport != nullptr ? "(Transport)" : "null") << "}"; + result << "{rtp_max_packet_size: " << rtp_max_packet_size.value_or(0) << "}"; return result.Release(); } diff --git a/api/transport/media/media_transport_config.h b/api/transport/media/media_transport_config.h index 6a12630295..7ef65453ae 100644 --- a/api/transport/media/media_transport_config.h +++ b/api/transport/media/media_transport_config.h @@ -14,7 +14,6 @@ #include #include "absl/types/optional.h" -#include "api/transport/media/media_transport_interface.h" namespace webrtc { @@ -25,18 +24,11 @@ struct MediaTransportConfig { // Default constructor for no-media transport scenarios. MediaTransportConfig() = default; - // Constructor for media transport scenarios. - // Note that |media_transport| may not be nullptr. - explicit MediaTransportConfig(MediaTransportInterface* media_transport); - // Constructor for datagram transport scenarios. explicit MediaTransportConfig(size_t rtp_max_packet_size); std::string DebugString() const; - // If provided, all media is sent through media_transport. - MediaTransportInterface* media_transport = nullptr; - // If provided, limits RTP packet size (excludes ICE, IP or network overhead). absl::optional rtp_max_packet_size; }; diff --git a/audio/BUILD.gn b/audio/BUILD.gn index e64b76fd4a..a6d7ed40a4 100644 --- a/audio/BUILD.gn +++ b/audio/BUILD.gn @@ -53,7 +53,6 @@ rtc_library("audio") { "../api/neteq:neteq_api", "../api/rtc_event_log", "../api/task_queue", - "../api/transport/media:media_transport_interface", "../api/transport/rtp:rtp_source", "../call:bitrate_allocator", "../call:call_interfaces", @@ -121,13 +120,11 @@ if (rtc_include_tests) { "mock_voe_channel_proxy.h", "remix_resample_unittest.cc", "test/audio_stats_test.cc", - "test/media_transport_test.cc", ] deps = [ ":audio", ":audio_end_to_end_test", "../api:libjingle_peerconnection_api", - "../api:loopback_media_transport", "../api:mock_audio_mixer", "../api:mock_frame_decryptor", "../api:mock_frame_encryptor", @@ -137,7 +134,6 @@ if (rtc_include_tests) { "../api/audio_codecs/opus:audio_encoder_opus", "../api/rtc_event_log", "../api/task_queue:default_task_queue_factory", - "../api/transport/media:media_transport_interface", "../api/units:time_delta", "../call:mock_bitrate_allocator", "../call:mock_call_interfaces", diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index e1041beb84..2e02388531 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -56,7 +56,6 @@ std::string AudioReceiveStream::Config::ToString() const { ss << "{rtp: " << rtp.ToString(); ss << ", rtcp_send_transport: " << (rtcp_send_transport ? "(Transport)" : "null"); - ss << ", media_transport_config: " << media_transport_config.DebugString(); if (!sync_group.empty()) { ss << ", sync_group: " << sync_group; } @@ -78,9 +77,8 @@ std::unique_ptr CreateChannelReceive( static_cast(audio_state); return voe::CreateChannelReceive( clock, module_process_thread, neteq_factory, - internal_audio_state->audio_device_module(), - config.media_transport_config, config.rtcp_send_transport, event_log, - config.rtp.local_ssrc, config.rtp.remote_ssrc, + internal_audio_state->audio_device_module(), config.rtcp_send_transport, + event_log, config.rtp.local_ssrc, config.rtp.remote_ssrc, config.jitter_buffer_max_packets, config.jitter_buffer_fast_accelerate, config.jitter_buffer_min_delay_ms, config.jitter_buffer_enable_rtx_handling, config.decoder_factory, @@ -129,16 +127,14 @@ AudioReceiveStream::AudioReceiveStream( module_process_thread_checker_.Detach(); - if (!config.media_transport_config.media_transport) { - RTC_DCHECK(receiver_controller); - RTC_DCHECK(packet_router); - // Configure bandwidth estimation. - channel_receive_->RegisterReceiverCongestionControlObjects(packet_router); + RTC_DCHECK(receiver_controller); + RTC_DCHECK(packet_router); + // Configure bandwidth estimation. + channel_receive_->RegisterReceiverCongestionControlObjects(packet_router); - // Register with transport. - rtp_stream_receiver_ = receiver_controller->CreateReceiver( - config.rtp.remote_ssrc, channel_receive_.get()); - } + // Register with transport. + rtp_stream_receiver_ = receiver_controller->CreateReceiver( + config.rtp.remote_ssrc, channel_receive_.get()); ConfigureStream(this, config, true); } @@ -147,9 +143,7 @@ AudioReceiveStream::~AudioReceiveStream() { RTC_LOG(LS_INFO) << "~AudioReceiveStream: " << config_.rtp.remote_ssrc; Stop(); channel_receive_->SetAssociatedSendChannel(nullptr); - if (!config_.media_transport_config.media_transport) { - channel_receive_->ResetReceiverCongestionControlObjects(); - } + channel_receive_->ResetReceiverCongestionControlObjects(); } void AudioReceiveStream::Reconfigure( diff --git a/audio/audio_receive_stream_unittest.cc b/audio/audio_receive_stream_unittest.cc index 473b387780..b8eff0a443 100644 --- a/audio/audio_receive_stream_unittest.cc +++ b/audio/audio_receive_stream_unittest.cc @@ -223,8 +223,7 @@ TEST(AudioReceiveStreamTest, ConfigToString) { "{rtp: {remote_ssrc: 1234, local_ssrc: 5678, transport_cc: off, nack: " "{rtp_history_ms: 0}, extensions: [{uri: " "urn:ietf:params:rtp-hdrext:ssrc-audio-level, id: 3}]}, " - "rtcp_send_transport: null, media_transport_config: {media_transport: " - "null}}", + "rtcp_send_transport: null}", config.ToString()); } diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 323b7a9ef2..90d72c4732 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -22,7 +22,6 @@ #include "api/crypto/frame_encryptor_interface.h" #include "api/function_view.h" #include "api/rtc_event_log/rtc_event_log.h" -#include "api/transport/media/media_transport_config.h" #include "audio/audio_state.h" #include "audio/channel_send.h" #include "audio/conversion.h" @@ -119,7 +118,6 @@ AudioSendStream::AudioSendStream( voe::CreateChannelSend(clock, task_queue_factory, module_process_thread, - config.media_transport_config, /*overhead_observer=*/this, config.send_transport, rtcp_rtt_stats, @@ -150,7 +148,7 @@ AudioSendStream::AudioSendStream( !field_trial::IsDisabled("WebRTC-Audio-AlrProbing")), send_side_bwe_with_overhead_( field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")), - config_(Config(/*send_transport=*/nullptr, MediaTransportConfig())), + config_(Config(/*send_transport=*/nullptr)), audio_state_(audio_state), channel_send_(std::move(channel_send)), event_log_(event_log), @@ -165,23 +163,8 @@ AudioSendStream::AudioSendStream( RTC_DCHECK(audio_state_); RTC_DCHECK(channel_send_); RTC_DCHECK(bitrate_allocator_); - // Currently we require the rtp transport even when media transport is used. RTC_DCHECK(rtp_transport); - // TODO(nisse): Eventually, we should have only media_transport. But for the - // time being, we can have either. When media transport is injected, there - // should be no rtp_transport, and below check should be strengthened to XOR - // (either rtp_transport or media_transport but not both). - RTC_DCHECK(rtp_transport || config.media_transport_config.media_transport); - if (config.media_transport_config.media_transport) { - // TODO(sukhanov): Currently media transport audio overhead is considered - // constant, we will not get overhead_observer calls when using - // media_transport. In the future when we introduce RTP media transport we - // should make audio overhead interface consistent and work for both RTP and - // non-RTP implementations. - audio_overhead_per_packet_bytes_ = - config.media_transport_config.media_transport->GetAudioPacketOverhead(); - } rtp_rtcp_module_ = channel_send_->GetRtpRtcp(); RTC_DCHECK(rtp_rtcp_module_); diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 6d6ec6a92b..95d7f7340e 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -144,7 +144,7 @@ struct ConfigHelper { ConfigHelper(bool audio_bwe_enabled, bool expect_set_encoder_call) : clock_(1000000), task_queue_factory_(CreateDefaultTaskQueueFactory()), - stream_config_(/*send_transport=*/nullptr, MediaTransportConfig()), + stream_config_(/*send_transport=*/nullptr), audio_processing_(new rtc::RefCountedObject()), bitrate_allocator_(&limit_observer_), worker_queue_(task_queue_factory_->CreateTaskQueue( @@ -347,8 +347,7 @@ std::unique_ptr CreateAudioFrame1kHzSineWave(int16_t audio_level, } // namespace TEST(AudioSendStreamTest, ConfigToString) { - AudioSendStream::Config config(/*send_transport=*/nullptr, - MediaTransportConfig()); + AudioSendStream::Config config(/*send_transport=*/nullptr); config.rtp.ssrc = kSsrc; config.rtp.c_name = kCName; config.min_bitrate_bps = 12000; @@ -367,7 +366,7 @@ TEST(AudioSendStreamTest, ConfigToString) { "{rtp: {ssrc: 1234, extmap-allow-mixed: true, extensions: [{uri: " "urn:ietf:params:rtp-hdrext:ssrc-audio-level, id: 2}], " "c_name: foo_name}, rtcp_report_interval_ms: 2500, " - "send_transport: null, media_transport_config: {media_transport: null}, " + "send_transport: null, " "min_bitrate_bps: 12000, max_bitrate_bps: 34000, " "send_codec_spec: {nack_enabled: true, transport_cc_enabled: false, " "cng_payload_type: 42, payload_type: 103, " diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index 502818f1c9..2ecc3cf7b3 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -58,21 +58,6 @@ constexpr double kAudioSampleDurationSeconds = 0.01; constexpr int kVoiceEngineMinMinPlayoutDelayMs = 0; constexpr int kVoiceEngineMaxMinPlayoutDelayMs = 10000; -RTPHeader CreateRTPHeaderForMediaTransportFrame( - const MediaTransportEncodedAudioFrame& frame, - uint64_t channel_id) { - webrtc::RTPHeader rtp_header; - rtp_header.payloadType = frame.payload_type(); - rtp_header.payload_type_frequency = frame.sampling_rate_hz(); - rtp_header.timestamp = frame.starting_sample_index(); - rtp_header.sequenceNumber = frame.sequence_number(); - - rtp_header.ssrc = static_cast(channel_id); - - // The rest are initialized by the RTPHeader constructor. - return rtp_header; -} - AudioCodingModule::Config AcmConfig( NetEqFactory* neteq_factory, rtc::scoped_refptr decoder_factory, @@ -90,15 +75,13 @@ AudioCodingModule::Config AcmConfig( return acm_config; } -class ChannelReceive : public ChannelReceiveInterface, - public MediaTransportAudioSinkInterface { +class ChannelReceive : public ChannelReceiveInterface { public: // Used for receive streams. ChannelReceive(Clock* clock, ProcessThread* module_process_thread, NetEqFactory* neteq_factory, AudioDeviceModule* audio_device_module, - const MediaTransportConfig& media_transport_config, Transport* rtcp_send_transport, RtcEventLog* rtc_event_log, uint32_t local_ssrc, @@ -177,12 +160,6 @@ class ChannelReceive : public ChannelReceiveInterface, // Used for obtaining RTT for a receive-only channel. void SetAssociatedSendChannel(const ChannelSendInterface* channel) override; - // TODO(sukhanov): Return const pointer. It requires making media transport - // getters like GetLatestTargetTransferRate to be also const. - MediaTransportInterface* media_transport() const { - return media_transport_config_.media_transport; - } - private: void ReceivePacket(const uint8_t* packet, size_t packet_length, @@ -193,10 +170,6 @@ class ChannelReceive : public ChannelReceiveInterface, int GetRtpTimestampRateHz() const; int64_t GetRTT() const; - // MediaTransportAudioSinkInterface override; - void OnData(uint64_t channel_id, - MediaTransportEncodedAudioFrame frame) override; - void OnReceivedPayloadData(rtc::ArrayView payload, const RTPHeader& rtpHeader); @@ -283,8 +256,6 @@ class ChannelReceive : public ChannelReceiveInterface, rtc::ThreadChecker construction_thread_; - MediaTransportConfig media_transport_config_; - // E2EE Audio Frame Decryption rtc::scoped_refptr frame_decryptor_; webrtc::CryptoOptions crypto_options_; @@ -293,9 +264,6 @@ class ChannelReceive : public ChannelReceiveInterface, void ChannelReceive::OnReceivedPayloadData( rtc::ArrayView payload, const RTPHeader& rtpHeader) { - // We should not be receiving any RTP packets if media_transport is set. - RTC_CHECK(!media_transport()); - if (!Playing()) { // Avoid inserting into NetEQ when we are not playing. Count the // packet as discarded. @@ -320,26 +288,6 @@ void ChannelReceive::OnReceivedPayloadData( } } -// MediaTransportAudioSinkInterface override. -void ChannelReceive::OnData(uint64_t channel_id, - MediaTransportEncodedAudioFrame frame) { - RTC_CHECK(media_transport()); - - if (!Playing()) { - // Avoid inserting into NetEQ when we are not playing. Count the - // packet as discarded. - return; - } - - // Send encoded audio frame to Decoder / NetEq. - if (acm_receiver_.InsertPacket( - CreateRTPHeaderForMediaTransportFrame(frame, channel_id), - frame.encoded_data()) != 0) { - RTC_DLOG(LS_ERROR) << "ChannelReceive::OnData: unable to " - "push data to the ACM"; - } -} - AudioMixer::Source::AudioFrameInfo ChannelReceive::GetAudioFrameWithInfo( int sample_rate_hz, AudioFrame* audio_frame) { @@ -460,7 +408,6 @@ ChannelReceive::ChannelReceive( ProcessThread* module_process_thread, NetEqFactory* neteq_factory, AudioDeviceModule* audio_device_module, - const MediaTransportConfig& media_transport_config, Transport* rtcp_send_transport, RtcEventLog* rtc_event_log, uint32_t local_ssrc, @@ -492,7 +439,6 @@ ChannelReceive::ChannelReceive( _audioDeviceModulePtr(audio_device_module), _outputGain(1.0f), associated_send_channel_(nullptr), - media_transport_config_(media_transport_config), frame_decryptor_(frame_decryptor), crypto_options_(crypto_options) { // TODO(nisse): Use _moduleProcessThreadPtr instead? @@ -526,19 +472,11 @@ ChannelReceive::ChannelReceive( // Ensure that RTCP is enabled for the created channel. _rtpRtcpModule->SetRTCPStatus(RtcpMode::kCompound); - - if (media_transport()) { - media_transport()->SetReceiveAudioSink(this); - } } ChannelReceive::~ChannelReceive() { RTC_DCHECK(construction_thread_.IsCurrent()); - if (media_transport()) { - media_transport()->SetReceiveAudioSink(nullptr); - } - StopPlayout(); if (_moduleProcessThreadPtr) @@ -931,14 +869,6 @@ int ChannelReceive::GetRtpTimestampRateHz() const { } int64_t ChannelReceive::GetRTT() const { - if (media_transport()) { - auto target_rate = media_transport()->GetLatestTargetTransferRate(); - if (target_rate.has_value()) { - return target_rate->network_estimate.round_trip_time.ms(); - } - - return 0; - } std::vector report_blocks; _rtpRtcpModule->RemoteRTCPStat(&report_blocks); @@ -973,7 +903,6 @@ std::unique_ptr CreateChannelReceive( ProcessThread* module_process_thread, NetEqFactory* neteq_factory, AudioDeviceModule* audio_device_module, - const MediaTransportConfig& media_transport_config, Transport* rtcp_send_transport, RtcEventLog* rtc_event_log, uint32_t local_ssrc, @@ -988,8 +917,8 @@ std::unique_ptr CreateChannelReceive( const webrtc::CryptoOptions& crypto_options) { return std::make_unique( clock, module_process_thread, neteq_factory, audio_device_module, - media_transport_config, rtcp_send_transport, rtc_event_log, local_ssrc, - remote_ssrc, jitter_buffer_max_packets, jitter_buffer_fast_playout, + rtcp_send_transport, rtc_event_log, local_ssrc, remote_ssrc, + jitter_buffer_max_packets, jitter_buffer_fast_playout, jitter_buffer_min_delay_ms, jitter_buffer_enable_rtx_handling, decoder_factory, codec_pair_id, frame_decryptor, crypto_options); } diff --git a/audio/channel_receive.h b/audio/channel_receive.h index 3cab489719..034ac7b059 100644 --- a/audio/channel_receive.h +++ b/audio/channel_receive.h @@ -23,8 +23,6 @@ #include "api/call/transport.h" #include "api/crypto/crypto_options.h" #include "api/neteq/neteq_factory.h" -#include "api/transport/media/media_transport_config.h" -#include "api/transport/media/media_transport_interface.h" #include "api/transport/rtp/rtp_source.h" #include "call/rtp_packet_sink_interface.h" #include "call/syncable.h" @@ -146,7 +144,6 @@ std::unique_ptr CreateChannelReceive( ProcessThread* module_process_thread, NetEqFactory* neteq_factory, AudioDeviceModule* audio_device_module, - const MediaTransportConfig& media_transport_config, Transport* rtcp_send_transport, RtcEventLog* rtc_event_log, uint32_t local_ssrc, diff --git a/audio/channel_send.cc b/audio/channel_send.cc index 184ea41458..5bb2cbeb2c 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -52,34 +52,14 @@ namespace { constexpr int64_t kMaxRetransmissionWindowMs = 1000; constexpr int64_t kMinRetransmissionWindowMs = 30; -MediaTransportEncodedAudioFrame::FrameType -MediaTransportFrameTypeForWebrtcFrameType(webrtc::AudioFrameType frame_type) { - switch (frame_type) { - case AudioFrameType::kAudioFrameSpeech: - return MediaTransportEncodedAudioFrame::FrameType::kSpeech; - break; - - case AudioFrameType::kAudioFrameCN: - return MediaTransportEncodedAudioFrame::FrameType:: - kDiscontinuousTransmission; - break; - - default: - RTC_CHECK(false) << "Unexpected frame type=" - << static_cast(frame_type); - break; - } -} - class RtpPacketSenderProxy; class TransportFeedbackProxy; class TransportSequenceNumberProxy; class VoERtcpObserver; class ChannelSend : public ChannelSendInterface, - public AudioPacketizationCallback, // receive encoded - // packets from the ACM - public TargetTransferRateObserver { + public AudioPacketizationCallback { // receive encoded + // packets from the ACM public: // TODO(nisse): Make OnUplinkPacketLossRate public, and delete friend // declaration. @@ -88,7 +68,6 @@ class ChannelSend : public ChannelSendInterface, ChannelSend(Clock* clock, TaskQueueFactory* task_queue_factory, ProcessThread* module_process_thread, - const MediaTransportConfig& media_transport_config, OverheadObserver* overhead_observer, Transport* rtp_transport, RtcpRttStats* rtcp_rtt_stats, @@ -188,21 +167,8 @@ class ChannelSend : public ChannelSendInterface, rtc::ArrayView payload) RTC_RUN_ON(encoder_queue_); - int32_t SendMediaTransportAudio(AudioFrameType frameType, - uint8_t payloadType, - uint32_t timeStamp, - rtc::ArrayView payload) - RTC_RUN_ON(encoder_queue_); - - // Return media transport or nullptr if using RTP. - MediaTransportInterface* media_transport() { - return media_transport_config_.media_transport; - } - void OnReceivedRtt(int64_t rtt_ms); - void OnTargetTransferRate(TargetTransferRate) override; - // Thread checkers document and lock usage of some methods on voe::Channel to // specific threads we know about. The goal is to eventually split up // voe::Channel into parts with single-threaded semantics, and thereby reduce @@ -251,20 +217,6 @@ class ChannelSend : public ChannelSendInterface, bool encoder_queue_is_active_ RTC_GUARDED_BY(encoder_queue_) = false; - MediaTransportConfig media_transport_config_; - int media_transport_sequence_number_ RTC_GUARDED_BY(encoder_queue_) = 0; - - rtc::CriticalSection media_transport_lock_; - // Currently set to local SSRC at construction. - uint64_t media_transport_channel_id_ RTC_GUARDED_BY(&media_transport_lock_) = - 0; - // Cache payload type and sampling frequency from most recent call to - // SetEncoder. Needed to set MediaTransportEncodedAudioFrame metadata, and - // invalidate on encoder change. - int media_transport_payload_type_ RTC_GUARDED_BY(&media_transport_lock_); - int media_transport_sampling_frequency_ - RTC_GUARDED_BY(&media_transport_lock_); - // E2EE Audio Frame Encryption rtc::scoped_refptr frame_encryptor_ RTC_GUARDED_BY(encoder_queue_); @@ -421,18 +373,7 @@ int32_t ChannelSend::SendData(AudioFrameType frameType, size_t payloadSize) { RTC_DCHECK_RUN_ON(&encoder_queue_); rtc::ArrayView payload(payloadData, payloadSize); - - if (media_transport() != nullptr) { - if (frameType == AudioFrameType::kEmptyFrame) { - // TODO(bugs.webrtc.org/9719): Media transport Send doesn't support - // sending empty frames. - return 0; - } - - return SendMediaTransportAudio(frameType, payloadType, timeStamp, payload); - } else { - return SendRtpAudio(frameType, payloadType, timeStamp, payload); - } + return SendRtpAudio(frameType, payloadType, timeStamp, payload); } int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType, @@ -512,64 +453,9 @@ int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType, return 0; } -int32_t ChannelSend::SendMediaTransportAudio( - AudioFrameType frameType, - uint8_t payloadType, - uint32_t timeStamp, - rtc::ArrayView payload) { - // TODO(nisse): Use null _transportPtr for MediaTransport. - // RTC_DCHECK(_transportPtr == nullptr); - uint64_t channel_id; - int sampling_rate_hz; - { - rtc::CritScope cs(&media_transport_lock_); - if (media_transport_payload_type_ != payloadType) { - // Payload type is being changed, media_transport_sampling_frequency_, - // no longer current. - return -1; - } - sampling_rate_hz = media_transport_sampling_frequency_; - channel_id = media_transport_channel_id_; - } - MediaTransportEncodedAudioFrame frame( - /*sampling_rate_hz=*/sampling_rate_hz, - - // TODO(nisse): Timestamp and sample index are the same for all supported - // audio codecs except G722. Refactor audio coding module to only use - // sample index, and leave translation to RTP time, when needed, for - // RTP-specific code. - /*starting_sample_index=*/timeStamp, - - // Sample count isn't conveniently available from the AudioCodingModule, - // and needs some refactoring to wire up in a good way. For now, left as - // zero. - /*samples_per_channel=*/0, - - /*sequence_number=*/media_transport_sequence_number_, - MediaTransportFrameTypeForWebrtcFrameType(frameType), payloadType, - std::vector(payload.begin(), payload.end())); - - // TODO(nisse): Introduce a MediaTransportSender object bound to a specific - // channel id. - RTCError rtc_error = - media_transport()->SendAudioFrame(channel_id, std::move(frame)); - - if (!rtc_error.ok()) { - RTC_LOG(LS_ERROR) << "Failed to send frame, rtc_error=" - << ToString(rtc_error.type()) << ", " - << rtc_error.message(); - return -1; - } - - ++media_transport_sequence_number_; - - return 0; -} - ChannelSend::ChannelSend(Clock* clock, TaskQueueFactory* task_queue_factory, ProcessThread* module_process_thread, - const MediaTransportConfig& media_transport_config, OverheadObserver* overhead_observer, Transport* rtp_transport, RtcpRttStats* rtcp_rtt_stats, @@ -591,7 +477,6 @@ ChannelSend::ChannelSend(Clock* clock, rtp_packet_pacer_proxy_(new RtpPacketSenderProxy()), retransmission_rate_limiter_( new RateLimiter(clock, kMaxRetransmissionWindowMs)), - media_transport_config_(media_transport_config), frame_encryptor_(frame_encryptor), crypto_options_(crypto_options), encoder_queue_(task_queue_factory->CreateTaskQueue( @@ -603,17 +488,9 @@ ChannelSend::ChannelSend(Clock* clock, audio_coding_.reset(AudioCodingModule::Create(AudioCodingModule::Config())); RtpRtcp::Configuration configuration; - - // We gradually remove codepaths that depend on RTP when using media - // transport. All of this logic should be moved to the future - // RTPMediaTransport. In this case it means that overhead and bandwidth - // observers should not be called when using media transport. - if (!media_transport_config.media_transport) { - configuration.overhead_observer = overhead_observer; - configuration.bandwidth_callback = rtcp_observer_.get(); - configuration.transport_feedback_callback = feedback_observer_proxy_.get(); - } - + configuration.overhead_observer = overhead_observer; + configuration.bandwidth_callback = rtcp_observer_.get(); + configuration.transport_feedback_callback = feedback_observer_proxy_.get(); configuration.clock = clock; configuration.audio = true; configuration.clock = Clock::GetRealTimeClock(); @@ -629,10 +506,6 @@ ChannelSend::ChannelSend(Clock* clock, configuration.rtcp_report_interval_ms = rtcp_report_interval_ms; configuration.local_media_ssrc = ssrc; - if (media_transport_config_.media_transport) { - rtc::CritScope cs(&media_transport_lock_); - media_transport_channel_id_ = ssrc; - } _rtpRtcpModule = RtpRtcp::Create(configuration); _rtpRtcpModule->SetSendingMediaStatus(false); @@ -640,17 +513,6 @@ ChannelSend::ChannelSend(Clock* clock, rtp_sender_audio_ = std::make_unique( configuration.clock, _rtpRtcpModule->RtpSender()); - // We want to invoke the 'TargetRateObserver' and |OnOverheadChanged| - // callbacks after the audio_coding_ is fully initialized. - if (media_transport_config.media_transport) { - RTC_DLOG(LS_INFO) << "Setting media_transport_ rate observers."; - media_transport_config.media_transport->AddTargetTransferRateObserver(this); - media_transport_config.media_transport->SetAudioOverheadObserver( - overhead_observer); - } else { - RTC_DLOG(LS_INFO) << "Not setting media_transport_ rate observers."; - } - _moduleProcessThreadPtr->RegisterModule(_rtpRtcpModule.get(), RTC_FROM_HERE); // Ensure that RTCP is enabled by default for the created channel. @@ -663,12 +525,6 @@ ChannelSend::ChannelSend(Clock* clock, ChannelSend::~ChannelSend() { RTC_DCHECK(construction_thread_.IsCurrent()); - if (media_transport_config_.media_transport) { - media_transport_config_.media_transport->RemoveTargetTransferRateObserver( - this); - media_transport_config_.media_transport->SetAudioOverheadObserver(nullptr); - } - StopSend(); int error = audio_coding_->RegisterTransportCallback(NULL); RTC_DCHECK_EQ(0, error); @@ -729,13 +585,6 @@ void ChannelSend::SetEncoder(int payload_type, encoder->RtpTimestampRateHz(), encoder->NumChannels(), 0); - if (media_transport_config_.media_transport) { - rtc::CritScope cs(&media_transport_lock_); - media_transport_payload_type_ = payload_type; - // TODO(nisse): Currently broken for G722, since timestamps passed through - // encoder use RTP clock rather than sample count, and they differ for G722. - media_transport_sampling_frequency_ = encoder->RtpTimestampRateHz(); - } audio_coding_->SetEncoder(std::move(encoder)); } @@ -785,13 +634,6 @@ void ChannelSend::OnUplinkPacketLossRate(float packet_loss_rate) { } void ChannelSend::ReceivedRTCPPacket(const uint8_t* data, size_t length) { - // May be called on either worker thread or network thread. - if (media_transport_config_.media_transport) { - // Ignore RTCP packets while media transport is used. - // Those packets should not arrive, but we are seeing occasional packets. - return; - } - // Deliver RTCP packet to RTP/RTCP module for parsing _rtpRtcpModule->IncomingRtcpPacket(data, length); @@ -1064,19 +906,6 @@ void ChannelSend::SetSendRtpHeaderExtension(bool enable, } int64_t ChannelSend::GetRTT() const { - if (media_transport_config_.media_transport) { - // GetRTT is generally used in the RTCP codepath, where media transport is - // not present and so it shouldn't be needed. But it's also invoked in - // 'GetStats' method, and for now returning media transport RTT here gives - // us "free" rtt stats for media transport. - auto target_rate = - media_transport_config_.media_transport->GetLatestTargetTransferRate(); - if (target_rate.has_value()) { - return target_rate.value().network_estimate.round_trip_time.ms(); - } - - return 0; - } std::vector report_blocks; _rtpRtcpModule->RemoteRTCPStat(&report_blocks); @@ -1106,14 +935,6 @@ void ChannelSend::SetFrameEncryptor( }); } -// TODO(sukhanov): Consider moving TargetTransferRate observer to -// AudioSendStream. Since AudioSendStream owns encoder and configures ANA, it -// makes sense to consolidate all rate (and overhead) calculation there. -void ChannelSend::OnTargetTransferRate(TargetTransferRate rate) { - RTC_DCHECK(media_transport_config_.media_transport); - OnReceivedRtt(rate.network_estimate.round_trip_time.ms()); -} - void ChannelSend::OnReceivedRtt(int64_t rtt_ms) { // Invoke audio encoders OnReceivedRtt(). CallEncoder( @@ -1126,7 +947,6 @@ std::unique_ptr CreateChannelSend( Clock* clock, TaskQueueFactory* task_queue_factory, ProcessThread* module_process_thread, - const MediaTransportConfig& media_transport_config, OverheadObserver* overhead_observer, Transport* rtp_transport, RtcpRttStats* rtcp_rtt_stats, @@ -1137,10 +957,9 @@ std::unique_ptr CreateChannelSend( int rtcp_report_interval_ms, uint32_t ssrc) { return std::make_unique( - clock, task_queue_factory, module_process_thread, media_transport_config, - overhead_observer, rtp_transport, rtcp_rtt_stats, rtc_event_log, - frame_encryptor, crypto_options, extmap_allow_mixed, - rtcp_report_interval_ms, ssrc); + clock, task_queue_factory, module_process_thread, overhead_observer, + rtp_transport, rtcp_rtt_stats, rtc_event_log, frame_encryptor, + crypto_options, extmap_allow_mixed, rtcp_report_interval_ms, ssrc); } } // namespace voe diff --git a/audio/channel_send.h b/audio/channel_send.h index 053b69a4e0..6f73c2b331 100644 --- a/audio/channel_send.h +++ b/audio/channel_send.h @@ -20,8 +20,6 @@ #include "api/crypto/crypto_options.h" #include "api/function_view.h" #include "api/task_queue/task_queue_factory.h" -#include "api/transport/media/media_transport_config.h" -#include "api/transport/media/media_transport_interface.h" #include "modules/rtp_rtcp/include/report_block_data.h" #include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/source/rtp_sender_audio.h" @@ -129,7 +127,6 @@ std::unique_ptr CreateChannelSend( Clock* clock, TaskQueueFactory* task_queue_factory, ProcessThread* module_process_thread, - const MediaTransportConfig& media_transport_config, OverheadObserver* overhead_observer, Transport* rtp_transport, RtcpRttStats* rtcp_rtt_stats, diff --git a/audio/test/media_transport_test.cc b/audio/test/media_transport_test.cc deleted file mode 100644 index bee0539fed..0000000000 --- a/audio/test/media_transport_test.cc +++ /dev/null @@ -1,160 +0,0 @@ -/* - * Copyright (c) 2018 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 - -#include "api/audio_codecs/audio_decoder_factory_template.h" -#include "api/audio_codecs/audio_encoder_factory_template.h" -#include "api/audio_codecs/opus/audio_decoder_opus.h" -#include "api/audio_codecs/opus/audio_encoder_opus.h" -#include "api/rtc_event_log/rtc_event_log.h" -#include "api/task_queue/default_task_queue_factory.h" -#include "api/test/loopback_media_transport.h" -#include "api/test/mock_audio_mixer.h" -#include "api/transport/media/media_transport_config.h" -#include "audio/audio_receive_stream.h" -#include "audio/audio_send_stream.h" -#include "call/rtp_transport_controller_send.h" -#include "call/test/mock_bitrate_allocator.h" -#include "modules/audio_device/include/test_audio_device.h" -#include "modules/audio_mixer/audio_mixer_impl.h" -#include "modules/audio_processing/include/mock_audio_processing.h" -#include "modules/utility/include/process_thread.h" -#include "rtc_base/time_utils.h" -#include "test/gtest.h" -#include "test/mock_transport.h" - -namespace webrtc { -namespace test { - -namespace { -using ::testing::NiceMock; - -constexpr int kPayloadTypeOpus = 17; -constexpr int kSamplingFrequency = 48000; -constexpr int kNumChannels = 2; -constexpr int kWantedSamples = 3000; -constexpr int kTestTimeoutMs = 2 * rtc::kNumMillisecsPerSec; - -class TestRenderer : public TestAudioDeviceModule::Renderer { - public: - TestRenderer(int sampling_frequency, int num_channels, size_t wanted_samples) - : sampling_frequency_(sampling_frequency), - num_channels_(num_channels), - wanted_samples_(wanted_samples) {} - ~TestRenderer() override = default; - - int SamplingFrequency() const override { return sampling_frequency_; } - int NumChannels() const override { return num_channels_; } - - bool Render(rtc::ArrayView data) override { - if (data.size() >= wanted_samples_) { - return false; - } - wanted_samples_ -= data.size(); - return true; - } - - private: - const int sampling_frequency_; - const int num_channels_; - size_t wanted_samples_; -}; - -} // namespace - -TEST(AudioWithMediaTransport, DeliversAudio) { - std::unique_ptr transport_thread = rtc::Thread::Create(); - transport_thread->Start(); - std::unique_ptr task_queue_factory = - CreateDefaultTaskQueueFactory(); - MediaTransportPair transport_pair(transport_thread.get()); - NiceMock rtcp_send_transport; - NiceMock send_transport; - RtcEventLogNull null_event_log; - NiceMock bitrate_allocator; - - rtc::scoped_refptr audio_device = - TestAudioDeviceModule::Create( - task_queue_factory.get(), - TestAudioDeviceModule::CreatePulsedNoiseCapturer( - /* max_amplitude= */ 10000, kSamplingFrequency, kNumChannels), - std::make_unique(kSamplingFrequency, kNumChannels, - kWantedSamples)); - - AudioState::Config audio_config; - audio_config.audio_mixer = AudioMixerImpl::Create(); - // TODO(nisse): Is a mock AudioProcessing enough? - audio_config.audio_processing = - new rtc::RefCountedObject(); - audio_config.audio_device_module = audio_device; - rtc::scoped_refptr audio_state = AudioState::Create(audio_config); - - // TODO(nisse): Use some lossless codec? - const SdpAudioFormat audio_format("opus", kSamplingFrequency, kNumChannels); - - // Setup receive stream; - webrtc::AudioReceiveStream::Config receive_config; - // TODO(nisse): Update AudioReceiveStream to not require rtcp_send_transport - // when a MediaTransport is provided. - receive_config.rtcp_send_transport = &rtcp_send_transport; - receive_config.media_transport_config.media_transport = - transport_pair.first(); - receive_config.decoder_map.emplace(kPayloadTypeOpus, audio_format); - receive_config.decoder_factory = - CreateAudioDecoderFactory(); - - std::unique_ptr receive_process_thread = - ProcessThread::Create("audio recv thread"); - - webrtc::internal::AudioReceiveStream receive_stream( - Clock::GetRealTimeClock(), - /*receiver_controller=*/nullptr, - /*packet_router=*/nullptr, receive_process_thread.get(), - /*neteq_factory=*/nullptr, receive_config, audio_state, &null_event_log); - - // TODO(nisse): Update AudioSendStream to not require send_transport when a - // MediaTransport is provided. - AudioSendStream::Config send_config( - &send_transport, webrtc::MediaTransportConfig(transport_pair.second())); - send_config.send_codec_spec = - AudioSendStream::Config::SendCodecSpec(kPayloadTypeOpus, audio_format); - send_config.encoder_factory = CreateAudioEncoderFactory(); - std::unique_ptr send_process_thread = - ProcessThread::Create("audio send thread"); - FieldTrialBasedConfig field_trials; - RtpTransportControllerSend rtp_transport( - Clock::GetRealTimeClock(), &null_event_log, nullptr, nullptr, - BitrateConstraints(), ProcessThread::Create("Pacer"), - task_queue_factory.get(), &field_trials); - webrtc::internal::AudioSendStream send_stream( - Clock::GetRealTimeClock(), send_config, audio_state, - task_queue_factory.get(), send_process_thread.get(), &rtp_transport, - &bitrate_allocator, &null_event_log, - /*rtcp_rtt_stats=*/nullptr, absl::optional()); - - audio_device->Init(); // Starts thread. - audio_device->RegisterAudioCallback(audio_state->audio_transport()); - - receive_stream.Start(); - send_stream.Start(); - audio_device->StartPlayout(); - audio_device->StartRecording(); - - EXPECT_TRUE(audio_device->WaitForPlayoutEnd(kTestTimeoutMs)); - - audio_device->StopRecording(); - audio_device->StopPlayout(); - receive_stream.Stop(); - send_stream.Stop(); -} - -} // namespace test -} // namespace webrtc diff --git a/call/BUILD.gn b/call/BUILD.gn index 26a0b377ce..81afe55c3a 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -47,7 +47,6 @@ rtc_library("call_interfaces") { "../api/transport:bitrate_settings", "../api/transport:network_control", "../api/transport:webrtc_key_value_config", - "../api/transport/media:media_transport_interface", "../api/transport/rtp:rtp_source", "../modules/audio_device", "../modules/audio_processing", @@ -292,7 +291,6 @@ rtc_library("video_stream_api") { "../api/crypto:frame_decryptor_interface", "../api/crypto:frame_encryptor_interface", "../api/crypto:options", - "../api/transport/media:media_transport_interface", "../api/transport/rtp:rtp_source", "../api/video:video_frame", "../api/video:video_rtp_headers", @@ -383,8 +381,6 @@ if (rtc_include_tests) { ":rtp_sender", ":simulated_network", "../api:array_view", - "../api:fake_media_transport", - "../api:fake_media_transport", "../api:mock_audio_mixer", "../api:rtp_headers", "../api:rtp_parameters", diff --git a/call/audio_receive_stream.h b/call/audio_receive_stream.h index 55c1af7f46..090fb82090 100644 --- a/call/audio_receive_stream.h +++ b/call/audio_receive_stream.h @@ -23,7 +23,6 @@ #include "api/crypto/frame_decryptor_interface.h" #include "api/rtp_parameters.h" #include "api/scoped_refptr.h" -#include "api/transport/media/media_transport_config.h" #include "api/transport/rtp/rtp_source.h" #include "call/rtp_config.h" @@ -125,8 +124,6 @@ class AudioReceiveStream { Transport* rtcp_send_transport = nullptr; - MediaTransportConfig media_transport_config; - // NetEq settings. size_t jitter_buffer_max_packets = 200; bool jitter_buffer_fast_accelerate = false; diff --git a/call/audio_send_stream.cc b/call/audio_send_stream.cc index 6fdb39c4aa..ddcba031a7 100644 --- a/call/audio_send_stream.cc +++ b/call/audio_send_stream.cc @@ -21,14 +21,8 @@ namespace webrtc { AudioSendStream::Stats::Stats() = default; AudioSendStream::Stats::~Stats() = default; -AudioSendStream::Config::Config( - Transport* send_transport, - const MediaTransportConfig& media_transport_config) - : send_transport(send_transport), - media_transport_config(media_transport_config) {} - AudioSendStream::Config::Config(Transport* send_transport) - : Config(send_transport, MediaTransportConfig()) {} + : send_transport(send_transport) {} AudioSendStream::Config::~Config() = default; @@ -38,7 +32,6 @@ std::string AudioSendStream::Config::ToString() const { ss << "{rtp: " << rtp.ToString(); ss << ", rtcp_report_interval_ms: " << rtcp_report_interval_ms; ss << ", send_transport: " << (send_transport ? "(Transport)" : "null"); - ss << ", media_transport_config: " << media_transport_config.DebugString(); ss << ", min_bitrate_bps: " << min_bitrate_bps; ss << ", max_bitrate_bps: " << max_bitrate_bps; ss << ", send_codec_spec: " diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h index f3730551dc..734be307f1 100644 --- a/call/audio_send_stream.h +++ b/call/audio_send_stream.h @@ -25,8 +25,6 @@ #include "api/crypto/frame_encryptor_interface.h" #include "api/rtp_parameters.h" #include "api/scoped_refptr.h" -#include "api/transport/media/media_transport_config.h" -#include "api/transport/media/media_transport_interface.h" #include "call/rtp_config.h" #include "modules/audio_processing/include/audio_processing_statistics.h" #include "modules/rtp_rtcp/include/report_block_data.h" @@ -76,8 +74,6 @@ class AudioSendStream { struct Config { Config() = delete; - Config(Transport* send_transport, - const MediaTransportConfig& media_transport_config); explicit Config(Transport* send_transport); ~Config(); std::string ToString() const; @@ -116,8 +112,6 @@ class AudioSendStream { // the entire life of the AudioSendStream and is owned by the API client. Transport* send_transport = nullptr; - MediaTransportConfig media_transport_config; - // Bitrate limits used for variable audio bitrate streams. Set both to -1 to // disable audio bitrate adaptation. // Note: This is still an experimental feature and not ready for real usage. diff --git a/call/call_perf_tests.cc b/call/call_perf_tests.cc index de91b66d97..7e59020b09 100644 --- a/call/call_perf_tests.cc +++ b/call/call_perf_tests.cc @@ -244,8 +244,7 @@ void CallPerfTest::TestAudioVideoSync(FecMode fec, CreateSendConfig(1, 0, 0, video_send_transport.get()); CreateMatchingReceiveConfigs(receive_transport.get()); - AudioSendStream::Config audio_send_config(audio_send_transport.get(), - MediaTransportConfig()); + AudioSendStream::Config audio_send_config(audio_send_transport.get()); audio_send_config.rtp.ssrc = kAudioSendSsrc; audio_send_config.send_codec_spec = AudioSendStream::Config::SendCodecSpec( kAudioSendPayloadType, {"ISAC", 16000, 1}); diff --git a/call/call_unittest.cc b/call/call_unittest.cc index cf2037ee65..754be81645 100644 --- a/call/call_unittest.cc +++ b/call/call_unittest.cc @@ -19,7 +19,6 @@ #include "api/audio_codecs/builtin_audio_decoder_factory.h" #include "api/rtc_event_log/rtc_event_log.h" #include "api/task_queue/default_task_queue_factory.h" -#include "api/test/fake_media_transport.h" #include "api/test/mock_audio_mixer.h" #include "audio/audio_receive_stream.h" #include "audio/audio_send_stream.h" @@ -68,7 +67,7 @@ TEST(CallTest, ConstructDestruct) { TEST(CallTest, CreateDestroy_AudioSendStream) { CallHelper call; MockTransport send_transport; - AudioSendStream::Config config(&send_transport, MediaTransportConfig()); + AudioSendStream::Config config(&send_transport); config.rtp.ssrc = 42; AudioSendStream* stream = call->CreateAudioSendStream(config); EXPECT_NE(stream, nullptr); @@ -91,7 +90,7 @@ TEST(CallTest, CreateDestroy_AudioReceiveStream) { TEST(CallTest, CreateDestroy_AudioSendStreams) { CallHelper call; MockTransport send_transport; - AudioSendStream::Config config(&send_transport, MediaTransportConfig()); + AudioSendStream::Config config(&send_transport); std::list streams; for (int i = 0; i < 2; ++i) { for (uint32_t ssrc = 0; ssrc < 1234567; ssrc += 34567) { @@ -150,7 +149,7 @@ TEST(CallTest, CreateDestroy_AssociateAudioSendReceiveStreams_RecvFirst) { EXPECT_NE(recv_stream, nullptr); MockTransport send_transport; - AudioSendStream::Config send_config(&send_transport, MediaTransportConfig()); + AudioSendStream::Config send_config(&send_transport); send_config.rtp.ssrc = 777; AudioSendStream* send_stream = call->CreateAudioSendStream(send_config); EXPECT_NE(send_stream, nullptr); @@ -169,7 +168,7 @@ TEST(CallTest, CreateDestroy_AssociateAudioSendReceiveStreams_RecvFirst) { TEST(CallTest, CreateDestroy_AssociateAudioSendReceiveStreams_SendFirst) { CallHelper call; MockTransport send_transport; - AudioSendStream::Config send_config(&send_transport, MediaTransportConfig()); + AudioSendStream::Config send_config(&send_transport); send_config.rtp.ssrc = 777; AudioSendStream* send_stream = call->CreateAudioSendStream(send_config); EXPECT_NE(send_stream, nullptr); @@ -273,7 +272,7 @@ TEST(CallTest, RecreatingAudioStreamWithSameSsrcReusesRtpState) { auto create_stream_and_get_rtp_state = [&](uint32_t ssrc) { MockTransport send_transport; - AudioSendStream::Config config(&send_transport, MediaTransportConfig()); + AudioSendStream::Config config(&send_transport); config.rtp.ssrc = ssrc; AudioSendStream* stream = call->CreateAudioSendStream(config); const RtpState rtp_state = diff --git a/call/video_receive_stream.cc b/call/video_receive_stream.cc index acda498b1d..c4895e465a 100644 --- a/call/video_receive_stream.cc +++ b/call/video_receive_stream.cc @@ -69,12 +69,8 @@ std::string VideoReceiveStream::Stats::ToString(int64_t time_ms) const { VideoReceiveStream::Config::Config(const Config&) = default; VideoReceiveStream::Config::Config(Config&&) = default; -VideoReceiveStream::Config::Config(Transport* rtcp_send_transport, - MediaTransportConfig media_transport_config) - : rtcp_send_transport(rtcp_send_transport), - media_transport_config(media_transport_config) {} VideoReceiveStream::Config::Config(Transport* rtcp_send_transport) - : Config(rtcp_send_transport, MediaTransportConfig()) {} + : rtcp_send_transport(rtcp_send_transport) {} VideoReceiveStream::Config& VideoReceiveStream::Config::operator=(Config&&) = default; diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index 2959f67c0d..96c60b519d 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -22,8 +22,6 @@ #include "api/crypto/frame_decryptor_interface.h" #include "api/rtp_headers.h" #include "api/rtp_parameters.h" -#include "api/transport/media/media_transport_config.h" -#include "api/transport/media/media_transport_interface.h" #include "api/transport/rtp/rtp_source.h" #include "api/video/video_content_type.h" #include "api/video/video_frame.h" @@ -139,8 +137,6 @@ class VideoReceiveStream { public: Config() = delete; Config(Config&&); - Config(Transport* rtcp_send_transport, - MediaTransportConfig media_transport_config); explicit Config(Transport* rtcp_send_transport); Config& operator=(Config&&); Config& operator=(const Config&) = delete; @@ -151,10 +147,6 @@ class VideoReceiveStream { std::string ToString() const; - MediaTransportInterface* media_transport() const { - return media_transport_config.media_transport; - } - // Decoders for every payload that we can receive. std::vector decoders; @@ -217,8 +209,6 @@ class VideoReceiveStream { // Transport for outgoing packets (RTCP). Transport* rtcp_send_transport = nullptr; - MediaTransportConfig media_transport_config; - // Must always be set. rtc::VideoSinkInterface* renderer = nullptr; diff --git a/call/video_send_stream.cc b/call/video_send_stream.cc index dac4029876..f495d085cf 100644 --- a/call/video_send_stream.cc +++ b/call/video_send_stream.cc @@ -75,14 +75,10 @@ std::string VideoSendStream::Stats::ToString(int64_t time_ms) const { VideoSendStream::Config::Config(const Config&) = default; VideoSendStream::Config::Config(Config&&) = default; -VideoSendStream::Config::Config(Transport* send_transport, - MediaTransportInterface* media_transport) +VideoSendStream::Config::Config(Transport* send_transport) : rtp(), encoder_settings(VideoEncoder::Capabilities(rtp.lntf.enabled)), - send_transport(send_transport), - media_transport(media_transport) {} -VideoSendStream::Config::Config(Transport* send_transport) - : Config(send_transport, nullptr) {} + send_transport(send_transport) {} VideoSendStream::Config& VideoSendStream::Config::operator=(Config&&) = default; VideoSendStream::Config::Config::~Config() = default; @@ -95,7 +91,6 @@ std::string VideoSendStream::Config::ToString() const { ss << ", rtp: " << rtp.ToString(); ss << ", rtcp_report_interval_ms: " << rtcp_report_interval_ms; ss << ", send_transport: " << (send_transport ? "(Transport)" : "nullptr"); - ss << ", media_transport: " << (media_transport ? "(Transport)" : "nullptr"); ss << ", render_delay_ms: " << render_delay_ms; ss << ", target_delay_ms: " << target_delay_ms; ss << ", suspend_below_min_bitrate: " diff --git a/call/video_send_stream.h b/call/video_send_stream.h index 478d73cf33..39abdfc808 100644 --- a/call/video_send_stream.h +++ b/call/video_send_stream.h @@ -21,7 +21,6 @@ #include "api/call/transport.h" #include "api/crypto/crypto_options.h" #include "api/rtp_parameters.h" -#include "api/transport/media/media_transport_interface.h" #include "api/video/video_content_type.h" #include "api/video/video_frame.h" #include "api/video/video_sink_interface.h" @@ -116,7 +115,6 @@ class VideoSendStream { public: Config() = delete; Config(Config&&); - Config(Transport* send_transport, MediaTransportInterface* media_transport); explicit Config(Transport* send_transport); Config& operator=(Config&&); @@ -139,8 +137,6 @@ class VideoSendStream { // Transport for outgoing packets. Transport* send_transport = nullptr; - MediaTransportInterface* media_transport = nullptr; - // Expected delay needed by the renderer, i.e. the frame will be delivered // this many milliseconds, if possible, earlier than expected render time. // Only valid if |local_renderer| is set. diff --git a/media/BUILD.gn b/media/BUILD.gn index 009741f088..9912d2995a 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -523,7 +523,6 @@ if (rtc_include_tests) { ":rtc_vp9_profile", "../:webrtc_common", "../api:create_simulcast_test_fixture_api", - "../api:fake_media_transport", "../api:libjingle_peerconnection_api", "../api:mock_video_bitrate_allocator", "../api:mock_video_bitrate_allocator_factory", diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 026d371f3a..185c8832e7 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -271,10 +271,6 @@ class MediaChannel : public sigslot::has_slots<> { return media_transport_config_; } - webrtc::MediaTransportInterface* media_transport() { - return media_transport_config_.media_transport; - } - // Corresponds to the SDP attribute extmap-allow-mixed, see RFC8285. // Set to true if it's allowed to mix one- and two-byte RTP header extensions // in the same stream. The setter and getter must only be called from diff --git a/media/base/rtp_data_engine_unittest.cc b/media/base/rtp_data_engine_unittest.cc index 79fb2b27c9..e46c83edd3 100644 --- a/media/base/rtp_data_engine_unittest.cc +++ b/media/base/rtp_data_engine_unittest.cc @@ -15,7 +15,6 @@ #include #include -#include "api/transport/media/media_transport_config.h" #include "media/base/fake_network_interface.h" #include "media/base/media_constants.h" #include "media/base/rtp_utils.h" diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 818be4a8c8..f36314fdd3 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -826,25 +826,8 @@ bool WebRtcVideoChannel::ApplyChangedParams( : send_params_.max_bandwidth_bps; } - if (media_transport()) { - webrtc::MediaTransportTargetRateConstraints constraints; - if (bitrate_config_.start_bitrate_bps >= 0) { - constraints.starting_bitrate = - webrtc::DataRate::bps(bitrate_config_.start_bitrate_bps); - } - if (bitrate_config_.max_bitrate_bps > 0) { - constraints.max_bitrate = - webrtc::DataRate::bps(bitrate_config_.max_bitrate_bps); - } - if (bitrate_config_.min_bitrate_bps >= 0) { - constraints.min_bitrate = - webrtc::DataRate::bps(bitrate_config_.min_bitrate_bps); - } - media_transport()->SetTargetBitrateLimits(constraints); - } else { - call_->GetTransportControllerSend()->SetSdpBitrateParameters( - bitrate_config_); - } + call_->GetTransportControllerSend()->SetSdpBitrateParameters( + bitrate_config_); } for (auto& kv : send_streams_) { @@ -1175,7 +1158,7 @@ bool WebRtcVideoChannel::AddSendStream(const StreamParams& sp) { for (uint32_t used_ssrc : sp.ssrcs) send_ssrcs_.insert(used_ssrc); - webrtc::VideoSendStream::Config config(this, media_transport()); + webrtc::VideoSendStream::Config config(this); for (const RidDescription& rid : sp.rids()) { config.rtp.rids.push_back(rid.rid); @@ -1308,7 +1291,7 @@ bool WebRtcVideoChannel::AddRecvStream(const StreamParams& sp, for (uint32_t used_ssrc : sp.ssrcs) receive_ssrcs_.insert(used_ssrc); - webrtc::VideoReceiveStream::Config config(this, media_transport_config()); + webrtc::VideoReceiveStream::Config config(this); webrtc::FlexfecReceiveStream::Config flexfec_config(this); ConfigureReceiverRtp(&config, &flexfec_config, sp); diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 9d6e449919..5c24454914 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -22,7 +22,6 @@ #include "api/rtc_event_log/rtc_event_log.h" #include "api/rtp_parameters.h" #include "api/task_queue/default_task_queue_factory.h" -#include "api/test/fake_media_transport.h" #include "api/test/mock_video_bitrate_allocator.h" #include "api/test/mock_video_bitrate_allocator_factory.h" #include "api/test/mock_video_decoder_factory.h" @@ -4249,82 +4248,6 @@ TEST_F(WebRtcVideoChannelTest, SetSendCodecsWithBitratesAndMaxSendBandwidth) { EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); } -// Test that when both the codec-specific bitrate params and max_bandwidth_bps -// are present in the same send parameters, the settings are combined correctly. -TEST_F(WebRtcVideoChannelTest, - SetSendCodecsWithBitratesAndMaxSendBandwidthForMediaTransport) { - // Same as SetSendCodecsWithBitratesAndMaxSendBandwidth but with Media - // Transport. - webrtc::MediaTransportSettings settings; - settings.is_caller = true; - webrtc::FakeMediaTransport fake_media_transport(settings); - std::unique_ptr network_interface( - new cricket::FakeNetworkInterface); - channel_->SetInterface(network_interface.get(), - webrtc::MediaTransportConfig(&fake_media_transport)); - - send_parameters_.codecs[0].params[kCodecParamMinBitrate] = "100"; - send_parameters_.codecs[0].params[kCodecParamStartBitrate] = "200"; - send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "300"; - send_parameters_.max_bandwidth_bps = 400000; - { - // We expect max_bandwidth_bps to take priority, if set. - ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); - ASSERT_EQ(1u, - fake_media_transport.target_rate_constraints_in_order().size()); - const webrtc::MediaTransportTargetRateConstraints& constraint = - fake_media_transport.target_rate_constraints_in_order()[0]; - ASSERT_EQ(webrtc::DataRate::bps(100000), constraint.min_bitrate); - ASSERT_EQ(webrtc::DataRate::bps(200000), constraint.starting_bitrate); - ASSERT_EQ(webrtc::DataRate::bps(400000), constraint.max_bitrate); - } - - { - // Decrease max_bandwidth_bps. - send_parameters_.max_bandwidth_bps = 350000; - ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); - ASSERT_EQ(2u, - fake_media_transport.target_rate_constraints_in_order().size()); - const webrtc::MediaTransportTargetRateConstraints& constraint = - fake_media_transport.target_rate_constraints_in_order()[1]; - - // Since the codec isn't changing, start_bitrate_bps should be 0. - ASSERT_EQ(webrtc::DataRate::bps(100000), constraint.min_bitrate); - ASSERT_EQ(absl::nullopt, constraint.starting_bitrate); - ASSERT_EQ(webrtc::DataRate::bps(350000), constraint.max_bitrate); - } - - { - // Now try again with the values flipped around. - send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "400"; - send_parameters_.max_bandwidth_bps = 300000; - ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); - ASSERT_EQ(3u, - fake_media_transport.target_rate_constraints_in_order().size()); - const webrtc::MediaTransportTargetRateConstraints& constraint = - fake_media_transport.target_rate_constraints_in_order()[2]; - - ASSERT_EQ(webrtc::DataRate::bps(100000), constraint.min_bitrate); - ASSERT_EQ(webrtc::DataRate::bps(200000), constraint.starting_bitrate); - ASSERT_EQ(webrtc::DataRate::bps(300000), constraint.max_bitrate); - } - - { - // Now try again with the values flipped around. - // If we change the codec max, max_bandwidth_bps should still apply. - send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "350"; - ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); - ASSERT_EQ(4u, - fake_media_transport.target_rate_constraints_in_order().size()); - const webrtc::MediaTransportTargetRateConstraints& constraint = - fake_media_transport.target_rate_constraints_in_order()[3]; - - ASSERT_EQ(webrtc::DataRate::bps(100000), constraint.min_bitrate); - ASSERT_EQ(webrtc::DataRate::bps(200000), constraint.starting_bitrate); - ASSERT_EQ(webrtc::DataRate::bps(300000), constraint.max_bitrate); - } -} - TEST_F(WebRtcVideoChannelTest, SetMaxSendBandwidthShouldPreserveOtherBitrates) { SetSendCodecsShouldWorkForBitrates("100", 100000, "150", 150000, "200", 200000); diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 201503afff..eab2bc860f 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -21,7 +21,6 @@ #include "absl/strings/match.h" #include "api/audio_codecs/audio_codec_pair_id.h" #include "api/call/audio_sink.h" -#include "api/transport/media/media_transport_interface.h" #include "media/base/audio_source.h" #include "media/base/media_constants.h" #include "media/base/stream_params.h" @@ -697,13 +696,12 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream const absl::optional& audio_network_adaptor_config, webrtc::Call* call, webrtc::Transport* send_transport, - const webrtc::MediaTransportConfig& media_transport_config, const rtc::scoped_refptr& encoder_factory, const absl::optional codec_pair_id, rtc::scoped_refptr frame_encryptor, const webrtc::CryptoOptions& crypto_options) : call_(call), - config_(send_transport, media_transport_config), + config_(send_transport), max_send_bitrate_bps_(max_send_bitrate_bps), rtp_parameters_(CreateRtpParametersWithOneEncoding()) { RTC_DCHECK(call); @@ -1052,7 +1050,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { const std::vector& extensions, webrtc::Call* call, webrtc::Transport* rtcp_send_transport, - const webrtc::MediaTransportConfig& media_transport_config, const rtc::scoped_refptr& decoder_factory, const std::map& decoder_map, absl::optional codec_pair_id, @@ -1070,7 +1067,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { config_.rtp.nack.rtp_history_ms = use_nack ? kNackRtpHistoryMs : 0; config_.rtp.extensions = extensions; config_.rtcp_send_transport = rtcp_send_transport; - config_.media_transport_config = media_transport_config; config_.jitter_buffer_max_packets = jitter_buffer_max_packets; config_.jitter_buffer_fast_accelerate = jitter_buffer_fast_accelerate; config_.jitter_buffer_min_delay_ms = jitter_buffer_min_delay_ms; @@ -1803,8 +1799,8 @@ bool WebRtcVoiceMediaChannel::AddSendStream(const StreamParams& sp) { ssrc, mid_, sp.cname, sp.id, send_codec_spec_, ExtmapAllowMixed(), send_rtp_extensions_, max_send_bitrate_bps_, audio_config_.rtcp_report_interval_ms, audio_network_adaptor_config, - call_, this, media_transport_config(), engine()->encoder_factory_, - codec_pair_id_, nullptr, crypto_options_); + call_, this, engine()->encoder_factory_, codec_pair_id_, nullptr, + crypto_options_); send_streams_.insert(std::make_pair(ssrc, stream)); // At this point the stream's local SSRC has been updated. If it is the first @@ -1884,9 +1880,8 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { ssrc, new WebRtcAudioReceiveStream( ssrc, receiver_reports_ssrc_, recv_transport_cc_enabled_, recv_nack_enabled_, sp.stream_ids(), recv_rtp_extensions_, - call_, this, media_transport_config(), - engine()->decoder_factory_, decoder_map_, codec_pair_id_, - engine()->audio_jitter_buffer_max_packets_, + call_, this, engine()->decoder_factory_, decoder_map_, + codec_pair_id_, engine()->audio_jitter_buffer_max_packets_, engine()->audio_jitter_buffer_fast_accelerate_, engine()->audio_jitter_buffer_min_delay_ms_, engine()->audio_jitter_buffer_enable_rtx_handling_, diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc index fd054df153..f72fad76e0 100644 --- a/media/engine/webrtc_voice_engine_unittest.cc +++ b/media/engine/webrtc_voice_engine_unittest.cc @@ -21,7 +21,6 @@ #include "api/rtp_parameters.h" #include "api/scoped_refptr.h" #include "api/task_queue/default_task_queue_factory.h" -#include "api/transport/media/media_transport_config.h" #include "call/call.h" #include "media/base/fake_media_engine.h" #include "media/base/fake_network_interface.h" diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 01bc47d567..301b86c066 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -47,8 +47,6 @@ rtc_library("rtc_p2p") { "base/ice_transport_internal.h", "base/mdns_message.cc", "base/mdns_message.h", - "base/no_op_dtls_transport.cc", - "base/no_op_dtls_transport.h", "base/p2p_constants.cc", "base/p2p_constants.h", "base/p2p_transport_channel.cc", diff --git a/p2p/base/no_op_dtls_transport.cc b/p2p/base/no_op_dtls_transport.cc deleted file mode 100644 index 0ce03b930c..0000000000 --- a/p2p/base/no_op_dtls_transport.cc +++ /dev/null @@ -1,162 +0,0 @@ -/* - * Copyright 2018 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 "p2p/base/no_op_dtls_transport.h" - -#include -#include -#include - -#include "absl/memory/memory.h" -#include "api/rtc_event_log/rtc_event_log.h" -#include "logging/rtc_event_log/events/rtc_event_dtls_transport_state.h" -#include "logging/rtc_event_log/events/rtc_event_dtls_writable_state.h" -#include "p2p/base/packet_transport_internal.h" -#include "rtc_base/buffer.h" -#include "rtc_base/checks.h" -#include "rtc_base/dscp.h" -#include "rtc_base/logging.h" -#include "rtc_base/message_queue.h" -#include "rtc_base/rtc_certificate.h" -#include "rtc_base/ssl_stream_adapter.h" -#include "rtc_base/stream.h" -#include "rtc_base/thread.h" - -namespace cricket { - -NoOpDtlsTransport::NoOpDtlsTransport( - IceTransportInternal* ice_transport, - const webrtc::CryptoOptions& crypto_options) - : crypto_options_(webrtc::CryptoOptions::NoGcm()), - ice_transport_(ice_transport) { - RTC_DCHECK(ice_transport_); - ice_transport_->SignalWritableState.connect( - this, &NoOpDtlsTransport::OnWritableState); - ice_transport_->SignalReadyToSend.connect(this, - &NoOpDtlsTransport::OnReadyToSend); - ice_transport_->SignalReceivingState.connect( - this, &NoOpDtlsTransport::OnReceivingState); - ice_transport_->SignalNetworkRouteChanged.connect( - this, &NoOpDtlsTransport::OnNetworkRouteChanged); -} - -NoOpDtlsTransport::~NoOpDtlsTransport() {} -const webrtc::CryptoOptions& NoOpDtlsTransport::crypto_options() const { - return crypto_options_; -} -DtlsTransportState NoOpDtlsTransport::dtls_state() const { - return DTLS_TRANSPORT_CONNECTED; -} -int NoOpDtlsTransport::component() const { - return kNoOpDtlsTransportComponent; -} -bool NoOpDtlsTransport::IsDtlsActive() const { - return true; -} -bool NoOpDtlsTransport::GetDtlsRole(rtc::SSLRole* role) const { - return false; -} -bool NoOpDtlsTransport::SetDtlsRole(rtc::SSLRole role) { - return false; -} -bool NoOpDtlsTransport::GetSslVersionBytes(int* version) const { - return false; -} -bool NoOpDtlsTransport::GetSrtpCryptoSuite(int* cipher) { - return false; -} -bool NoOpDtlsTransport::GetSslCipherSuite(int* cipher) { - return false; -} -rtc::scoped_refptr NoOpDtlsTransport::GetLocalCertificate() - const { - return rtc::scoped_refptr(); -} -bool NoOpDtlsTransport::SetLocalCertificate( - const rtc::scoped_refptr& certificate) { - return false; -} -std::unique_ptr NoOpDtlsTransport::GetRemoteSSLCertChain() - const { - return std::unique_ptr(); -} -bool NoOpDtlsTransport::ExportKeyingMaterial(const std::string& label, - const uint8_t* context, - size_t context_len, - bool use_context, - uint8_t* result, - size_t result_len) { - return false; -} -bool NoOpDtlsTransport::SetRemoteFingerprint(const std::string& digest_alg, - const uint8_t* digest, - size_t digest_len) { - return true; -} -bool NoOpDtlsTransport::SetSslMaxProtocolVersion( - rtc::SSLProtocolVersion version) { - return true; -} -IceTransportInternal* NoOpDtlsTransport::ice_transport() { - return ice_transport_; -} - -void NoOpDtlsTransport::OnReadyToSend(rtc::PacketTransportInternal* transport) { - RTC_DCHECK_RUN_ON(&thread_checker_); - if (is_writable_) { - SignalReadyToSend(this); - } -} - -void NoOpDtlsTransport::OnWritableState( - rtc::PacketTransportInternal* transport) { - RTC_DCHECK_RUN_ON(&thread_checker_); - is_writable_ = ice_transport_->writable(); - if (is_writable_) { - SignalWritableState(this); - } -} -const std::string& NoOpDtlsTransport::transport_name() const { - return ice_transport_->transport_name(); -} -bool NoOpDtlsTransport::writable() const { - return ice_transport_->writable(); -} -bool NoOpDtlsTransport::receiving() const { - return ice_transport_->receiving(); -} -int NoOpDtlsTransport::SendPacket(const char* data, - size_t len, - const rtc::PacketOptions& options, - int flags) { - return 0; -} - -int NoOpDtlsTransport::SetOption(rtc::Socket::Option opt, int value) { - return ice_transport_->SetOption(opt, value); -} - -int NoOpDtlsTransport::GetError() { - return ice_transport_->GetError(); -} - -void NoOpDtlsTransport::OnNetworkRouteChanged( - absl::optional network_route) { - RTC_DCHECK_RUN_ON(&thread_checker_); - SignalNetworkRouteChanged(network_route); -} - -void NoOpDtlsTransport::OnReceivingState( - rtc::PacketTransportInternal* transport) { - RTC_DCHECK_RUN_ON(&thread_checker_); - SignalReceivingState(this); -} - -} // namespace cricket diff --git a/p2p/base/no_op_dtls_transport.h b/p2p/base/no_op_dtls_transport.h deleted file mode 100644 index f8829dbfa9..0000000000 --- a/p2p/base/no_op_dtls_transport.h +++ /dev/null @@ -1,112 +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. - */ - -#ifndef P2P_BASE_NO_OP_DTLS_TRANSPORT_H_ -#define P2P_BASE_NO_OP_DTLS_TRANSPORT_H_ - -#include -#include -#include - -#include "api/crypto/crypto_options.h" -#include "p2p/base/dtls_transport_internal.h" -#include "p2p/base/ice_transport_internal.h" -#include "p2p/base/packet_transport_internal.h" -#include "rtc_base/buffer.h" -#include "rtc_base/buffer_queue.h" -#include "rtc_base/constructor_magic.h" -#include "rtc_base/ssl_stream_adapter.h" -#include "rtc_base/stream.h" -#include "rtc_base/strings/string_builder.h" -#include "rtc_base/thread_checker.h" - -namespace cricket { - -constexpr int kNoOpDtlsTransportComponent = -1; - -// This implementation wraps a cricket::DtlsTransport, and takes -// ownership of it. -// The implementation does not perform any operations, except of being -// "connected". The purpose of this implementation is to disable RTP transport -// while MediaTransport is used. -// -// This implementation is only temporary. Long-term we will refactor and disable -// RTP transport entirely when MediaTransport is used. Always connected (after -// ICE), no-op, dtls transport. This is used when DTLS is disabled. -// -// MaybeCreateJsepTransport controller expects DTLS connection to send a -// 'connected' signal _after_ it is created (if it is created in a connected -// state, that would not be noticed by jsep transport controller). Therefore, -// the no-op dtls transport will wait for ICE event "writable", and then -// immediately report that it's connected (emulating 0-rtt connection). -// -// We could simply not set a dtls to active (not set a certificate on the DTLS), -// and it would use an underyling connection instead. -// However, when MediaTransport is used, we want to entirely disable -// dtls/srtp/rtp, in order to avoid multiplexing issues, such as "Failed to -// unprotect RTCP packet". -class NoOpDtlsTransport : public DtlsTransportInternal { - public: - NoOpDtlsTransport(IceTransportInternal* ice_transport, - const webrtc::CryptoOptions& crypto_options); - - ~NoOpDtlsTransport() override; - const webrtc::CryptoOptions& crypto_options() const override; - DtlsTransportState dtls_state() const override; - int component() const override; - bool IsDtlsActive() const override; - bool GetDtlsRole(rtc::SSLRole* role) const override; - bool SetDtlsRole(rtc::SSLRole role) override; - bool GetSslVersionBytes(int* version) const override; - bool GetSrtpCryptoSuite(int* cipher) override; - bool GetSslCipherSuite(int* cipher) override; - rtc::scoped_refptr GetLocalCertificate() const override; - bool SetLocalCertificate( - const rtc::scoped_refptr& certificate) override; - std::unique_ptr GetRemoteSSLCertChain() const override; - bool ExportKeyingMaterial(const std::string& label, - const uint8_t* context, - size_t context_len, - bool use_context, - uint8_t* result, - size_t result_len) override; - bool SetRemoteFingerprint(const std::string& digest_alg, - const uint8_t* digest, - size_t digest_len) override; - bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) override; - IceTransportInternal* ice_transport() override; - - const std::string& transport_name() const override; - bool writable() const override; - bool receiving() const override; - - private: - void OnReadyToSend(rtc::PacketTransportInternal* transport); - void OnWritableState(rtc::PacketTransportInternal* transport); - void OnNetworkRouteChanged(absl::optional network_route); - void OnReceivingState(rtc::PacketTransportInternal* transport); - - int SendPacket(const char* data, - size_t len, - const rtc::PacketOptions& options, - int flags) override; - int SetOption(rtc::Socket::Option opt, int value) override; - int GetError() override; - - rtc::ThreadChecker thread_checker_; - - webrtc::CryptoOptions crypto_options_; - IceTransportInternal* ice_transport_; - bool is_writable_ = false; -}; - -} // namespace cricket - -#endif // P2P_BASE_NO_OP_DTLS_TRANSPORT_H_ diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 4d0b61f9fe..bc44bbbebd 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -606,7 +606,6 @@ if (rtc_include_tests) { ":libjingle_peerconnection", ":pc_test_utils", "../api:callfactory_api", - "../api:fake_media_transport", "../api:rtc_event_log_output_file", "../api:rtc_stats_api", "../api:rtp_parameters", diff --git a/pc/channel.cc b/pc/channel.cc index 83927750e5..fc5337a7fd 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -150,10 +150,6 @@ BaseChannel::~BaseChannel() { TRACE_EVENT0("webrtc", "BaseChannel::~BaseChannel"); RTC_DCHECK_RUN_ON(worker_thread_); - if (media_transport_config_.media_transport) { - media_transport_config_.media_transport->RemoveNetworkChangeCallback(this); - } - // Eats any outstanding messages or packets. worker_thread_->Clear(&invoker_); worker_thread_->Clear(this); @@ -171,15 +167,8 @@ bool BaseChannel::ConnectToRtpTransport() { } rtp_transport_->SignalReadyToSend.connect( this, &BaseChannel::OnTransportReadyToSend); - - // If media transport is used, it's responsible for providing network - // route changed callbacks. - if (!media_transport_config_.media_transport) { - rtp_transport_->SignalNetworkRouteChanged.connect( - this, &BaseChannel::OnNetworkRouteChanged); - } - // TODO(bugs.webrtc.org/9719): Media transport should also be used to provide - // 'writable' state here. + rtp_transport_->SignalNetworkRouteChanged.connect( + this, &BaseChannel::OnNetworkRouteChanged); rtp_transport_->SignalWritableState.connect(this, &BaseChannel::OnWritableState); rtp_transport_->SignalSentPacket.connect(this, @@ -208,12 +197,6 @@ void BaseChannel::Init_w( // Both RTP and RTCP channels should be set, we can call SetInterface on // the media channel and it can set network options. media_channel_->SetInterface(this, media_transport_config); - - RTC_LOG(LS_INFO) << "BaseChannel::Init_w, media_transport_config=" - << media_transport_config.DebugString(); - if (media_transport_config_.media_transport) { - media_transport_config_.media_transport->AddNetworkChangeCallback(this); - } } void BaseChannel::Deinit() { @@ -802,9 +785,6 @@ VoiceChannel::VoiceChannel(rtc::Thread* worker_thread, ssrc_generator) {} VoiceChannel::~VoiceChannel() { - if (media_transport()) { - media_transport()->SetFirstAudioPacketReceivedObserver(nullptr); - } TRACE_EVENT0("webrtc", "VoiceChannel::~VoiceChannel"); // this can't be done in the base class, since it calls a virtual DisableMedia_w(); @@ -817,24 +797,10 @@ void BaseChannel::UpdateMediaSendRecvState() { [this] { UpdateMediaSendRecvState_w(); }); } -void BaseChannel::OnNetworkRouteChanged( - const rtc::NetworkRoute& network_route) { - OnNetworkRouteChanged(absl::make_optional(network_route)); -} - void VoiceChannel::Init_w( webrtc::RtpTransportInternal* rtp_transport, const webrtc::MediaTransportConfig& media_transport_config) { BaseChannel::Init_w(rtp_transport, media_transport_config); - if (media_transport_config.media_transport) { - media_transport_config.media_transport->SetFirstAudioPacketReceivedObserver( - this); - } -} - -void VoiceChannel::OnFirstAudioPacketReceived(int64_t channel_id) { - has_received_packet_ = true; - signaling_thread()->Post(RTC_FROM_HERE, this, MSG_FIRSTPACKETRECEIVED); } void VoiceChannel::UpdateMediaSendRecvState_w() { diff --git a/pc/channel.h b/pc/channel.h index 62fcaa25d6..c2b9e40dec 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -74,8 +74,7 @@ class BaseChannel : public ChannelInterface, public rtc::MessageHandler, public sigslot::has_slots<>, public MediaChannel::NetworkInterface, - public webrtc::RtpPacketSinkInterface, - public webrtc::MediaTransportNetworkChangeCallback { + public webrtc::RtpPacketSinkInterface { public: // If |srtp_required| is true, the channel will not send or receive any // RTP/RTCP packets without using SRTP (either using SDES or DTLS-SRTP). @@ -156,11 +155,6 @@ class BaseChannel : public ChannelInterface, // Fired on the network thread. sigslot::signal1 SignalRtcpMuxFullyActive; - // Returns media transport, can be null if media transport is not available. - webrtc::MediaTransportInterface* media_transport() { - return media_transport_config_.media_transport; - } - // From RtpTransport - public for testing only void OnTransportReadyToSend(bool ready); @@ -287,9 +281,6 @@ class BaseChannel : public ChannelInterface, void SignalSentPacket_n(const rtc::SentPacket& sent_packet); bool IsReadyToSendMedia_n() const; - // MediaTransportNetworkChangeCallback override. - void OnNetworkRouteChanged(const rtc::NetworkRoute& network_route) override; - rtc::Thread* const worker_thread_; rtc::Thread* const network_thread_; rtc::Thread* const signaling_thread_; @@ -337,8 +328,7 @@ class BaseChannel : public ChannelInterface, // VoiceChannel is a specialization that adds support for early media, DTMF, // and input/output level monitoring. -class VoiceChannel : public BaseChannel, - public webrtc::AudioPacketReceivedObserver { +class VoiceChannel : public BaseChannel { public: VoiceChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, @@ -372,8 +362,6 @@ class VoiceChannel : public BaseChannel, webrtc::SdpType type, std::string* error_desc) override; - void OnFirstAudioPacketReceived(int64_t channel_id) override; - // Last AudioSendParameters sent down to the media_channel() via // SetSendParameters. AudioSendParameters last_send_params_; diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc index ab3b88b76e..90785131f9 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc @@ -13,7 +13,6 @@ #include #include "api/rtc_error.h" -#include "api/test/fake_media_transport.h" #include "api/transport/media/media_transport_config.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "media/base/fake_media_engine.h" @@ -74,18 +73,6 @@ class ChannelManagerTest : public ::testing::Test { return dtls_srtp_transport; } - std::unique_ptr CreateMediaTransport( - rtc::PacketTransportInternal* packet_transport) { - webrtc::MediaTransportSettings settings; - settings.is_caller = true; - auto media_transport_result = - fake_media_transport_factory_.CreateMediaTransport( - packet_transport, network_.get(), - /*is_caller=*/settings); - RTC_CHECK(media_transport_result.ok()); - return media_transport_result.MoveValue(); - } - void TestCreateDestroyChannels( webrtc::RtpTransportInternal* rtp_transport, webrtc::MediaTransportConfig media_transport_config) { @@ -122,7 +109,6 @@ class ChannelManagerTest : public ::testing::Test { cricket::FakeDataEngine* fdme_; std::unique_ptr cm_; cricket::FakeCall fake_call_; - webrtc::FakeMediaTransportFactory fake_media_transport_factory_; rtc::UniqueRandomIdGenerator ssrc_generator_; }; @@ -192,14 +178,6 @@ TEST_F(ChannelManagerTest, CreateDestroyChannels) { webrtc::MediaTransportConfig()); } -TEST_F(ChannelManagerTest, CreateDestroyChannelsWithMediaTransport) { - EXPECT_TRUE(cm_->Init()); - auto rtp_transport = CreateDtlsSrtpTransport(); - auto media_transport = CreateMediaTransport(rtp_dtls_transport_.get()); - TestCreateDestroyChannels( - rtp_transport.get(), webrtc::MediaTransportConfig(media_transport.get())); -} - TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { network_->Start(); worker_->Start(); diff --git a/pc/datagram_rtp_transport.h b/pc/datagram_rtp_transport.h index 8aadf977bb..f9684c69c0 100644 --- a/pc/datagram_rtp_transport.h +++ b/pc/datagram_rtp_transport.h @@ -18,6 +18,7 @@ #include "api/crypto/crypto_options.h" #include "api/transport/datagram_transport_interface.h" +#include "api/transport/media/media_transport_interface.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "p2p/base/ice_transport_internal.h" diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc index 79b933c9e5..37f31628dd 100644 --- a/pc/jsep_transport.cc +++ b/pc/jsep_transport.cc @@ -111,7 +111,6 @@ JsepTransport::JsepTransport( std::unique_ptr rtp_dtls_transport, std::unique_ptr rtcp_dtls_transport, std::unique_ptr sctp_transport, - std::unique_ptr media_transport, std::unique_ptr datagram_transport, webrtc::DataChannelTransportInterface* data_channel_transport) : network_thread_(rtc::Thread::Current()), @@ -139,7 +138,6 @@ JsepTransport::JsepTransport( ? new rtc::RefCountedObject( std::move(sctp_transport)) : nullptr), - media_transport_(std::move(media_transport)), datagram_transport_(std::move(datagram_transport)), datagram_rtp_transport_(std::move(datagram_rtp_transport)), data_channel_transport_(data_channel_transport) { @@ -149,7 +147,6 @@ JsepTransport::JsepTransport( // present. RTC_DCHECK_EQ((rtcp_ice_transport_ != nullptr), (rtcp_dtls_transport_ != nullptr)); - RTC_DCHECK(!datagram_transport_ || !media_transport_); // Verify the "only one out of these three can be set" invariant. if (unencrypted_rtp_transport_) { RTC_DCHECK(!sdes_transport); @@ -173,10 +170,6 @@ JsepTransport::JsepTransport( datagram_rtp_transport_.get(), default_rtp_transport()}); } - if (media_transport_) { - media_transport_->SetMediaTransportStateCallback(this); - } - if (data_channel_transport_ && sctp_data_channel_transport_) { composite_data_channel_transport_ = std::make_unique( @@ -186,11 +179,6 @@ JsepTransport::JsepTransport( } JsepTransport::~JsepTransport() { - // Disconnect media transport state callbacks. - if (media_transport_) { - media_transport_->SetMediaTransportStateCallback(nullptr); - } - if (sctp_transport_) { sctp_transport_->Clear(); } @@ -784,18 +772,6 @@ bool JsepTransport::GetTransportStats(DtlsTransportInternal* dtls_transport, return true; } -void JsepTransport::OnStateChanged(webrtc::MediaTransportState state) { - // TODO(bugs.webrtc.org/9719) This method currently fires on the network - // thread, but media transport does not make such guarantees. We need to make - // sure this callback is guaranteed to be executed on the network thread. - RTC_DCHECK_RUN_ON(network_thread_); - { - rtc::CritScope scope(&accessor_lock_); - media_transport_state_ = state; - } - SignalMediaTransportStateChanged(); -} - void JsepTransport::NegotiateDatagramTransport(SdpType type) { RTC_DCHECK(type == SdpType::kAnswer || type == SdpType::kPrAnswer); rtc::CritScope lock(&accessor_lock_); diff --git a/pc/jsep_transport.h b/pc/jsep_transport.h index 658e8e7b72..5f7d46f915 100644 --- a/pc/jsep_transport.h +++ b/pc/jsep_transport.h @@ -21,7 +21,6 @@ #include "api/ice_transport_interface.h" #include "api/jsep.h" #include "api/transport/datagram_transport_interface.h" -#include "api/transport/media/media_transport_interface.h" #include "media/sctp/sctp_transport_internal.h" #include "p2p/base/dtls_transport.h" #include "p2p/base/p2p_constants.h" @@ -89,16 +88,11 @@ struct JsepTransportDescription { // // On Threading: JsepTransport performs work solely on the network thread, and // so its methods should only be called on the network thread. -class JsepTransport : public sigslot::has_slots<>, - public webrtc::MediaTransportStateCallback { +class JsepTransport : public sigslot::has_slots<> { public: // |mid| is just used for log statements in order to identify the Transport. // Note that |local_certificate| is allowed to be null since a remote // description may be set before a local certificate is generated. - // - // |media_trasport| is optional (experimental). If available it will be used - // to send / receive encoded audio and video frames instead of RTP. - // Currently |media_transport| can co-exist with RTP / RTCP transports. JsepTransport( const std::string& mid, const rtc::scoped_refptr& local_certificate, @@ -111,7 +105,6 @@ class JsepTransport : public sigslot::has_slots<>, std::unique_ptr rtp_dtls_transport, std::unique_ptr rtcp_dtls_transport, std::unique_ptr sctp_transport, - std::unique_ptr media_transport, std::unique_ptr datagram_transport, webrtc::DataChannelTransportInterface* data_channel_transport); @@ -246,34 +239,17 @@ class JsepTransport : public sigslot::has_slots<>, return data_channel_transport_; } - // Returns media transport, if available. - // Note that media transport is owned by jseptransport and the pointer - // to media transport will becomes invalid after destruction of jseptransport. - webrtc::MediaTransportInterface* media_transport() const { - rtc::CritScope scope(&accessor_lock_); - return media_transport_.get(); - } - // Returns datagram transport, if available. webrtc::DatagramTransportInterface* datagram_transport() const { rtc::CritScope scope(&accessor_lock_); return datagram_transport_.get(); } - // Returns the latest media transport state. - webrtc::MediaTransportState media_transport_state() const { - rtc::CritScope scope(&accessor_lock_); - return media_transport_state_; - } - // This is signaled when RTCP-mux becomes active and // |rtcp_dtls_transport_| is destroyed. The JsepTransportController will // handle the signal and update the aggregate transport states. sigslot::signal<> SignalRtcpMuxActive; - // This is signaled for changes in |media_transport_| state. - sigslot::signal<> SignalMediaTransportStateChanged; - // Signals that a data channel transport was negotiated and may be used to // send data. The first parameter is |this|. The second parameter is the // transport that was negotiated, or null if negotiation rejected the data @@ -338,9 +314,6 @@ class JsepTransport : public sigslot::has_slots<>, bool GetTransportStats(DtlsTransportInternal* dtls_transport, TransportStats* stats); - // Invoked whenever the state of the media transport changes. - void OnStateChanged(webrtc::MediaTransportState state) override; - // Deactivates, signals removal, and deletes |composite_rtp_transport_| if the // current state of negotiation is sufficient to determine which rtp_transport // and data channel transport to use. @@ -418,10 +391,6 @@ class JsepTransport : public sigslot::has_slots<>, absl::optional> recv_extension_ids_ RTC_GUARDED_BY(network_thread_); - // Optional media transport (experimental). - std::unique_ptr media_transport_ - RTC_GUARDED_BY(accessor_lock_); - // Optional datagram transport (experimental). std::unique_ptr datagram_transport_ RTC_GUARDED_BY(accessor_lock_); @@ -429,9 +398,8 @@ class JsepTransport : public sigslot::has_slots<>, std::unique_ptr datagram_rtp_transport_ RTC_GUARDED_BY(accessor_lock_); - // Non-SCTP data channel transport. Set to one of |media_transport_| or - // |datagram_transport_| if that transport should be used for data chanels. - // Unset if neither should be used for data channels. + // Non-SCTP data channel transport. Set to |datagram_transport_| if that + // transport should be used for data chanels. Unset otherwise. webrtc::DataChannelTransportInterface* data_channel_transport_ RTC_GUARDED_BY(accessor_lock_) = nullptr; @@ -439,15 +407,6 @@ class JsepTransport : public sigslot::has_slots<>, std::unique_ptr composite_data_channel_transport_ RTC_GUARDED_BY(accessor_lock_); - // If |media_transport_| is provided, this variable represents the state of - // media transport. - // - // NOTE: datagram transport state is handled by DatagramDtlsAdaptor, because - // DatagramDtlsAdaptor owns DatagramTransport. This state only represents - // media transport. - webrtc::MediaTransportState media_transport_state_ - RTC_GUARDED_BY(accessor_lock_) = webrtc::MediaTransportState::kPending; - RTC_DISALLOW_COPY_AND_ASSIGN(JsepTransport); }; diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 590aa6b10c..f62cd87bb1 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -18,7 +18,6 @@ #include "api/transport/datagram_transport_interface.h" #include "api/transport/media/media_transport_interface.h" #include "p2p/base/ice_transport_internal.h" -#include "p2p/base/no_op_dtls_transport.h" #include "p2p/base/port.h" #include "pc/datagram_rtp_transport.h" #include "pc/srtp_filter.h" @@ -148,22 +147,12 @@ MediaTransportConfig JsepTransportController::GetMediaTransportConfig( return MediaTransportConfig(); } - MediaTransportInterface* media_transport = nullptr; - if (config_.use_media_transport_for_media) { - media_transport = jsep_transport->media_transport(); - } - DatagramTransportInterface* datagram_transport = nullptr; if (config_.use_datagram_transport) { datagram_transport = jsep_transport->datagram_transport(); } - // Media transport and datagram transports can not be used together. - RTC_DCHECK(!media_transport || !datagram_transport); - - if (media_transport) { - return MediaTransportConfig(media_transport); - } else if (datagram_transport) { + if (datagram_transport) { return MediaTransportConfig( /*rtp_max_packet_size=*/datagram_transport->GetLargestDatagramSize()); } else { @@ -180,15 +169,6 @@ DataChannelTransportInterface* JsepTransportController::GetDataChannelTransport( return jsep_transport->data_channel_transport(); } -MediaTransportState JsepTransportController::GetMediaTransportState( - const std::string& mid) const { - auto jsep_transport = GetJsepTransportForMid(mid); - if (!jsep_transport) { - return MediaTransportState::kPending; - } - return jsep_transport->media_transport_state(); -} - cricket::DtlsTransportInternal* JsepTransportController::GetDtlsTransport( const std::string& mid) { auto jsep_transport = GetJsepTransportForMid(mid); @@ -446,26 +426,9 @@ void JsepTransportController::SetActiveResetSrtpParams( } void JsepTransportController::SetMediaTransportSettings( - bool use_media_transport_for_media, - bool use_media_transport_for_data_channels, bool use_datagram_transport, bool use_datagram_transport_for_data_channels, bool use_datagram_transport_for_data_channels_receive_only) { - RTC_DCHECK(use_media_transport_for_media == - config_.use_media_transport_for_media || - jsep_transports_by_name_.empty()) - << "You can only change media transport configuration before creating " - "the first transport."; - - RTC_DCHECK(use_media_transport_for_data_channels == - config_.use_media_transport_for_data_channels || - jsep_transports_by_name_.empty()) - << "You can only change media transport configuration before creating " - "the first transport."; - - config_.use_media_transport_for_media = use_media_transport_for_media; - config_.use_media_transport_for_data_channels = - use_media_transport_for_data_channels; config_.use_datagram_transport = use_datagram_transport; config_.use_datagram_transport_for_data_channels = use_datagram_transport_for_data_channels; @@ -514,14 +477,6 @@ JsepTransportController::CreateDtlsTransport( if (datagram_transport) { RTC_DCHECK(config_.use_datagram_transport || config_.use_datagram_transport_for_data_channels); - } else if (config_.media_transport_factory && - config_.use_media_transport_for_media && - config_.use_media_transport_for_data_channels) { - // If media transport is used for both media and data channels, - // then we don't need to create DTLS. - // Otherwise, DTLS is still created. - dtls = std::make_unique(ice, - config_.crypto_options); } else if (config_.dtls_transport_factory) { dtls = config_.dtls_transport_factory->CreateDtlsTransport( ice, config_.crypto_options); @@ -916,13 +871,12 @@ bool JsepTransportController::SetTransportForMid( mid_to_transport_[mid] = jsep_transport; return config_.transport_observer->OnTransportChanged( mid, jsep_transport->rtp_transport(), jsep_transport->RtpDtlsTransport(), - jsep_transport->media_transport(), jsep_transport->data_channel_transport()); } void JsepTransportController::RemoveTransportForMid(const std::string& mid) { - bool ret = config_.transport_observer->OnTransportChanged( - mid, nullptr, nullptr, nullptr, nullptr); + bool ret = config_.transport_observer->OnTransportChanged(mid, nullptr, + nullptr, nullptr); // Calling OnTransportChanged with nullptr should always succeed, since it is // only expected to fail when adding media to a transport (not removing). RTC_DCHECK(ret); @@ -1102,76 +1056,6 @@ cricket::JsepTransport* JsepTransportController::GetJsepTransportByName( return (it == jsep_transports_by_name_.end()) ? nullptr : it->second.get(); } -std::unique_ptr -JsepTransportController::MaybeCreateMediaTransport( - const cricket::ContentInfo& content_info, - const cricket::SessionDescription& description, - bool local) { - if (config_.media_transport_factory == nullptr) { - return nullptr; - } - - if (!config_.use_media_transport_for_media && - !config_.use_media_transport_for_data_channels) { - return nullptr; - } - - // Caller (offerer) media transport. - if (local) { - if (offer_media_transport_) { - RTC_LOG(LS_INFO) << "Offered media transport has now been activated."; - return std::move(offer_media_transport_); - } else { - RTC_LOG(LS_INFO) - << "Not returning media transport. Either SDES wasn't enabled, or " - "media transport didn't return an offer earlier."; - // Offer wasn't generated. Either because media transport didn't want it, - // or because SDES wasn't enabled. - return nullptr; - } - } - - // Remote offer. If no x-mt lines, do not create media transport. - if (description.MediaTransportSettings().empty()) { - return nullptr; - } - - // When bundle is enabled, two JsepTransports are created, and then - // the second transport is destroyed (right away). - // For media transport, we don't want to create the second - // media transport in the first place. - RTC_LOG(LS_INFO) << "Returning new, client media transport."; - - RTC_DCHECK(!local) - << "If media transport is used, you must call " - "GenerateOrGetLastMediaTransportOffer before SetLocalDescription. You " - "also " - "must use kRtcpMuxPolicyRequire and kBundlePolicyMaxBundle with media " - "transport."; - MediaTransportSettings settings; - settings.is_caller = local; - if (config_.use_media_transport_for_media) { - settings.event_log = config_.event_log; - } - - // Assume there is only one media transport (or if more, use the first one). - if (!local && !description.MediaTransportSettings().empty() && - config_.media_transport_factory->GetTransportName() == - description.MediaTransportSettings()[0].transport_name) { - settings.remote_transport_parameters = - description.MediaTransportSettings()[0].transport_setting; - } - - auto media_transport_result = - config_.media_transport_factory->CreateMediaTransport(network_thread_, - settings); - - // TODO(sukhanov): Proper error handling. - RTC_CHECK(media_transport_result.ok()); - - return media_transport_result.MoveValue(); -} - // TODO(sukhanov): Refactor to avoid code duplication for Media and Datagram // transports setup. std::unique_ptr @@ -1259,13 +1143,6 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( CreateIceTransport(content_info.name, /*rtcp=*/false); RTC_DCHECK(ice); - std::unique_ptr media_transport = - MaybeCreateMediaTransport(content_info, description, local); - if (media_transport) { - media_transport_created_once_ = true; - media_transport->Connect(ice->internal()); - } - std::unique_ptr datagram_transport = MaybeCreateDatagramTransport(content_info, description, local); if (datagram_transport) { @@ -1285,7 +1162,6 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( if (config_.rtcp_mux_policy != PeerConnectionInterface::kRtcpMuxPolicyRequire && content_info.type == cricket::MediaProtocolType::kRtp) { - RTC_DCHECK(media_transport == nullptr); RTC_DCHECK(datagram_transport == nullptr); rtcp_ice = CreateIceTransport(content_info.name, /*rtcp=*/true); rtcp_dtls_transport = @@ -1335,8 +1211,6 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( DataChannelTransportInterface* data_channel_transport = nullptr; if (config_.use_datagram_transport_for_data_channels) { data_channel_transport = datagram_transport.get(); - } else if (config_.use_media_transport_for_data_channels) { - data_channel_transport = media_transport.get(); } std::unique_ptr jsep_transport = @@ -1345,16 +1219,14 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( std::move(unencrypted_rtp_transport), std::move(sdes_transport), std::move(dtls_srtp_transport), std::move(datagram_rtp_transport), std::move(rtp_dtls_transport), std::move(rtcp_dtls_transport), - std::move(sctp_transport), std::move(media_transport), - std::move(datagram_transport), data_channel_transport); + std::move(sctp_transport), std::move(datagram_transport), + data_channel_transport); jsep_transport->rtp_transport()->SignalRtcpPacketReceived.connect( this, &JsepTransportController::OnRtcpPacketReceived_n); jsep_transport->SignalRtcpMuxActive.connect( this, &JsepTransportController::UpdateAggregateStates_n); - jsep_transport->SignalMediaTransportStateChanged.connect( - this, &JsepTransportController::OnMediaTransportStateChanged_n); jsep_transport->SignalDataChannelTransportNegotiated.connect( this, &JsepTransportController::OnDataChannelTransportNegotiated_n); SetTransportForMid(content_info.name, jsep_transport.get()); @@ -1387,8 +1259,8 @@ void JsepTransportController::DestroyAllJsepTransports_n() { RTC_DCHECK(network_thread_->IsCurrent()); for (const auto& jsep_transport : jsep_transports_by_name_) { - config_.transport_observer->OnTransportChanged( - jsep_transport.first, nullptr, nullptr, nullptr, nullptr); + config_.transport_observer->OnTransportChanged(jsep_transport.first, + nullptr, nullptr, nullptr); } jsep_transports_by_name_.clear(); @@ -1559,10 +1431,6 @@ void JsepTransportController::OnTransportStateChanged_n( UpdateAggregateStates_n(); } -void JsepTransportController::OnMediaTransportStateChanged_n() { - UpdateAggregateStates_n(); -} - void JsepTransportController::OnDataChannelTransportNegotiated_n( cricket::JsepTransport* transport, DataChannelTransportInterface* data_channel_transport) { @@ -1570,7 +1438,7 @@ void JsepTransportController::OnDataChannelTransportNegotiated_n( if (it.second == transport) { config_.transport_observer->OnTransportChanged( it.first, transport->rtp_transport(), transport->RtpDtlsTransport(), - transport->media_transport(), data_channel_transport); + data_channel_transport); } } } @@ -1587,10 +1455,6 @@ void JsepTransportController::UpdateAggregateStates_n() { PeerConnectionInterface::PeerConnectionState::kNew; cricket::IceGatheringState new_gathering_state = cricket::kIceGatheringNew; bool any_failed = false; - - // TODO(http://bugs.webrtc.org/9719) If(when) media_transport disables - // dtls_transports entirely, the below line will have to be changed to account - // for the fact that dtls transports might be absent. bool all_connected = !dtls_transports.empty(); bool all_completed = !dtls_transports.empty(); bool any_gathering = false; @@ -1620,35 +1484,6 @@ void JsepTransportController::UpdateAggregateStates_n() { ice_state_counts[dtls->ice_transport()->GetIceTransportState()]++; } - // Don't indicate that the call failed or isn't connected due to media - // transport state unless the media transport is used for media. If it's only - // used for data channels, it will signal those separately. - if (config_.use_media_transport_for_media || config_.use_datagram_transport) { - for (auto it = jsep_transports_by_name_.begin(); - it != jsep_transports_by_name_.end(); ++it) { - auto jsep_transport = it->second.get(); - if (!jsep_transport->media_transport()) { - continue; - } - - // There is no 'kIceConnectionDisconnected', so we only need to handle - // connected and completed. - // We treat kClosed as failed, because if it happens before shutting down - // media transports it means that there was a failure. - // MediaTransportInterface allows to flip back and forth between kWritable - // and kPending, but there does not exist an implementation that does - // that, and the contract of jsep transport controller doesn't quite - // expect that. When this happens, we would go from connected to - // connecting state, but this may change in future. - any_failed |= jsep_transport->media_transport_state() == - webrtc::MediaTransportState::kClosed; - all_completed &= jsep_transport->media_transport_state() == - webrtc::MediaTransportState::kWritable; - all_connected &= jsep_transport->media_transport_state() == - webrtc::MediaTransportState::kWritable; - } - } - if (any_failed) { new_connection_state = cricket::kIceConnectionFailed; } else if (all_completed) { @@ -1809,67 +1644,6 @@ void JsepTransportController::OnDtlsHandshakeError( SignalDtlsHandshakeError(error); } -absl::optional -JsepTransportController::GenerateOrGetLastMediaTransportOffer() { - if (media_transport_created_once_) { - RTC_LOG(LS_INFO) << "Not regenerating media transport for the new offer in " - "existing session."; - return media_transport_offer_settings_; - } - - RTC_LOG(LS_INFO) << "Generating media transport offer!"; - - absl::optional transport_parameters; - - // Check that media transport is supposed to be used. - // Note that ICE is not available when media transport is created. It will - // only be available in 'Connect'. This may be a potential server config, if - // we decide to use this peer connection as a caller, not as a callee. - // TODO(sukhanov): Avoid code duplication with CreateMedia/MediaTransport. - if (config_.use_media_transport_for_media || - config_.use_media_transport_for_data_channels) { - RTC_DCHECK(config_.media_transport_factory != nullptr); - RTC_DCHECK(!config_.use_datagram_transport); - webrtc::MediaTransportSettings settings; - settings.is_caller = true; - settings.pre_shared_key = rtc::CreateRandomString(32); - if (config_.use_media_transport_for_media) { - settings.event_log = config_.event_log; - } - auto media_transport_or_error = - config_.media_transport_factory->CreateMediaTransport(network_thread_, - settings); - - if (media_transport_or_error.ok()) { - offer_media_transport_ = std::move(media_transport_or_error.value()); - transport_parameters = - offer_media_transport_->GetTransportParametersOffer(); - } else { - RTC_LOG(LS_INFO) << "Unable to create media transport, error=" - << media_transport_or_error.error().message(); - } - } - - if (!offer_media_transport_) { - RTC_LOG(LS_INFO) << "Media and data transports do not exist"; - return absl::nullopt; - } - - if (!transport_parameters) { - RTC_LOG(LS_INFO) << "Media transport didn't generate the offer"; - // Media transport didn't generate the offer, and is not supposed to be - // used. Destroy the temporary media transport. - offer_media_transport_ = nullptr; - return absl::nullopt; - } - - cricket::SessionDescription::MediaTransportSetting setting; - setting.transport_name = config_.media_transport_factory->GetTransportName(); - setting.transport_setting = *transport_parameters; - media_transport_offer_settings_ = setting; - return setting; -} - absl::optional JsepTransportController::GetTransportParameters(const std::string& mid) { if (!(config_.use_datagram_transport || diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index b7121e78dc..9c3f691302 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -23,7 +23,6 @@ #include "api/peer_connection_interface.h" #include "api/rtc_event_log/rtc_event_log.h" #include "api/transport/media/media_transport_config.h" -#include "api/transport/media/media_transport_interface.h" #include "media/sctp/sctp_transport_internal.h" #include "p2p/base/dtls_transport.h" #include "p2p/base/dtls_transport_factory.h" @@ -72,7 +71,6 @@ class JsepTransportController : public sigslot::has_slots<> { const std::string& mid, RtpTransportInternal* rtp_transport, rtc::scoped_refptr dtls_transport, - MediaTransportInterface* media_transport, DataChannelTransportInterface* data_channel_transport) = 0; }; @@ -106,12 +104,6 @@ class JsepTransportController : public sigslot::has_slots<> { // Factory for SCTP transports. cricket::SctpTransportInternalFactory* sctp_factory = nullptr; - // Whether media transport is used for media. - bool use_media_transport_for_media = false; - - // Whether media transport is used for data channels. - bool use_media_transport_for_data_channels = false; - // Whether an RtpMediaTransport should be created as default, when no // MediaTransportFactory is provided. bool use_rtp_media_transport = false; @@ -128,13 +120,13 @@ class JsepTransportController : public sigslot::has_slots<> { bool use_datagram_transport_for_data_channels_receive_only = false; // Optional media transport factory (experimental). If provided it will be - // used to create media_transport (as long as either - // |use_media_transport_for_media| or - // |use_media_transport_for_data_channels| is set to true). However, whether - // it will be used to send / receive audio and video frames instead of RTP - // is determined by |use_media_transport_for_media|. Note that currently - // media_transport co-exists with RTP / RTCP transports and may use the same - // underlying ICE transport. + // used to create datagram_transport (as long as either + // |use_datagram_transport| or + // |use_datagram_transport_for_data_channels| is set to true). However, + // whether it will be used to send / receive audio and video frames instead + // of RTP is determined by |use_datagram_transport|. Note that currently + // datagram_transport co-exists with RTP / RTCP transports and may use the + // same underlying ICE transport. MediaTransportFactory* media_transport_factory = nullptr; }; @@ -174,13 +166,6 @@ class JsepTransportController : public sigslot::has_slots<> { DataChannelTransportInterface* GetDataChannelTransport( const std::string& mid) const; - // TODO(sukhanov): Deprecate, return only config. - MediaTransportInterface* GetMediaTransport(const std::string& mid) const { - return GetMediaTransportConfig(mid).media_transport; - } - - MediaTransportState GetMediaTransportState(const std::string& mid) const; - /********************* * ICE-related methods ********************/ @@ -235,8 +220,6 @@ class JsepTransportController : public sigslot::has_slots<> { // you did not call 'GetMediaTransport' or 'MaybeCreateJsepTransport'. Once // Jsep transport is created, you can't change this setting. void SetMediaTransportSettings( - bool use_media_transport_for_media, - bool use_media_transport_for_data_channels, bool use_datagram_transport, bool use_datagram_transport_for_data_channels, bool use_datagram_transport_for_data_channels_receive_only); @@ -245,13 +228,6 @@ class JsepTransportController : public sigslot::has_slots<> { // and deletes unused transports, but doesn't consider anything more complex. void RollbackTransportForMids(const std::vector& mids); - // If media transport is present enabled and supported, - // when this method is called, it creates a media transport and generates its - // offer. The new offer is then returned, and the created media transport will - // subsequently be used. - absl::optional - GenerateOrGetLastMediaTransportOffer(); - // Gets the transport parameters for the transport identified by |mid|. // If |mid| is bundled, returns the parameters for the bundled transport. // If the transport for |mid| has not been created yet, it may be allocated in @@ -371,16 +347,6 @@ class JsepTransportController : public sigslot::has_slots<> { const cricket::ContentInfo& content_info, const cricket::SessionDescription& description); - // Creates media transport if config wants to use it, and a=x-mt line is - // present for the current media transport. Returned MediaTransportInterface - // is not connected, and must be connected to ICE. You must call - // |GenerateOrGetLastMediaTransportOffer| on the caller before calling - // MaybeCreateMediaTransport. - std::unique_ptr MaybeCreateMediaTransport( - const cricket::ContentInfo& content_info, - const cricket::SessionDescription& description, - bool local); - // Creates datagram transport if config wants to use it, and a=x-mt line is // present for the current media transport. Returned // DatagramTransportInterface is not connected, and must be connected to ICE. @@ -441,7 +407,6 @@ class JsepTransportController : public sigslot::has_slots<> { const cricket::Candidates& candidates); void OnTransportRoleConflict_n(cricket::IceTransportInternal* transport); void OnTransportStateChanged_n(cricket::IceTransportInternal* transport); - void OnMediaTransportStateChanged_n(); void OnTransportCandidatePairChanged_n( const cricket::CandidatePairChangeEvent& event); void OnDataChannelTransportNegotiated_n( @@ -480,21 +445,6 @@ class JsepTransportController : public sigslot::has_slots<> { Config config_; - // Early on in the call we don't know if media transport is going to be used, - // but we need to get the server-supported parameters to add to an SDP. - // This server media transport will be promoted to the used media transport - // after the local description is set, and the ownership will be transferred - // to the actual JsepTransport. - // This "offer" media transport is not created if it's done on the party that - // provides answer. This offer media transport is only created once at the - // beginning of the connection, and never again. - std::unique_ptr offer_media_transport_ = nullptr; - - // Contains the offer of the |offer_media_transport_|, in case if it needs to - // be repeated. - absl::optional - media_transport_offer_settings_; - // Early on in the call we don't know if datagram transport is going to be // used, but we need to get the server-supported parameters to add to an SDP. // This server datagram transport will be promoted to the used datagram @@ -506,24 +456,6 @@ class JsepTransportController : public sigslot::has_slots<> { std::unique_ptr offer_datagram_transport_ = nullptr; - // Contains the offer of the |offer_datagram_transport_|, in case if it needs - // to be repeated. - absl::optional - datagram_transport_offer_settings_; - - // When the new offer is regenerated (due to upgrade), we don't want to - // re-create media transport. New streams might be created; but media - // transport stays the same. This flag prevents re-creation of the transport - // on the offerer. - // The first media transport is created in jsep transport controller as the - // |offer_media_transport_|, and then the ownership is moved to the - // appropriate JsepTransport, at which point |offer_media_transport_| is - // zeroed out. On the callee (answerer), the first media transport is not even - // assigned to |offer_media_transport_|. Both offerer and answerer can - // recreate the Offer (e.g. after adding streams in Plan B), and so we want to - // prevent recreation of the media transport when that happens. - bool media_transport_created_once_ = false; - const cricket::SessionDescription* local_desc_ = nullptr; const cricket::SessionDescription* remote_desc_ = nullptr; absl::optional initial_offerer_; diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index b96a999375..196be131c8 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -19,7 +19,6 @@ #include "p2p/base/dtls_transport_factory.h" #include "p2p/base/fake_dtls_transport.h" #include "p2p/base/fake_ice_transport.h" -#include "p2p/base/no_op_dtls_transport.h" #include "p2p/base/transport_info.h" #include "rtc_base/gunit.h" #include "rtc_base/thread.h" @@ -331,7 +330,6 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, const std::string& mid, RtpTransportInternal* rtp_transport, rtc::scoped_refptr dtls_transport, - MediaTransportInterface* media_transport, DataChannelTransportInterface* data_channel_transport) override { changed_rtp_transport_by_mid_[mid] = rtp_transport; if (dtls_transport) { @@ -339,7 +337,6 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, } else { changed_dtls_transport_by_mid_[mid] = nullptr; } - changed_media_transport_by_mid_[mid] = media_transport; return true; } @@ -373,8 +370,6 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, std::map changed_rtp_transport_by_mid_; std::map changed_dtls_transport_by_mid_; - std::map - changed_media_transport_by_mid_; // Transport controller needs to be destroyed first, because it may issue // callbacks that modify the changed_*_by_mid in the destructor. @@ -443,46 +438,6 @@ TEST_F(JsepTransportControllerTest, GetDtlsTransportWithRtcpMux) { EXPECT_EQ(nullptr, transport_controller_->GetRtcpDtlsTransport(kAudioMid1)); EXPECT_NE(nullptr, transport_controller_->GetDtlsTransport(kVideoMid1)); EXPECT_EQ(nullptr, transport_controller_->GetRtcpDtlsTransport(kVideoMid1)); - EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kAudioMid1)); -} - -TEST_F(JsepTransportControllerTest, - DtlsIsStillCreatedIfMediaTransportIsOnlyUsedForDataChannels) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_data_channels = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - - EXPECT_NE(absl::nullopt, - transport_controller_->GenerateOrGetLastMediaTransportOffer()); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, description.get()) - .ok()); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetDataChannelTransport(kAudioMid1)); - - ASSERT_NE(nullptr, media_transport); - - // After SetLocalDescription, media transport should be created as caller. - EXPECT_TRUE(media_transport->is_caller()); - EXPECT_TRUE(media_transport->pre_shared_key().has_value()); - - // Return nullptr for non-existing mids. - EXPECT_EQ(nullptr, - transport_controller_->GetDataChannelTransport(kVideoMid2)); - - EXPECT_EQ(cricket::ICE_CANDIDATE_COMPONENT_RTP, - transport_controller_->GetDtlsTransport(kAudioMid1)->component()) - << "Media transport for media was not enabled, and so DTLS transport " - "should be created."; } TEST_F(JsepTransportControllerTest, @@ -575,339 +530,6 @@ TEST_F(JsepTransportControllerTest, CannotBundleDifferentAltProtocols) { .ok()); } -TEST_F(JsepTransportControllerTest, GetMediaTransportInCaller) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - - EXPECT_NE(absl::nullopt, - transport_controller_->GenerateOrGetLastMediaTransportOffer()); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, description.get()) - .ok()); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - - ASSERT_NE(nullptr, media_transport); - - // After SetLocalDescription, media transport should be created as caller. - EXPECT_TRUE(media_transport->is_caller()); - // We set the pre-shared key on the caller. - EXPECT_TRUE(media_transport->pre_shared_key().has_value()); - EXPECT_TRUE(media_transport->is_connected()); - - // Return nullptr for non-existing mids. - EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kVideoMid2)); - - EXPECT_EQ(cricket::kNoOpDtlsTransportComponent, - transport_controller_->GetDtlsTransport(kAudioMid1)->component()) - << "Because media transport is used, expected no-op DTLS transport."; -} - -TEST_F(JsepTransportControllerTest, - GetMediaTransportOfferInTheConfigOnSubsequentCalls) { - FakeMediaTransportFactory fake_media_transport_factory; - WrapperMediaTransportFactory wrapping_factory(&fake_media_transport_factory); - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &wrapping_factory; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - - absl::optional settings = - transport_controller_->GenerateOrGetLastMediaTransportOffer(); - ASSERT_NE(absl::nullopt, settings); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, description.get()) - .ok()); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - - ASSERT_NE(nullptr, media_transport); - - absl::optional - new_settings = - transport_controller_->GenerateOrGetLastMediaTransportOffer(); - ASSERT_NE(absl::nullopt, new_settings); - EXPECT_EQ(settings->transport_name, new_settings->transport_name); - EXPECT_EQ(settings->transport_setting, new_settings->transport_setting); - EXPECT_EQ(1, wrapping_factory.created_transport_count()); -} - -TEST_F(JsepTransportControllerTest, GetMediaTransportInCallee) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - description->AddMediaTransportSetting("fake", "fake-remote-settings"); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kOffer, description.get()) - .ok()); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - - ASSERT_NE(nullptr, media_transport); - - // After SetRemoteDescription, media transport should be created as callee. - EXPECT_FALSE(media_transport->is_caller()); - // We do not set pre-shared key on the callee, it comes in media transport - // settings. - EXPECT_EQ(absl::nullopt, media_transport->settings().pre_shared_key); - EXPECT_TRUE(media_transport->is_connected()); - - // Return nullptr for non-existing mids. - EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kVideoMid2)); - - EXPECT_EQ(cricket::kNoOpDtlsTransportComponent, - transport_controller_->GetDtlsTransport(kAudioMid1)->component()) - << "Because media transport is used, expected no-op DTLS transport."; -} - -TEST_F(JsepTransportControllerTest, GetMediaTransportInCalleePassesSdp) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - description->AddMediaTransportSetting("fake", "this-is-a-test-setting"); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kOffer, description.get()) - .ok()); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - - ASSERT_NE(nullptr, media_transport); - - EXPECT_EQ("this-is-a-test-setting", - media_transport->settings().remote_transport_parameters); -} - -// Caller generates the offer if media transport returns empty offer (no -// parameters). -TEST_F(JsepTransportControllerTest, MediaTransportGeneratesSessionDescription) { - FakeMediaTransportFactory fake_media_transport_factory( - /*transport_offer=*/""); - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - absl::optional settings = - transport_controller_->GenerateOrGetLastMediaTransportOffer(); - - ASSERT_TRUE(settings.has_value()); - EXPECT_EQ("fake", settings->transport_name); - // Fake media transport returns empty settings (but not nullopt settings!) - EXPECT_EQ("", settings->transport_setting); -} - -// Caller generates the offer if media transport returns offer with parameters. -TEST_F(JsepTransportControllerTest, - MediaTransportGeneratesSessionDescriptionWithOfferParams) { - FakeMediaTransportFactory fake_media_transport_factory( - /*transport_offer=*/"offer-params"); - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - absl::optional settings = - transport_controller_->GenerateOrGetLastMediaTransportOffer(); - - ASSERT_TRUE(settings.has_value()); - EXPECT_EQ("fake", settings->transport_name); - EXPECT_EQ("offer-params", settings->transport_setting); -} - -// Caller skips the offer if media transport requests it. -TEST_F(JsepTransportControllerTest, - MediaTransportGeneratesSkipsSessionDescription) { - FakeMediaTransportFactory fake_media_transport_factory( - /*transport_offer=*/absl::nullopt); - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - absl::optional settings = - transport_controller_->GenerateOrGetLastMediaTransportOffer(); - - // Fake media transport returns nullopt settings - ASSERT_EQ(absl::nullopt, settings); -} - -// Caller ignores its own outgoing parameters. -TEST_F(JsepTransportControllerTest, - GetMediaTransportInCallerIgnoresXmtSection) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - EXPECT_NE(absl::nullopt, - transport_controller_->GenerateOrGetLastMediaTransportOffer()); - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, description.get()) - .ok()); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - - ASSERT_NE(nullptr, media_transport); - - // Remote parameters are nullopt, because we are the offerer (we don't) - // have the remote transport parameters, only ours. - EXPECT_EQ(absl::nullopt, - media_transport->settings().remote_transport_parameters); -} - -TEST_F(JsepTransportControllerTest, - GetMediaTransportInCalleeIgnoresDifferentTransport) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - description->AddMediaTransportSetting("not-a-fake-transport", - "this-is-a-test-setting"); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kOffer, description.get()) - .ok()); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - - ASSERT_NE(nullptr, media_transport); - - EXPECT_EQ(absl::nullopt, - media_transport->settings().remote_transport_parameters); -} - -TEST_F(JsepTransportControllerTest, GetMediaTransportIsNotSetIfNoSdes) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyNegotiate; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kOffer, description.get()) - .ok()); - - EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kAudioMid1)); - - // Even if we set local description with crypto now (after the remote offer - // was set), media transport won't be provided. - auto description2 = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description2.get()); - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kAnswer, description2.get()) - .ok()); - - EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kAudioMid1)); - EXPECT_EQ(cricket::ICE_CANDIDATE_COMPONENT_RTP, - transport_controller_->GetDtlsTransport(kAudioMid1)->component()) - << "Because media transport is NOT used (fallback to RTP), expected " - "actual DTLS transport for RTP"; -} - -TEST_F(JsepTransportControllerTest, - AfterSettingAnswerTheSameMediaTransportIsReturnedCallee) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.media_transport_factory = &fake_media_transport_factory; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - description->AddMediaTransportSetting("fake", "fake-settings"); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kOffer, description.get()) - .ok()); - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - EXPECT_NE(nullptr, media_transport); - EXPECT_FALSE(media_transport->pre_shared_key().has_value()) - << "On the callee, preshared key is passed through the media-transport " - "settings (x-mt)"; - - // Even if we set local description with crypto now (after the remote offer - // was set), media transport won't be provided. - auto description2 = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description2.get()); - - RTCError result = transport_controller_->SetLocalDescription( - SdpType::kAnswer, description2.get()); - EXPECT_TRUE(result.ok()) << result.message(); - - // Media transport did not change. - EXPECT_EQ(media_transport, - transport_controller_->GetMediaTransport(kAudioMid1)); -} - TEST_F(JsepTransportControllerTest, SetIceConfig) { CreateJsepTransportController(JsepTransportController::Config()); auto description = CreateSessionDescriptionWithoutBundle(); @@ -1190,164 +812,6 @@ TEST_F(JsepTransportControllerTest, EXPECT_EQ(3, combined_connection_state_signal_count_); } -TEST_F(JsepTransportControllerTest, - SignalConnectionStateConnectedWithMediaTransportAndNoDtlsCaller) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - config.media_transport_factory = &fake_media_transport_factory; - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.use_media_transport_for_data_channels = true; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - - // Media Transport is only used with bundle. - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - EXPECT_NE(absl::nullopt, - transport_controller_->GenerateOrGetLastMediaTransportOffer()); - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, description.get()) - .ok()); - - auto fake_audio_ice = static_cast( - transport_controller_->GetDtlsTransport(kAudioMid1)->ice_transport()); - auto fake_video_ice = static_cast( - transport_controller_->GetDtlsTransport(kVideoMid1)->ice_transport()); - EXPECT_EQ(fake_audio_ice, fake_video_ice); - fake_audio_ice->SetConnectionCount(2); - fake_audio_ice->SetConnectionCount(1); - fake_video_ice->SetConnectionCount(2); - fake_video_ice->SetConnectionCount(1); - fake_audio_ice->SetWritable(true); - fake_video_ice->SetWritable(true); - - // Still not connected, because we are waiting for media transport. - EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_, - kTimeout); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - - ASSERT_NE(nullptr, media_transport); - - media_transport->SetState(webrtc::MediaTransportState::kWritable); - // Only one media transport. - EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout); -} - -TEST_F(JsepTransportControllerTest, - SignalConnectionStateConnectedWithMediaTransportCaller) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - config.media_transport_factory = &fake_media_transport_factory; - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - - // Media Transport is only used with bundle. - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - EXPECT_NE(absl::nullopt, - transport_controller_->GenerateOrGetLastMediaTransportOffer()); - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, description.get()) - .ok()); - - auto fake_audio_dtls = static_cast( - transport_controller_->GetDtlsTransport(kAudioMid1)); - auto fake_video_dtls = static_cast( - transport_controller_->GetDtlsTransport(kVideoMid1)); - - auto fake_audio_ice = static_cast( - transport_controller_->GetDtlsTransport(kAudioMid1)->ice_transport()); - auto fake_video_ice = static_cast( - transport_controller_->GetDtlsTransport(kVideoMid1)->ice_transport()); - fake_audio_ice->SetConnectionCount(2); - fake_audio_ice->SetConnectionCount(1); - fake_video_ice->SetConnectionCount(2); - fake_video_ice->SetConnectionCount(1); - fake_audio_ice->SetWritable(true); - fake_video_ice->SetWritable(true); - fake_audio_dtls->SetWritable(true); - fake_video_dtls->SetWritable(true); - - // Still not connected, because we are waiting for media transport. - EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_, - kTimeout); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - - ASSERT_NE(nullptr, media_transport); - - media_transport->SetState(webrtc::MediaTransportState::kWritable); - EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_, - kTimeout); - - // Still waiting for the second media transport. - media_transport = static_cast( - transport_controller_->GetMediaTransport(kVideoMid1)); - media_transport->SetState(webrtc::MediaTransportState::kWritable); - - EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout); -} - -TEST_F(JsepTransportControllerTest, - SignalConnectionStateFailedWhenMediaTransportClosedCaller) { - FakeMediaTransportFactory fake_media_transport_factory; - JsepTransportController::Config config; - config.media_transport_factory = &fake_media_transport_factory; - config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - config.use_media_transport_for_media = true; - CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithBundleGroup(); - AddCryptoSettings(description.get()); - EXPECT_NE(absl::nullopt, - transport_controller_->GenerateOrGetLastMediaTransportOffer()); - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, description.get()) - .ok()); - - auto fake_audio_dtls = static_cast( - transport_controller_->GetDtlsTransport(kAudioMid1)); - auto fake_video_dtls = static_cast( - transport_controller_->GetDtlsTransport(kVideoMid1)); - - auto fake_audio_ice = static_cast( - transport_controller_->GetDtlsTransport(kAudioMid1)->ice_transport()); - auto fake_video_ice = static_cast( - transport_controller_->GetDtlsTransport(kVideoMid1)->ice_transport()); - fake_audio_ice->SetWritable(true); - fake_video_ice->SetWritable(true); - // Decreasing connection count from 2 to 1 triggers connection state event. - fake_audio_ice->SetConnectionCount(2); - fake_audio_ice->SetConnectionCount(1); - fake_video_ice->SetConnectionCount(2); - fake_video_ice->SetConnectionCount(1); - - fake_audio_dtls->SetWritable(true); - fake_video_dtls->SetWritable(true); - - FakeMediaTransport* media_transport = static_cast( - transport_controller_->GetMediaTransport(kAudioMid1)); - ASSERT_NE(nullptr, media_transport); - media_transport->SetState(webrtc::MediaTransportState::kWritable); - - media_transport = static_cast( - transport_controller_->GetMediaTransport(kVideoMid1)); - ASSERT_NE(nullptr, media_transport); - - media_transport->SetState(webrtc::MediaTransportState::kWritable); - - EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout); - - media_transport->SetState(webrtc::MediaTransportState::kClosed); - EXPECT_EQ_WAIT(cricket::kIceConnectionFailed, connection_state_, kTimeout); -} - TEST_F(JsepTransportControllerTest, SignalConnectionStateComplete) { CreateJsepTransportController(JsepTransportController::Config()); auto description = CreateSessionDescriptionWithoutBundle(); diff --git a/pc/jsep_transport_unittest.cc b/pc/jsep_transport_unittest.cc index 87d6e87212..c4193e5974 100644 --- a/pc/jsep_transport_unittest.cc +++ b/pc/jsep_transport_unittest.cc @@ -114,11 +114,6 @@ class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { RTC_NOTREACHED(); } - // TODO(sukhanov): Currently there is no media_transport specific - // logic in jseptransport, so jseptransport unittests are created with - // media_transport = nullptr. In the future we will probably add - // more logic that require unit tests. Note that creation of media_transport - // is covered in jseptransportcontroller_unittest. auto jsep_transport = std::make_unique( kTransportName, /*local_certificate=*/nullptr, std::move(ice), std::move(rtcp_ice), std::move(unencrypted_rtp_transport), @@ -126,7 +121,6 @@ class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { /*datagram_rtp_transport=*/nullptr, std::move(rtp_dtls_transport), std::move(rtcp_dtls_transport), /*sctp_transport=*/nullptr, - /*media_transport=*/nullptr, /*datagram_transport=*/nullptr, /*data_channel_transport=*/nullptr); diff --git a/pc/media_session.cc b/pc/media_session.cc index 873f27dad5..59f140f951 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -1523,12 +1523,6 @@ std::unique_ptr MediaSessionDescriptionFactory::CreateOffer( offer->set_extmap_allow_mixed(session_options.offer_extmap_allow_mixed); - if (session_options.media_transport_settings.has_value()) { - offer->AddMediaTransportSetting( - session_options.media_transport_settings->transport_name, - session_options.media_transport_settings->transport_setting); - } - return offer; } diff --git a/pc/media_session.h b/pc/media_session.h index f91729aa28..235945c4f9 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -115,11 +115,6 @@ struct MediaSessionOptions { std::vector media_description_options; std::vector pooled_ice_credentials; - // An optional media transport settings. - // In the future we may consider using a vector here, to indicate multiple - // supported transports. - absl::optional - media_transport_settings; // Use the draft-ietf-mmusic-sctp-sdp-03 obsolete syntax for SCTP // datachannels. // Default is true for backwards compatibility with clients that use diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index f5f51c43f9..c24bd2e88a 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1135,7 +1135,6 @@ bool PeerConnection::Initialize( const PeerConnectionInterface::RTCConfiguration& configuration, PeerConnectionDependencies dependencies) { RTC_DCHECK_RUN_ON(signaling_thread()); - RTC_DCHECK_RUNS_SERIALIZED(&use_media_transport_race_checker_); TRACE_EVENT0("webrtc", "PeerConnection::Initialize"); RTCError config_error = ValidateConfiguration(configuration); @@ -1260,37 +1259,15 @@ bool PeerConnection::Initialize( use_datagram_transport_for_data_channels_receive_only_ = configuration.use_datagram_transport_for_data_channels_receive_only .value_or(datagram_transport_data_channel_config_.receive_only); - if (use_datagram_transport_ || use_datagram_transport_for_data_channels_ || - configuration.use_media_transport || - configuration.use_media_transport_for_data_channels) { + if (use_datagram_transport_ || use_datagram_transport_for_data_channels_) { if (!factory_->media_transport_factory()) { RTC_DCHECK(false) - << "PeerConnecton is initialized with use_media_transport = true or " - << "use_media_transport_for_data_channels = true " + << "PeerConnecton is initialized with use_datagram_transport = true " + "or use_datagram_transport_for_data_channels = true " << "but media transport factory is not set in PeerConnectionFactory"; return false; } - if (configuration.use_media_transport || - configuration.use_media_transport_for_data_channels) { - // TODO(bugs.webrtc.org/9719): This check will eventually go away, when - // RTP media transport is introduced. But until then, we require SDES to - // be enabled. - if (configuration.enable_dtls_srtp.has_value() && - configuration.enable_dtls_srtp.value()) { - RTC_LOG(LS_WARNING) - << "When media transport is used, SDES must be enabled. Set " - "configuration.enable_dtls_srtp to false. use_media_transport=" - << configuration.use_media_transport - << ", use_media_transport_for_data_channels=" - << configuration.use_media_transport_for_data_channels; - return false; - } - } - - config.use_media_transport_for_media = configuration.use_media_transport; - config.use_media_transport_for_data_channels = - configuration.use_media_transport_for_data_channels; config.use_datagram_transport = use_datagram_transport_; config.use_datagram_transport_for_data_channels = use_datagram_transport_for_data_channels_; @@ -1336,14 +1313,6 @@ bool PeerConnection::Initialize( data_channel_type_ = cricket::DCT_DATA_CHANNEL_TRANSPORT_SCTP; config.sctp_factory = sctp_factory_.get(); } - } else if (configuration.use_media_transport_for_data_channels) { - if (configuration.enable_rtp_data_channel) { - RTC_LOG(LS_ERROR) << "enable_rtp_data_channel and " - "use_media_transport_for_data_channels are " - "incompatible and cannot both be set to true"; - return false; - } - data_channel_type_ = cricket::DCT_MEDIA_TRANSPORT; } else if (configuration.enable_rtp_data_channel) { // Enable creation of RTP data channels if the kEnableRtpDataChannels is // set. It takes precendence over the disable_sctp_data_channels @@ -1385,7 +1354,6 @@ bool PeerConnection::Initialize( stats_collector_ = RTCStatsCollector::Create(this); configuration_ = configuration; - use_media_transport_ = configuration.use_media_transport; transport_controller_->SetIceConfig(ParseIceConfig(configuration)); @@ -3928,7 +3896,6 @@ PeerConnectionInterface::RTCConfiguration PeerConnection::GetConfiguration() { RTCError PeerConnection::SetConfiguration( const RTCConfiguration& configuration) { RTC_DCHECK_RUN_ON(signaling_thread()); - RTC_DCHECK_RUNS_SERIALIZED(&use_media_transport_race_checker_); TRACE_EVENT0("webrtc", "PeerConnection::SetConfiguration"); if (IsClosed()) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_STATE, @@ -3945,36 +3912,6 @@ RTCError PeerConnection::SetConfiguration( "SetLocalDescription."); } - if (local_description() && - configuration.use_media_transport != configuration_.use_media_transport) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, - "Can't change media_transport after calling " - "SetLocalDescription."); - } - - if (remote_description() && - configuration.use_media_transport != configuration_.use_media_transport) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, - "Can't change media_transport after calling " - "SetRemoteDescription."); - } - - if (local_description() && - configuration.use_media_transport_for_data_channels != - configuration_.use_media_transport_for_data_channels) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, - "Can't change media_transport_for_data_channels " - "after calling SetLocalDescription."); - } - - if (remote_description() && - configuration.use_media_transport_for_data_channels != - configuration_.use_media_transport_for_data_channels) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, - "Can't change media_transport_for_data_channels " - "after calling SetRemoteDescription."); - } - if (local_description() && configuration.crypto_options != configuration_.crypto_options) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, @@ -4034,9 +3971,7 @@ RTCError PeerConnection::SetConfiguration( "after calling SetRemoteDescription."); } - if (configuration.use_media_transport_for_data_channels || - configuration.use_media_transport || - (configuration.use_datagram_transport && + if ((configuration.use_datagram_transport && *configuration.use_datagram_transport) || (configuration.use_datagram_transport_for_data_channels && *configuration.use_datagram_transport_for_data_channels)) { @@ -4072,9 +4007,6 @@ RTCError PeerConnection::SetConfiguration( modified_config.network_preference = configuration.network_preference; modified_config.active_reset_srtp_params = configuration.active_reset_srtp_params; - modified_config.use_media_transport = configuration.use_media_transport; - modified_config.use_media_transport_for_data_channels = - configuration.use_media_transport_for_data_channels; modified_config.use_datagram_transport = configuration.use_datagram_transport; modified_config.use_datagram_transport_for_data_channels = configuration.use_datagram_transport_for_data_channels; @@ -4158,8 +4090,6 @@ RTCError PeerConnection::SetConfiguration( modified_config.use_datagram_transport_for_data_channels_receive_only .value_or(datagram_transport_data_channel_config_.receive_only); transport_controller_->SetMediaTransportSettings( - modified_config.use_media_transport, - modified_config.use_media_transport_for_data_channels, use_datagram_transport_, use_datagram_transport_for_data_channels_, use_datagram_transport_for_data_channels_receive_only_); @@ -4178,7 +4108,6 @@ RTCError PeerConnection::SetConfiguration( } configuration_ = modified_config; - use_media_transport_ = configuration.use_media_transport; return RTCError::OK(); } @@ -4967,12 +4896,6 @@ void PeerConnection::GetOptionsForOffer( session_options->offer_extmap_allow_mixed = configuration_.offer_extmap_allow_mixed; - if (configuration_.use_media_transport || - configuration_.use_media_transport_for_data_channels) { - session_options->media_transport_settings = - transport_controller_->GenerateOrGetLastMediaTransportOffer(); - } - // If datagram transport is in use, add opaque transport parameters. if (use_datagram_transport_ || use_datagram_transport_for_data_channels_) { for (auto& options : session_options->media_description_options) { @@ -5476,7 +5399,6 @@ absl::optional PeerConnection::GetDataMid() const { } return rtp_data_channel_->content_name(); case cricket::DCT_SCTP: - case cricket::DCT_MEDIA_TRANSPORT: case cricket::DCT_DATA_CHANNEL_TRANSPORT: case cricket::DCT_DATA_CHANNEL_TRANSPORT_SCTP: return sctp_mid_; @@ -7106,7 +7028,6 @@ bool PeerConnection::CreateDataChannel(const std::string& mid) { case cricket::DCT_SCTP: case cricket::DCT_DATA_CHANNEL_TRANSPORT_SCTP: case cricket::DCT_DATA_CHANNEL_TRANSPORT: - case cricket::DCT_MEDIA_TRANSPORT: if (!network_thread()->Invoke( RTC_FROM_HERE, rtc::Bind(&PeerConnection::SetupDataChannelTransport_n, this, @@ -7777,20 +7698,14 @@ bool PeerConnection::OnTransportChanged( const std::string& mid, RtpTransportInternal* rtp_transport, rtc::scoped_refptr dtls_transport, - MediaTransportInterface* media_transport, DataChannelTransportInterface* data_channel_transport) { RTC_DCHECK_RUN_ON(network_thread()); - RTC_DCHECK_RUNS_SERIALIZED(&use_media_transport_race_checker_); bool ret = true; auto base_channel = GetChannel(mid); if (base_channel) { ret = base_channel->SetRtpTransport(rtp_transport); } - if (use_media_transport_) { - RTC_LOG(LS_ERROR) << "Media transport isn't supported."; - } - if (data_channel_transport_ && mid == sctp_mid_ && data_channel_transport_ != data_channel_transport) { // Changed which data channel transport is used for |sctp_mid_| (eg. now diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 3126348788..9bc6119461 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -20,7 +20,6 @@ #include "api/peer_connection_interface.h" #include "api/transport/data_channel_transport_interface.h" -#include "api/transport/media/media_transport_interface.h" #include "api/turn_customizer.h" #include "pc/ice_server_parsing.h" #include "pc/jsep_transport_controller.h" @@ -1201,7 +1200,6 @@ class PeerConnection : public PeerConnectionInternal, const std::string& mid, RtpTransportInternal* rtp_transport, rtc::scoped_refptr dtls_transport, - MediaTransportInterface* media_transport, DataChannelTransportInterface* data_channel_transport) override; // RtpSenderBase::SetStreamsObserver override. @@ -1289,14 +1287,6 @@ class PeerConnection : public PeerConnectionInternal, bool use_datagram_transport_for_data_channels_receive_only_ RTC_GUARDED_BY(signaling_thread()) = false; - // Cache configuration_.use_media_transport so that we can access it from - // other threads. - // TODO(bugs.webrtc.org/9987): Caching just this bool and allowing the data - // it's derived from to change is not necessarily sound. Stop doing it. - rtc::RaceChecker use_media_transport_race_checker_; - bool use_media_transport_ RTC_GUARDED_BY(use_media_transport_race_checker_) = - configuration_.use_media_transport; - // TODO(zstein): |async_resolver_factory_| can currently be nullptr if it // is not injected. It should be required once chromium supplies it. std::unique_ptr async_resolver_factory_ diff --git a/pc/peer_connection_data_channel_unittest.cc b/pc/peer_connection_data_channel_unittest.cc index a902c76654..b063c39307 100644 --- a/pc/peer_connection_data_channel_unittest.cc +++ b/pc/peer_connection_data_channel_unittest.cc @@ -22,8 +22,6 @@ #include "api/peer_connection_proxy.h" #include "api/scoped_refptr.h" #include "api/task_queue/default_task_queue_factory.h" -#include "api/test/fake_media_transport.h" -#include "api/transport/media/media_transport_interface.h" #include "media/base/codec.h" #include "media/base/fake_media_engine.h" #include "media/base/media_constants.h" @@ -65,8 +63,7 @@ PeerConnectionFactoryDependencies CreatePeerConnectionFactoryDependencies( rtc::Thread* worker_thread, rtc::Thread* signaling_thread, std::unique_ptr media_engine, - std::unique_ptr call_factory, - std::unique_ptr media_transport_factory) { + std::unique_ptr call_factory) { PeerConnectionFactoryDependencies deps; deps.network_thread = network_thread; deps.worker_thread = worker_thread; @@ -74,7 +71,6 @@ PeerConnectionFactoryDependencies CreatePeerConnectionFactoryDependencies( deps.task_queue_factory = CreateDefaultTaskQueueFactory(); deps.media_engine = std::move(media_engine); deps.call_factory = std::move(call_factory); - deps.media_transport_factory = std::move(media_transport_factory); return deps; } @@ -90,8 +86,7 @@ class PeerConnectionFactoryForDataChannelTest rtc::Thread::Current(), rtc::Thread::Current(), std::make_unique(), - CreateCallFactory(), - std::make_unique())) {} + CreateCallFactory())) {} std::unique_ptr CreateSctpTransportInternalFactory() { @@ -385,50 +380,6 @@ TEST_P(PeerConnectionDataChannelTest, SctpPortPropagatedFromSdpToTransport) { EXPECT_EQ(kNewRecvPort, callee_transport->local_port()); } -TEST_P(PeerConnectionDataChannelTest, - NoSctpTransportCreatedIfMediaTransportDataChannelsEnabled) { - RTCConfiguration config; - config.use_media_transport_for_data_channels = true; - config.enable_dtls_srtp = false; // SDES is required to use media transport. - auto caller = CreatePeerConnectionWithDataChannel(config); - - ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer())); - EXPECT_FALSE(caller->sctp_transport_factory()->last_fake_sctp_transport()); -} - -TEST_P(PeerConnectionDataChannelTest, - MediaTransportDataChannelCreatedEvenIfSctpAvailable) { - RTCConfiguration config; - config.use_media_transport_for_data_channels = true; - config.enable_dtls_srtp = false; // SDES is required to use media transport. - PeerConnectionFactoryInterface::Options options; - options.disable_sctp_data_channels = false; - auto caller = CreatePeerConnectionWithDataChannel(config, options); - - ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer())); - EXPECT_FALSE(caller->sctp_transport_factory()->last_fake_sctp_transport()); -} - -TEST_P(PeerConnectionDataChannelTest, - CannotEnableBothMediaTransportAndRtpDataChannels) { - RTCConfiguration config; - config.enable_rtp_data_channel = true; - config.use_media_transport_for_data_channels = true; - config.enable_dtls_srtp = false; // SDES is required to use media transport. - EXPECT_EQ(CreatePeerConnection(config), nullptr); -} - -// This test now DCHECKs, instead of failing to SetLocalDescription. -TEST_P(PeerConnectionDataChannelTest, MediaTransportWithoutSdesFails) { - RTCConfiguration config; - config.use_media_transport_for_data_channels = true; - config.enable_dtls_srtp = true; // Disables SDES for data sections. - - auto caller = CreatePeerConnectionWithDataChannel(config); - - EXPECT_EQ(nullptr, caller); -} - TEST_P(PeerConnectionDataChannelTest, ModernSdpSyntaxByDefault) { PeerConnectionInterface::RTCOfferAnswerOptions options; auto caller = CreatePeerConnectionWithDataChannel(); diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index e59ce9a2a8..dd06b65ffa 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -4278,331 +4278,6 @@ TEST_P(PeerConnectionIntegrationTest, ASSERT_TRUE(ExpectNewFrames(media_expectations)); } -// This test sets up a call between two parties with a media transport data -// channel. -TEST_P(PeerConnectionIntegrationTest, MediaTransportDataChannelEndToEnd) { - PeerConnectionInterface::RTCConfiguration rtc_config; - rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - rtc_config.use_media_transport_for_data_channels = true; - rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( - rtc_config, rtc_config, loopback_media_transports()->first_factory(), - loopback_media_transports()->second_factory())); - ConnectFakeSignaling(); - - // Expect that data channel created on caller side will show up for callee as - // well. - caller()->CreateDataChannel(); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - // Ensure that the media transport is ready. - loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable); - loopback_media_transports()->FlushAsyncInvokes(); - - // Caller data channel should already exist (it created one). Callee data - // channel may not exist yet, since negotiation happens in-band, not in SDP. - ASSERT_NE(nullptr, caller()->data_channel()); - ASSERT_TRUE_WAIT(callee()->data_channel() != nullptr, kDefaultTimeout); - EXPECT_TRUE_WAIT(caller()->data_observer()->IsOpen(), kDefaultTimeout); - EXPECT_TRUE_WAIT(callee()->data_observer()->IsOpen(), kDefaultTimeout); - - // Ensure data can be sent in both directions. - std::string data = "hello world"; - caller()->data_channel()->Send(DataBuffer(data)); - EXPECT_EQ_WAIT(data, callee()->data_observer()->last_message(), - kDefaultTimeout); - callee()->data_channel()->Send(DataBuffer(data)); - EXPECT_EQ_WAIT(data, caller()->data_observer()->last_message(), - kDefaultTimeout); -} - -// Tests that 'zero-rtt' data channel transports (which are ready-to-send as -// soon as they're created) work correctly. -TEST_P(PeerConnectionIntegrationTest, MediaTransportDataChannelZeroRtt) { - PeerConnectionInterface::RTCConfiguration rtc_config; - rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - rtc_config.use_media_transport_for_data_channels = true; - rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( - rtc_config, rtc_config, loopback_media_transports()->first_factory(), - loopback_media_transports()->second_factory())); - ConnectFakeSignaling(); - - // Ensure that the callee's media transport is ready-to-send immediately. - // Note that only the callee can become writable in zero RTTs. The caller - // must wait for the callee's answer. - loopback_media_transports()->SetSecondStateAfterConnect( - webrtc::MediaTransportState::kWritable); - loopback_media_transports()->FlushAsyncInvokes(); - - // Expect that data channel created on caller side will show up for callee as - // well. - caller()->CreateDataChannel(); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - loopback_media_transports()->SetFirstState( - webrtc::MediaTransportState::kWritable); - loopback_media_transports()->FlushAsyncInvokes(); - - // Caller data channel should already exist (it created one). Callee data - // channel may not exist yet, since negotiation happens in-band, not in SDP. - ASSERT_NE(nullptr, caller()->data_channel()); - ASSERT_TRUE_WAIT(callee()->data_channel() != nullptr, kDefaultTimeout); - EXPECT_TRUE_WAIT(caller()->data_observer()->IsOpen(), kDefaultTimeout); - EXPECT_TRUE_WAIT(callee()->data_observer()->IsOpen(), kDefaultTimeout); - - // Ensure data can be sent in both directions. - std::string data = "hello world"; - caller()->data_channel()->Send(DataBuffer(data)); - EXPECT_EQ_WAIT(data, callee()->data_observer()->last_message(), - kDefaultTimeout); - callee()->data_channel()->Send(DataBuffer(data)); - EXPECT_EQ_WAIT(data, caller()->data_observer()->last_message(), - kDefaultTimeout); -} - -// Ensure that when the callee closes a media transport data channel, the -// closing procedure results in the data channel being closed for the caller -// as well. -TEST_P(PeerConnectionIntegrationTest, MediaTransportDataChannelCalleeCloses) { - PeerConnectionInterface::RTCConfiguration rtc_config; - rtc_config.use_media_transport_for_data_channels = true; - rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( - rtc_config, rtc_config, loopback_media_transports()->first_factory(), - loopback_media_transports()->second_factory())); - ConnectFakeSignaling(); - - // Create a data channel on the caller and signal it to the callee. - caller()->CreateDataChannel(); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - // Ensure that the media transport is ready. - loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable); - loopback_media_transports()->FlushAsyncInvokes(); - - // Data channels exist and open on both ends of the connection. - ASSERT_NE(nullptr, caller()->data_channel()); - ASSERT_TRUE_WAIT(callee()->data_channel() != nullptr, kDefaultTimeout); - ASSERT_TRUE_WAIT(caller()->data_observer()->IsOpen(), kDefaultTimeout); - ASSERT_TRUE_WAIT(callee()->data_observer()->IsOpen(), kDefaultTimeout); - - // Close the data channel on the callee side, and wait for it to reach the - // "closed" state on both sides. - callee()->data_channel()->Close(); - EXPECT_TRUE_WAIT(!caller()->data_observer()->IsOpen(), kDefaultTimeout); - EXPECT_TRUE_WAIT(!callee()->data_observer()->IsOpen(), kDefaultTimeout); -} - -TEST_P(PeerConnectionIntegrationTest, - MediaTransportDataChannelConfigSentToOtherSide) { - PeerConnectionInterface::RTCConfiguration rtc_config; - rtc_config.use_media_transport_for_data_channels = true; - rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( - rtc_config, rtc_config, loopback_media_transports()->first_factory(), - loopback_media_transports()->second_factory())); - ConnectFakeSignaling(); - - // Create a data channel with a non-default configuration and signal it to the - // callee. - webrtc::DataChannelInit init; - init.id = 53; - init.maxRetransmits = 52; - caller()->CreateDataChannel("data-channel", &init); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - // Ensure that the media transport is ready. - loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable); - loopback_media_transports()->FlushAsyncInvokes(); - - // Ensure that the data channel exists on the callee with the correct - // configuration. - ASSERT_TRUE_WAIT(callee()->data_channel() != nullptr, kDefaultTimeout); - ASSERT_TRUE_WAIT(callee()->data_observer()->IsOpen(), kDefaultTimeout); - // Since "negotiate" is false, the "id" parameter is ignored. - EXPECT_NE(init.id, callee()->data_channel()->id()); - EXPECT_EQ("data-channel", callee()->data_channel()->label()); - EXPECT_EQ(init.maxRetransmits, callee()->data_channel()->maxRetransmits()); - EXPECT_FALSE(callee()->data_channel()->negotiated()); -} - -TEST_P(PeerConnectionIntegrationTest, MediaTransportOfferUpgrade) { - PeerConnectionInterface::RTCConfiguration rtc_config; - rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - rtc_config.use_media_transport = true; - rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( - rtc_config, rtc_config, loopback_media_transports()->first_factory(), - loopback_media_transports()->second_factory())); - ConnectFakeSignaling(); - - // Do initial offer/answer with just a video track. - caller()->AddVideoTrack(); - callee()->AddVideoTrack(); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - // Ensure that the media transport is ready. - loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable); - loopback_media_transports()->FlushAsyncInvokes(); - - // Now add an audio track and do another offer/answer. - caller()->AddAudioTrack(); - callee()->AddAudioTrack(); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - // Ensure both audio and video frames are received end-to-end. - MediaExpectations media_expectations; - media_expectations.ExpectBidirectionalAudioAndVideo(); - ASSERT_TRUE(ExpectNewFrames(media_expectations)); - - // The second offer should not have generated another media transport. - // Media transport was kept alive, and was not recreated. - EXPECT_EQ(1, loopback_media_transports()->first_factory_transport_count()); - EXPECT_EQ(1, loopback_media_transports()->second_factory_transport_count()); -} - -TEST_P(PeerConnectionIntegrationTest, MediaTransportOfferUpgradeOnTheCallee) { - PeerConnectionInterface::RTCConfiguration rtc_config; - rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - rtc_config.use_media_transport = true; - rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( - rtc_config, rtc_config, loopback_media_transports()->first_factory(), - loopback_media_transports()->second_factory())); - ConnectFakeSignaling(); - - // Do initial offer/answer with just a video track. - caller()->AddVideoTrack(); - callee()->AddVideoTrack(); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - // Ensure that the media transport is ready. - loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable); - loopback_media_transports()->FlushAsyncInvokes(); - - // Now add an audio track and do another offer/answer. - caller()->AddAudioTrack(); - callee()->AddAudioTrack(); - callee()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - // Ensure both audio and video frames are received end-to-end. - MediaExpectations media_expectations; - media_expectations.ExpectBidirectionalAudioAndVideo(); - ASSERT_TRUE(ExpectNewFrames(media_expectations)); - - // The second offer should not have generated another media transport. - // Media transport was kept alive, and was not recreated. - EXPECT_EQ(1, loopback_media_transports()->first_factory_transport_count()); - EXPECT_EQ(1, loopback_media_transports()->second_factory_transport_count()); -} - -TEST_P(PeerConnectionIntegrationTest, MediaTransportBidirectionalAudio) { - PeerConnectionInterface::RTCConfiguration rtc_config; - rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; - rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - rtc_config.use_media_transport = true; - rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( - rtc_config, rtc_config, loopback_media_transports()->first_factory(), - loopback_media_transports()->second_factory())); - ConnectFakeSignaling(); - - caller()->AddAudioTrack(); - callee()->AddAudioTrack(); - // Start offer/answer exchange and wait for it to complete. - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - // Ensure that the media transport is ready. - loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable); - loopback_media_transports()->FlushAsyncInvokes(); - - MediaExpectations media_expectations; - media_expectations.ExpectBidirectionalAudio(); - ASSERT_TRUE(ExpectNewFrames(media_expectations)); - - webrtc::MediaTransportPair::Stats first_stats = - loopback_media_transports()->FirstStats(); - webrtc::MediaTransportPair::Stats second_stats = - loopback_media_transports()->SecondStats(); - - EXPECT_GT(first_stats.received_audio_frames, 0); - EXPECT_GE(second_stats.sent_audio_frames, first_stats.received_audio_frames); - - EXPECT_GT(second_stats.received_audio_frames, 0); - EXPECT_GE(first_stats.sent_audio_frames, second_stats.received_audio_frames); -} - -TEST_P(PeerConnectionIntegrationTest, MediaTransportBidirectionalVideo) { - PeerConnectionInterface::RTCConfiguration rtc_config; - rtc_config.use_media_transport = true; - rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( - rtc_config, rtc_config, loopback_media_transports()->first_factory(), - loopback_media_transports()->second_factory())); - ConnectFakeSignaling(); - - caller()->AddVideoTrack(); - callee()->AddVideoTrack(); - // Start offer/answer exchange and wait for it to complete. - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - // Ensure that the media transport is ready. - loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable); - loopback_media_transports()->FlushAsyncInvokes(); - - MediaExpectations media_expectations; - media_expectations.ExpectBidirectionalVideo(); - ASSERT_TRUE(ExpectNewFrames(media_expectations)); - - webrtc::MediaTransportPair::Stats first_stats = - loopback_media_transports()->FirstStats(); - webrtc::MediaTransportPair::Stats second_stats = - loopback_media_transports()->SecondStats(); - - EXPECT_GT(first_stats.received_video_frames, 0); - EXPECT_GE(second_stats.sent_video_frames, first_stats.received_video_frames); - - EXPECT_GT(second_stats.received_video_frames, 0); - EXPECT_GE(first_stats.sent_video_frames, second_stats.received_video_frames); -} - -TEST_P(PeerConnectionIntegrationTest, - MediaTransportDataChannelUsesRtpBidirectionalVideo) { - PeerConnectionInterface::RTCConfiguration rtc_config; - rtc_config.use_media_transport = false; - rtc_config.use_media_transport_for_data_channels = true; - rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( - rtc_config, rtc_config, loopback_media_transports()->first_factory(), - loopback_media_transports()->second_factory())); - ConnectFakeSignaling(); - - caller()->AddVideoTrack(); - callee()->AddVideoTrack(); - // Start offer/answer exchange and wait for it to complete. - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - - MediaExpectations media_expectations; - media_expectations.ExpectBidirectionalVideo(); - ASSERT_TRUE(ExpectNewFrames(media_expectations)); -} - // Test that the ICE connection and gathering states eventually reach // "complete". TEST_P(PeerConnectionIntegrationTest, IceStatesReachCompletion) { diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc index 5a01430b95..7f42b8cb95 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -1421,15 +1421,15 @@ TEST_P(PeerConnectionInterfaceTest, GetConfigurationAfterSetConfiguration) { PeerConnectionInterface::RTCConfiguration config = pc_->GetConfiguration(); config.type = PeerConnectionInterface::kRelay; - config.use_media_transport = true; - config.use_media_transport_for_data_channels = true; + config.use_datagram_transport = true; + config.use_datagram_transport_for_data_channels = true; EXPECT_TRUE(pc_->SetConfiguration(config).ok()); PeerConnectionInterface::RTCConfiguration returned_config = pc_->GetConfiguration(); EXPECT_EQ(PeerConnectionInterface::kRelay, returned_config.type); - EXPECT_TRUE(returned_config.use_media_transport); - EXPECT_TRUE(returned_config.use_media_transport_for_data_channels); + EXPECT_TRUE(returned_config.use_datagram_transport); + EXPECT_TRUE(returned_config.use_datagram_transport_for_data_channels); } TEST_P(PeerConnectionInterfaceTest, SetConfigurationFailsAfterClose) { diff --git a/pc/peer_connection_media_unittest.cc b/pc/peer_connection_media_unittest.cc index 62368a29a1..077c4a3e43 100644 --- a/pc/peer_connection_media_unittest.cc +++ b/pc/peer_connection_media_unittest.cc @@ -20,7 +20,6 @@ #include "api/call/call_factory_interface.h" #include "api/rtc_event_log/rtc_event_log_factory.h" #include "api/task_queue/default_task_queue_factory.h" -#include "api/test/fake_media_transport.h" #include "media/base/fake_media_engine.h" #include "p2p/base/fake_port_allocator.h" #include "pc/media_session.h" @@ -85,8 +84,6 @@ class PeerConnectionMediaBaseTest : public ::testing::Test { } // Creates PeerConnectionFactory and PeerConnection for given configuration. - // Note that PeerConnectionFactory is created with MediaTransportFactory, - // because some tests pass config.use_media_transport = true. WrapperPtr CreatePeerConnection( const RTCConfiguration& config, std::unique_ptr media_engine) { @@ -103,8 +100,6 @@ class PeerConnectionMediaBaseTest : public ::testing::Test { factory_dependencies.event_log_factory = std::make_unique( factory_dependencies.task_queue_factory.get()); - factory_dependencies.media_transport_factory = - std::make_unique(); auto pc_factory = CreateModularPeerConnectionFactory(std::move(factory_dependencies)); @@ -1244,128 +1239,6 @@ TEST_P(PeerConnectionMediaTest, audio_options.combined_audio_video_bwe); } -TEST_P(PeerConnectionMediaTest, MediaTransportPropagatedToVoiceEngine) { - RTCConfiguration config; - - // Setup PeerConnection to use media transport. - config.use_media_transport = true; - - // Force SDES. - config.enable_dtls_srtp = false; - - auto caller = CreatePeerConnectionWithAudio(config); - auto callee = CreatePeerConnectionWithAudio(config); - - ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); - auto answer = callee->CreateAnswer(); - ASSERT_TRUE(callee->SetLocalDescription(std::move(answer))); - - auto caller_voice = caller->media_engine()->GetVoiceChannel(0); - auto callee_voice = callee->media_engine()->GetVoiceChannel(0); - ASSERT_TRUE(caller_voice); - ASSERT_TRUE(callee_voice); - - // Make sure media transport is propagated to voice channel. - FakeMediaTransport* caller_voice_media_transport = - static_cast(caller_voice->media_transport()); - FakeMediaTransport* callee_voice_media_transport = - static_cast(callee_voice->media_transport()); - ASSERT_NE(nullptr, caller_voice_media_transport); - ASSERT_NE(nullptr, callee_voice_media_transport); - - // Make sure media transport is created with correct is_caller. - EXPECT_TRUE(caller_voice_media_transport->is_caller()); - EXPECT_FALSE(callee_voice_media_transport->is_caller()); - - // TODO(sukhanov): Propagate media transport to video channel. - // This test does NOT set up video channels, because currently it causes - // us to create two media transports. -} - -TEST_P(PeerConnectionMediaTest, MediaTransportOnlyForDataChannels) { - RTCConfiguration config; - - // Setup PeerConnection to use media transport for data channels. - config.use_media_transport_for_data_channels = true; - - // Force SDES. - config.enable_dtls_srtp = false; - - auto caller = CreatePeerConnectionWithAudio(config); - auto callee = CreatePeerConnectionWithAudio(config); - - ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); - ASSERT_TRUE(callee->SetLocalDescription(callee->CreateAnswer())); - - auto caller_voice = caller->media_engine()->GetVoiceChannel(0); - auto callee_voice = callee->media_engine()->GetVoiceChannel(0); - ASSERT_TRUE(caller_voice); - ASSERT_TRUE(callee_voice); - - // Make sure media transport is not propagated to voice channel. - EXPECT_EQ(nullptr, caller_voice->media_transport()); - EXPECT_EQ(nullptr, callee_voice->media_transport()); -} - -TEST_P(PeerConnectionMediaTest, MediaTransportForMediaAndDataChannels) { - RTCConfiguration config; - - // Setup PeerConnection to use media transport for both media and data - // channels. - config.use_media_transport = true; - config.use_media_transport_for_data_channels = true; - - // Force SDES. - config.enable_dtls_srtp = false; - - auto caller = CreatePeerConnectionWithAudio(config); - auto callee = CreatePeerConnectionWithAudio(config); - - ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); - ASSERT_TRUE(callee->SetLocalDescription(callee->CreateAnswer())); - - auto caller_voice = caller->media_engine()->GetVoiceChannel(0); - auto callee_voice = callee->media_engine()->GetVoiceChannel(0); - ASSERT_TRUE(caller_voice); - ASSERT_TRUE(callee_voice); - - // Make sure media transport is propagated to voice channel. - FakeMediaTransport* caller_voice_media_transport = - static_cast(caller_voice->media_transport()); - FakeMediaTransport* callee_voice_media_transport = - static_cast(callee_voice->media_transport()); - ASSERT_NE(nullptr, caller_voice_media_transport); - ASSERT_NE(nullptr, callee_voice_media_transport); - - // Make sure media transport is created with correct is_caller. - EXPECT_TRUE(caller_voice_media_transport->is_caller()); - EXPECT_FALSE(callee_voice_media_transport->is_caller()); -} - -TEST_P(PeerConnectionMediaTest, MediaTransportNotPropagatedToVoiceEngine) { - auto caller = CreatePeerConnectionWithAudioVideo(); - auto callee = CreatePeerConnectionWithAudioVideo(); - - ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); - auto answer = callee->CreateAnswer(); - ASSERT_TRUE(callee->SetLocalDescription(std::move(answer))); - - auto caller_voice = caller->media_engine()->GetVoiceChannel(0); - auto callee_voice = callee->media_engine()->GetVoiceChannel(0); - ASSERT_TRUE(caller_voice); - ASSERT_TRUE(callee_voice); - - // Since we did not setup PeerConnection to use media transport, media - // transport should not be created / propagated to the voice engine. - ASSERT_EQ(nullptr, caller_voice->media_transport()); - ASSERT_EQ(nullptr, callee_voice->media_transport()); - - auto caller_video = caller->media_engine()->GetVideoChannel(0); - auto callee_video = callee->media_engine()->GetVideoChannel(0); - ASSERT_EQ(nullptr, caller_video->media_transport()); - ASSERT_EQ(nullptr, callee_video->media_transport()); -} - template bool CompareCodecs(const std::vector& capabilities, const std::vector& codecs) { diff --git a/pc/session_description.h b/pc/session_description.h index f5e3635eaf..7546d12bcb 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -515,8 +515,6 @@ class SessionDescription { std::unique_ptr Clone() const; - struct MediaTransportSetting; - // Content accessors. const ContentInfos& contents() const { return contents_; } ContentInfos& contents() { return contents_; } @@ -627,32 +625,6 @@ class SessionDescription { } bool extmap_allow_mixed() const { return extmap_allow_mixed_; } - // Adds the media transport setting. - // Media transport name uniquely identifies the type of media transport. - // The name cannot be empty, or repeated in the previously added transport - // settings. - void AddMediaTransportSetting(const std::string& media_transport_name, - const std::string& media_transport_setting) { - RTC_DCHECK(!media_transport_name.empty()); - for (const auto& setting : media_transport_settings_) { - RTC_DCHECK(media_transport_name != setting.transport_name) - << "MediaTransportSetting was already registered, transport_name=" - << setting.transport_name; - } - media_transport_settings_.push_back( - {media_transport_name, media_transport_setting}); - } - - // Gets the media transport settings, in order of preference. - const std::vector& MediaTransportSettings() const { - return media_transport_settings_; - } - - struct MediaTransportSetting { - std::string transport_name; - std::string transport_setting; - }; - private: SessionDescription(const SessionDescription&); @@ -669,8 +641,6 @@ class SessionDescription { // correctly. If it's included in offer to us we will respond that we support // it. bool extmap_allow_mixed_ = false; - - std::vector media_transport_settings_; }; // Indicates whether a session description was sent by the local client or diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 7a42dcaa0a..c0e959a53d 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -229,13 +229,6 @@ static const char kApplicationSpecificMaximum[] = "AS"; static const char kDefaultSctpmapProtocol[] = "webrtc-datachannel"; -// This is a non-standardized media transport settings. -// This setting is going to be set in the offer. There may be one or more -// a=x-mt: settings, and they are in the priority order (the most preferred on -// top). x-mt setting format depends on the media transport, and is generated by -// |MediaTransportInterface::GetTransportParametersOffer|. -static const char kMediaTransportSettingLine[] = "x-mt"; - // This is a non-standardized setting for plugin transports. static const char kOpaqueTransportParametersLine[] = "x-opaque"; @@ -530,17 +523,6 @@ static void InitAttrLine(const std::string& attribute, rtc::StringBuilder* os) { InitLine(kLineTypeAttributes, attribute, os); } -// Writes an x-mt SDP attribute line based on the media transport settings. -static void AddMediaTransportLine( - const cricket::SessionDescription::MediaTransportSetting& setting, - std::string* message) { - rtc::StringBuilder os; - InitAttrLine(kMediaTransportSettingLine, &os); - os << kSdpDelimiterColon << setting.transport_name << kSdpDelimiterColon - << rtc::Base64::Encode(setting.transport_setting); - AddLine(os.str(), message); -} - // Adds an x-otp SDP attribute line based on opaque transport parameters. static void AddOpaqueTransportLine( const cricket::OpaqueTransportParameters params, @@ -902,11 +884,6 @@ std::string SdpSerialize(const JsepSessionDescription& jdesc) { // Time Description. AddLine(kTimeDescription, &message); - for (const cricket::SessionDescription::MediaTransportSetting& settings : - desc->MediaTransportSettings()) { - AddMediaTransportLine(settings, &message); - } - // Group if (desc->HasGroup(cricket::GROUP_TYPE_BUNDLE)) { std::string group_line = kAttrGroup; @@ -2122,28 +2099,6 @@ bool ParseConnectionData(const std::string& line, return true; } -bool ParseMediaTransportLine(const std::string& line, - std::string* transport_name, - std::string* transport_setting, - SdpParseError* error) { - std::string value; - if (!GetValue(line, kMediaTransportSettingLine, &value, error)) { - return false; - } - std::string media_transport_settings_base64; - if (!rtc::tokenize_first(value, kSdpDelimiterColonChar, transport_name, - &media_transport_settings_base64)) { - return ParseFailedGetValue(line, kMediaTransportSettingLine, error); - } - if (!rtc::Base64::Decode(media_transport_settings_base64, - rtc::Base64::DO_STRICT, transport_setting, - nullptr)) { - return ParseFailedGetValue(line, kMediaTransportSettingLine, error); - } - - return true; -} - bool ParseOpaqueTransportLine(const std::string& line, std::string* protocol, std::string* transport_parameters, @@ -2327,24 +2282,6 @@ bool ParseSessionDescription(const std::string& message, return false; } session_extmaps->push_back(extmap); - } else if (HasAttribute(line, kMediaTransportSettingLine)) { - std::string transport_name; - std::string transport_setting; - if (!ParseMediaTransportLine(line, &transport_name, &transport_setting, - error)) { - return false; - } - - for (const auto& setting : desc->MediaTransportSettings()) { - if (setting.transport_name == transport_name) { - // Ignore repeated transport names rather than failing to parse so - // that in the future the same transport could have multiple configs. - RTC_LOG(INFO) << "x-mt line with repeated transport, transport_name=" - << transport_name; - return true; - } - } - desc->AddMediaTransportSetting(transport_name, transport_setting); } } diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 5c7e7836fb..e8e937a7a5 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -4640,121 +4640,6 @@ TEST_F(WebRtcSdpTest, ParseNoMid) { Field("name", &cricket::ContentInfo::name, ""))); } -// Test that the media transport name and base64-decoded setting is parsed from -// an a=x-mt line. -TEST_F(WebRtcSdpTest, ParseMediaTransport) { - JsepSessionDescription output(kDummyType); - std::string sdp = kSdpSessionString; - sdp += "a=x-mt:rtp:dGVzdDY0\r\n"; - SdpParseError error; - - ASSERT_TRUE(webrtc::SdpDeserialize(sdp, &output, &error)) - << error.description; - const auto& settings = output.description()->MediaTransportSettings(); - ASSERT_EQ(1u, settings.size()); - EXPECT_EQ("rtp", settings[0].transport_name); - EXPECT_EQ("test64", settings[0].transport_setting); -} - -// Test that an a=x-mt line fails to parse if its setting is invalid base 64. -TEST_F(WebRtcSdpTest, ParseMediaTransportInvalidBase64) { - JsepSessionDescription output(kDummyType); - std::string sdp = kSdpSessionString; - sdp += "a=x-mt:rtp:ThisIsInvalidBase64\r\n"; - SdpParseError error; - - ASSERT_FALSE(webrtc::SdpDeserialize(sdp, &output, &error)); -} - -// Test that multiple a=x-mt lines are parsed in the order of preference (the -// order of the lines in the SDP). -TEST_F(WebRtcSdpTest, ParseMediaTransportMultipleLines) { - JsepSessionDescription output(kDummyType); - std::string sdp = kSdpSessionString; - sdp += - "a=x-mt:rtp:dGVzdDY0\r\n" - "a=x-mt:generic:Z2VuZXJpY3NldHRpbmc=\r\n"; - SdpParseError error; - - ASSERT_TRUE(webrtc::SdpDeserialize(sdp, &output, &error)) - << error.description; - const auto& settings = output.description()->MediaTransportSettings(); - ASSERT_EQ(2u, settings.size()); - EXPECT_EQ("rtp", settings[0].transport_name); - EXPECT_EQ("test64", settings[0].transport_setting); - EXPECT_EQ("generic", settings[1].transport_name); - EXPECT_EQ("genericsetting", settings[1].transport_setting); -} - -// Test that only the first a=x-mt line associated with a transport name is -// parsed and the rest ignored. -TEST_F(WebRtcSdpTest, ParseMediaTransportSkipRepeatedTransport) { - JsepSessionDescription output(kDummyType); - std::string sdp = kSdpSessionString; - sdp += - "a=x-mt:rtp:dGVzdDY0\r\n" - "a=x-mt:rtp:Z2VuZXJpY3NldHRpbmc=\r\n"; - SdpParseError error; - - // Repeated 'rtp' transport setting. We still parse the SDP successfully, - // but ignore the repeated transport. - ASSERT_TRUE(webrtc::SdpDeserialize(sdp, &output, &error)); - const auto& settings = output.description()->MediaTransportSettings(); - EXPECT_EQ("test64", settings[0].transport_setting); -} - -// Test that an a=x-mt line fails to parse if it is missing a setting. -TEST_F(WebRtcSdpTest, ParseMediaTransportMalformedLine) { - JsepSessionDescription output(kDummyType); - std::string sdp = kSdpSessionString; - sdp += "a=x-mt:rtp\r\n"; - SdpParseError error; - - ASSERT_FALSE(webrtc::SdpDeserialize(sdp, &output, &error)); -} - -// Test that an a=x-mt line fails to parse if its missing a name and setting. -TEST_F(WebRtcSdpTest, ParseMediaTransportMalformedLine2) { - JsepSessionDescription output(kDummyType); - std::string sdp = kSdpSessionString; - sdp += "a=x-mt\r\n"; - SdpParseError error; - - ASSERT_FALSE(webrtc::SdpDeserialize(sdp, &output, &error)); -} - -TEST_F(WebRtcSdpTest, ParseMediaTransportIgnoreNonsenseAttributeLines) { - JsepSessionDescription output(kDummyType); - std::string sdp = kSdpSessionString; - sdp += "a=x-nonsense:rtp:dGVzdDY0\r\n"; - SdpParseError error; - - ASSERT_TRUE(webrtc::SdpDeserialize(sdp, &output, &error)) - << error.description; - EXPECT_TRUE(output.description()->MediaTransportSettings().empty()); -} - -TEST_F(WebRtcSdpTest, SerializeMediaTransportSettings) { - auto description = std::make_unique(); - - JsepSessionDescription output(SdpType::kOffer); - // JsepSessionDescription takes ownership of the description. - output.Initialize(std::move(description), "session_id", "session_version"); - output.description()->AddMediaTransportSetting("foo", "bar"); - std::string serialized_out; - output.ToString(&serialized_out); - ASSERT_THAT(serialized_out, ::testing::HasSubstr("\r\na=x-mt:foo:YmFy\r\n")); -} - -TEST_F(WebRtcSdpTest, SerializeMediaTransportSettingsTestCopy) { - cricket::SessionDescription description; - description.AddMediaTransportSetting("name", "setting"); - std::unique_ptr copy = description.Clone(); - ASSERT_EQ(1u, copy->MediaTransportSettings().size()); - EXPECT_EQ("name", copy->MediaTransportSettings()[0].transport_name); - EXPECT_EQ("setting", copy->MediaTransportSettings()[0].transport_setting); -} - TEST_F(WebRtcSdpTest, SerializeWithDefaultSctpProtocol) { AddSctpDataChannel(false); // Don't use sctpmap JsepSessionDescription jsep_desc(kDummyType); diff --git a/test/call_test.cc b/test/call_test.cc index 9f26cc679f..10b631aacf 100644 --- a/test/call_test.cc +++ b/test/call_test.cc @@ -36,7 +36,7 @@ CallTest::CallTest() task_queue_factory_(CreateDefaultTaskQueueFactory()), send_event_log_(std::make_unique()), recv_event_log_(std::make_unique()), - audio_send_config_(/*send_transport=*/nullptr, MediaTransportConfig()), + audio_send_config_(/*send_transport=*/nullptr), audio_send_stream_(nullptr), frame_generator_capturer_(nullptr), fake_encoder_factory_([this]() { @@ -275,8 +275,7 @@ void CallTest::CreateAudioAndFecSendConfigs(size_t num_audio_streams, RTC_DCHECK_LE(num_audio_streams, 1); RTC_DCHECK_LE(num_flexfec_streams, 1); if (num_audio_streams > 0) { - AudioSendStream::Config audio_send_config(send_transport, - MediaTransportConfig()); + AudioSendStream::Config audio_send_config(send_transport); audio_send_config.rtp.ssrc = kAudioSendSsrc; audio_send_config.send_codec_spec = AudioSendStream::Config::SendCodecSpec( kAudioSendPayloadType, {"opus", 48000, 2, {{"stereo", "1"}}}); diff --git a/test/peer_scenario/peer_scenario_client.cc b/test/peer_scenario/peer_scenario_client.cc index 28cbb6e22c..d8f2b65ac7 100644 --- a/test/peer_scenario/peer_scenario_client.cc +++ b/test/peer_scenario/peer_scenario_client.cc @@ -185,7 +185,6 @@ PeerScenarioClient::PeerScenarioClient( pcf_deps.fec_controller_factory = nullptr; pcf_deps.network_controller_factory = nullptr; pcf_deps.network_state_predictor_factory = nullptr; - pcf_deps.media_transport_factory = nullptr; pc_factory_ = CreateModularPeerConnectionFactory(std::move(pcf_deps)); diff --git a/test/scenario/audio_stream.cc b/test/scenario/audio_stream.cc index f5d21167ff..2738f6952c 100644 --- a/test/scenario/audio_stream.cc +++ b/test/scenario/audio_stream.cc @@ -73,8 +73,7 @@ SendAudioStream::SendAudioStream( rtc::scoped_refptr encoder_factory, Transport* send_transport) : sender_(sender), config_(config) { - AudioSendStream::Config send_config(send_transport, - webrtc::MediaTransportConfig()); + AudioSendStream::Config send_config(send_transport); ssrc_ = sender->GetNextAudioSsrc(); send_config.rtp.ssrc = ssrc_; SdpAudioFormat::Parameters sdp_params; diff --git a/test/scenario/stats_collection.h b/test/scenario/stats_collection.h index 64cb58cbe9..908385e763 100644 --- a/test/scenario/stats_collection.h +++ b/test/scenario/stats_collection.h @@ -15,6 +15,7 @@ #include "absl/types/optional.h" #include "call/call.h" +#include "rtc_base/thread.h" #include "test/logging/log_writer.h" #include "test/scenario/performance_stats.h" diff --git a/video/encoder_rtcp_feedback.cc b/video/encoder_rtcp_feedback.cc index 19a8f64054..a736d83b82 100644 --- a/video/encoder_rtcp_feedback.cc +++ b/video/encoder_rtcp_feedback.cc @@ -67,16 +67,6 @@ void EncoderRtcpFeedback::OnReceivedIntraFrameRequest(uint32_t ssrc) { video_stream_encoder_->SendKeyFrame(); } -void EncoderRtcpFeedback::OnKeyFrameRequested(uint64_t channel_id) { - if (channel_id != ssrcs_[0]) { - RTC_LOG(LS_INFO) << "Key frame request on unknown channel id " << channel_id - << " expected " << ssrcs_[0]; - return; - } - - video_stream_encoder_->SendKeyFrame(); -} - void EncoderRtcpFeedback::OnReceivedLossNotification( uint32_t ssrc, uint16_t seq_num_of_last_decodable, diff --git a/video/encoder_rtcp_feedback.h b/video/encoder_rtcp_feedback.h index 21624dbdaa..b5dd0288f3 100644 --- a/video/encoder_rtcp_feedback.h +++ b/video/encoder_rtcp_feedback.h @@ -12,7 +12,6 @@ #include -#include "api/transport/media/media_transport_interface.h" #include "api/video/video_stream_encoder_interface.h" #include "call/rtp_video_sender_interface.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -24,12 +23,9 @@ namespace webrtc { class VideoStreamEncoderInterface; // This class passes feedback (such as key frame requests or loss notifications) -// from either Mediatransport or the RtpRtcp module. -// TODO(bugs.webrtc.org/9719): Should be eliminated when RtpMediaTransport is -// implemented. +// from the RtpRtcp module. class EncoderRtcpFeedback : public RtcpIntraFrameObserver, - public RtcpLossNotificationObserver, - public MediaTransportKeyFrameRequestCallback { + public RtcpLossNotificationObserver { public: EncoderRtcpFeedback(Clock* clock, const std::vector& ssrcs, @@ -40,9 +36,6 @@ class EncoderRtcpFeedback : public RtcpIntraFrameObserver, void OnReceivedIntraFrameRequest(uint32_t ssrc) override; - // Implements MediaTransportKeyFrameRequestCallback - void OnKeyFrameRequested(uint64_t channel_id) override; - // Implements RtcpLossNotificationObserver. void OnReceivedLossNotification(uint32_t ssrc, uint16_t seq_num_of_last_decodable, diff --git a/video/encoder_rtcp_feedback_unittest.cc b/video/encoder_rtcp_feedback_unittest.cc index b49a0b9aa1..81ac22b6c6 100644 --- a/video/encoder_rtcp_feedback_unittest.cc +++ b/video/encoder_rtcp_feedback_unittest.cc @@ -55,9 +55,4 @@ TEST_F(VieKeyRequestTest, TooManyOnReceivedIntraFrameRequest) { encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc); } -TEST_F(VieKeyRequestTest, TriggerRequestFromMediaTransport) { - EXPECT_CALL(encoder_, SendKeyFrame()).Times(1); - encoder_rtcp_feedback_.OnKeyFrameRequested(kSsrc); -} - } // namespace webrtc diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 8f7d612453..ad8c808088 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -22,7 +22,6 @@ #include "api/rtc_event_log_output_file.h" #include "api/task_queue/default_task_queue_factory.h" #include "api/task_queue/task_queue_base.h" -#include "api/transport/media/media_transport_config.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "api/video_codecs/video_encoder.h" #include "call/fake_network_pipe.h" @@ -1410,8 +1409,7 @@ void VideoQualityTest::InitializeAudioDevice(Call::Config* send_call_config, } void VideoQualityTest::SetupAudio(Transport* transport) { - AudioSendStream::Config audio_send_config(transport, - webrtc::MediaTransportConfig()); + AudioSendStream::Config audio_send_config(transport); audio_send_config.rtp.ssrc = kAudioSendSsrc; // Add extension to enable audio send side BWE, and allow audio bit rate diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index a683f7ddb5..8213c64bf3 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -125,40 +125,6 @@ class NullVideoDecoder : public webrtc::VideoDecoder { const char* ImplementationName() const override { return "NullVideoDecoder"; } }; -// Inherit video_coding::EncodedFrame, which is the class used by -// video_coding::FrameBuffer and other components in the receive pipeline. It's -// a subclass of EncodedImage, and it always owns the buffer. -class EncodedFrameForMediaTransport : public video_coding::EncodedFrame { - public: - explicit EncodedFrameForMediaTransport( - MediaTransportEncodedVideoFrame frame) { - // TODO(nisse): This is ugly. We copy the EncodedImage (a base class of - // ours, in several steps), to get all the meta data. We should be using - // std::move in some way. Then we also need to handle the case of an unowned - // buffer, in which case we need to make an owned copy. - *static_cast(this) = frame.encoded_image(); - - // If we don't already own the buffer, make a copy. - Retain(); - - _payloadType = static_cast(frame.payload_type()); - - // TODO(nisse): frame_id and picture_id are probably not the same thing. For - // a single layer, this should be good enough. - id.picture_id = frame.frame_id(); - id.spatial_layer = frame.encoded_image().SpatialIndex().value_or(0); - num_references = std::min(static_cast(kMaxFrameReferences), - frame.referenced_frame_ids().size()); - for (size_t i = 0; i < num_references; i++) { - references[i] = frame.referenced_frame_ids()[i]; - } - } - - // TODO(nisse): Implement. Not sure how they are used. - int64_t ReceivedTime() const override { return 0; } - int64_t RenderTime() const override { return 0; } -}; - // TODO(https://bugs.webrtc.org/9974): Consider removing this workaround. // Maximum time between frames before resetting the FrameBuffer to avoid RTP // timestamps wraparound to affect FrameBuffer. @@ -238,23 +204,18 @@ VideoReceiveStream::VideoReceiveStream( new video_coding::FrameBuffer(clock_, timing_.get(), &stats_proxy_)); process_thread_->RegisterModule(&rtp_stream_sync_, RTC_FROM_HERE); - if (config_.media_transport()) { - config_.media_transport()->SetReceiveVideoSink(this); - config_.media_transport()->AddRttObserver(this); + // Register with RtpStreamReceiverController. + media_receiver_ = receiver_controller->CreateReceiver( + config_.rtp.remote_ssrc, &rtp_video_stream_receiver_); + if (config_.rtp.rtx_ssrc) { + rtx_receive_stream_ = std::make_unique( + &rtp_video_stream_receiver_, config.rtp.rtx_associated_payload_types, + config_.rtp.remote_ssrc, rtp_receive_statistics_.get()); + rtx_receiver_ = receiver_controller->CreateReceiver( + config_.rtp.rtx_ssrc, rtx_receive_stream_.get()); } else { - // Register with RtpStreamReceiverController. - media_receiver_ = receiver_controller->CreateReceiver( - config_.rtp.remote_ssrc, &rtp_video_stream_receiver_); - if (config_.rtp.rtx_ssrc) { - rtx_receive_stream_ = std::make_unique( - &rtp_video_stream_receiver_, config.rtp.rtx_associated_payload_types, - config_.rtp.remote_ssrc, rtp_receive_statistics_.get()); - rtx_receiver_ = receiver_controller->CreateReceiver( - config_.rtp.rtx_ssrc, rtx_receive_stream_.get()); - } else { - rtp_receive_statistics_->EnableRetransmitDetection(config.rtp.remote_ssrc, - true); - } + rtp_receive_statistics_->EnableRetransmitDetection(config.rtp.remote_ssrc, + true); } } @@ -281,10 +242,6 @@ VideoReceiveStream::~VideoReceiveStream() { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); RTC_LOG(LS_INFO) << "~VideoReceiveStream: " << config_.ToString(); Stop(); - if (config_.media_transport()) { - config_.media_transport()->SetReceiveVideoSink(nullptr); - config_.media_transport()->RemoveRttObserver(this); - } process_thread_->DeRegisterModule(&rtp_stream_sync_); } @@ -536,11 +493,7 @@ void VideoReceiveStream::SendNack(const std::vector& sequence_numbers, } void VideoReceiveStream::RequestKeyFrame(int64_t timestamp_ms) { - if (config_.media_transport()) { - config_.media_transport()->RequestKeyFrame(config_.rtp.remote_ssrc); - } else { - rtp_video_stream_receiver_.RequestKeyFrame(); - } + rtp_video_stream_receiver_.RequestKeyFrame(); last_keyframe_request_ms_ = timestamp_ms; } @@ -573,22 +526,12 @@ void VideoReceiveStream::OnCompleteFrame( rtp_video_stream_receiver_.FrameContinuous(last_continuous_pid); } -void VideoReceiveStream::OnData(uint64_t channel_id, - MediaTransportEncodedVideoFrame frame) { - OnCompleteFrame( - std::make_unique(std::move(frame))); -} - void VideoReceiveStream::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) { RTC_DCHECK_RUN_ON(&module_process_sequence_checker_); frame_buffer_->UpdateRtt(max_rtt_ms); rtp_video_stream_receiver_.UpdateRtt(max_rtt_ms); } -void VideoReceiveStream::OnRttUpdated(int64_t rtt_ms) { - frame_buffer_->UpdateRtt(rtt_ms); -} - int VideoReceiveStream::id() const { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); return config_.rtp.remote_ssrc; diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h index 7c6856381a..2a4e0d1de5 100644 --- a/video/video_receive_stream.h +++ b/video/video_receive_stream.h @@ -15,7 +15,6 @@ #include #include "api/task_queue/task_queue_factory.h" -#include "api/transport/media/media_transport_interface.h" #include "call/rtp_packet_sink_interface.h" #include "call/syncable.h" #include "call/video_receive_stream.h" @@ -49,9 +48,7 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, public NackSender, public video_coding::OnCompleteFrameCallback, public Syncable, - public CallStatsObserver, - public MediaTransportVideoSinkInterface, - public MediaTransportRttObserver { + public CallStatsObserver { public: VideoReceiveStream(TaskQueueFactory* task_queue_factory, RtpStreamReceiverControllerInterface* receiver_controller, @@ -110,17 +107,9 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, void OnCompleteFrame( std::unique_ptr frame) override; - // Implements MediaTransportVideoSinkInterface, converts the received frame to - // OnCompleteFrameCallback - void OnData(uint64_t channel_id, - MediaTransportEncodedVideoFrame frame) override; - // Implements CallStatsObserver::OnRttUpdate void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; - // Implements MediaTransportRttObserver::OnRttUpdated - void OnRttUpdated(int64_t rtt_ms) override; - // Implements Syncable. int id() const override; absl::optional GetInfo() const override; diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index 12d9dac5e9..8fae407bc1 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -103,7 +103,7 @@ VideoSendStream::VideoSendStream( event_log, &config_, encoder_config.max_bitrate_bps, encoder_config.bitrate_priority, suspended_ssrcs, suspended_payload_states, encoder_config.content_type, - std::move(fec_controller), config_.media_transport)); + std::move(fec_controller))); }, [this]() { thread_sync_event_.Set(); })); diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index 31dddcceca..97f3bb7f4c 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -181,8 +181,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( std::map suspended_ssrcs, std::map suspended_payload_states, VideoEncoderConfig::ContentType content_type, - std::unique_ptr fec_controller, - MediaTransportInterface* media_transport) + std::unique_ptr fec_controller) : clock_(clock), has_alr_probing_(config->periodic_alr_bandwidth_probing || GetAlrSettings(content_type)), @@ -216,8 +215,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( event_log, std::move(fec_controller), CreateFrameEncryptionConfig(config_))), - weak_ptr_factory_(this), - media_transport_(media_transport) { + weak_ptr_factory_(this) { video_stream_encoder->SetFecControllerOverride(rtp_video_sender_); RTC_DCHECK_RUN_ON(worker_queue_); RTC_LOG(LS_INFO) << "VideoSendStreamInternal: " << config_->ToString(); @@ -225,14 +223,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( encoder_feedback_.SetRtpVideoSender(rtp_video_sender_); - if (media_transport_) { - // The configured ssrc is interpreted as a channel id, so there must be - // exactly one. - RTC_DCHECK_EQ(config_->rtp.ssrcs.size(), 1); - media_transport_->SetKeyFrameRequestCallback(&encoder_feedback_); - } else { - RTC_DCHECK(!config_->rtp.ssrcs.empty()); - } + RTC_DCHECK(!config_->rtp.ssrcs.empty()); RTC_DCHECK(call_stats_); RTC_DCHECK(transport_); RTC_DCHECK_NE(initial_encoder_max_bitrate, 0); @@ -310,9 +301,6 @@ VideoSendStreamImpl::~VideoSendStreamImpl() { << "VideoSendStreamImpl::Stop not called"; RTC_LOG(LS_INFO) << "~VideoSendStreamInternal: " << config_->ToString(); transport_->DestroyRtpVideoSender(rtp_video_sender_); - if (media_transport_) { - media_transport_->SetKeyFrameRequestCallback(nullptr); - } } void VideoSendStreamImpl::RegisterProcessThread( @@ -581,31 +569,8 @@ EncodedImageCallback::Result VideoSendStreamImpl::OnEncodedImage( } EncodedImageCallback::Result result(EncodedImageCallback::Result::OK); - if (media_transport_) { - int64_t frame_id; - { - // TODO(nisse): Responsibility for allocation of frame ids should move to - // VideoStreamEncoder. - rtc::CritScope cs(&media_transport_id_lock_); - frame_id = media_transport_frame_id_++; - } - // TODO(nisse): Responsibility for reference meta data should be moved - // upstream, ideally close to the encoders, but probably VideoStreamEncoder - // will need to do some translation to produce reference info using frame - // ids. - std::vector referenced_frame_ids; - if (encoded_image._frameType != VideoFrameType::kVideoFrameKey) { - RTC_DCHECK_GT(frame_id, 0); - referenced_frame_ids.push_back(frame_id - 1); - } - media_transport_->SendVideoFrame( - config_->rtp.ssrcs[0], webrtc::MediaTransportEncodedVideoFrame( - frame_id, referenced_frame_ids, - config_->rtp.payload_type, encoded_image)); - } else { - result = rtp_video_sender_->OnEncodedImage( - encoded_image, codec_specific_info, fragmentation); - } + result = rtp_video_sender_->OnEncodedImage(encoded_image, codec_specific_info, + fragmentation); // Check if there's a throttled VideoBitrateAllocation that we should try // sending. rtc::WeakPtr send_stream = weak_ptr_; diff --git a/video/video_send_stream_impl.h b/video/video_send_stream_impl.h index 091ac0f8da..4195efcf82 100644 --- a/video/video_send_stream_impl.h +++ b/video/video_send_stream_impl.h @@ -87,8 +87,7 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, std::map suspended_ssrcs, std::map suspended_payload_states, VideoEncoderConfig::ContentType content_type, - std::unique_ptr fec_controller, - MediaTransportInterface* media_transport); + std::unique_ptr fec_controller); ~VideoSendStreamImpl() override; // RegisterProcessThread register |module_process_thread| with those objects @@ -199,10 +198,6 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, }; absl::optional video_bitrate_allocation_context_ RTC_GUARDED_BY(worker_queue_); - MediaTransportInterface* const media_transport_; - rtc::CriticalSection media_transport_id_lock_; - int64_t media_transport_frame_id_ RTC_GUARDED_BY(media_transport_id_lock_) = - 0; }; } // namespace internal } // namespace webrtc diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index ce88a36470..1c44cc8dd4 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -137,8 +137,7 @@ class VideoSendStreamImplTest : public ::testing::Test { &video_stream_encoder_, &event_log_, &config_, initial_encoder_max_bitrate, initial_encoder_bitrate_priority, suspended_ssrcs, suspended_payload_states, content_type, - std::make_unique(&clock_), - /*media_transport=*/nullptr); + std::make_unique(&clock_)); } protected: