From 365381fdf1cd83e6dc7ce1debaa8a97ede6f4e21 Mon Sep 17 00:00:00 2001 From: Zhi Huang Date: Fri, 13 Apr 2018 16:44:34 -0700 Subject: [PATCH] Replace BundleFilter with RtpDemuxer in RtpTransport. BundleFilter is replaced by RtpDemuxer in RtpTransport for payload type-based demuxing. RtpTransport will support MID-based demuxing later. Each BaseChannel has its own RTP demuxing criteria and when connecting to the RtpTransport, BaseChannel will register itself as a demuxer sink. The inheritance model is changed. New inheritance chain: DtlsSrtpTransport->SrtpTransport->RtpTranpsort The JsepTransport2 is renamed to JsepTransport. NOTE: When RTCP packets are received, Call::DeliverRtcp will be called for multiple times (webrtc:9035) which is an existing issue. With this CL, it will become more of a problem and should be fixed. Bug: webrtc:8587 Change-Id: Ibd880e7b744bd912336a691309950bc18e42cf62 Reviewed-on: https://webrtc-review.googlesource.com/65786 Commit-Queue: Zhi Huang Reviewed-by: Taylor Brandstetter Reviewed-by: Benjamin Wright Cr-Commit-Position: refs/heads/master@{#22867} --- media/base/rtputils.cc | 10 + media/base/rtputils.h | 1 + ortc/rtptransportadapter.cc | 4 +- ortc/rtptransportadapter.h | 13 +- ortc/rtptransportcontrolleradapter.cc | 4 - pc/BUILD.gn | 14 +- pc/channel.cc | 133 ++++++++--- pc/channel.h | 56 ++--- pc/channel_unittest.cc | 31 +-- pc/channelmanager_unittest.cc | 6 +- pc/dtlssrtptransport.cc | 97 ++------ pc/dtlssrtptransport.h | 38 +-- pc/dtlssrtptransport_unittest.cc | 66 +++--- pc/{jseptransport2.cc => jseptransport.cc} | 58 ++--- pc/{jseptransport2.h => jseptransport.h} | 16 +- ..._unittest.cc => jseptransport_unittest.cc} | 31 +-- pc/jseptransportcontroller.cc | 105 +++++---- pc/jseptransportcontroller.h | 60 ++--- pc/jseptransportcontroller_unittest.cc | 19 +- pc/mediasession.h | 2 +- pc/peerconnection.cc | 20 +- pc/peerconnection.h | 11 +- pc/peerconnection_bundle_unittest.cc | 36 +++ pc/peerconnection_integrationtest.cc | 3 +- pc/peerconnection_media_unittest.cc | 8 +- pc/rtpsenderreceiver_unittest.cc | 6 +- pc/rtptransport.cc | 142 ++++++----- pc/rtptransport.h | 59 +++-- pc/rtptransport_unittest.cc | 55 +++-- pc/rtptransportinternal.h | 30 ++- pc/rtptransportinternaladapter.h | 14 ++ pc/rtptransporttestutil.h | 64 +++-- pc/srtptransport.cc | 223 +++++++++--------- pc/srtptransport.h | 64 ++--- pc/srtptransport_unittest.cc | 87 +++---- 35 files changed, 861 insertions(+), 725 deletions(-) rename pc/{jseptransport2.cc => jseptransport.cc} (93%) rename pc/{jseptransport2.h => jseptransport.h} (96%) rename pc/{jseptransport2_unittest.cc => jseptransport_unittest.cc} (98%) diff --git a/media/base/rtputils.cc b/media/base/rtputils.cc index d0ba1cf72b..e8fd10d8bb 100644 --- a/media/base/rtputils.cc +++ b/media/base/rtputils.cc @@ -275,6 +275,16 @@ bool IsRtpPacket(const void* data, size_t len) { return (static_cast(data)[0] >> 6) == kRtpVersion; } +// Check the RTP payload type. If 63 < payload type < 96, it's RTCP. +// For additional details, see http://tools.ietf.org/html/rfc5761. +bool IsRtcpPacket(const char* data, size_t len) { + if (len < 2) { + return false; + } + char pt = data[1] & 0x7F; + return (63 < pt) && (pt < 96); +} + bool IsValidRtpPayloadType(int payload_type) { return payload_type >= 0 && payload_type <= 127; } diff --git a/media/base/rtputils.h b/media/base/rtputils.h index 0b7205cf8f..5258c0be26 100644 --- a/media/base/rtputils.h +++ b/media/base/rtputils.h @@ -55,6 +55,7 @@ bool SetRtpHeader(void* data, size_t len, const RtpHeader& header); bool IsRtpPacket(const void* data, size_t len); +bool IsRtcpPacket(const char* data, size_t len); // True if |payload type| is 0-127. bool IsValidRtpPayloadType(int payload_type); diff --git a/ortc/rtptransportadapter.cc b/ortc/rtptransportadapter.cc index acd71db25b..7705e176fe 100644 --- a/ortc/rtptransportadapter.cc +++ b/ortc/rtptransportadapter.cc @@ -171,8 +171,8 @@ RtpTransportAdapter::RtpTransportAdapter( transport_->SignalReadyToSend.connect(this, &RtpTransportAdapter::OnReadyToSend); - transport_->SignalPacketReceived.connect( - this, &RtpTransportAdapter::OnPacketReceived); + transport_->SignalRtcpPacketReceived.connect( + this, &RtpTransportAdapter::OnRtcpPacketReceived); transport_->SignalWritableState.connect( this, &RtpTransportAdapter::OnWritableState); } diff --git a/ortc/rtptransportadapter.h b/ortc/rtptransportadapter.h index 72bd3f61ab..c1b8efaf05 100644 --- a/ortc/rtptransportadapter.h +++ b/ortc/rtptransportadapter.h @@ -14,13 +14,11 @@ #include #include -#include "api/ortc/srtptransportinterface.h" #include "api/rtcerror.h" #include "media/base/streamparams.h" #include "ortc/rtptransportcontrolleradapter.h" #include "pc/channel.h" -#include "pc/rtptransport.h" -#include "pc/rtptransportinternal.h" +#include "pc/rtptransportinternaladapter.h" #include "pc/srtptransport.h" #include "rtc_base/constructormagic.h" #include "rtc_base/sigslot.h" @@ -82,8 +80,6 @@ class RtpTransportAdapter : public RtpTransportInternalAdapter { bool IsSrtpActive() const override { return transport_->IsSrtpActive(); } - bool IsSrtpTransport() const { return srtp_transport_ != nullptr; } - protected: RtpTransportAdapter* GetInternal() override { return this; } @@ -96,10 +92,9 @@ class RtpTransportAdapter : public RtpTransportInternalAdapter { void OnReadyToSend(bool ready) { SignalReadyToSend(ready); } - void OnPacketReceived(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketTime& time) { - SignalPacketReceived(rtcp, packet, time); + void OnRtcpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& time) { + SignalRtcpPacketReceived(packet, time); } void OnWritableState(bool writable) { SignalWritableState(writable); } diff --git a/ortc/rtptransportcontrolleradapter.cc b/ortc/rtptransportcontrolleradapter.cc index b772ffadd8..f69edd5803 100644 --- a/ortc/rtptransportcontrolleradapter.cc +++ b/ortc/rtptransportcontrolleradapter.cc @@ -869,8 +869,6 @@ void RtpTransportControllerAdapter::CreateVoiceChannel() { cricket::AudioOptions()); RTC_DCHECK(voice_channel_); voice_channel_->Enable(true); - voice_channel_->DisableEncryption( - !inner_audio_transport_->GetInternal()->IsSrtpTransport()); } void RtpTransportControllerAdapter::CreateVideoChannel() { @@ -880,8 +878,6 @@ void RtpTransportControllerAdapter::CreateVideoChannel() { cricket::VideoOptions()); RTC_DCHECK(video_channel_); video_channel_->Enable(true); - video_channel_->DisableEncryption( - !inner_video_transport_->GetInternal()->IsSrtpTransport()); } void RtpTransportControllerAdapter::DestroyVoiceChannel() { diff --git a/pc/BUILD.gn b/pc/BUILD.gn index b59de48ab3..b9b2f84116 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -30,8 +30,6 @@ rtc_static_library("rtc_pc_base") { defines = [] sources = [ "audiomonitor.h", - "bundlefilter.cc", - "bundlefilter.h", "channel.cc", "channel.h", "channelmanager.cc", @@ -42,8 +40,8 @@ rtc_static_library("rtc_pc_base") { "dtlssrtptransport.h", "externalhmac.cc", "externalhmac.h", - "jseptransport2.cc", - "jseptransport2.h", + "jseptransport.cc", + "jseptransport.h", "jseptransportcontroller.cc", "jseptransportcontroller.h", "mediasession.cc", @@ -76,10 +74,13 @@ rtc_static_library("rtc_pc_base") { "../api:optional", "../api:ortc_api", "../api:video_frame_api", + "../call:rtp_interfaces", + "../call:rtp_receiver", "../common_video:common_video", "../media:rtc_data", "../media:rtc_h264_profile_id", "../media:rtc_media_base", + "../modules/rtp_rtcp:rtp_rtcp_format", "../p2p:rtc_p2p", "../rtc_base:checks", "../rtc_base:rtc_base", @@ -270,12 +271,11 @@ if (rtc_include_tests) { testonly = true sources = [ - "bundlefilter_unittest.cc", "channel_unittest.cc", "channelmanager_unittest.cc", "currentspeakermonitor_unittest.cc", "dtlssrtptransport_unittest.cc", - "jseptransport2_unittest.cc", + "jseptransport_unittest.cc", "jseptransportcontroller_unittest.cc", "mediasession_unittest.cc", "rtcpmuxfilter_unittest.cc", @@ -308,9 +308,11 @@ if (rtc_include_tests) { "../api:array_view", "../api:fakemetricsobserver", "../api:libjingle_peerconnection_api", + "../call:rtp_interfaces", "../logging:rtc_event_log_api", "../media:rtc_media_base", "../media:rtc_media_tests_utils", + "../modules/rtp_rtcp:rtp_rtcp_format", "../p2p:p2p_test_utils", "../p2p:rtc_p2p", "../rtc_base:checks", diff --git a/pc/channel.cc b/pc/channel.cc index a1869c43ca..041412582c 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -17,6 +17,7 @@ #include "api/call/audio_sink.h" #include "media/base/mediaconstants.h" #include "media/base/rtputils.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/bind.h" #include "rtc_base/byteorder.h" #include "rtc_base/checks.h" @@ -108,13 +109,13 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread, crypto_options_(crypto_options), media_channel_(std::move(media_channel)) { RTC_DCHECK_RUN_ON(worker_thread_); + demuxer_criteria_.mid = content_name; RTC_LOG(LS_INFO) << "Created channel for " << content_name; } BaseChannel::~BaseChannel() { TRACE_EVENT0("webrtc", "BaseChannel::~BaseChannel"); RTC_DCHECK_RUN_ON(worker_thread_); - Deinit(); // Eats any outstanding messages or packets. worker_thread_->Clear(&invoker_); worker_thread_->Clear(this); @@ -125,15 +126,15 @@ BaseChannel::~BaseChannel() { RTC_LOG(LS_INFO) << "Destroyed channel: " << content_name_; } -void BaseChannel::ConnectToRtpTransport() { +bool BaseChannel::ConnectToRtpTransport() { RTC_DCHECK(rtp_transport_); + if (!RegisterRtpDemuxerSink()) { + return false; + } rtp_transport_->SignalReadyToSend.connect( this, &BaseChannel::OnTransportReadyToSend); - // TODO(zstein): RtpTransport::SignalPacketReceived will probably be replaced - // with a callback interface later so that the demuxer can select which - // channel to signal. - rtp_transport_->SignalPacketReceived.connect(this, - &BaseChannel::OnPacketReceived); + rtp_transport_->SignalRtcpPacketReceived.connect( + this, &BaseChannel::OnRtcpPacketReceived); rtp_transport_->SignalNetworkRouteChanged.connect( this, &BaseChannel::OnNetworkRouteChanged); rtp_transport_->SignalWritableState.connect(this, @@ -146,12 +147,14 @@ void BaseChannel::ConnectToRtpTransport() { if (metrics_observer_) { rtp_transport_->SetMetricsObserver(metrics_observer_); } + return true; } void BaseChannel::DisconnectFromRtpTransport() { RTC_DCHECK(rtp_transport_); + rtp_transport_->UnregisterRtpDemuxerSink(this); rtp_transport_->SignalReadyToSend.disconnect(this); - rtp_transport_->SignalPacketReceived.disconnect(this); + rtp_transport_->SignalRtcpPacketReceived.disconnect(this); rtp_transport_->SignalNetworkRouteChanged.disconnect(this); rtp_transport_->SignalWritableState.disconnect(this); rtp_transport_->SignalSentPacket.disconnect(this); @@ -160,9 +163,8 @@ void BaseChannel::DisconnectFromRtpTransport() { void BaseChannel::Init_w(webrtc::RtpTransportInternal* rtp_transport) { RTC_DCHECK_RUN_ON(worker_thread_); - network_thread_->Invoke(RTC_FROM_HERE, [&] { - SetRtpTransport(rtp_transport); - }); + network_thread_->Invoke( + RTC_FROM_HERE, [this, rtp_transport] { SetRtpTransport(rtp_transport); }); // Both RTP and RTCP channels should be set, we can call SetInterface on // the media channel and it can set network options. @@ -187,12 +189,15 @@ void BaseChannel::Deinit() { }); } -void BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { +bool BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { + if (rtp_transport == rtp_transport_) { + return true; + } + if (!network_thread_->IsCurrent()) { - network_thread_->Invoke(RTC_FROM_HERE, [&] { - SetRtpTransport(rtp_transport); + return network_thread_->Invoke(RTC_FROM_HERE, [this, rtp_transport] { + return SetRtpTransport(rtp_transport); }); - return; } if (rtp_transport_) { @@ -204,7 +209,10 @@ void BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { RTC_DCHECK(rtp_transport_->rtp_packet_transport()); transport_name_ = rtp_transport_->rtp_packet_transport()->transport_name(); - ConnectToRtpTransport(); + if (!ConnectToRtpTransport()) { + RTC_LOG(LS_ERROR) << "Failed to connect to the new RtpTransport."; + return false; + } OnTransportReadyToSend(rtp_transport_->IsReadyToSend()); UpdateWritableState_n(); @@ -220,6 +228,7 @@ void BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { } } } + return true; } void BaseChannel::SetMetricsObserver( @@ -239,11 +248,19 @@ bool BaseChannel::Enable(bool enable) { } bool BaseChannel::AddRecvStream(const StreamParams& sp) { + demuxer_criteria_.ssrcs.insert(sp.first_ssrc()); + if (!RegisterRtpDemuxerSink()) { + return false; + } return InvokeOnWorker(RTC_FROM_HERE, Bind(&BaseChannel::AddRecvStream_w, this, sp)); } bool BaseChannel::RemoveRecvStream(uint32_t ssrc) { + demuxer_criteria_.ssrcs.erase(ssrc); + if (!RegisterRtpDemuxerSink()) { + return false; + } return InvokeOnWorker( RTC_FROM_HERE, Bind(&BaseChannel::RemoveRecvStream_w, this, ssrc)); } @@ -295,7 +312,7 @@ bool BaseChannel::IsReadyToSendMedia_n() const { return enabled() && webrtc::RtpTransceiverDirectionHasRecv(remote_content_direction_) && webrtc::RtpTransceiverDirectionHasSend(local_content_direction_) && - was_ever_writable() && (srtp_active() || encryption_disabled_); + was_ever_writable(); } bool BaseChannel::SendPacket(rtc::CopyOnWriteBuffer* packet, @@ -430,23 +447,56 @@ bool BaseChannel::SendPacket(bool rtcp, : rtp_transport_->SendRtpPacket(packet, options, PF_SRTP_BYPASS); } -bool BaseChannel::HandlesPayloadType(int packet_type) const { - return bundle_filter_.FindPayloadType(packet_type); +void BaseChannel::OnRtpPacket(const webrtc::RtpPacketReceived& parsed_packet) { + // Reconstruct the PacketTime from the |parsed_packet|. + // RtpPacketReceived.arrival_time_ms = (PacketTime + 500) / 1000; + // Note: The |not_before| field is always 0 here. This field is not currently + // used, so it should be fine. + int64_t timestamp = -1; + if (parsed_packet.arrival_time_ms() > 0) { + timestamp = parsed_packet.arrival_time_ms() * 1000; + } + rtc::PacketTime packet_time(timestamp, /*not_before=*/0); + + OnPacketReceived(/*rtcp=*/false, parsed_packet.Buffer(), packet_time); +} + +void BaseChannel::UpdateRtpHeaderExtensionMap( + const RtpHeaderExtensions& header_extensions) { + RTC_DCHECK(rtp_transport_); + // Update the header extension map on network thread in case there is data + // race. + // TODO(zhihuang): Add an rtc::ThreadChecker make sure to RtpTransport won't + // be accessed from different threads. + // + // NOTE: This doesn't take the BUNDLE case in account meaning the RTP header + // extension maps are not merged when BUNDLE is enabled. This is fine because + // the ID for MID should be consistent among all the RTP transports. + network_thread_->Invoke(RTC_FROM_HERE, [this, &header_extensions] { + rtp_transport_->UpdateRtpHeaderExtensionMap(header_extensions); + }); +} + +bool BaseChannel::RegisterRtpDemuxerSink() { + RTC_DCHECK(rtp_transport_); + return network_thread_->Invoke(RTC_FROM_HERE, [this] { + return rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria_, this); + }); +} + +void BaseChannel::OnRtcpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time) { + OnPacketReceived(/*rtcp=*/true, *packet, packet_time); } void BaseChannel::OnPacketReceived(bool rtcp, - rtc::CopyOnWriteBuffer* packet, + const rtc::CopyOnWriteBuffer& packet, const rtc::PacketTime& packet_time) { if (!has_received_packet_ && !rtcp) { has_received_packet_ = true; signaling_thread()->Post(RTC_FROM_HERE, this, MSG_FIRSTPACKETRECEIVED); } - // Filter out the packet this channel cannot handle. - if (!rtcp && !bundle_filter_.DemuxPacket(packet->data(), packet->size())) { - return; - } - if (!srtp_active() && srtp_required_) { // Our session description indicates that SRTP is required, but we got a // packet before our SRTP filter is active. This means either that @@ -467,7 +517,7 @@ void BaseChannel::OnPacketReceived(bool rtcp, invoker_.AsyncInvoke( RTC_FROM_HERE, worker_thread_, - Bind(&BaseChannel::ProcessPacket, this, rtcp, *packet, packet_time)); + Bind(&BaseChannel::ProcessPacket, this, rtcp, packet, packet_time)); } void BaseChannel::ProcessPacket(bool rtcp, @@ -595,7 +645,9 @@ bool BaseChannel::UpdateRemoteStreams_w( // the unsignaled stream params that are cached. if ((!it->has_ssrcs() && !HasStreamWithNoSsrcs(streams)) || !GetStreamBySsrc(streams, it->first_ssrc())) { - if (!RemoveRecvStream_w(it->first_ssrc())) { + if (RemoveRecvStream_w(it->first_ssrc())) { + RTC_LOG(LS_INFO) << "Remove remote ssrc: " << it->first_ssrc(); + } else { std::ostringstream desc; desc << "Failed to remove remote stream with ssrc " << it->first_ssrc() << "."; @@ -604,6 +656,7 @@ bool BaseChannel::UpdateRemoteStreams_w( } } } + demuxer_criteria_.ssrcs.clear(); // Check for new streams. for (StreamParamsVec::const_iterator it = streams.begin(); it != streams.end(); ++it) { @@ -621,7 +674,11 @@ bool BaseChannel::UpdateRemoteStreams_w( ret = false; } } + // Update the receiving SSRCs. + demuxer_criteria_.ssrcs.insert(it->ssrcs.begin(), it->ssrcs.end()); } + // Re-register the sink to update the receiving ssrcs. + RegisterRtpDemuxerSink(); remote_streams_ = streams; return ret; } @@ -663,7 +720,7 @@ void BaseChannel::OnMessage(rtc::Message *pmsg) { } void BaseChannel::AddHandledPayloadType(int payload_type) { - bundle_filter_.AddPayloadType(payload_type); + demuxer_criteria_.payload_types.insert(static_cast(payload_type)); } void BaseChannel::FlushRtcpMessages_n() { @@ -752,6 +809,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, RtpHeaderExtensions rtp_header_extensions = GetFilteredRtpHeaderExtensions(audio->rtp_header_extensions()); + UpdateRtpHeaderExtensionMap(rtp_header_extensions); AudioRecvParameters recv_params = last_recv_params_; RtpParametersFromMediaDescription(audio, rtp_header_extensions, &recv_params); @@ -763,6 +821,12 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, for (const AudioCodec& codec : audio->codecs()) { AddHandledPayloadType(codec.id); } + // Need to re-register the sink to update the handled payload. + if (!RegisterRtpDemuxerSink()) { + RTC_LOG(LS_ERROR) << "Failed to set up audio demuxing."; + return false; + } + last_recv_params_ = recv_params; // TODO(pthatcher): Move local streams into AudioSendParameters, and @@ -880,6 +944,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, RtpHeaderExtensions rtp_header_extensions = GetFilteredRtpHeaderExtensions(video->rtp_header_extensions()); + UpdateRtpHeaderExtensionMap(rtp_header_extensions); VideoRecvParameters recv_params = last_recv_params_; RtpParametersFromMediaDescription(video, rtp_header_extensions, &recv_params); @@ -891,6 +956,12 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, for (const VideoCodec& codec : video->codecs()) { AddHandledPayloadType(codec.id); } + // Need to re-register the sink to update the handled payload. + if (!RegisterRtpDemuxerSink()) { + RTC_LOG(LS_ERROR) << "Failed to set up video demuxing."; + return false; + } + last_recv_params_ = recv_params; // TODO(pthatcher): Move local streams into VideoSendParameters, and @@ -1039,6 +1110,12 @@ bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content, for (const DataCodec& codec : data->codecs()) { AddHandledPayloadType(codec.id); } + // Need to re-register the sink to update the handled payload. + if (!RegisterRtpDemuxerSink()) { + RTC_LOG(LS_ERROR) << "Failed to set up data demuxing."; + return false; + } + last_recv_params_ = recv_params; // TODO(pthatcher): Move local streams into DataSendParameters, and diff --git a/pc/channel.h b/pc/channel.h index cf04b5c0b0..50356fae08 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -23,13 +23,13 @@ #include "api/rtpreceiverinterface.h" #include "api/videosinkinterface.h" #include "api/videosourceinterface.h" +#include "call/rtp_packet_sink_interface.h" #include "media/base/mediachannel.h" #include "media/base/mediaengine.h" #include "media/base/streamparams.h" #include "p2p/base/dtlstransportinternal.h" #include "p2p/base/packettransportinternal.h" #include "pc/audiomonitor.h" -#include "pc/bundlefilter.h" #include "pc/dtlssrtptransport.h" #include "pc/mediasession.h" #include "pc/rtptransport.h" @@ -68,9 +68,10 @@ class MediaContentDescription; // vtable, and the media channel's thread using BaseChannel as the // NetworkInterface. -class BaseChannel - : public rtc::MessageHandler, public sigslot::has_slots<>, - public MediaChannel::NetworkInterface { +class BaseChannel : public rtc::MessageHandler, + public sigslot::has_slots<>, + public MediaChannel::NetworkInterface, + 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). @@ -108,7 +109,7 @@ class BaseChannel // encryption, an SrtpTransport for SDES or a DtlsSrtpTransport for DTLS-SRTP. // This can be called from any thread and it hops to the network thread // internally. It would replace the |SetTransports| and its variants. - void SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport); + bool SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport); // Channel control bool SetLocalContent(const MediaContentDescription* content, @@ -120,7 +121,7 @@ class BaseChannel bool Enable(bool enable); - // Multiplexing + // TODO(zhihuang): These methods are used for testing and can be removed. bool AddRecvStream(const StreamParams& sp); bool RemoveRecvStream(uint32_t ssrc); bool AddSendStream(const StreamParams& sp); @@ -172,10 +173,8 @@ class BaseChannel virtual cricket::MediaType media_type() = 0; - // Public for testing. - // TODO(zstein): Remove this once channels register themselves with - // an RtpTransport in a more explicit way. - bool HandlesPayloadType(int payload_type) const; + // RtpPacketSinkInterface overrides. + void OnRtpPacket(const webrtc::RtpPacketReceived& packet) override; // Used by the RTCStatsCollector tests to set the transport name without // creating RtpTransports. @@ -186,8 +185,6 @@ class BaseChannel void SetMetricsObserver( rtc::scoped_refptr metrics_observer); - void DisableEncryption(bool disabled) { encryption_disabled_ = disabled; } - protected: virtual MediaChannel* media_channel() const { return media_channel_.get(); } @@ -232,12 +229,11 @@ class BaseChannel rtc::CopyOnWriteBuffer* packet, const rtc::PacketOptions& options); - bool WantsPacket(bool rtcp, const rtc::CopyOnWriteBuffer* packet); - void HandlePacket(bool rtcp, rtc::CopyOnWriteBuffer* packet, - const rtc::PacketTime& packet_time); - // TODO(zstein): packet can be const once the RtpTransport handles protection. + void OnRtcpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time); + void OnPacketReceived(bool rtcp, - rtc::CopyOnWriteBuffer* packet, + const rtc::CopyOnWriteBuffer& packet, const rtc::PacketTime& packet_time); void ProcessPacket(bool rtcp, const rtc::CopyOnWriteBuffer& packet, @@ -282,11 +278,6 @@ class BaseChannel RtpHeaderExtensions GetFilteredRtpHeaderExtensions( const RtpHeaderExtensions& extensions); - // Helper method to get RTP Absoulute SendTime extension header id if - // present in remote supported extensions list. - void MaybeCacheRtpAbsSendTimeHeaderExtension_w( - const std::vector& extensions); - // From MessageHandler void OnMessage(rtc::Message* pmsg) override; @@ -298,8 +289,13 @@ class BaseChannel void AddHandledPayloadType(int payload_type); + void UpdateRtpHeaderExtensionMap( + const RtpHeaderExtensions& header_extensions); + + bool RegisterRtpDemuxerSink(); + private: - void ConnectToRtpTransport(); + bool ConnectToRtpTransport(); void DisconnectFromRtpTransport(); void SignalSentPacket_n(const rtc::SentPacket& sent_packet); void SignalSentPacket_w(const rtc::SentPacket& sent_packet); @@ -317,11 +313,6 @@ class BaseChannel rtc::scoped_refptr metrics_observer_; webrtc::RtpTransportInternal* rtp_transport_ = nullptr; - // Only one of these transports is non-null at a time. One for DTLS-SRTP, one - // for SDES and one for unencrypted RTP. - std::unique_ptr sdes_transport_; - std::unique_ptr dtls_srtp_transport_; - std::unique_ptr unencrypted_rtp_transport_; std::vector > socket_options_; std::vector > rtcp_socket_options_; @@ -345,14 +336,7 @@ class BaseChannel webrtc::RtpTransceiverDirection remote_content_direction_ = webrtc::RtpTransceiverDirection::kInactive; - // The cached encrypted header extension IDs. - rtc::Optional> cached_send_extension_ids_; - rtc::Optional> cached_recv_extension_ids_; - - // TODO(zhihuang): These two variables can be removed once switching to - // RtpDemuxer. - BundleFilter bundle_filter_; - bool encryption_disabled_ = false; + webrtc::RtpDemuxerCriteria demuxer_criteria_; }; // VoiceChannel is a specialization that adds support for early media, DTMF, diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index c398f07263..0d8fdc3ed4 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -42,6 +42,8 @@ const cricket::VideoCodec kH264SvcCodec(99, "H264-SVC"); const cricket::DataCodec kGoogleDataCodec(101, "google-data"); const uint32_t kSsrc1 = 0x1111; const uint32_t kSsrc2 = 0x2222; +const uint32_t kSsrc3 = 0x3333; +const uint32_t kSsrc4 = 0x4444; const int kAudioPts[] = {0, 8}; const int kVideoPts[] = {97, 99}; enum class NetworkIsWorker { Yes, No }; @@ -239,13 +241,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { if (flags2 & SSRC_MUX) { AddLegacyStreamInContent(kSsrc2, flags2, &remote_media_content2_); } - - if (!(flags1 & DTLS)) { - channel1_->DisableEncryption(true); - } - if (!(flags2 & DTLS)) { - channel2_->DisableEncryption(true); - } } std::unique_ptr CreateChannel( rtc::Thread* worker_thread, @@ -289,9 +284,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr CreateUnencryptedTransport( rtc::PacketTransportInternal* rtp_packet_transport, rtc::PacketTransportInternal* rtcp_packet_transport) { - bool rtcp_mux_enabled = (rtcp_packet_transport == nullptr); auto rtp_transport = - rtc::MakeUnique(rtcp_mux_enabled); + rtc::MakeUnique(rtcp_packet_transport == nullptr); rtp_transport->SetRtpPacketTransport(rtp_packet_transport); if (rtcp_packet_transport) { @@ -303,11 +297,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr CreateDtlsSrtpTransport( cricket::DtlsTransportInternal* rtp_dtls_transport, cricket::DtlsTransportInternal* rtcp_dtls_transport) { - bool rtcp_mux_enabled = (rtcp_dtls_transport == nullptr); - auto srtp_transport = - rtc::MakeUnique(rtcp_mux_enabled); - auto dtls_srtp_transport = - rtc::MakeUnique(std::move(srtp_transport)); + auto dtls_srtp_transport = rtc::MakeUnique( + rtcp_dtls_transport == nullptr); dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport, rtcp_dtls_transport); @@ -1177,10 +1168,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateChannels(flags, flags); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - EXPECT_TRUE(channel1_->HandlesPayloadType(pl_type1)); - EXPECT_TRUE(channel2_->HandlesPayloadType(pl_type1)); - EXPECT_FALSE(channel1_->HandlesPayloadType(pl_type2)); - EXPECT_FALSE(channel2_->HandlesPayloadType(pl_type2)); // Both channels can receive pl_type1 only. SendCustomRtp1(kSsrc1, ++sequence_number1_1, pl_type1); @@ -1191,11 +1178,11 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(CheckNoRtp1()); EXPECT_TRUE(CheckNoRtp2()); - SendCustomRtp1(kSsrc1, ++sequence_number1_1, pl_type2); - SendCustomRtp2(kSsrc2, ++sequence_number2_2, pl_type2); + SendCustomRtp1(kSsrc3, ++sequence_number1_1, pl_type2); + SendCustomRtp2(kSsrc4, ++sequence_number2_2, pl_type2); WaitForThreads(); - EXPECT_FALSE(CheckCustomRtp2(kSsrc1, sequence_number1_1, pl_type2)); - EXPECT_FALSE(CheckCustomRtp1(kSsrc2, sequence_number2_2, pl_type2)); + EXPECT_FALSE(CheckCustomRtp2(kSsrc3, sequence_number1_1, pl_type2)); + EXPECT_FALSE(CheckCustomRtp1(kSsrc4, sequence_number2_2, pl_type2)); // RTCP test SendCustomRtcp1(kSsrc1); diff --git a/pc/channelmanager_unittest.cc b/pc/channelmanager_unittest.cc index 38da139448..b65b18bbf2 100644 --- a/pc/channelmanager_unittest.cc +++ b/pc/channelmanager_unittest.cc @@ -55,12 +55,8 @@ class ChannelManagerTest : public testing::Test { std::unique_ptr CreateDtlsSrtpTransport() { rtp_dtls_transport_ = rtc::MakeUnique( "fake_dtls_transport", cricket::ICE_CANDIDATE_COMPONENT_RTP); - auto rtp_transport = - rtc::MakeUnique(/*rtcp_mux_required=*/true); - auto srtp_transport = - rtc::MakeUnique(std::move(rtp_transport)); auto dtls_srtp_transport = - rtc::MakeUnique(std::move(srtp_transport)); + rtc::MakeUnique(/*rtcp_mux_required=*/true); dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport_.get(), /*rtcp_dtls_transport=*/nullptr); return dtls_srtp_transport; diff --git a/pc/dtlssrtptransport.cc b/pc/dtlssrtptransport.cc index f40c96f925..4ccec73e6c 100644 --- a/pc/dtlssrtptransport.cc +++ b/pc/dtlssrtptransport.cc @@ -24,22 +24,8 @@ static const char kDtlsSrtpExporterLabel[] = "EXTRACTOR-dtls_srtp"; namespace webrtc { -DtlsSrtpTransport::DtlsSrtpTransport( - std::unique_ptr srtp_transport) - : RtpTransportInternalAdapter(srtp_transport.get()) { - srtp_transport_ = std::move(srtp_transport); - RTC_DCHECK(srtp_transport_); - srtp_transport_->SignalPacketReceived.connect( - this, &DtlsSrtpTransport::OnPacketReceived); - srtp_transport_->SignalReadyToSend.connect(this, - &DtlsSrtpTransport::OnReadyToSend); - srtp_transport_->SignalNetworkRouteChanged.connect( - this, &DtlsSrtpTransport::OnNetworkRouteChanged); - srtp_transport_->SignalWritableState.connect( - this, &DtlsSrtpTransport::OnWritableState); - srtp_transport_->SignalSentPacket.connect(this, - &DtlsSrtpTransport::OnSentPacket); -} +DtlsSrtpTransport::DtlsSrtpTransport(bool rtcp_mux_enabled) + : SrtpTransport(rtcp_mux_enabled) {} void DtlsSrtpTransport::SetDtlsTransports( cricket::DtlsTransportInternal* rtp_dtls_transport, @@ -54,7 +40,7 @@ void DtlsSrtpTransport::SetDtlsTransports( // DtlsTransport changes and wait until the DTLS handshake is complete to set // the newly negotiated parameters. if (IsSrtpActive()) { - srtp_transport_->ResetParams(); + ResetParams(); } const std::string transport_name = @@ -76,13 +62,13 @@ void DtlsSrtpTransport::SetDtlsTransports( SetRtpDtlsTransport(rtp_dtls_transport); SetRtpPacketTransport(rtp_dtls_transport); - UpdateWritableStateAndMaybeSetupDtlsSrtp(); + MaybeSetupDtlsSrtp(); } void DtlsSrtpTransport::SetRtcpMuxEnabled(bool enable) { - srtp_transport_->SetRtcpMuxEnabled(enable); + SrtpTransport::SetRtcpMuxEnabled(enable); if (enable) { - UpdateWritableStateAndMaybeSetupDtlsSrtp(); + MaybeSetupDtlsSrtp(); } } @@ -128,10 +114,9 @@ bool DtlsSrtpTransport::IsDtlsConnected() { } bool DtlsSrtpTransport::IsDtlsWritable() { - auto rtp_packet_transport = srtp_transport_->rtp_packet_transport(); auto rtcp_packet_transport = - rtcp_mux_enabled() ? nullptr : srtp_transport_->rtcp_packet_transport(); - return rtp_packet_transport && rtp_packet_transport->writable() && + rtcp_mux_enabled() ? nullptr : rtcp_dtls_transport_; + return rtp_dtls_transport_ && rtp_dtls_transport_->writable() && (!rtcp_packet_transport || rtcp_packet_transport->writable()); } @@ -140,7 +125,7 @@ bool DtlsSrtpTransport::DtlsHandshakeCompleted() { } void DtlsSrtpTransport::MaybeSetupDtlsSrtp() { - if (IsSrtpActive() || !DtlsHandshakeCompleted()) { + if (IsSrtpActive() || !IsDtlsWritable()) { return; } @@ -170,11 +155,10 @@ void DtlsSrtpTransport::SetupRtpDtlsSrtp() { if (!ExtractParams(rtp_dtls_transport_, &selected_crypto_suite, &send_key, &recv_key) || - !srtp_transport_->SetRtpParams( - selected_crypto_suite, &send_key[0], - static_cast(send_key.size()), send_extension_ids, - selected_crypto_suite, &recv_key[0], - static_cast(recv_key.size()), recv_extension_ids)) { + !SetRtpParams(selected_crypto_suite, &send_key[0], + static_cast(send_key.size()), send_extension_ids, + selected_crypto_suite, &recv_key[0], + static_cast(recv_key.size()), recv_extension_ids)) { SignalDtlsSrtpSetupFailure(this, /*rtcp=*/false); RTC_LOG(LS_WARNING) << "DTLS-SRTP key installation for RTP failed"; } @@ -202,11 +186,11 @@ void DtlsSrtpTransport::SetupRtcpDtlsSrtp() { rtc::ZeroOnFreeBuffer rtcp_recv_key; if (!ExtractParams(rtcp_dtls_transport_, &selected_crypto_suite, &rtcp_send_key, &rtcp_recv_key) || - !srtp_transport_->SetRtcpParams( - selected_crypto_suite, &rtcp_send_key[0], - static_cast(rtcp_send_key.size()), send_extension_ids, - selected_crypto_suite, &rtcp_recv_key[0], - static_cast(rtcp_recv_key.size()), recv_extension_ids)) { + !SetRtcpParams(selected_crypto_suite, &rtcp_send_key[0], + static_cast(rtcp_send_key.size()), send_extension_ids, + selected_crypto_suite, &rtcp_recv_key[0], + static_cast(rtcp_recv_key.size()), + recv_extension_ids)) { SignalDtlsSrtpSetupFailure(this, /*rtcp=*/true); RTC_LOG(LS_WARNING) << "DTLS-SRTP key installation for RTCP failed"; } @@ -307,59 +291,22 @@ void DtlsSrtpTransport::SetRtcpDtlsTransport( SetDtlsTransport(rtcp_dtls_transport, &rtcp_dtls_transport_); } -void DtlsSrtpTransport::UpdateWritableStateAndMaybeSetupDtlsSrtp() { - bool writable = IsDtlsWritable(); - SetWritable(writable); - if (writable) { - MaybeSetupDtlsSrtp(); - } -} - -void DtlsSrtpTransport::SetWritable(bool writable) { - // Only fire the signal if the writable state changes. - if (writable_ != writable) { - writable_ = writable; - SignalWritableState(writable_); - } -} - void DtlsSrtpTransport::OnDtlsState(cricket::DtlsTransportInternal* transport, cricket::DtlsTransportState state) { RTC_DCHECK(transport == rtp_dtls_transport_ || transport == rtcp_dtls_transport_); if (state != cricket::DTLS_TRANSPORT_CONNECTED) { - srtp_transport_->ResetParams(); + ResetParams(); return; } MaybeSetupDtlsSrtp(); } -void DtlsSrtpTransport::OnWritableState(bool writable) { - SetWritable(writable); - if (writable) { - MaybeSetupDtlsSrtp(); - } -} - -void DtlsSrtpTransport::OnSentPacket(const rtc::SentPacket& sent_packet) { - SignalSentPacket(sent_packet); -} - -void DtlsSrtpTransport::OnPacketReceived(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketTime& packet_time) { - SignalPacketReceived(rtcp, packet, packet_time); -} - -void DtlsSrtpTransport::OnReadyToSend(bool ready) { - SignalReadyToSend(ready); -} - -void DtlsSrtpTransport::OnNetworkRouteChanged( - rtc::Optional network_route) { - SignalNetworkRouteChanged(network_route); +void DtlsSrtpTransport::OnWritableState( + rtc::PacketTransportInternal* packet_transport) { + MaybeSetupDtlsSrtp(); } } // namespace webrtc diff --git a/pc/dtlssrtptransport.h b/pc/dtlssrtptransport.h index 889bd22234..c24034a146 100644 --- a/pc/dtlssrtptransport.h +++ b/pc/dtlssrtptransport.h @@ -16,20 +16,17 @@ #include #include "p2p/base/dtlstransportinternal.h" -#include "pc/rtptransportinternaladapter.h" #include "pc/srtptransport.h" #include "rtc_base/buffer.h" namespace webrtc { -// This class is intended to be used as an RtpTransport and it wraps both an -// SrtpTransport and DtlsTransports(RTP/RTCP). When the DTLS handshake is -// finished, it extracts the keying materials from DtlsTransport and sets them -// to SrtpTransport. -class DtlsSrtpTransport : public RtpTransportInternalAdapter { +// The subclass of SrtpTransport is used for DTLS-SRTP. When the DTLS handshake +// is finished, it extracts the keying materials from DtlsTransport and +// configures the SrtpSessions in the base class. +class DtlsSrtpTransport : public SrtpTransport { public: - explicit DtlsSrtpTransport( - std::unique_ptr srtp_transport); + explicit DtlsSrtpTransport(bool rtcp_mux_enabled); // Set P2P layer RTP/RTCP DtlsTransports. When using RTCP-muxing, // |rtcp_dtls_transport| is null. @@ -45,15 +42,6 @@ class DtlsSrtpTransport : public RtpTransportInternalAdapter { void UpdateRecvEncryptedHeaderExtensionIds( const std::vector& recv_extension_ids); - bool IsSrtpActive() const override { return srtp_transport_->IsSrtpActive(); } - - // Cache RTP Absoulute SendTime extension header ID. This is only used when - // external authentication is enabled. - void CacheRtpAbsSendTimeHeaderExtension(int rtp_abs_sendtime_extn_id) { - srtp_transport_->CacheRtpAbsSendTimeHeaderExtension( - rtp_abs_sendtime_extn_id); - } - sigslot::signal2 SignalDtlsSrtpSetupFailure; RTCError SetSrtpSendKey(const cricket::CryptoParams& params) override { @@ -82,23 +70,13 @@ class DtlsSrtpTransport : public RtpTransportInternalAdapter { void SetRtpDtlsTransport(cricket::DtlsTransportInternal* rtp_dtls_transport); void SetRtcpDtlsTransport( cricket::DtlsTransportInternal* rtcp_dtls_transport); - void UpdateWritableStateAndMaybeSetupDtlsSrtp(); - // Set the writability and fire the SignalWritableState if the writability - // changes. - void SetWritable(bool writable); void OnDtlsState(cricket::DtlsTransportInternal* dtls_transport, cricket::DtlsTransportState state); - void OnWritableState(bool writable); - void OnSentPacket(const rtc::SentPacket& sent_packet); - void OnPacketReceived(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketTime& packet_time); - void OnReadyToSend(bool ready); - void OnNetworkRouteChanged(rtc::Optional network_route); - bool writable_ = false; - std::unique_ptr srtp_transport_; + // Override the SrtpTransport::OnWritableState. + void OnWritableState(rtc::PacketTransportInternal* packet_transport) override; + // Owned by the TransportController. cricket::DtlsTransportInternal* rtp_dtls_transport_ = nullptr; cricket::DtlsTransportInternal* rtcp_dtls_transport_ = nullptr; diff --git a/pc/dtlssrtptransport_unittest.cc b/pc/dtlssrtptransport_unittest.cc index f0e4f28b04..37d517b236 100644 --- a/pc/dtlssrtptransport_unittest.cc +++ b/pc/dtlssrtptransport_unittest.cc @@ -33,47 +33,26 @@ using webrtc::RtpTransport; const int kRtpAuthTagLen = 10; -class TransportObserver : public sigslot::has_slots<> { - public: - void OnPacketReceived(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketTime& packet_time) { - rtcp ? last_recv_rtcp_packet_ = *packet : last_recv_rtp_packet_ = *packet; - } - - void OnReadyToSend(bool ready) { ready_to_send_ = ready; } - - rtc::CopyOnWriteBuffer last_recv_rtp_packet() { - return last_recv_rtp_packet_; - } - - rtc::CopyOnWriteBuffer last_recv_rtcp_packet() { - return last_recv_rtcp_packet_; - } - - bool ready_to_send() { return ready_to_send_; } - - private: - rtc::CopyOnWriteBuffer last_recv_rtp_packet_; - rtc::CopyOnWriteBuffer last_recv_rtcp_packet_; - bool ready_to_send_ = false; -}; - class DtlsSrtpTransportTest : public testing::Test, public sigslot::has_slots<> { protected: DtlsSrtpTransportTest() {} + ~DtlsSrtpTransportTest() { + if (dtls_srtp_transport1_) { + dtls_srtp_transport1_->UnregisterRtpDemuxerSink(&transport_observer1_); + } + if (dtls_srtp_transport2_) { + dtls_srtp_transport2_->UnregisterRtpDemuxerSink(&transport_observer2_); + } + } + std::unique_ptr MakeDtlsSrtpTransport( FakeDtlsTransport* rtp_dtls, FakeDtlsTransport* rtcp_dtls, bool rtcp_mux_enabled) { - auto rtp_transport = rtc::MakeUnique(rtcp_mux_enabled); - - auto srtp_transport = - rtc::MakeUnique(std::move(rtp_transport)); auto dtls_srtp_transport = - rtc::MakeUnique(std::move(srtp_transport)); + rtc::MakeUnique(rtcp_mux_enabled); dtls_srtp_transport->SetDtlsTransports(rtp_dtls, rtcp_dtls); @@ -90,15 +69,24 @@ class DtlsSrtpTransportTest : public testing::Test, dtls_srtp_transport2_ = MakeDtlsSrtpTransport(rtp_dtls2, rtcp_dtls2, rtcp_mux_enabled); - dtls_srtp_transport1_->SignalPacketReceived.connect( - &transport_observer1_, &TransportObserver::OnPacketReceived); + dtls_srtp_transport1_->SignalRtcpPacketReceived.connect( + &transport_observer1_, + &webrtc::TransportObserver::OnRtcpPacketReceived); dtls_srtp_transport1_->SignalReadyToSend.connect( - &transport_observer1_, &TransportObserver::OnReadyToSend); + &transport_observer1_, &webrtc::TransportObserver::OnReadyToSend); - dtls_srtp_transport2_->SignalPacketReceived.connect( - &transport_observer2_, &TransportObserver::OnPacketReceived); + dtls_srtp_transport2_->SignalRtcpPacketReceived.connect( + &transport_observer2_, + &webrtc::TransportObserver::OnRtcpPacketReceived); dtls_srtp_transport2_->SignalReadyToSend.connect( - &transport_observer2_, &TransportObserver::OnReadyToSend); + &transport_observer2_, &webrtc::TransportObserver::OnReadyToSend); + webrtc::RtpDemuxerCriteria demuxer_criteria; + // 0x00 is the payload type used in kPcmuFrame. + demuxer_criteria.payload_types = {0x00}; + dtls_srtp_transport1_->RegisterRtpDemuxerSink(demuxer_criteria, + &transport_observer1_); + dtls_srtp_transport2_->RegisterRtpDemuxerSink(demuxer_criteria, + &transport_observer2_); } void CompleteDtlsHandshake(FakeDtlsTransport* fake_dtls1, @@ -248,8 +236,8 @@ class DtlsSrtpTransportTest : public testing::Test, std::unique_ptr dtls_srtp_transport1_; std::unique_ptr dtls_srtp_transport2_; - TransportObserver transport_observer1_; - TransportObserver transport_observer2_; + webrtc::TransportObserver transport_observer1_; + webrtc::TransportObserver transport_observer2_; int sequence_number_ = 0; }; diff --git a/pc/jseptransport2.cc b/pc/jseptransport.cc similarity index 93% rename from pc/jseptransport2.cc rename to pc/jseptransport.cc index a4a03487bd..ab785affb1 100644 --- a/pc/jseptransport2.cc +++ b/pc/jseptransport.cc @@ -8,7 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "pc/jseptransport2.h" +#include "pc/jseptransport.h" #include #include // for std::pair @@ -21,6 +21,7 @@ #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/ptr_util.h" +#include "rtc_base/strings/string_builder.h" using webrtc::SdpType; @@ -86,7 +87,7 @@ JsepTransportDescription& JsepTransportDescription::operator=( return *this; } -JsepTransport2::JsepTransport2( +JsepTransport::JsepTransport( const std::string& mid, const rtc::scoped_refptr& local_certificate, std::unique_ptr unencrypted_rtp_transport, @@ -115,9 +116,9 @@ JsepTransport2::JsepTransport2( } } -JsepTransport2::~JsepTransport2() {} +JsepTransport::~JsepTransport() {} -webrtc::RTCError JsepTransport2::SetLocalJsepTransportDescription( +webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription( const JsepTransportDescription& jsep_description, SdpType type) { webrtc::RTCError error; @@ -195,7 +196,7 @@ webrtc::RTCError JsepTransport2::SetLocalJsepTransportDescription( return webrtc::RTCError::OK(); } -webrtc::RTCError JsepTransport2::SetRemoteJsepTransportDescription( +webrtc::RTCError JsepTransport::SetRemoteJsepTransportDescription( const JsepTransportDescription& jsep_description, webrtc::SdpType type) { webrtc::RTCError error; @@ -251,7 +252,7 @@ webrtc::RTCError JsepTransport2::SetRemoteJsepTransportDescription( return webrtc::RTCError::OK(); } -webrtc::RTCError JsepTransport2::AddRemoteCandidates( +webrtc::RTCError JsepTransport::AddRemoteCandidates( const Candidates& candidates) { if (!local_description_ || !remote_description_) { return webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE, @@ -276,14 +277,14 @@ webrtc::RTCError JsepTransport2::AddRemoteCandidates( return webrtc::RTCError::OK(); } -void JsepTransport2::SetNeedsIceRestartFlag() { +void JsepTransport::SetNeedsIceRestartFlag() { if (!needs_ice_restart_) { needs_ice_restart_ = true; RTC_LOG(LS_VERBOSE) << "needs-ice-restart flag set for transport " << mid(); } } -rtc::Optional JsepTransport2::GetDtlsRole() const { +rtc::Optional JsepTransport::GetDtlsRole() const { RTC_DCHECK(rtp_dtls_transport_); rtc::SSLRole dtls_role; if (!rtp_dtls_transport_->GetDtlsRole(&dtls_role)) { @@ -293,7 +294,7 @@ rtc::Optional JsepTransport2::GetDtlsRole() const { return rtc::Optional(dtls_role); } -bool JsepTransport2::GetStats(TransportStats* stats) { +bool JsepTransport::GetStats(TransportStats* stats) { stats->transport_name = mid(); stats->channel_stats.clear(); bool ret = GetTransportStats(rtp_dtls_transport_.get(), stats); @@ -303,7 +304,7 @@ bool JsepTransport2::GetStats(TransportStats* stats) { return ret; } -webrtc::RTCError JsepTransport2::VerifyCertificateFingerprint( +webrtc::RTCError JsepTransport::VerifyCertificateFingerprint( const rtc::RTCCertificate* certificate, const rtc::SSLFingerprint* fingerprint) const { if (!fingerprint) { @@ -320,22 +321,23 @@ webrtc::RTCError JsepTransport2::VerifyCertificateFingerprint( if (*fp_tmp == *fingerprint) { return webrtc::RTCError::OK(); } - std::ostringstream desc; + char ss_buf[1024]; + rtc::SimpleStringBuilder desc(ss_buf); desc << "Local fingerprint does not match identity. Expected: "; desc << fp_tmp->ToString(); desc << " Got: " << fingerprint->ToString(); - return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, desc.str()); + return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, + std::string(desc.str())); } -void JsepTransport2::SetLocalIceParameters( - IceTransportInternal* ice_transport) { +void JsepTransport::SetLocalIceParameters(IceTransportInternal* ice_transport) { RTC_DCHECK(ice_transport); RTC_DCHECK(local_description_); ice_transport->SetIceParameters( local_description_->transport_desc.GetIceParameters()); } -void JsepTransport2::SetRemoteIceParameters( +void JsepTransport::SetRemoteIceParameters( IceTransportInternal* ice_transport) { RTC_DCHECK(ice_transport); RTC_DCHECK(remote_description_); @@ -344,7 +346,7 @@ void JsepTransport2::SetRemoteIceParameters( ice_transport->SetRemoteIceMode(remote_description_->transport_desc.ice_mode); } -webrtc::RTCError JsepTransport2::SetNegotiatedDtlsParameters( +webrtc::RTCError JsepTransport::SetNegotiatedDtlsParameters( DtlsTransportInternal* dtls_transport, rtc::Optional dtls_role, rtc::SSLFingerprint* remote_fingerprint) { @@ -367,9 +369,9 @@ webrtc::RTCError JsepTransport2::SetNegotiatedDtlsParameters( return webrtc::RTCError::OK(); } -bool JsepTransport2::SetRtcpMux(bool enable, - webrtc::SdpType type, - ContentSource source) { +bool JsepTransport::SetRtcpMux(bool enable, + webrtc::SdpType type, + ContentSource source) { bool ret = false; switch (type) { case SdpType::kOffer: @@ -399,7 +401,7 @@ bool JsepTransport2::SetRtcpMux(bool enable, return ret; } -void JsepTransport2::ActivateRtcpMux() { +void JsepTransport::ActivateRtcpMux() { if (unencrypted_rtp_transport_) { RTC_DCHECK(!sdes_transport_); RTC_DCHECK(!dtls_srtp_transport_); @@ -420,10 +422,10 @@ void JsepTransport2::ActivateRtcpMux() { SignalRtcpMuxActive(); } -bool JsepTransport2::SetSdes(const std::vector& cryptos, - const std::vector& encrypted_extension_ids, - webrtc::SdpType type, - ContentSource source) { +bool JsepTransport::SetSdes(const std::vector& cryptos, + const std::vector& encrypted_extension_ids, + webrtc::SdpType type, + ContentSource source) { bool ret = false; ret = sdes_negotiator_.Process(cryptos, type, source); if (!ret) { @@ -464,7 +466,7 @@ bool JsepTransport2::SetSdes(const std::vector& cryptos, return ret; } -webrtc::RTCError JsepTransport2::NegotiateAndSetDtlsParameters( +webrtc::RTCError JsepTransport::NegotiateAndSetDtlsParameters( SdpType local_description_type) { if (!local_description_ || !remote_description_) { return webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE, @@ -516,7 +518,7 @@ webrtc::RTCError JsepTransport2::NegotiateAndSetDtlsParameters( return error; } -webrtc::RTCError JsepTransport2::NegotiateDtlsRole( +webrtc::RTCError JsepTransport::NegotiateDtlsRole( SdpType local_description_type, ConnectionRole local_connection_role, ConnectionRole remote_connection_role, @@ -604,8 +606,8 @@ webrtc::RTCError JsepTransport2::NegotiateDtlsRole( return webrtc::RTCError::OK(); } -bool JsepTransport2::GetTransportStats(DtlsTransportInternal* dtls_transport, - TransportStats* stats) { +bool JsepTransport::GetTransportStats(DtlsTransportInternal* dtls_transport, + TransportStats* stats) { RTC_DCHECK(dtls_transport); TransportChannelStats substats; substats.component = dtls_transport == rtcp_dtls_transport_.get() diff --git a/pc/jseptransport2.h b/pc/jseptransport.h similarity index 96% rename from pc/jseptransport2.h rename to pc/jseptransport.h index 8656cf5747..290ac4a2db 100644 --- a/pc/jseptransport2.h +++ b/pc/jseptransport.h @@ -8,8 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef PC_JSEPTRANSPORT2_H_ -#define PC_JSEPTRANSPORT2_H_ +#ifndef PC_JSEPTRANSPORT_H_ +#define PC_JSEPTRANSPORT_H_ #include #include @@ -68,14 +68,14 @@ struct JsepTransportDescription { // JSEP. Each transport consists of DTLS and ICE transport channels for RTP // (and possibly RTCP, if rtcp-mux isn't used). // -// On Threading: JsepTransport performs work solely on the network thread, and +// On Threading: JsepTransport performs work solely on the network thread, and // so its methods should only be called on the network thread. -class JsepTransport2 : public sigslot::has_slots<> { +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. - JsepTransport2( + JsepTransport( const std::string& mid, const rtc::scoped_refptr& local_certificate, std::unique_ptr unencrypted_rtp_transport, @@ -84,7 +84,7 @@ class JsepTransport2 : public sigslot::has_slots<> { std::unique_ptr rtp_dtls_transport, std::unique_ptr rtcp_dtls_transport); - ~JsepTransport2() override; + ~JsepTransport() override; // Returns the MID of this transport. This is only used for logging. const std::string& mid() const { return mid_; } @@ -239,9 +239,9 @@ class JsepTransport2 : public sigslot::has_slots<> { rtc::Optional> send_extension_ids_; rtc::Optional> recv_extension_ids_; - RTC_DISALLOW_COPY_AND_ASSIGN(JsepTransport2); + RTC_DISALLOW_COPY_AND_ASSIGN(JsepTransport); }; } // namespace cricket -#endif // PC_JSEPTRANSPORT2_H_ +#endif // PC_JSEPTRANSPORT_H_ diff --git a/pc/jseptransport2_unittest.cc b/pc/jseptransport_unittest.cc similarity index 98% rename from pc/jseptransport2_unittest.cc rename to pc/jseptransport_unittest.cc index 2f015cdc69..f6f3e1eb2e 100644 --- a/pc/jseptransport2_unittest.cc +++ b/pc/jseptransport_unittest.cc @@ -15,7 +15,7 @@ #include "media/base/fakertp.h" #include "p2p/base/fakedtlstransport.h" #include "p2p/base/fakeicetransport.h" -#include "pc/jseptransport2.h" +#include "pc/jseptransport.h" #include "rtc_base/gunit.h" namespace cricket { @@ -44,9 +44,8 @@ class JsepTransport2Test : public testing::Test, public sigslot::has_slots<> { std::unique_ptr CreateSdesTransport( rtc::PacketTransportInternal* rtp_packet_transport, rtc::PacketTransportInternal* rtcp_packet_transport) { - bool rtcp_mux_enabled = (rtcp_packet_transport == nullptr); - auto srtp_transport = - rtc::MakeUnique(rtcp_mux_enabled); + auto srtp_transport = rtc::MakeUnique( + rtcp_packet_transport == nullptr); srtp_transport->SetRtpPacketTransport(rtp_packet_transport); if (rtcp_packet_transport) { @@ -58,21 +57,17 @@ class JsepTransport2Test : public testing::Test, public sigslot::has_slots<> { std::unique_ptr CreateDtlsSrtpTransport( cricket::DtlsTransportInternal* rtp_dtls_transport, cricket::DtlsTransportInternal* rtcp_dtls_transport) { - bool rtcp_mux_enabled = (rtcp_dtls_transport == nullptr); - auto srtp_transport = - rtc::MakeUnique(rtcp_mux_enabled); - auto dtls_srtp_transport = - rtc::MakeUnique(std::move(srtp_transport)); - + auto dtls_srtp_transport = rtc::MakeUnique( + rtcp_dtls_transport == nullptr); dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport, rtcp_dtls_transport); return dtls_srtp_transport; } - // Create a new JsepTransport2 with a FakeDtlsTransport and a + // Create a new JsepTransport with a FakeDtlsTransport and a // FakeIceTransport. - std::unique_ptr CreateJsepTransport2(bool rtcp_mux_enabled, - SrtpMode srtp_mode) { + std::unique_ptr CreateJsepTransport2(bool rtcp_mux_enabled, + SrtpMode srtp_mode) { auto ice = rtc::MakeUnique(kTransportName, ICE_CANDIDATE_COMPONENT_RTP); auto rtp_dtls_transport = @@ -102,7 +97,7 @@ class JsepTransport2Test : public testing::Test, public sigslot::has_slots<> { RTC_NOTREACHED(); } - auto jsep_transport = rtc::MakeUnique( + auto jsep_transport = rtc::MakeUnique( kTransportName, /*local_certificate=*/nullptr, std::move(unencrypted_rtp_transport), std::move(sdes_transport), std::move(dtls_srtp_transport), std::move(rtp_dtls_transport), @@ -144,7 +139,7 @@ class JsepTransport2Test : public testing::Test, public sigslot::has_slots<> { void OnRtcpMuxActive() { signal_rtcp_mux_active_received_ = true; } - std::unique_ptr jsep_transport_; + std::unique_ptr jsep_transport_; bool signal_rtcp_mux_active_received_ = false; // The SrtpTransport is owned by |jsep_transport_|. Keep a raw pointer here // for testing. @@ -1096,7 +1091,7 @@ class JsepTransport2HeaderExtensionTest } void TestOneWaySendRecvPacketWithEncryptedHeaderExtension( - JsepTransport2* sender_transport) { + JsepTransport* sender_transport) { size_t rtp_len = sizeof(kPcmuFrameWithExtensions); size_t packet_size = rtp_len + GetRtpAuthLen(); rtc::Buffer rtp_packet_buffer(packet_size); @@ -1119,8 +1114,8 @@ class JsepTransport2HeaderExtensionTest int sequence_number_ = 0; int received_packet_count_ = 0; - std::unique_ptr jsep_transport1_; - std::unique_ptr jsep_transport2_; + std::unique_ptr jsep_transport1_; + std::unique_ptr jsep_transport2_; std::vector recv_encrypted_headers1_; std::vector recv_encrypted_headers2_; }; diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc index b5ce5fe0a7..b8234fa413 100644 --- a/pc/jseptransportcontroller.cc +++ b/pc/jseptransportcontroller.cc @@ -94,7 +94,10 @@ JsepTransportController::JsepTransportController( : signaling_thread_(signaling_thread), network_thread_(network_thread), port_allocator_(port_allocator), - config_(config) {} + config_(config) { + // The |transport_observer| is assumed to be non-null. + RTC_DCHECK(config_.transport_observer); +} JsepTransportController::~JsepTransportController() { // Channel destructors may try to send packets, so this needs to happen on @@ -181,7 +184,7 @@ void JsepTransportController::SetNeedsIceRestartFlag() { bool JsepTransportController::NeedsIceRestart( const std::string& transport_name) const { - const cricket::JsepTransport2* transport = + const cricket::JsepTransport* transport = GetJsepTransportByName(transport_name); if (!transport) { return false; @@ -196,7 +199,7 @@ rtc::Optional JsepTransportController::GetDtlsRole( RTC_FROM_HERE, [&] { return GetDtlsRole(mid); }); } - const cricket::JsepTransport2* t = GetJsepTransportForMid(mid); + const cricket::JsepTransport* t = GetJsepTransportForMid(mid); if (!t) { return rtc::Optional(); } @@ -237,7 +240,7 @@ JsepTransportController::GetLocalCertificate( RTC_FROM_HERE, [&] { return GetLocalCertificate(transport_name); }); } - const cricket::JsepTransport2* t = GetJsepTransportByName(transport_name); + const cricket::JsepTransport* t = GetJsepTransportByName(transport_name); if (!t) { return nullptr; } @@ -329,7 +332,7 @@ RTCError JsepTransportController::RemoveRemoteCandidates( for (const auto& kv : candidates_by_transport_name) { const std::string& transport_name = kv.first; const cricket::Candidates& candidates = kv.second; - cricket::JsepTransport2* jsep_transport = + cricket::JsepTransport* jsep_transport = GetJsepTransportByName(transport_name); if (!jsep_transport) { RTC_LOG(LS_WARNING) @@ -355,7 +358,7 @@ bool JsepTransportController::GetStats(const std::string& transport_name, RTC_FROM_HERE, [=] { return GetStats(transport_name, stats); }); } - cricket::JsepTransport2* transport = GetJsepTransportByName(transport_name); + cricket::JsepTransport* transport = GetJsepTransportByName(transport_name); if (!transport) { return false; } @@ -467,15 +470,12 @@ JsepTransportController::CreateDtlsSrtpTransport( cricket::DtlsTransportInternal* rtp_dtls_transport, cricket::DtlsTransportInternal* rtcp_dtls_transport) { RTC_DCHECK(network_thread_->IsCurrent()); - auto srtp_transport = - rtc::MakeUnique(rtcp_dtls_transport == nullptr); + auto dtls_srtp_transport = rtc::MakeUnique( + rtcp_dtls_transport == nullptr); if (config_.enable_external_auth) { - srtp_transport->EnableExternalAuth(); + dtls_srtp_transport->EnableExternalAuth(); } - auto dtls_srtp_transport = - rtc::MakeUnique(std::move(srtp_transport)); - dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport, rtcp_dtls_transport); return dtls_srtp_transport; @@ -574,12 +574,18 @@ RTCError JsepTransportController::ApplyDescription_n( const cricket::TransportInfo& transport_info = description->transport_infos()[i]; if (content_info.rejected) { - HandleRejectedContent(content_info, description); + if (!HandleRejectedContent(content_info, description)) { + return RTCError(RTCErrorType::INVALID_PARAMETER, + "Failed to process the rejected m= section."); + } continue; } if (IsBundled(content_info.name) && content_info.name != *bundled_mid()) { - HandleBundledContent(content_info); + if (!HandleBundledContent(content_info)) { + return RTCError(RTCErrorType::INVALID_PARAMETER, + "Failed to process the bundled m= section."); + } continue; } @@ -598,7 +604,7 @@ RTCError JsepTransportController::ApplyDescription_n( int rtp_abs_sendtime_extn_id = GetRtpAbsSendTimeHeaderExtensionId(content_info); - cricket::JsepTransport2* transport = + cricket::JsepTransport* transport = GetJsepTransportForMid(content_info.name); RTC_DCHECK(transport); @@ -736,21 +742,20 @@ RTCError JsepTransportController::ValidateContent( return RTCError::OK(); } -void JsepTransportController::HandleRejectedContent( +bool JsepTransportController::HandleRejectedContent( const cricket::ContentInfo& content_info, const cricket::SessionDescription* description) { + bool ret = true; // If the content is rejected, let the // BaseChannel/SctpTransport change the RtpTransport/DtlsTransport first, - // then destroy the cricket::JsepTransport2. - RemoveTransportForMid(content_info.name, content_info.type); - // If the answerer rejects the first content, which other contents are bundled - // on, all the other contents in the bundle group will be rejected. + // then destroy the cricket::JsepTransport. + ret = RemoveTransportForMid(content_info.name, content_info.type); if (content_info.name == bundled_mid()) { for (auto content_name : bundle_group_->content_names()) { const cricket::ContentInfo* content_in_group = description->GetContentByName(content_name); RTC_DCHECK(content_in_group); - RemoveTransportForMid(content_name, content_in_group->type); + ret = ret && RemoveTransportForMid(content_name, content_in_group->type); } bundle_group_.reset(); } else if (IsBundled(content_info.name)) { @@ -761,45 +766,60 @@ void JsepTransportController::HandleRejectedContent( bundle_group_.reset(); } } - MaybeDestroyJsepTransport(content_info.name); + if (ret) { + MaybeDestroyJsepTransport(content_info.name); + } + return ret; } -void JsepTransportController::HandleBundledContent( +bool JsepTransportController::HandleBundledContent( const cricket::ContentInfo& content_info) { auto jsep_transport = GetJsepTransportByName(*bundled_mid()); RTC_DCHECK(jsep_transport); // If the content is bundled, let the // BaseChannel/SctpTransport change the RtpTransport/DtlsTransport first, - // then destroy the cricket::JsepTransport2. - SetTransportForMid(content_info.name, jsep_transport, content_info.type); - MaybeDestroyJsepTransport(content_info.name); + // then destroy the cricket::JsepTransport. + if (SetTransportForMid(content_info.name, jsep_transport, + content_info.type)) { + MaybeDestroyJsepTransport(content_info.name); + return true; + } + return false; } -void JsepTransportController::SetTransportForMid( +bool JsepTransportController::SetTransportForMid( const std::string& mid, - cricket::JsepTransport2* jsep_transport, + cricket::JsepTransport* jsep_transport, cricket::MediaProtocolType protocol_type) { + RTC_DCHECK(jsep_transport); if (mid_to_transport_[mid] == jsep_transport) { - return; + return true; } + bool ret = true; mid_to_transport_[mid] = jsep_transport; if (protocol_type == cricket::MediaProtocolType::kRtp) { - SignalRtpTransportChanged(mid, jsep_transport->rtp_transport()); + ret = config_.transport_observer->OnRtpTransportChanged( + mid, jsep_transport->rtp_transport()); } else { - SignalDtlsTransportChanged(mid, jsep_transport->rtp_dtls_transport()); + config_.transport_observer->OnDtlsTransportChanged( + mid, jsep_transport->rtp_dtls_transport()); } + return ret; } -void JsepTransportController::RemoveTransportForMid( +bool JsepTransportController::RemoveTransportForMid( const std::string& mid, cricket::MediaProtocolType protocol_type) { + bool ret = true; if (protocol_type == cricket::MediaProtocolType::kRtp) { - SignalRtpTransportChanged(mid, nullptr); + ret = config_.transport_observer->OnRtpTransportChanged(mid, nullptr); + RTC_DCHECK(ret); } else { - SignalDtlsTransportChanged(mid, nullptr); + config_.transport_observer->OnDtlsTransportChanged(mid, nullptr); } mid_to_transport_.erase(mid); + return ret; } cricket::JsepTransportDescription @@ -905,25 +925,25 @@ int JsepTransportController::GetRtpAbsSendTimeHeaderExtensionId( return send_time_extension ? send_time_extension->id : -1; } -const cricket::JsepTransport2* JsepTransportController::GetJsepTransportForMid( +const cricket::JsepTransport* JsepTransportController::GetJsepTransportForMid( const std::string& mid) const { auto it = mid_to_transport_.find(mid); return it == mid_to_transport_.end() ? nullptr : it->second; } -cricket::JsepTransport2* JsepTransportController::GetJsepTransportForMid( +cricket::JsepTransport* JsepTransportController::GetJsepTransportForMid( const std::string& mid) { auto it = mid_to_transport_.find(mid); return it == mid_to_transport_.end() ? nullptr : it->second; } -const cricket::JsepTransport2* JsepTransportController::GetJsepTransportByName( +const cricket::JsepTransport* JsepTransportController::GetJsepTransportByName( const std::string& transport_name) const { auto it = jsep_transports_by_name_.find(transport_name); return (it == jsep_transports_by_name_.end()) ? nullptr : it->second.get(); } -cricket::JsepTransport2* JsepTransportController::GetJsepTransportByName( +cricket::JsepTransport* JsepTransportController::GetJsepTransportByName( const std::string& transport_name) { auto it = jsep_transports_by_name_.find(transport_name); return (it == jsep_transports_by_name_.end()) ? nullptr : it->second.get(); @@ -932,8 +952,7 @@ cricket::JsepTransport2* JsepTransportController::GetJsepTransportByName( RTCError JsepTransportController::MaybeCreateJsepTransport( const cricket::ContentInfo& content_info) { RTC_DCHECK(network_thread_->IsCurrent()); - cricket::JsepTransport2* transport = - GetJsepTransportByName(content_info.name); + cricket::JsepTransport* transport = GetJsepTransportByName(content_info.name); if (transport) { return RTCError::OK(); } @@ -970,8 +989,8 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( content_info.name, rtp_dtls_transport.get(), rtcp_dtls_transport.get()); } - std::unique_ptr jsep_transport = - rtc::MakeUnique( + std::unique_ptr jsep_transport = + rtc::MakeUnique( content_info.name, certificate_, std::move(unencrypted_rtp_transport), std::move(sdes_transport), std::move(dtls_srtp_transport), std::move(rtp_dtls_transport), std::move(rtcp_dtls_transport)); @@ -1018,7 +1037,7 @@ void JsepTransportController::SetIceRole_n(cricket::IceRole ice_role) { } cricket::IceRole JsepTransportController::DetermineIceRole( - cricket::JsepTransport2* jsep_transport, + cricket::JsepTransport* jsep_transport, const cricket::TransportInfo& transport_info, SdpType type, bool local) { diff --git a/pc/jseptransportcontroller.h b/pc/jseptransportcontroller.h index 6e44534f08..c3c7919f50 100644 --- a/pc/jseptransportcontroller.h +++ b/pc/jseptransportcontroller.h @@ -25,7 +25,7 @@ #include "p2p/base/transportfactoryinterface.h" #include "pc/channel.h" #include "pc/dtlssrtptransport.h" -#include "pc/jseptransport2.h" +#include "pc/jseptransport.h" #include "pc/rtptransport.h" #include "pc/srtptransport.h" #include "rtc_base/asyncinvoker.h" @@ -44,6 +44,23 @@ namespace webrtc { class JsepTransportController : public sigslot::has_slots<>, public rtc::MessageHandler { public: + // Used when the RtpTransport/DtlsTransport of the m= section is changed + // because the section is rejected or BUNDLE is enabled. + class Observer { + public: + virtual ~Observer() {} + + // Returns true if media associated with |mid| was successfully set up to be + // demultiplexed on |rtp_transport|. Could return false if two bundled m= + // sections use the same SSRC, for example. + virtual bool OnRtpTransportChanged(const std::string& mid, + RtpTransportInternal* rtp_transport) = 0; + + virtual void OnDtlsTransportChanged( + const std::string& mid, + cricket::DtlsTransportInternal* dtls_transport) = 0; + }; + struct Config { // If |redetermine_role_on_ice_restart| is true, ICE role is redetermined // upon setting a local transport description that indicates an ICE @@ -61,6 +78,7 @@ class JsepTransportController : public sigslot::has_slots<>, bool enable_external_auth = false; // Used to inject the ICE/DTLS transports created externally. cricket::TransportFactoryInterface* external_transport_factory = nullptr; + Observer* transport_observer = nullptr; }; // The ICE related events are signaled on the |signaling_thread|. @@ -136,6 +154,7 @@ class JsepTransportController : public sigslot::has_slots<>, void SetMetricsObserver(webrtc::MetricsObserverInterface* metrics_observer); bool initial_offerer() const { return initial_offerer_ && *initial_offerer_; } + // All of these signals are fired on the signaling thread. // If any transport failed => failed, @@ -158,19 +177,6 @@ class JsepTransportController : public sigslot::has_slots<>, sigslot::signal1 SignalDtlsHandshakeError; - // This will be fired when BUNDLE is enabled, the PeerConnection will handle - // the signal and set the RtpTransport for the BaseChannel. - // The first argument is the MID and the second is the new RtpTransport. - // Before firing this signal, the previous RtpTransport must no longer be - // referenced. - sigslot::signal2 - SignalRtpTransportChanged; - - // SCTP version of the signal above. PeerConnection will set a new - // DtlsTransport for the SctpTransport. - sigslot::signal2 - SignalDtlsTransportChanged; - private: void OnMessage(rtc::Message* pmsg) override; @@ -183,14 +189,14 @@ class JsepTransportController : public sigslot::has_slots<>, const cricket::SessionDescription* description); RTCError ValidateContent(const cricket::ContentInfo& content_info); - void HandleRejectedContent(const cricket::ContentInfo& content_info, + bool HandleRejectedContent(const cricket::ContentInfo& content_info, const cricket::SessionDescription* description); - void HandleBundledContent(const cricket::ContentInfo& content_info); + bool HandleBundledContent(const cricket::ContentInfo& content_info); - void SetTransportForMid(const std::string& mid, - cricket::JsepTransport2* jsep_transport, + bool SetTransportForMid(const std::string& mid, + cricket::JsepTransport* jsep_transport, cricket::MediaProtocolType protocol_type); - void RemoveTransportForMid(const std::string& mid, + bool RemoveTransportForMid(const std::string& mid, cricket::MediaProtocolType protocol_type); cricket::JsepTransportDescription CreateJsepTransportDescription( @@ -226,15 +232,15 @@ class JsepTransportController : public sigslot::has_slots<>, // destroyed because of BUNDLE, it would return the transport which other // transports are bundled on (In current implementation, it is the first // content in the BUNDLE group). - const cricket::JsepTransport2* GetJsepTransportForMid( + const cricket::JsepTransport* GetJsepTransportForMid( const std::string& mid) const; - cricket::JsepTransport2* GetJsepTransportForMid(const std::string& mid); + cricket::JsepTransport* GetJsepTransportForMid(const std::string& mid); // Get the JsepTransport without considering the BUNDLE group. Return nullptr // if the JsepTransport is destroyed. - const cricket::JsepTransport2* GetJsepTransportByName( + const cricket::JsepTransport* GetJsepTransportByName( const std::string& transport_name) const; - cricket::JsepTransport2* GetJsepTransportByName( + cricket::JsepTransport* GetJsepTransportByName( const std::string& transport_name); RTCError MaybeCreateJsepTransport(const cricket::ContentInfo& content_info); @@ -244,7 +250,7 @@ class JsepTransportController : public sigslot::has_slots<>, void SetIceRole_n(cricket::IceRole ice_role); cricket::IceRole DetermineIceRole( - cricket::JsepTransport2* jsep_transport, + cricket::JsepTransport* jsep_transport, const cricket::TransportInfo& transport_info, SdpType type, bool local); @@ -291,11 +297,11 @@ class JsepTransportController : public sigslot::has_slots<>, rtc::Thread* const network_thread_ = nullptr; cricket::PortAllocator* const port_allocator_ = nullptr; - std::map> + std::map> jsep_transports_by_name_; // This keeps track of the mapping between media section - // (BaseChannel/SctpTransport) and the JsepTransport2 underneath. - std::map mid_to_transport_; + // (BaseChannel/SctpTransport) and the JsepTransport underneath. + std::map mid_to_transport_; // Aggregate state for Transports. cricket::IceConnectionState ice_connection_state_ = diff --git a/pc/jseptransportcontroller_unittest.cc b/pc/jseptransportcontroller_unittest.cc index 16a829f2dc..b8ec316d72 100644 --- a/pc/jseptransportcontroller_unittest.cc +++ b/pc/jseptransportcontroller_unittest.cc @@ -59,7 +59,8 @@ class FakeTransportFactory : public cricket::TransportFactoryInterface { } }; -class JsepTransportControllerTest : public testing::Test, +class JsepTransportControllerTest : public JsepTransportController::Observer, + public testing::Test, public sigslot::has_slots<> { public: JsepTransportControllerTest() : signaling_thread_(rtc::Thread::Current()) { @@ -71,6 +72,7 @@ class JsepTransportControllerTest : public testing::Test, rtc::Thread* signaling_thread = rtc::Thread::Current(), rtc::Thread* network_thread = rtc::Thread::Current(), cricket::PortAllocator* port_allocator = nullptr) { + config.transport_observer = this; // The tests only works with |fake_transport_factory|; config.external_transport_factory = fake_transport_factory_.get(); transport_controller_ = rtc::MakeUnique( @@ -85,10 +87,6 @@ class JsepTransportControllerTest : public testing::Test, this, &JsepTransportControllerTest::OnGatheringState); transport_controller_->SignalIceCandidatesGathered.connect( this, &JsepTransportControllerTest::OnCandidatesGathered); - transport_controller_->SignalRtpTransportChanged.connect( - this, &JsepTransportControllerTest::OnRtpTransportChanged); - transport_controller_->SignalDtlsTransportChanged.connect( - this, &JsepTransportControllerTest::OnDtlsTransportChanged); } std::unique_ptr @@ -262,13 +260,16 @@ class JsepTransportControllerTest : public testing::Test, ++candidates_signal_count_; } - void OnRtpTransportChanged(const std::string& mid, - RtpTransportInternal* rtp_transport) { + // JsepTransportController::Observer overrides. + bool OnRtpTransportChanged(const std::string& mid, + RtpTransportInternal* rtp_transport) override { changed_rtp_transport_by_mid_[mid] = rtp_transport; + return true; } - void OnDtlsTransportChanged(const std::string& mid, - cricket::DtlsTransportInternal* dtls_transport) { + void OnDtlsTransportChanged( + const std::string& mid, + cricket::DtlsTransportInternal* dtls_transport) override { changed_dtls_transport_by_mid_[mid] = dtls_transport; } diff --git a/pc/mediasession.h b/pc/mediasession.h index 5b945392cb..997a4692d3 100644 --- a/pc/mediasession.h +++ b/pc/mediasession.h @@ -22,7 +22,7 @@ #include "media/base/mediaconstants.h" #include "media/base/mediaengine.h" // For DataChannelType #include "p2p/base/transportdescriptionfactory.h" -#include "pc/jseptransport2.h" +#include "pc/jseptransport.h" #include "pc/sessiondescription.h" namespace cricket { diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 2721063999..c7dc73b23f 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -932,6 +932,7 @@ bool PeerConnection::Initialize( config.bundle_policy = configuration.bundle_policy; config.rtcp_mux_policy = configuration.rtcp_mux_policy; config.crypto_options = options.crypto_options; + config.transport_observer = this; #if defined(ENABLE_EXTERNAL_AUTH) config.enable_external_auth = true; #endif @@ -947,10 +948,6 @@ bool PeerConnection::Initialize( this, &PeerConnection::OnTransportControllerCandidatesRemoved); transport_controller_->SignalDtlsHandshakeError.connect( this, &PeerConnection::OnTransportControllerDtlsHandshakeError); - transport_controller_->SignalRtpTransportChanged.connect( - this, &PeerConnection::OnRtpTransportChanged); - transport_controller_->SignalDtlsTransportChanged.connect( - this, &PeerConnection::OnDtlsTransportChanged); sctp_factory_ = factory_->CreateSctpTransportInternalFactory(); @@ -5430,9 +5427,6 @@ cricket::VoiceChannel* PeerConnection::CreateVoiceChannel( voice_channel->SignalSentPacket.connect(this, &PeerConnection::OnSentPacket_w); voice_channel->SetRtpTransport(rtp_transport); - if (factory_->options().disable_encryption) { - voice_channel->DisableEncryption(true); - } if (uma_observer_) { voice_channel->SetMetricsObserver(uma_observer_); } @@ -5458,9 +5452,6 @@ cricket::VideoChannel* PeerConnection::CreateVideoChannel( video_channel->SignalSentPacket.connect(this, &PeerConnection::OnSentPacket_w); video_channel->SetRtpTransport(rtp_transport); - if (factory_->options().disable_encryption) { - video_channel->DisableEncryption(true); - } if (uma_observer_) { video_channel->SetMetricsObserver(uma_observer_); } @@ -5500,9 +5491,6 @@ bool PeerConnection::CreateDataChannel(const std::string& mid) { rtp_data_channel_->SignalSentPacket.connect( this, &PeerConnection::OnSentPacket_w); rtp_data_channel_->SetRtpTransport(rtp_transport); - if (factory_->options().disable_encryption) { - rtp_data_channel_->DisableEncryption(true); - } if (uma_observer_) { rtp_data_channel_->SetMetricsObserver(uma_observer_); } @@ -6100,7 +6088,6 @@ void PeerConnection::DestroyDataChannel() { void PeerConnection::DestroyBaseChannel(cricket::BaseChannel* channel) { RTC_DCHECK(channel); - switch (channel->media_type()) { case cricket::MEDIA_TYPE_AUDIO: channel_manager()->DestroyVoiceChannel( @@ -6120,13 +6107,14 @@ void PeerConnection::DestroyBaseChannel(cricket::BaseChannel* channel) { } } -void PeerConnection::OnRtpTransportChanged( +bool PeerConnection::OnRtpTransportChanged( const std::string& mid, RtpTransportInternal* rtp_transport) { auto base_channel = GetChannel(mid); if (base_channel) { - base_channel->SetRtpTransport(rtp_transport); + return base_channel->SetRtpTransport(rtp_transport); } + return true; } void PeerConnection::OnDtlsTransportChanged( diff --git a/pc/peerconnection.h b/pc/peerconnection.h index fa46a4e151..c019ef9bc9 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -51,6 +51,7 @@ class RtcEventLog; // - Generating stats. class PeerConnection : public PeerConnectionInternal, public DataChannelProviderInterface, + public JsepTransportController::Observer, public rtc::MessageHandler, public sigslot::has_slots<> { public: @@ -877,11 +878,13 @@ class PeerConnection : public PeerConnectionInternal, // method is called. void DestroyBaseChannel(cricket::BaseChannel* channel); - void OnRtpTransportChanged(const std::string& mid, - RtpTransportInternal* rtp_transport); + // JsepTransportController::Observer override. + bool OnRtpTransportChanged(const std::string& mid, + RtpTransportInternal* rtp_transport) override; - void OnDtlsTransportChanged(const std::string& mid, - cricket::DtlsTransportInternal* dtls_transport); + void OnDtlsTransportChanged( + const std::string& mid, + cricket::DtlsTransportInternal* dtls_transport) override; sigslot::signal1 SignalDataChannelCreated_; diff --git a/pc/peerconnection_bundle_unittest.cc b/pc/peerconnection_bundle_unittest.cc index a1b69d8ae1..dfd47cde79 100644 --- a/pc/peerconnection_bundle_unittest.cc +++ b/pc/peerconnection_bundle_unittest.cc @@ -666,6 +666,42 @@ TEST_P(PeerConnectionBundleTest, BundleOnFirstMidInAnswer) { EXPECT_EQ(caller->voice_rtp_transport(), caller->video_rtp_transport()); } +// This tests that applying description with conflicted RTP demuxing criteria +// will fail. +TEST_P(PeerConnectionBundleTest, + ApplyDescriptionWithConflictedDemuxCriteriaFail) { + auto caller = CreatePeerConnectionWithAudioVideo(); + auto callee = CreatePeerConnectionWithAudioVideo(); + + RTCOfferAnswerOptions options; + options.use_rtp_mux = false; + auto offer = caller->CreateOffer(options); + // Modified the SDP to make two m= sections have the same SSRC. + ASSERT_GE(offer->description()->contents().size(), 2U); + offer->description() + ->contents()[0] + .description->mutable_streams()[0] + .ssrcs[0] = 1111222; + offer->description() + ->contents()[1] + .description->mutable_streams()[0] + .ssrcs[0] = 1111222; + EXPECT_TRUE( + caller->SetLocalDescription(CloneSessionDescription(offer.get()))); + EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer))); + EXPECT_TRUE(callee->CreateAnswerAndSetAsLocal(options)); + + // Enable BUNDLE in subsequent offer/answer exchange and two m= sections are + // expectd to use one RtpTransport underneath. + options.use_rtp_mux = true; + EXPECT_TRUE( + callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(options))); + auto answer = callee->CreateAnswer(options); + // When BUNDLE is enabled, applying the description is expected to fail + // because the demuxing criteria is conflicted. + EXPECT_FALSE(callee->SetLocalDescription(std::move(answer))); +} + // This tests that changing the pre-negotiated BUNDLE tag is not supported. TEST_P(PeerConnectionBundleTest, RejectDescriptionChangingBundleTag) { RTCConfiguration config; diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index 7379c711cf..583765d9a6 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -2273,6 +2273,8 @@ TEST_F(PeerConnectionIntegrationTestUnifiedPlan, // Test that if two video tracks are sent (from caller to callee, in this test), // they're transmitted correctly end-to-end. +// TODO(zhihuang): Enable this test in Unified Plan mode once the MID-based +// demuxing is ready. TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithTwoVideoTracks) { ASSERT_TRUE(CreatePeerConnectionWrappers()); ConnectFakeSignaling(); @@ -2395,7 +2397,6 @@ TEST_P(PeerConnectionIntegrationTest, GetBytesReceivedStatsWithOldStatsApi) { TEST_P(PeerConnectionIntegrationTest, GetBytesSentStatsWithOldStatsApi) { ASSERT_TRUE(CreatePeerConnectionWrappers()); ConnectFakeSignaling(); - caller()->AddAudioVideoTracks(); auto audio_track = caller()->CreateLocalAudioTrack(); auto video_track = caller()->CreateLocalVideoTrack(); caller()->AddTrack(audio_track); diff --git a/pc/peerconnection_media_unittest.cc b/pc/peerconnection_media_unittest.cc index 228243c5a3..f59d6aa1bc 100644 --- a/pc/peerconnection_media_unittest.cc +++ b/pc/peerconnection_media_unittest.cc @@ -967,8 +967,8 @@ void RenameContent(cricket::SessionDescription* desc, // Tests that an answer responds with the same MIDs as the offer. TEST_P(PeerConnectionMediaTest, AnswerHasSameMidsAsOffer) { - const std::string kAudioMid = "not default1"; - const std::string kVideoMid = "not default2"; + const std::string kAudioMid = "notdefault1"; + const std::string kVideoMid = "notdefault2"; auto caller = CreatePeerConnectionWithAudioVideo(); auto callee = CreatePeerConnectionWithAudioVideo(); @@ -988,8 +988,8 @@ TEST_P(PeerConnectionMediaTest, AnswerHasSameMidsAsOffer) { // Test that if the callee creates a re-offer, the MIDs are the same as the // original offer. TEST_P(PeerConnectionMediaTest, ReOfferHasSameMidsAsFirstOffer) { - const std::string kAudioMid = "not default1"; - const std::string kVideoMid = "not default2"; + const std::string kAudioMid = "notdefault1"; + const std::string kVideoMid = "notdefault2"; auto caller = CreatePeerConnectionWithAudioVideo(); auto callee = CreatePeerConnectionWithAudioVideo(); diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index 69089a5dbb..d9b543b8c2 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -113,12 +113,8 @@ class RtpSenderReceiverTest : public testing::Test, } std::unique_ptr CreateDtlsSrtpTransport() { - auto rtp_transport = - rtc::MakeUnique(/*rtcp_mux_required=*/true); - auto srtp_transport = - rtc::MakeUnique(std::move(rtp_transport)); auto dtls_srtp_transport = - rtc::MakeUnique(std::move(srtp_transport)); + rtc::MakeUnique(/*rtcp_mux_required=*/true); dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport_.get(), /*rtcp_dtls_transport=*/nullptr); return dtls_srtp_transport; diff --git a/pc/rtptransport.cc b/pc/rtptransport.cc index ce6996c6ad..366d6e2fc8 100644 --- a/pc/rtptransport.cc +++ b/pc/rtptransport.cc @@ -10,7 +10,10 @@ #include "pc/rtptransport.h" +#include + #include "media/base/rtputils.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/packettransportinterface.h" #include "rtc_base/checks.h" @@ -44,7 +47,7 @@ void RtpTransport::SetRtpPacketTransport( new_packet_transport->SignalReadPacket.connect(this, &RtpTransport::OnReadPacket); new_packet_transport->SignalNetworkRouteChanged.connect( - this, &RtpTransport::OnNetworkRouteChange); + this, &RtpTransport::OnNetworkRouteChanged); new_packet_transport->SignalWritableState.connect( this, &RtpTransport::OnWritableState); new_packet_transport->SignalSentPacket.connect(this, @@ -80,7 +83,7 @@ void RtpTransport::SetRtcpPacketTransport( new_packet_transport->SignalReadPacket.connect(this, &RtpTransport::OnReadPacket); new_packet_transport->SignalNetworkRouteChanged.connect( - this, &RtpTransport::OnNetworkRouteChange); + this, &RtpTransport::OnNetworkRouteChanged); new_packet_transport->SignalWritableState.connect( this, &RtpTransport::OnWritableState); new_packet_transport->SignalSentPacket.connect(this, @@ -134,6 +137,29 @@ bool RtpTransport::SendPacket(bool rtcp, return true; } +void RtpTransport::UpdateRtpHeaderExtensionMap( + const cricket::RtpHeaderExtensions& header_extensions) { + header_extension_map_ = RtpHeaderExtensionMap(header_extensions); +} + +bool RtpTransport::RegisterRtpDemuxerSink(const RtpDemuxerCriteria& criteria, + RtpPacketSinkInterface* sink) { + rtp_demuxer_.RemoveSink(sink); + if (!rtp_demuxer_.AddSink(criteria, sink)) { + RTC_LOG(LS_ERROR) << "Failed to register the sink for RTP demuxer."; + return false; + } + return true; +} + +bool RtpTransport::UnregisterRtpDemuxerSink(RtpPacketSinkInterface* sink) { + if (!rtp_demuxer_.RemoveSink(sink)) { + RTC_LOG(LS_ERROR) << "Failed to unregister the sink for RTP demuxer."; + return false; + } + return true; +} + RTCError RtpTransport::SetParameters(const RtpTransportParameters& parameters) { if (parameters_.rtcp.mux && !parameters.rtcp.mux) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_STATE, @@ -160,11 +186,26 @@ RtpTransportParameters RtpTransport::GetParameters() const { return parameters_; } +void RtpTransport::DemuxPacket(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& time) { + webrtc::RtpPacketReceived parsed_packet(&header_extension_map_); + if (!parsed_packet.Parse(std::move(*packet))) { + RTC_LOG(LS_ERROR) + << "Failed to parse the incoming RTP packet before demuxing. Drop it."; + return; + } + + if (time.timestamp != -1) { + parsed_packet.set_arrival_time_ms((time.timestamp + 500) / 1000); + } + rtp_demuxer_.OnRtpPacket(parsed_packet); +} + RtpTransportAdapter* RtpTransport::GetInternal() { return nullptr; } -bool RtpTransport::IsRtpTransportWritable() { +bool RtpTransport::IsTransportWritable() { auto rtcp_packet_transport = rtcp_mux_enabled_ ? nullptr : rtcp_packet_transport_; return rtp_packet_transport_ && rtp_packet_transport_->writable() && @@ -175,7 +216,7 @@ void RtpTransport::OnReadyToSend(rtc::PacketTransportInternal* transport) { SetReadyToSend(transport == rtcp_packet_transport_, true); } -void RtpTransport::OnNetworkRouteChange( +void RtpTransport::OnNetworkRouteChanged( rtc::Optional network_route) { SignalNetworkRouteChanged(network_route); } @@ -184,7 +225,7 @@ void RtpTransport::OnWritableState( rtc::PacketTransportInternal* packet_transport) { RTC_DCHECK(packet_transport == rtp_packet_transport_ || packet_transport == rtcp_packet_transport_); - SignalWritableState(IsRtpTransportWritable()); + SignalWritableState(IsTransportWritable()); } void RtpTransport::OnSentPacket(rtc::PacketTransportInternal* packet_transport, @@ -194,6 +235,49 @@ void RtpTransport::OnSentPacket(rtc::PacketTransportInternal* packet_transport, SignalSentPacket(sent_packet); } +void RtpTransport::OnRtpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time) { + DemuxPacket(packet, packet_time); +} + +void RtpTransport::OnRtcpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time) { + SignalRtcpPacketReceived(packet, packet_time); +} + +void RtpTransport::OnReadPacket(rtc::PacketTransportInternal* transport, + const char* data, + size_t len, + const rtc::PacketTime& packet_time, + int flags) { + TRACE_EVENT0("webrtc", "RtpTransport::OnReadPacket"); + + // When using RTCP multiplexing we might get RTCP packets on the RTP + // transport. We check the RTP payload type to determine if it is RTCP. + bool rtcp = + transport == rtcp_packet_transport() || cricket::IsRtcpPacket(data, len); + + // Filter out the packet that is neither RTP nor RTCP. + if (!rtcp && !cricket::IsRtpPacket(data, len)) { + return; + } + + rtc::CopyOnWriteBuffer packet(data, len); + // Protect ourselves against crazy data. + if (!cricket::IsValidRtpRtcpPacketSize(rtcp, packet.size())) { + RTC_LOG(LS_ERROR) << "Dropping incoming " + << cricket::RtpRtcpStringLiteral(rtcp) + << " packet: wrong size=" << packet.size(); + return; + } + + if (rtcp) { + OnRtcpPacketReceived(&packet, packet_time); + } else { + OnRtpPacketReceived(&packet, packet_time); + } +} + void RtpTransport::SetReadyToSend(bool rtcp, bool ready) { if (rtcp) { rtcp_ready_to_send_ = ready; @@ -213,52 +297,4 @@ void RtpTransport::MaybeSignalReadyToSend() { } } -// Check the RTP payload type. If 63 < payload type < 96, it's RTCP. -// For additional details, see http://tools.ietf.org/html/rfc5761. -bool IsRtcp(const char* data, int len) { - if (len < 2) { - return false; - } - char pt = data[1] & 0x7F; - return (63 < pt) && (pt < 96); -} - -void RtpTransport::OnReadPacket(rtc::PacketTransportInternal* transport, - const char* data, - size_t len, - const rtc::PacketTime& packet_time, - int flags) { - TRACE_EVENT0("webrtc", "RtpTransport::OnReadPacket"); - - if (!cricket::IsRtpPacket(data, len) && - !IsRtcp(data, static_cast(len))) { - return; - } - - // When using RTCP multiplexing we might get RTCP packets on the RTP - // transport. We check the RTP payload type to determine if it is RTCP. - bool rtcp = transport == rtcp_packet_transport() || - IsRtcp(data, static_cast(len)); - rtc::CopyOnWriteBuffer packet(data, len); - - if (!WantsPacket(rtcp, &packet)) { - return; - } - - // This mutates |packet| if it is protected. - SignalPacketReceived(rtcp, &packet, packet_time); -} - -bool RtpTransport::WantsPacket(bool rtcp, - const rtc::CopyOnWriteBuffer* packet) { - // Protect ourselves against crazy data. - if (!packet || !cricket::IsValidRtpRtcpPacketSize(rtcp, packet->size())) { - RTC_LOG(LS_ERROR) << "Dropping incoming " - << cricket::RtpRtcpStringLiteral(rtcp) - << " packet: wrong size=" << packet->size(); - return false; - } - return true; -} - } // namespace webrtc diff --git a/pc/rtptransport.h b/pc/rtptransport.h index f878980e00..6620095e15 100644 --- a/pc/rtptransport.h +++ b/pc/rtptransport.h @@ -13,7 +13,8 @@ #include -#include "api/ortc/rtptransportinterface.h" +#include "call/rtp_demuxer.h" +#include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "pc/rtptransportinternal.h" #include "rtc_base/sigslot.h" @@ -74,6 +75,14 @@ class RtpTransport : public RtpTransportInternal { bool IsSrtpActive() const override { return false; } + void UpdateRtpHeaderExtensionMap( + const cricket::RtpHeaderExtensions& header_extensions) override; + + bool RegisterRtpDemuxerSink(const RtpDemuxerCriteria& criteria, + RtpPacketSinkInterface* sink) override; + + bool UnregisterRtpDemuxerSink(RtpPacketSinkInterface* sink) override; + void SetMetricsObserver( rtc::scoped_refptr metrics_observer) override {} @@ -81,15 +90,33 @@ class RtpTransport : public RtpTransportInternal { // TODO(zstein): Remove this when we remove RtpTransportAdapter. RtpTransportAdapter* GetInternal() override; - private: - bool IsRtpTransportWritable(); - bool HandlesPacket(const uint8_t* data, size_t len); + // These methods will be used in the subclasses. + void DemuxPacket(rtc::CopyOnWriteBuffer* packet, const rtc::PacketTime& time); + bool SendPacket(bool rtcp, + rtc::CopyOnWriteBuffer* packet, + const rtc::PacketOptions& options, + int flags); + + // Overridden by SrtpTransport. + virtual void OnNetworkRouteChanged( + rtc::Optional network_route); + virtual void OnRtpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time); + virtual void OnRtcpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time); + // Overridden by SrtpTransport and DtlsSrtpTransport. + virtual void OnWritableState(rtc::PacketTransportInternal* packet_transport); + + private: void OnReadyToSend(rtc::PacketTransportInternal* transport); - void OnNetworkRouteChange(rtc::Optional network_route); - void OnWritableState(rtc::PacketTransportInternal* packet_transport); void OnSentPacket(rtc::PacketTransportInternal* packet_transport, const rtc::SentPacket& sent_packet); + void OnReadPacket(rtc::PacketTransportInternal* transport, + const char* data, + size_t len, + const rtc::PacketTime& packet_time, + int flags); // Updates "ready to send" for an individual channel and fires // SignalReadyToSend. @@ -97,19 +124,11 @@ class RtpTransport : public RtpTransportInternal { void MaybeSignalReadyToSend(); - bool SendPacket(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketOptions& options, - int flags); - - void OnReadPacket(rtc::PacketTransportInternal* transport, - const char* data, - size_t len, - const rtc::PacketTime& packet_time, - int flags); - - bool WantsPacket(bool rtcp, const rtc::CopyOnWriteBuffer* packet); + bool IsTransportWritable(); + // SRTP specific methods. + // TODO(zhihuang): Improve the inheritance model so that the RtpTransport + // doesn't need to implement SRTP specfic methods. RTCError SetSrtpSendKey(const cricket::CryptoParams& params) override { RTC_NOTREACHED(); return RTCError::OK(); @@ -129,6 +148,10 @@ class RtpTransport : public RtpTransportInternal { bool rtcp_ready_to_send_ = false; RtpTransportParameters parameters_; + RtpDemuxer rtp_demuxer_; + + // Used for identifying the MID for RtpDemuxer. + RtpHeaderExtensionMap header_extension_map_; }; } // namespace webrtc diff --git a/pc/rtptransport_unittest.cc b/pc/rtptransport_unittest.cc index 0425942925..97ea2e4e23 100644 --- a/pc/rtptransport_unittest.cc +++ b/pc/rtptransport_unittest.cc @@ -250,49 +250,37 @@ TEST(RtpTransportTest, RtcpPacketSentOverCorrectTransport) { EXPECT_EQ(1, observer.rtp_transport_sent_count()); } -class SignalCounter : public sigslot::has_slots<> { - public: - explicit SignalCounter(RtpTransport* transport) { - transport->SignalReadyToSend.connect(this, &SignalCounter::OnReadyToSend); - } - int count() const { return count_; } - void OnReadyToSend(bool ready) { ++count_; } - - private: - int count_ = 0; -}; - TEST(RtpTransportTest, ChangingReadyToSendStateOnlySignalsWhenChanged) { RtpTransport transport(kMuxEnabled); - SignalCounter observer(&transport); + TransportObserver observer(&transport); rtc::FakePacketTransport fake_rtp("fake_rtp"); fake_rtp.SetWritable(true); // State changes, so we should signal. transport.SetRtpPacketTransport(&fake_rtp); - EXPECT_EQ(observer.count(), 1); + EXPECT_EQ(observer.ready_to_send_signal_count(), 1); // State does not change, so we should not signal. transport.SetRtpPacketTransport(&fake_rtp); - EXPECT_EQ(observer.count(), 1); + EXPECT_EQ(observer.ready_to_send_signal_count(), 1); // State does not change, so we should not signal. transport.SetRtcpMuxEnabled(true); - EXPECT_EQ(observer.count(), 1); + EXPECT_EQ(observer.ready_to_send_signal_count(), 1); // State changes, so we should signal. transport.SetRtcpMuxEnabled(false); - EXPECT_EQ(observer.count(), 2); + EXPECT_EQ(observer.ready_to_send_signal_count(), 2); } // Test that SignalPacketReceived fires with rtcp=true when a RTCP packet is // received. TEST(RtpTransportTest, SignalDemuxedRtcp) { RtpTransport transport(kMuxDisabled); - SignalPacketReceivedCounter observer(&transport); rtc::FakePacketTransport fake_rtp("fake_rtp"); fake_rtp.SetDestination(&fake_rtp, true); transport.SetRtpPacketTransport(&fake_rtp); + TransportObserver observer(&transport); // An rtcp packet. const char data[] = {0, 73, 0, 0}; @@ -312,10 +300,14 @@ static const int kRtpLen = 12; // handled payload type is received. TEST(RtpTransportTest, SignalHandledRtpPayloadType) { RtpTransport transport(kMuxDisabled); - SignalPacketReceivedCounter observer(&transport); rtc::FakePacketTransport fake_rtp("fake_rtp"); fake_rtp.SetDestination(&fake_rtp, true); transport.SetRtpPacketTransport(&fake_rtp); + TransportObserver observer(&transport); + RtpDemuxerCriteria demuxer_criteria; + // Add a handled payload type. + demuxer_criteria.payload_types = {0x11}; + transport.RegisterRtpDemuxerSink(demuxer_criteria, &observer); // An rtp packet. const rtc::PacketOptions options; @@ -324,6 +316,31 @@ TEST(RtpTransportTest, SignalHandledRtpPayloadType) { fake_rtp.SendPacket(rtp_data.data(), kRtpLen, options, flags); EXPECT_EQ(1, observer.rtp_count()); EXPECT_EQ(0, observer.rtcp_count()); + // Remove the sink before destroying the transport. + transport.UnregisterRtpDemuxerSink(&observer); +} + +// Test that SignalPacketReceived does not fire when a RTP packet with an +// unhandled payload type is received. +TEST(RtpTransportTest, DontSignalUnhandledRtpPayloadType) { + RtpTransport transport(kMuxDisabled); + rtc::FakePacketTransport fake_rtp("fake_rtp"); + fake_rtp.SetDestination(&fake_rtp, true); + transport.SetRtpPacketTransport(&fake_rtp); + TransportObserver observer(&transport); + RtpDemuxerCriteria demuxer_criteria; + // Add an unhandled payload type. + demuxer_criteria.payload_types = {0x12}; + transport.RegisterRtpDemuxerSink(demuxer_criteria, &observer); + + const rtc::PacketOptions options; + const int flags = 0; + rtc::Buffer rtp_data(kRtpData, kRtpLen); + fake_rtp.SendPacket(rtp_data.data(), kRtpLen, options, flags); + EXPECT_EQ(0, observer.rtp_count()); + EXPECT_EQ(0, observer.rtcp_count()); + // Remove the sink before destroying the transport. + transport.UnregisterRtpDemuxerSink(&observer); } } // namespace webrtc diff --git a/pc/rtptransportinternal.h b/pc/rtptransportinternal.h index a354936188..7845d42a9c 100644 --- a/pc/rtptransportinternal.h +++ b/pc/rtptransportinternal.h @@ -15,7 +15,9 @@ #include "api/ortc/srtptransportinterface.h" #include "api/umametrics.h" +#include "call/rtp_demuxer.h" #include "p2p/base/icetransportinternal.h" +#include "pc/sessiondescription.h" #include "rtc_base/networkroute.h" #include "rtc_base/sigslot.h" #include "rtc_base/sslstreamadapter.h" @@ -55,11 +57,11 @@ class RtpTransportInternal : public SrtpTransportInterface, // than just "writable"; it means the last send didn't return ENOTCONN. sigslot::signal1 SignalReadyToSend; - // TODO(zstein): Consider having two signals - RtpPacketReceived and - // RtcpPacketReceived. - // The first argument is true for RTCP packets and false for RTP packets. - sigslot::signal3 - SignalPacketReceived; + // Called whenever an RTCP packet is received. There is no equivalent signal + // for RTP packets because they would be forwarded to the BaseChannel through + // the RtpDemuxer callback. + sigslot::signal2 + SignalRtcpPacketReceived; // Called whenever the network route of the P2P layer transport changes. // The argument is an optional network route. @@ -83,10 +85,28 @@ class RtpTransportInternal : public SrtpTransportInterface, const rtc::PacketOptions& options, int flags) = 0; + // This method updates the RTP header extension map so that the RTP transport + // can parse the received packets and identify the MID. This is called by the + // BaseChannel when setting the content description. + // + // TODO(zhihuang): Merging and replacing following methods handling header + // extensions with SetParameters: + // UpdateRtpHeaderExtensionMap, + // UpdateSendEncryptedHeaderExtensionIds, + // UpdateRecvEncryptedHeaderExtensionIds, + // CacheRtpAbsSendTimeHeaderExtension, + virtual void UpdateRtpHeaderExtensionMap( + const cricket::RtpHeaderExtensions& header_extensions) = 0; + virtual bool IsSrtpActive() const = 0; virtual void SetMetricsObserver( rtc::scoped_refptr metrics_observer) = 0; + + virtual bool RegisterRtpDemuxerSink(const RtpDemuxerCriteria& criteria, + RtpPacketSinkInterface* sink) = 0; + + virtual bool UnregisterRtpDemuxerSink(RtpPacketSinkInterface* sink) = 0; }; } // namespace webrtc diff --git a/pc/rtptransportinternaladapter.h b/pc/rtptransportinternaladapter.h index a1f4bfc02e..ed7258dde9 100644 --- a/pc/rtptransportinternaladapter.h +++ b/pc/rtptransportinternaladapter.h @@ -71,6 +71,20 @@ class RtpTransportInternalAdapter : public RtpTransportInternal { return transport_->SendRtcpPacket(packet, options, flags); } + void UpdateRtpHeaderExtensionMap( + const cricket::RtpHeaderExtensions& header_extensions) override { + transport_->UpdateRtpHeaderExtensionMap(header_extensions); + } + + bool RegisterRtpDemuxerSink(const RtpDemuxerCriteria& criteria, + RtpPacketSinkInterface* sink) override { + return transport_->RegisterRtpDemuxerSink(criteria, sink); + } + + bool UnregisterRtpDemuxerSink(RtpPacketSinkInterface* sink) override { + return transport_->UnregisterRtpDemuxerSink(sink); + } + // RtpTransportInterface overrides. PacketTransportInterface* GetRtpPacketTransport() const override { return transport_->GetRtpPacketTransport(); diff --git a/pc/rtptransporttestutil.h b/pc/rtptransporttestutil.h index c2bdaad23e..2707e1cd75 100644 --- a/pc/rtptransporttestutil.h +++ b/pc/rtptransporttestutil.h @@ -11,32 +11,66 @@ #ifndef PC_RTPTRANSPORTTESTUTIL_H_ #define PC_RTPTRANSPORTTESTUTIL_H_ +#include "call/rtp_packet_sink_interface.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "pc/rtptransportinternal.h" #include "rtc_base/sigslot.h" namespace webrtc { -class SignalPacketReceivedCounter : public sigslot::has_slots<> { +// Used to handle the signals when the RtpTransport receives an RTP/RTCP packet. +// Used in Rtp/Srtp/DtlsTransport unit tests. +class TransportObserver : public RtpPacketSinkInterface, + public sigslot::has_slots<> { public: - explicit SignalPacketReceivedCounter(RtpTransportInternal* transport) { - transport->SignalPacketReceived.connect( - this, &SignalPacketReceivedCounter::OnPacketReceived); + TransportObserver() {} + + explicit TransportObserver(RtpTransportInternal* rtp_transport) { + rtp_transport->SignalRtcpPacketReceived.connect( + this, &TransportObserver::OnRtcpPacketReceived); + rtp_transport->SignalReadyToSend.connect(this, + &TransportObserver::OnReadyToSend); } - int rtcp_count() const { return rtcp_count_; } + + // RtpPacketInterface override. + void OnRtpPacket(const RtpPacketReceived& packet) override { + rtp_count_++; + last_recv_rtp_packet_ = packet.Buffer(); + } + + void OnRtcpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time) { + rtcp_count_++; + last_recv_rtcp_packet_ = *packet; + } + int rtp_count() const { return rtp_count_; } + int rtcp_count() const { return rtcp_count_; } + + rtc::CopyOnWriteBuffer last_recv_rtp_packet() { + return last_recv_rtp_packet_; + } + + rtc::CopyOnWriteBuffer last_recv_rtcp_packet() { + return last_recv_rtcp_packet_; + } + + void OnReadyToSend(bool ready) { + ready_to_send_signal_count_++; + ready_to_send_ = ready; + } + + bool ready_to_send() { return ready_to_send_; } + + int ready_to_send_signal_count() { return ready_to_send_signal_count_; } private: - void OnPacketReceived(bool rtcp, - rtc::CopyOnWriteBuffer*, - const rtc::PacketTime&) { - if (rtcp) { - ++rtcp_count_; - } else { - ++rtp_count_; - } - } - int rtcp_count_ = 0; + bool ready_to_send_ = false; int rtp_count_ = 0; + int rtcp_count_ = 0; + int ready_to_send_signal_count_ = 0; + rtc::CopyOnWriteBuffer last_recv_rtp_packet_; + rtc::CopyOnWriteBuffer last_recv_rtcp_packet_; }; } // namespace webrtc diff --git a/pc/srtptransport.cc b/pc/srtptransport.cc index 3e7b154fc0..1fe0cc812e 100644 --- a/pc/srtptransport.cc +++ b/pc/srtptransport.cc @@ -19,6 +19,7 @@ #include "rtc_base/asyncpacketsocket.h" #include "rtc_base/base64.h" #include "rtc_base/copyonwritebuffer.h" +#include "rtc_base/numerics/safe_conversions.h" #include "rtc_base/ptr_util.h" #include "rtc_base/trace_event.h" #include "rtc_base/zero_memory.h" @@ -26,31 +27,7 @@ namespace webrtc { SrtpTransport::SrtpTransport(bool rtcp_mux_enabled) - : RtpTransportInternalAdapter(new RtpTransport(rtcp_mux_enabled)) { - // Own the raw pointer |transport| from the base class. - rtp_transport_.reset(static_cast(transport_)); - RTC_DCHECK(rtp_transport_); - ConnectToRtpTransport(); -} - -SrtpTransport::SrtpTransport(std::unique_ptr rtp_transport) - : RtpTransportInternalAdapter(rtp_transport.get()), - rtp_transport_(std::move(rtp_transport)) { - RTC_DCHECK(rtp_transport_); - ConnectToRtpTransport(); -} - -void SrtpTransport::ConnectToRtpTransport() { - rtp_transport_->SignalPacketReceived.connect( - this, &SrtpTransport::OnPacketReceived); - rtp_transport_->SignalReadyToSend.connect(this, - &SrtpTransport::OnReadyToSend); - rtp_transport_->SignalNetworkRouteChanged.connect( - this, &SrtpTransport::OnNetworkRouteChanged); - rtp_transport_->SignalWritableState.connect(this, - &SrtpTransport::OnWritableState); - rtp_transport_->SignalSentPacket.connect(this, &SrtpTransport::OnSentPacket); -} + : RtpTransport(rtcp_mux_enabled) {} RTCError SrtpTransport::SetSrtpSendKey(const cricket::CryptoParams& params) { if (send_params_) { @@ -135,125 +112,128 @@ RTCError SrtpTransport::SetSrtpReceiveKey(const cricket::CryptoParams& params) { bool SrtpTransport::SendRtpPacket(rtc::CopyOnWriteBuffer* packet, const rtc::PacketOptions& options, int flags) { - return SendPacket(false, packet, options, flags); -} - -bool SrtpTransport::SendRtcpPacket(rtc::CopyOnWriteBuffer* packet, - const rtc::PacketOptions& options, - int flags) { - return SendPacket(true, packet, options, flags); -} - -bool SrtpTransport::SendPacket(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketOptions& options, - int flags) { if (!IsSrtpActive()) { RTC_LOG(LS_ERROR) << "Failed to send the packet because SRTP transport is inactive."; return false; } - rtc::PacketOptions updated_options = options; - rtc::CopyOnWriteBuffer cp = *packet; TRACE_EVENT0("webrtc", "SRTP Encode"); bool res; uint8_t* data = packet->data(); - int len = static_cast(packet->size()); - if (!rtcp) { + int len = rtc::checked_cast(packet->size()); // If ENABLE_EXTERNAL_AUTH flag is on then packet authentication is not done // inside libsrtp for a RTP packet. A external HMAC module will be writing // a fake HMAC value. This is ONLY done for a RTP packet. // Socket layer will update rtp sendtime extension header if present in // packet with current time before updating the HMAC. #if !defined(ENABLE_EXTERNAL_AUTH) - res = ProtectRtp(data, len, static_cast(packet->capacity()), &len); + res = ProtectRtp(data, len, static_cast(packet->capacity()), &len); #else - if (!IsExternalAuthActive()) { - res = ProtectRtp(data, len, static_cast(packet->capacity()), &len); - } else { - updated_options.packet_time_params.rtp_sendtime_extension_id = - rtp_abs_sendtime_extn_id_; - res = ProtectRtp(data, len, static_cast(packet->capacity()), &len, - &updated_options.packet_time_params.srtp_packet_index); - // If protection succeeds, let's get auth params from srtp. + if (!IsExternalAuthActive()) { + res = ProtectRtp(data, len, static_cast(packet->capacity()), &len); + } else { + updated_options.packet_time_params.rtp_sendtime_extension_id = + rtp_abs_sendtime_extn_id_; + res = ProtectRtp(data, len, static_cast(packet->capacity()), &len, + &updated_options.packet_time_params.srtp_packet_index); + // If protection succeeds, let's get auth params from srtp. + if (res) { + uint8_t* auth_key = nullptr; + int key_len = 0; + res = GetRtpAuthParams( + &auth_key, &key_len, + &updated_options.packet_time_params.srtp_auth_tag_len); if (res) { - uint8_t* auth_key = NULL; - int key_len; - res = GetRtpAuthParams( - &auth_key, &key_len, - &updated_options.packet_time_params.srtp_auth_tag_len); - if (res) { - updated_options.packet_time_params.srtp_auth_key.resize(key_len); - updated_options.packet_time_params.srtp_auth_key.assign( - auth_key, auth_key + key_len); - } + updated_options.packet_time_params.srtp_auth_key.resize(key_len); + updated_options.packet_time_params.srtp_auth_key.assign( + auth_key, auth_key + key_len); } } + } #endif - if (!res) { - int seq_num = -1; - uint32_t ssrc = 0; - cricket::GetRtpSeqNum(data, len, &seq_num); - cricket::GetRtpSsrc(data, len, &ssrc); - RTC_LOG(LS_ERROR) << "Failed to protect RTP packet: size=" << len - << ", seqnum=" << seq_num << ", SSRC=" << ssrc; - return false; - } - } else { - res = ProtectRtcp(data, len, static_cast(packet->capacity()), &len); - if (!res) { - int type = -1; - cricket::GetRtcpType(data, len, &type); - RTC_LOG(LS_ERROR) << "Failed to protect RTCP packet: size=" << len - << ", type=" << type; - return false; - } + if (!res) { + int seq_num = -1; + uint32_t ssrc = 0; + cricket::GetRtpSeqNum(data, len, &seq_num); + cricket::GetRtpSsrc(data, len, &ssrc); + RTC_LOG(LS_ERROR) << "Failed to protect RTP packet: size=" << len + << ", seqnum=" << seq_num << ", SSRC=" << ssrc; + return false; } // Update the length of the packet now that we've added the auth tag. packet->SetSize(len); - return rtcp ? rtp_transport_->SendRtcpPacket(packet, updated_options, flags) - : rtp_transport_->SendRtpPacket(packet, updated_options, flags); + return SendPacket(/*rtcp=*/false, packet, updated_options, flags); } -void SrtpTransport::OnPacketReceived(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketTime& packet_time) { +bool SrtpTransport::SendRtcpPacket(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketOptions& options, + int flags) { + if (!IsSrtpActive()) { + RTC_LOG(LS_ERROR) + << "Failed to send the packet because SRTP transport is inactive."; + return false; + } + + TRACE_EVENT0("webrtc", "SRTP Encode"); + uint8_t* data = packet->data(); + int len = rtc::checked_cast(packet->size()); + if (!ProtectRtcp(data, len, static_cast(packet->capacity()), &len)) { + int type = -1; + cricket::GetRtcpType(data, len, &type); + RTC_LOG(LS_ERROR) << "Failed to protect RTCP packet: size=" << len + << ", type=" << type; + return false; + } + // Update the length of the packet now that we've added the auth tag. + packet->SetSize(len); + + return SendPacket(/*rtcp=*/true, packet, options, flags); +} + +void SrtpTransport::OnRtpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time) { if (!IsSrtpActive()) { RTC_LOG(LS_WARNING) - << "Inactive SRTP transport received a packet. Drop it."; + << "Inactive SRTP transport received an RTP packet. Drop it."; return; } - TRACE_EVENT0("webrtc", "SRTP Decode"); char* data = packet->data(); - int len = static_cast(packet->size()); - bool res; - if (!rtcp) { - res = UnprotectRtp(data, len, &len); - if (!res) { - int seq_num = -1; - uint32_t ssrc = 0; - cricket::GetRtpSeqNum(data, len, &seq_num); - cricket::GetRtpSsrc(data, len, &ssrc); - RTC_LOG(LS_ERROR) << "Failed to unprotect RTP packet: size=" << len - << ", seqnum=" << seq_num << ", SSRC=" << ssrc; - return; - } - } else { - res = UnprotectRtcp(data, len, &len); - if (!res) { - int type = -1; - cricket::GetRtcpType(data, len, &type); - RTC_LOG(LS_ERROR) << "Failed to unprotect RTCP packet: size=" << len - << ", type=" << type; - return; - } + int len = rtc::checked_cast(packet->size()); + if (!UnprotectRtp(data, len, &len)) { + int seq_num = -1; + uint32_t ssrc = 0; + cricket::GetRtpSeqNum(data, len, &seq_num); + cricket::GetRtpSsrc(data, len, &ssrc); + RTC_LOG(LS_ERROR) << "Failed to unprotect RTP packet: size=" << len + << ", seqnum=" << seq_num << ", SSRC=" << ssrc; + return; } - packet->SetSize(len); - SignalPacketReceived(rtcp, packet, packet_time); + DemuxPacket(packet, packet_time); +} + +void SrtpTransport::OnRtcpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time) { + if (!IsSrtpActive()) { + RTC_LOG(LS_WARNING) + << "Inactive SRTP transport received an RTCP packet. Drop it."; + return; + } + TRACE_EVENT0("webrtc", "SRTP Decode"); + char* data = packet->data(); + int len = rtc::checked_cast(packet->size()); + if (!UnprotectRtcp(data, len, &len)) { + int type = -1; + cricket::GetRtcpType(data, len, &type); + RTC_LOG(LS_ERROR) << "Failed to unprotect RTCP packet: size=" << len + << ", type=" << type; + return; + } + packet->SetSize(len); + SignalRtcpPacketReceived(packet, packet_time); } void SrtpTransport::OnNetworkRouteChanged( @@ -269,6 +249,11 @@ void SrtpTransport::OnNetworkRouteChanged( SignalNetworkRouteChanged(network_route); } +void SrtpTransport::OnWritableState( + rtc::PacketTransportInternal* packet_transport) { + SignalWritableState(IsWritable(/*rtcp=*/true) && IsWritable(/*rtcp=*/true)); +} + bool SrtpTransport::SetRtpParams(int send_cs, const uint8_t* send_key, int send_key_len, @@ -309,6 +294,7 @@ bool SrtpTransport::SetRtpParams(int send_cs, RTC_LOG(LS_INFO) << "SRTP " << (new_sessions ? "activated" : "updated") << " with negotiated parameters: send cipher_suite " << send_cs << " recv cipher_suite " << recv_cs; + MaybeUpdateWritableState(); return true; } @@ -347,7 +333,7 @@ bool SrtpTransport::SetRtcpParams(int send_cs, RTC_LOG(LS_INFO) << "SRTCP activated with negotiated parameters:" " send cipher_suite " << send_cs << " recv cipher_suite " << recv_cs; - + MaybeUpdateWritableState(); return true; } @@ -355,11 +341,16 @@ bool SrtpTransport::IsSrtpActive() const { return send_session_ && recv_session_; } +bool SrtpTransport::IsWritable(bool rtcp) const { + return IsSrtpActive() && RtpTransport::IsWritable(rtcp); +} + void SrtpTransport::ResetParams() { send_session_ = nullptr; recv_session_ = nullptr; send_rtcp_session_ = nullptr; recv_rtcp_session_ = nullptr; + MaybeUpdateWritableState(); RTC_LOG(LS_INFO) << "The params in SRTP transport are reset."; } @@ -530,7 +521,15 @@ void SrtpTransport::SetMetricsObserver( if (recv_rtcp_session_) { recv_rtcp_session_->SetMetricsObserver(metrics_observer_); } - rtp_transport_->SetMetricsObserver(metrics_observer); +} + +void SrtpTransport::MaybeUpdateWritableState() { + bool writable = IsWritable(/*rtcp=*/true) && IsWritable(/*rtcp=*/false); + // Only fire the signal if the writable state changes. + if (writable_ != writable) { + writable_ = writable; + SignalWritableState(writable_); + } } } // namespace webrtc diff --git a/pc/srtptransport.h b/pc/srtptransport.h index 5bc9970530..3266e54a05 100644 --- a/pc/srtptransport.h +++ b/pc/srtptransport.h @@ -20,38 +20,20 @@ #include "p2p/base/dtlstransportinternal.h" #include "p2p/base/icetransportinternal.h" #include "pc/rtptransport.h" -#include "pc/rtptransportinternaladapter.h" #include "pc/srtpsession.h" #include "rtc_base/buffer.h" #include "rtc_base/checks.h" namespace webrtc { -// This class will eventually be a wrapper around RtpTransportInternal -// that protects and unprotects sent and received RTP packets. -class SrtpTransport : public RtpTransportInternalAdapter { +// This subclass of the RtpTransport is used for SRTP which is reponsible for +// protecting/unprotecting the packets. It provides interfaces to set the crypto +// parameters for the SrtpSession underneath. +class SrtpTransport : public RtpTransport { public: explicit SrtpTransport(bool rtcp_mux_enabled); - explicit SrtpTransport(std::unique_ptr rtp_transport); - - virtual ~SrtpTransport() {} - - // SrtpTransportInterface overrides. - PacketTransportInterface* GetRtpPacketTransport() const override { - return rtp_transport_->GetRtpPacketTransport(); - } - PacketTransportInterface* GetRtcpPacketTransport() const override { - return rtp_transport_->GetRtcpPacketTransport(); - } - - // TODO(zstein): Use these RtcpParameters for configuration elsewhere. - RTCError SetParameters(const RtpTransportParameters& parameters) override { - return rtp_transport_->SetParameters(parameters); - } - RtpTransportParameters GetParameters() const override { - return rtp_transport_->GetParameters(); - } + virtual ~SrtpTransport() = default; // SrtpTransportInterface specific implementation. RTCError SetSrtpSendKey(const cricket::CryptoParams& params) override; @@ -69,6 +51,8 @@ class SrtpTransport : public RtpTransportInternalAdapter { // created. bool IsSrtpActive() const override; + bool IsWritable(bool rtcp) const override; + // Create new send/recv sessions and set the negotiated crypto keys for RTP // packet encryption. The keys can either come from SDES negotiation or DTLS // handshake. @@ -120,29 +104,23 @@ class SrtpTransport : public RtpTransportInternalAdapter { rtp_abs_sendtime_extn_id_ = rtp_abs_sendtime_extn_id; } - void SetMetricsObserver( - rtc::scoped_refptr metrics_observer) override; + protected: + // If the writable state changed, fire the SignalWritableState. + void MaybeUpdateWritableState(); private: void ConnectToRtpTransport(); void CreateSrtpSessions(); - bool SendPacket(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketOptions& options, - int flags); + void OnRtpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time) override; + void OnRtcpPacketReceived(rtc::CopyOnWriteBuffer* packet, + const rtc::PacketTime& packet_time) override; + void OnNetworkRouteChanged( + rtc::Optional network_route) override; - void OnPacketReceived(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketTime& packet_time); - void OnReadyToSend(bool ready) { SignalReadyToSend(ready); } - void OnNetworkRouteChanged(rtc::Optional network_route); - - void OnWritableState(bool writable) { SignalWritableState(writable); } - - void OnSentPacket(const rtc::SentPacket& sent_packet) { - SignalSentPacket(sent_packet); - } + // Override the RtpTransport::OnWritableState. + void OnWritableState(rtc::PacketTransportInternal* packet_transport) override; bool ProtectRtp(void* data, int in_len, int max_len, int* out_len); @@ -163,8 +141,10 @@ class SrtpTransport : public RtpTransportInternalAdapter { bool MaybeSetKeyParams(); bool ParseKeyParams(const std::string& key_params, uint8_t* key, size_t len); + void SetMetricsObserver( + rtc::scoped_refptr metrics_observer) override; + const std::string content_name_; - std::unique_ptr rtp_transport_; std::unique_ptr send_session_; std::unique_ptr recv_session_; @@ -178,6 +158,8 @@ class SrtpTransport : public RtpTransportInternalAdapter { rtc::ZeroOnFreeBuffer send_key_; rtc::ZeroOnFreeBuffer recv_key_; + bool writable_ = false; + bool external_auth_enabled_ = false; int rtp_abs_sendtime_extn_id_ = -1; diff --git a/pc/srtptransport_unittest.cc b/pc/srtptransport_unittest.cc index 30e30f656f..5af45c8111 100644 --- a/pc/srtptransport_unittest.cc +++ b/pc/srtptransport_unittest.cc @@ -42,8 +42,6 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { protected: SrtpTransportTest() { bool rtcp_mux_enabled = true; - auto rtp_transport1 = rtc::MakeUnique(rtcp_mux_enabled); - auto rtp_transport2 = rtc::MakeUnique(rtcp_mux_enabled); rtp_packet_transport1_ = rtc::MakeUnique("fake_packet_transport1"); @@ -54,32 +52,32 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { rtp_packet_transport1_->SetDestination(rtp_packet_transport2_.get(), asymmetric); - rtp_transport1->SetRtpPacketTransport(rtp_packet_transport1_.get()); - rtp_transport2->SetRtpPacketTransport(rtp_packet_transport2_.get()); + srtp_transport1_ = rtc::MakeUnique(rtcp_mux_enabled); + srtp_transport2_ = rtc::MakeUnique(rtcp_mux_enabled); - srtp_transport1_ = - rtc::MakeUnique(std::move(rtp_transport1)); - srtp_transport2_ = - rtc::MakeUnique(std::move(rtp_transport2)); + srtp_transport1_->SetRtpPacketTransport(rtp_packet_transport1_.get()); + srtp_transport2_->SetRtpPacketTransport(rtp_packet_transport2_.get()); - srtp_transport1_->SignalPacketReceived.connect( - this, &SrtpTransportTest::OnPacketReceived1); - srtp_transport2_->SignalPacketReceived.connect( - this, &SrtpTransportTest::OnPacketReceived2); + srtp_transport1_->SignalRtcpPacketReceived.connect( + &rtp_sink1_, &TransportObserver::OnRtcpPacketReceived); + srtp_transport2_->SignalRtcpPacketReceived.connect( + &rtp_sink2_, &TransportObserver::OnRtcpPacketReceived); + + RtpDemuxerCriteria demuxer_criteria; + // 0x00 is the payload type used in kPcmuFrame. + demuxer_criteria.payload_types = {0x00}; + + srtp_transport1_->RegisterRtpDemuxerSink(demuxer_criteria, &rtp_sink1_); + srtp_transport2_->RegisterRtpDemuxerSink(demuxer_criteria, &rtp_sink2_); } - void OnPacketReceived1(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketTime& packet_time) { - RTC_LOG(LS_INFO) << "SrtpTransport1 Received a packet."; - last_recv_packet1_ = *packet; - } - - void OnPacketReceived2(bool rtcp, - rtc::CopyOnWriteBuffer* packet, - const rtc::PacketTime& packet_time) { - RTC_LOG(LS_INFO) << "SrtpTransport2 Received a packet."; - last_recv_packet2_ = *packet; + ~SrtpTransportTest() { + if (srtp_transport1_) { + srtp_transport1_->UnregisterRtpDemuxerSink(&rtp_sink1_); + } + if (srtp_transport2_) { + srtp_transport2_->UnregisterRtpDemuxerSink(&rtp_sink2_); + } } // With external auth enabled, SRTP doesn't write the auth tag and @@ -136,9 +134,9 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { if (srtp_transport1_->IsExternalAuthActive()) { TestRtpAuthParams(srtp_transport1_.get(), cipher_suite_name); } else { - ASSERT_TRUE(last_recv_packet2_.data()); - EXPECT_EQ(0, - memcmp(last_recv_packet2_.data(), original_rtp_data, rtp_len)); + ASSERT_TRUE(rtp_sink2_.last_recv_rtp_packet().data()); + EXPECT_EQ(0, memcmp(rtp_sink2_.last_recv_rtp_packet().data(), + original_rtp_data, rtp_len)); // Get the encrypted packet from underneath packet transport and verify // the data is actually encrypted. auto fake_rtp_packet_transport = static_cast( @@ -153,9 +151,9 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { if (srtp_transport2_->IsExternalAuthActive()) { TestRtpAuthParams(srtp_transport2_.get(), cipher_suite_name); } else { - ASSERT_TRUE(last_recv_packet1_.data()); - EXPECT_EQ(0, - memcmp(last_recv_packet1_.data(), original_rtp_data, rtp_len)); + ASSERT_TRUE(rtp_sink1_.last_recv_rtp_packet().data()); + EXPECT_EQ(0, memcmp(rtp_sink1_.last_recv_rtp_packet().data(), + original_rtp_data, rtp_len)); auto fake_rtp_packet_transport = static_cast( srtp_transport2_->rtp_packet_transport()); EXPECT_NE(0, memcmp(fake_rtp_packet_transport->last_sent_packet()->data(), @@ -164,12 +162,12 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { } void TestSendRecvRtcpPacket(const std::string& cipher_suite_name) { - size_t rtcp_len = sizeof(kRtcpReport); + size_t rtcp_len = sizeof(::kRtcpReport); size_t packet_size = rtcp_len + 4 + rtc::rtcp_auth_tag_len(cipher_suite_name); rtc::Buffer rtcp_packet_buffer(packet_size); char* rtcp_packet_data = rtcp_packet_buffer.data(); - memcpy(rtcp_packet_data, kRtcpReport, rtcp_len); + memcpy(rtcp_packet_data, ::kRtcpReport, rtcp_len); rtc::CopyOnWriteBuffer rtcp_packet1to2(rtcp_packet_data, rtcp_len, packet_size); @@ -181,8 +179,9 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { // that the packet can be successfully received and decrypted. ASSERT_TRUE(srtp_transport1_->SendRtcpPacket(&rtcp_packet1to2, options, cricket::PF_SRTP_BYPASS)); - ASSERT_TRUE(last_recv_packet2_.data()); - EXPECT_EQ(0, memcmp(last_recv_packet2_.data(), rtcp_packet_data, rtcp_len)); + ASSERT_TRUE(rtp_sink2_.last_recv_rtcp_packet().data()); + EXPECT_EQ(0, memcmp(rtp_sink2_.last_recv_rtcp_packet().data(), + rtcp_packet_data, rtcp_len)); // Get the encrypted packet from underneath packet transport and verify the // data is actually encrypted. auto fake_rtp_packet_transport = static_cast( @@ -193,8 +192,9 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { // Do the same thing in the opposite direction; ASSERT_TRUE(srtp_transport2_->SendRtcpPacket(&rtcp_packet2to1, options, cricket::PF_SRTP_BYPASS)); - ASSERT_TRUE(last_recv_packet1_.data()); - EXPECT_EQ(0, memcmp(last_recv_packet1_.data(), rtcp_packet_data, rtcp_len)); + ASSERT_TRUE(rtp_sink1_.last_recv_rtcp_packet().data()); + EXPECT_EQ(0, memcmp(rtp_sink1_.last_recv_rtcp_packet().data(), + rtcp_packet_data, rtcp_len)); fake_rtp_packet_transport = static_cast( srtp_transport2_->rtp_packet_transport()); EXPECT_NE(0, memcmp(fake_rtp_packet_transport->last_sent_packet()->data(), @@ -261,8 +261,9 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { // that the packet can be successfully received and decrypted. ASSERT_TRUE(srtp_transport1_->SendRtpPacket(&rtp_packet1to2, options, cricket::PF_SRTP_BYPASS)); - ASSERT_TRUE(last_recv_packet2_.data()); - EXPECT_EQ(0, memcmp(last_recv_packet2_.data(), original_rtp_data, rtp_len)); + ASSERT_TRUE(rtp_sink2_.last_recv_rtp_packet().data()); + EXPECT_EQ(0, memcmp(rtp_sink2_.last_recv_rtp_packet().data(), + original_rtp_data, rtp_len)); // Get the encrypted packet from underneath packet transport and verify the // data and header extension are actually encrypted. auto fake_rtp_packet_transport = static_cast( @@ -278,8 +279,9 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { // Do the same thing in the opposite direction; ASSERT_TRUE(srtp_transport2_->SendRtpPacket(&rtp_packet2to1, options, cricket::PF_SRTP_BYPASS)); - ASSERT_TRUE(last_recv_packet1_.data()); - EXPECT_EQ(0, memcmp(last_recv_packet1_.data(), original_rtp_data, rtp_len)); + ASSERT_TRUE(rtp_sink1_.last_recv_rtp_packet().data()); + EXPECT_EQ(0, memcmp(rtp_sink1_.last_recv_rtp_packet().data(), + original_rtp_data, rtp_len)); fake_rtp_packet_transport = static_cast( srtp_transport2_->rtp_packet_transport()); EXPECT_NE(0, memcmp(fake_rtp_packet_transport->last_sent_packet()->data(), @@ -322,8 +324,9 @@ class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr rtp_packet_transport1_; std::unique_ptr rtp_packet_transport2_; - rtc::CopyOnWriteBuffer last_recv_packet1_; - rtc::CopyOnWriteBuffer last_recv_packet2_; + TransportObserver rtp_sink1_; + TransportObserver rtp_sink2_; + int sequence_number_ = 0; };