Adjust RtcpReceiver to be testable with callbacks:
Instead of full RtpRtcpImpl takes interface of all functions it needs from it.
Added single function for parsing packets and sending feedback, moving that
logic from RtpRtcpImpl to RtcpReceiver.
BUG=webrtc:5260
Review-Url: https://codereview.webrtc.org/2274573002
Cr-Commit-Position: refs/heads/master@{#13960}
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
index 9c0d66d..4d30bb0 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -20,7 +20,6 @@
#include "webrtc/base/trace_event.h"
#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h"
-#include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h"
#include "webrtc/modules/rtp_rtcp/source/time_util.h"
#include "webrtc/modules/rtp_rtcp/source/tmmbr_help.h"
#include "webrtc/system_wrappers/include/ntp_time.h"
@@ -46,7 +45,7 @@
RtcpBandwidthObserver* rtcp_bandwidth_observer,
RtcpIntraFrameObserver* rtcp_intra_frame_observer,
TransportFeedbackObserver* transport_feedback_observer,
- ModuleRtpRtcpImpl* owner)
+ ModuleRtpRtcp* owner)
: _clock(clock),
receiver_only_(receiver_only),
_lastReceived(0),
@@ -97,6 +96,20 @@
}
}
+bool RTCPReceiver::IncomingPacket(const uint8_t* packet, size_t packet_size) {
+ // Allow receive of non-compound RTCP packets.
+ RTCPUtility::RTCPParserV2 rtcp_parser(packet, packet_size, true);
+
+ if (!rtcp_parser.IsValid()) {
+ LOG(LS_WARNING) << "Incoming invalid RTCP packet";
+ return false;
+ }
+ RTCPHelp::RTCPPacketInformation rtcp_packet_information;
+ IncomingRTCPPacket(rtcp_packet_information, &rtcp_parser);
+ TriggerCallbacksFromRTCPPacket(rtcp_packet_information);
+ return true;
+}
+
int64_t RTCPReceiver::LastReceived() {
rtc::CritScope lock(&_criticalSectionRTCPReceiver);
return _lastReceived;
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h
index 747a75e..3659374 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h
@@ -20,23 +20,39 @@
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h"
#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h"
-#include "webrtc/modules/rtp_rtcp/source/rtp_utility.h"
#include "webrtc/typedefs.h"
namespace webrtc {
-class ModuleRtpRtcpImpl;
+namespace rtcp {
+class TmmbItem;
+} // namespace rtcp
class RTCPReceiver {
public:
+ class ModuleRtpRtcp {
+ public:
+ virtual void SetTmmbn(std::vector<rtcp::TmmbItem> bounding_set) = 0;
+ virtual void OnRequestSendReport() = 0;
+ virtual void OnReceivedNack(
+ const std::vector<uint16_t>& nack_sequence_numbers) = 0;
+ virtual void OnReceivedRtcpReportBlocks(
+ const ReportBlockList& report_blocks) = 0;
+
+ protected:
+ virtual ~ModuleRtpRtcp() = default;
+ };
+
RTCPReceiver(Clock* clock,
bool receiver_only,
RtcpPacketTypeCounterObserver* packet_type_counter_observer,
RtcpBandwidthObserver* rtcp_bandwidth_observer,
RtcpIntraFrameObserver* rtcp_intra_frame_observer,
TransportFeedbackObserver* transport_feedback_observer,
- ModuleRtpRtcpImpl* owner);
+ ModuleRtpRtcp* owner);
virtual ~RTCPReceiver();
+ bool IncomingPacket(const uint8_t* packet, size_t packet_size);
+
int64_t LastReceived();
int64_t LastReceivedReceiverReport() const;
@@ -257,7 +273,7 @@
Clock* const _clock;
const bool receiver_only_;
int64_t _lastReceived;
- ModuleRtpRtcpImpl& _rtpRtcp;
+ ModuleRtpRtcp& _rtpRtcp;
rtc::CriticalSection _criticalSectionFeedbacks;
RtcpBandwidthObserver* const _cbRtcpBandwidthObserver;
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index ba7ca7e..78ad5a0 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -223,21 +223,7 @@
int32_t ModuleRtpRtcpImpl::IncomingRtcpPacket(
const uint8_t* rtcp_packet,
const size_t length) {
- // Allow receive of non-compound RTCP packets.
- RTCPUtility::RTCPParserV2 rtcp_parser(rtcp_packet, length, true);
-
- const bool valid_rtcpheader = rtcp_parser.IsValid();
- if (!valid_rtcpheader) {
- LOG(LS_WARNING) << "Incoming invalid RTCP packet";
- return -1;
- }
- RTCPHelp::RTCPPacketInformation rtcp_packet_information;
- int32_t ret_val =
- rtcp_receiver_.IncomingRTCPPacket(rtcp_packet_information, &rtcp_parser);
- if (ret_val == 0) {
- rtcp_receiver_.TriggerCallbacksFromRTCPPacket(rtcp_packet_information);
- }
- return ret_val;
+ return rtcp_receiver_.IncomingPacket(rtcp_packet, length) ? 0 : -1;
}
int32_t ModuleRtpRtcpImpl::RegisterSendPayload(
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index 80e76e3..6d2e641 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -26,7 +26,7 @@
namespace webrtc {
-class ModuleRtpRtcpImpl : public RtpRtcp {
+class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
public:
explicit ModuleRtpRtcpImpl(const RtpRtcp::Configuration& configuration);
@@ -204,7 +204,7 @@
void SetTMMBRStatus(bool enable) override;
- void SetTmmbn(std::vector<rtcp::TmmbItem> bounding_set);
+ void SetTmmbn(std::vector<rtcp::TmmbItem> bounding_set) override;
uint16_t MaxPayloadLength() const override;
@@ -311,10 +311,11 @@
StreamDataCountersCallback* GetSendChannelRtpStatisticsCallback()
const override;
- void OnReceivedNack(const std::vector<uint16_t>& nack_sequence_numbers);
- void OnReceivedRtcpReportBlocks(const ReportBlockList& report_blocks);
-
- void OnRequestSendReport();
+ void OnReceivedNack(
+ const std::vector<uint16_t>& nack_sequence_numbers) override;
+ void OnReceivedRtcpReportBlocks(
+ const ReportBlockList& report_blocks) override;
+ void OnRequestSendReport() override;
protected:
bool UpdateRTCPReceiveInformationTimers();