From c0e58a3438c7bcedf9a197d232d1ca6a530ce15a Mon Sep 17 00:00:00 2001 From: mflodman Date: Mon, 25 Apr 2016 01:26:26 -0700 Subject: [PATCH] Move receive RtpRtcp ownership from ViEChannel to ViEReceiver. This is a first CL in a row of CLs to move everything related to receiving RTP from ViEChanel to ViEReceiver, rename the classes and move ViEReceiver ownership to VideoReceiveStream. Review URL: https://codereview.webrtc.org/1912133002 Cr-Commit-Position: refs/heads/master@{#12486} --- webrtc/video/vie_channel.cc | 72 ++++-------------------------------- webrtc/video/vie_channel.h | 51 ++----------------------- webrtc/video/vie_receiver.cc | 66 ++++++++++++++++++++++++++++++--- webrtc/video/vie_receiver.h | 58 +++++++++++++++++++++++++---- 4 files changed, 121 insertions(+), 126 deletions(-) diff --git a/webrtc/video/vie_channel.cc b/webrtc/video/vie_channel.cc index 5416e39472..a8ee4bcad0 100644 --- a/webrtc/video/vie_channel.cc +++ b/webrtc/video/vie_channel.cc @@ -20,8 +20,6 @@ #include "webrtc/common_video/include/frame_callback.h" #include "webrtc/common_video/include/incoming_video_stream.h" #include "webrtc/common_video/libyuv/include/webrtc_libyuv.h" -#include "webrtc/modules/pacing/paced_sender.h" -#include "webrtc/modules/pacing/packet_router.h" #include "webrtc/modules/rtp_rtcp/include/rtp_receiver.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/utility/include/process_thread.h" @@ -38,44 +36,6 @@ namespace webrtc { static const int kMaxPacketAgeToNack = 450; static const int kMaxNackListSize = 250; -namespace { - -std::unique_ptr CreateRtpRtcpModule( - ReceiveStatistics* receive_statistics, - Transport* outgoing_transport, - RtcpRttStats* rtt_stats, - RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer, - RemoteBitrateEstimator* remote_bitrate_estimator, - RtpPacketSender* paced_sender, - TransportSequenceNumberAllocator* transport_sequence_number_allocator) { - RtpRtcp::Configuration configuration; - configuration.audio = false; - configuration.receiver_only = true; - configuration.receive_statistics = receive_statistics; - configuration.outgoing_transport = outgoing_transport; - configuration.intra_frame_callback = nullptr; - configuration.rtt_stats = rtt_stats; - configuration.rtcp_packet_type_counter_observer = - rtcp_packet_type_counter_observer; - configuration.paced_sender = paced_sender; - configuration.transport_sequence_number_allocator = - transport_sequence_number_allocator; - configuration.send_bitrate_observer = nullptr; - configuration.send_frame_count_observer = nullptr; - configuration.send_side_delay_observer = nullptr; - configuration.bandwidth_callback = nullptr; - configuration.transport_feedback_callback = nullptr; - - std::unique_ptr rtp_rtcp(RtpRtcp::CreateRtpRtcp(configuration)); - rtp_rtcp->SetSendingStatus(false); - rtp_rtcp->SetSendingMediaStatus(false); - rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound); - - return rtp_rtcp; -} - -} // namespace - // Helper class receiving statistics callbacks. class ChannelStatsObserver : public CallStatsObserver { public: @@ -100,24 +60,15 @@ ViEChannel::ViEChannel(Transport* transport, PacketRouter* packet_router) : module_process_thread_(module_process_thread), video_receiver_(video_receiver), - vie_receiver_(video_receiver_, remote_bitrate_estimator, this), + vie_receiver_(video_receiver, remote_bitrate_estimator, this, transport, + rtt_stats, paced_sender, packet_router), + rtp_rtcp_(vie_receiver_.rtp_rtcp()), stats_observer_(new ChannelStatsObserver(this)), receive_stats_callback_(nullptr), incoming_video_stream_(nullptr), - rtt_stats_(rtt_stats), - paced_sender_(paced_sender), - packet_router_(packet_router), max_nack_reordering_threshold_(kMaxPacketAgeToNack), pre_render_callback_(nullptr), - last_rtt_ms_(0), - rtp_rtcp_(CreateRtpRtcpModule(vie_receiver_.GetReceiveStatistics(), - transport, - rtt_stats_, - &rtcp_packet_type_counter_observer_, - remote_bitrate_estimator, - paced_sender_, - packet_router_)) { - vie_receiver_.Init(rtp_rtcp_.get()); + last_rtt_ms_(0) { RTC_DCHECK(video_receiver_); video_receiver_->SetNackSettings(kMaxNackListSize, max_nack_reordering_threshold_, 0); @@ -126,12 +77,8 @@ ViEChannel::ViEChannel(Transport* transport, int32_t ViEChannel::Init() { static const int kDefaultRenderDelayMs = 10; module_process_thread_->RegisterModule(vie_receiver_.GetReceiveStatistics()); + module_process_thread_->RegisterModule(rtp_rtcp_); - // RTP/RTCP initialization. - module_process_thread_->RegisterModule(rtp_rtcp_.get()); - packet_router_->AddRtpModule(rtp_rtcp_.get()); - - rtp_rtcp_->SetKeyFrameRequestMethod(kKeyFrameReqPliRtcp); if (video_receiver_->RegisterReceiveCallback(this) != 0) { return -1; } @@ -148,8 +95,7 @@ ViEChannel::~ViEChannel() { module_process_thread_->DeRegisterModule( vie_receiver_.GetReceiveStatistics()); - packet_router_->RemoveRtpModule(rtp_rtcp_.get()); - module_process_thread_->DeRegisterModule(rtp_rtcp_.get()); + module_process_thread_->DeRegisterModule(rtp_rtcp_); } void ViEChannel::SetProtectionMode(bool enable_nack, @@ -224,11 +170,7 @@ RtpState ViEChannel::GetRtpStateForSsrc(uint32_t ssrc) const { void ViEChannel::RegisterRtcpPacketTypeCounterObserver( RtcpPacketTypeCounterObserver* observer) { - rtcp_packet_type_counter_observer_.Set(observer); -} - -RtpRtcp* ViEChannel::rtp_rtcp() const { - return rtp_rtcp_.get(); + vie_receiver_.RegisterRtcpPacketTypeCounterObserver(observer); } ViEReceiver* ViEChannel::vie_receiver() { diff --git a/webrtc/video/vie_channel.h b/webrtc/video/vie_channel.h index 98c3f85b4c..597ac5bfe0 100644 --- a/webrtc/video/vie_channel.h +++ b/webrtc/video/vie_channel.h @@ -75,6 +75,8 @@ class ViEChannel : public VCMFrameTypeCallback, int32_t Init(); + RtpRtcp* rtp_rtcp() const { return rtp_rtcp_; } + void SetProtectionMode(bool enable_nack, bool enable_fec, int payload_type_red, @@ -92,7 +94,6 @@ class ViEChannel : public VCMFrameTypeCallback, void OnIncomingCSRCChanged(const uint32_t CSRC, const bool added) override; // Gets the module used by the channel. - RtpRtcp* rtp_rtcp() const; ViEReceiver* vie_receiver(); CallStatsObserver* GetStatsObserver(); @@ -153,47 +154,6 @@ class ViEChannel : public VCMFrameTypeCallback, // Compute NACK list parameters for the buffering mode. int GetRequiredNackListSize(int target_delay_ms); - // ViEChannel exposes methods that allow to modify observers and callbacks - // to be modified. Such an API-style is cumbersome to implement and maintain - // at all the levels when comparing to only setting them at construction. As - // so this class instantiates its children with a wrapper that can be modified - // at a later time. - template - class RegisterableCallback : public T { - public: - RegisterableCallback() : callback_(nullptr) {} - - void Set(T* callback) { - rtc::CritScope lock(&critsect_); - callback_ = callback; - } - - protected: - // Note: this should be implemented with a RW-lock to allow simultaneous - // calls into the callback. However that doesn't seem to be needed for the - // current type of callbacks covered by this class. - rtc::CriticalSection critsect_; - T* callback_ GUARDED_BY(critsect_); - - private: - RTC_DISALLOW_COPY_AND_ASSIGN(RegisterableCallback); - }; - - class RegisterableRtcpPacketTypeCounterObserver - : public RegisterableCallback { - public: - void RtcpPacketTypesCounterUpdated( - uint32_t ssrc, - const RtcpPacketTypeCounter& packet_counter) override { - rtc::CritScope lock(&critsect_); - if (callback_) - callback_->RtcpPacketTypesCounterUpdated(ssrc, packet_counter); - } - - private: - } rtcp_packet_type_counter_observer_; - - ProcessThread* const module_process_thread_; // Used for all registered callbacks except rendering. @@ -201,6 +161,7 @@ class ViEChannel : public VCMFrameTypeCallback, vcm::VideoReceiver* const video_receiver_; ViEReceiver vie_receiver_; + RtpRtcp* const rtp_rtcp_; // Helper to report call statistics. std::unique_ptr stats_observer_; @@ -209,17 +170,11 @@ class ViEChannel : public VCMFrameTypeCallback, ReceiveStatisticsProxy* receive_stats_callback_ GUARDED_BY(crit_); FrameCounts receive_frame_counts_ GUARDED_BY(crit_); IncomingVideoStream* incoming_video_stream_ GUARDED_BY(crit_); - RtcpRttStats* const rtt_stats_; - PacedSender* const paced_sender_; - PacketRouter* const packet_router_; int max_nack_reordering_threshold_; I420FrameCallback* pre_render_callback_ GUARDED_BY(crit_); int64_t last_rtt_ms_ GUARDED_BY(crit_); - - // RtpRtcp module, declared last as it use other members on construction. - const std::unique_ptr rtp_rtcp_; }; } // namespace webrtc diff --git a/webrtc/video/vie_receiver.cc b/webrtc/video/vie_receiver.cc index d6a5aa506e..95d2f6fc8e 100644 --- a/webrtc/video/vie_receiver.cc +++ b/webrtc/video/vie_receiver.cc @@ -14,6 +14,7 @@ #include "webrtc/base/logging.h" #include "webrtc/config.h" +#include "webrtc/modules/pacing/packet_router.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/rtp_rtcp/include/fec_receiver.h" #include "webrtc/modules/rtp_rtcp/include/receive_statistics.h" @@ -29,14 +30,54 @@ namespace webrtc { +std::unique_ptr CreateRtpRtcpModule( + ReceiveStatistics* receive_statistics, + Transport* outgoing_transport, + RtcpRttStats* rtt_stats, + RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer, + RemoteBitrateEstimator* remote_bitrate_estimator, + RtpPacketSender* paced_sender, + TransportSequenceNumberAllocator* transport_sequence_number_allocator) { + RtpRtcp::Configuration configuration; + configuration.audio = false; + configuration.receiver_only = true; + configuration.receive_statistics = receive_statistics; + configuration.outgoing_transport = outgoing_transport; + configuration.intra_frame_callback = nullptr; + configuration.rtt_stats = rtt_stats; + configuration.rtcp_packet_type_counter_observer = + rtcp_packet_type_counter_observer; + configuration.paced_sender = paced_sender; + configuration.transport_sequence_number_allocator = + transport_sequence_number_allocator; + configuration.send_bitrate_observer = nullptr; + configuration.send_frame_count_observer = nullptr; + configuration.send_side_delay_observer = nullptr; + configuration.bandwidth_callback = nullptr; + configuration.transport_feedback_callback = nullptr; + + std::unique_ptr rtp_rtcp(RtpRtcp::CreateRtpRtcp(configuration)); + rtp_rtcp->SetSendingStatus(false); + rtp_rtcp->SetSendingMediaStatus(false); + rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound); + + return rtp_rtcp; +} + + static const int kPacketLogIntervalMs = 10000; ViEReceiver::ViEReceiver(vcm::VideoReceiver* video_receiver, RemoteBitrateEstimator* remote_bitrate_estimator, - RtpFeedback* rtp_feedback) + RtpFeedback* rtp_feedback, + Transport* transport, + RtcpRttStats* rtt_stats, + PacedSender* paced_sender, + PacketRouter* packet_router) : clock_(Clock::GetRealTimeClock()), video_receiver_(video_receiver), remote_bitrate_estimator_(remote_bitrate_estimator), + packet_router_(packet_router), ntp_estimator_(clock_), rtp_payload_registry_(RTPPayloadStrategy::CreateStrategy(false)), rtp_header_parser_(RtpHeaderParser::Create()), @@ -48,9 +89,20 @@ ViEReceiver::ViEReceiver(vcm::VideoReceiver* video_receiver, fec_receiver_(FecReceiver::Create(this)), receiving_(false), restored_packet_in_use_(false), - last_packet_log_ms_(-1) {} + last_packet_log_ms_(-1), + rtp_rtcp_(CreateRtpRtcpModule(rtp_receive_statistics_.get(), + transport, + rtt_stats, + &rtcp_packet_type_counter_observer_, + remote_bitrate_estimator_, + paced_sender, + packet_router)) { + packet_router_->AddRtpModule(rtp_rtcp_.get()); + rtp_rtcp_->SetKeyFrameRequestMethod(kKeyFrameReqPliRtcp); +} ViEReceiver::~ViEReceiver() { + packet_router_->RemoveRtpModule(rtp_rtcp_.get()); UpdateHistograms(); } @@ -124,10 +176,6 @@ int ViEReceiver::GetCsrcs(uint32_t* csrcs) const { return rtp_receiver_->CSRCs(csrcs); } -void ViEReceiver::Init(RtpRtcp* rtp_rtcp) { - rtp_rtcp_ = rtp_rtcp; -} - RtpReceiver* ViEReceiver::GetRtpReceiver() const { return rtp_receiver_.get(); } @@ -139,6 +187,12 @@ void ViEReceiver::EnableReceiveRtpHeaderExtension(const std::string& extension, StringToRtpExtensionType(extension), id)); } +void ViEReceiver::RegisterRtcpPacketTypeCounterObserver( + RtcpPacketTypeCounterObserver* observer) { + rtcp_packet_type_counter_observer_.Set(observer); +} + + int32_t ViEReceiver::OnReceivedPayloadData(const uint8_t* payload_data, const size_t payload_size, const WebRtcRTPHeader* rtp_header) { diff --git a/webrtc/video/vie_receiver.h b/webrtc/video/vie_receiver.h index 97a0fe4993..6803254e22 100644 --- a/webrtc/video/vie_receiver.h +++ b/webrtc/video/vie_receiver.h @@ -21,19 +21,23 @@ #include "webrtc/modules/rtp_rtcp/include/receive_statistics.h" #include "webrtc/modules/rtp_rtcp/include/remote_ntp_time_estimator.h" #include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h" +#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/typedefs.h" namespace webrtc { class FecReceiver; +class PacedSender; +class PacketRouter; class RemoteNtpTimeEstimator; class ReceiveStatistics; class RemoteBitrateEstimator; +class RtcpRttStats; class RtpHeaderParser; class RTPPayloadRegistry; class RtpReceiver; -class RtpRtcp; +class Transport; namespace vcm { class VideoReceiver; @@ -43,7 +47,11 @@ class ViEReceiver : public RtpData { public: ViEReceiver(vcm::VideoReceiver* video_receiver, RemoteBitrateEstimator* remote_bitrate_estimator, - RtpFeedback* rtp_feedback); + RtpFeedback* rtp_feedback, + Transport* transport, + RtcpRttStats* rtt_stats, + PacedSender* paced_sender, + PacketRouter* packet_router); ~ViEReceiver(); bool SetReceiveCodec(const VideoCodec& video_codec); @@ -63,11 +71,12 @@ class ViEReceiver : public RtpData { uint32_t GetRemoteSsrc() const; int GetCsrcs(uint32_t* csrcs) const; - void Init(RtpRtcp* rtp_rtcp); - RtpReceiver* GetRtpReceiver() const; + RtpRtcp* rtp_rtcp() const { return rtp_rtcp_.get(); } void EnableReceiveRtpHeaderExtension(const std::string& extension, int id); + void RegisterRtcpPacketTypeCounterObserver( + RtcpPacketTypeCounterObserver* observer); void StartReceive(); void StopReceive(); @@ -85,6 +94,41 @@ class ViEReceiver : public RtpData { ReceiveStatistics* GetReceiveStatistics() const; + template + class RegisterableCallback : public T { + public: + RegisterableCallback() : callback_(nullptr) {} + + void Set(T* callback) { + rtc::CritScope lock(&critsect_); + callback_ = callback; + } + + protected: + // Note: this should be implemented with a RW-lock to allow simultaneous + // calls into the callback. However that doesn't seem to be needed for the + // current type of callbacks covered by this class. + rtc::CriticalSection critsect_; + T* callback_ GUARDED_BY(critsect_); + + private: + RTC_DISALLOW_COPY_AND_ASSIGN(RegisterableCallback); + }; + + class RegisterableRtcpPacketTypeCounterObserver + : public RegisterableCallback { + public: + void RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) override { + rtc::CritScope lock(&critsect_); + if (callback_) + callback_->RtcpPacketTypesCounterUpdated(ssrc, packet_counter); + } + + private: + } rtcp_packet_type_counter_observer_; + private: bool ReceivePacket(const uint8_t* packet, size_t packet_length, @@ -103,9 +147,7 @@ class ViEReceiver : public RtpData { Clock* const clock_; vcm::VideoReceiver* const video_receiver_; RemoteBitrateEstimator* const remote_bitrate_estimator_; - - // TODO(pbos): Make const and set on construction. - RtpRtcp* rtp_rtcp_; // Owned by ViEChannel + PacketRouter* const packet_router_; RemoteNtpTimeEstimator ntp_estimator_; RTPPayloadRegistry rtp_payload_registry_; @@ -120,6 +162,8 @@ class ViEReceiver : public RtpData { uint8_t restored_packet_[IP_PACKET_SIZE] GUARDED_BY(receive_cs_); bool restored_packet_in_use_ GUARDED_BY(receive_cs_); int64_t last_packet_log_ms_ GUARDED_BY(receive_cs_); + + const std::unique_ptr rtp_rtcp_; }; } // namespace webrtc