[video] Plumbing of ReportBlockData from RTCPReceiver to MediaSenderInfo
This is part of implementing RTCRemoteInboundRtpStreamStats. The CL was
split up into smaller pieces for reviewability. Spec:
https://w3c.github.io/webrtc-stats/#dom-rtcremoteinboundrtpstreamstats
In [1], ReportBlockData was implemented and tested.
This CL adds the plumbing to make it available in MediaSenderInfo for
video streams, but the code is not wired up to make use of these stats.
In follow-up CL [2], ReportBlockData will be used to implement
RTCRemoteInboundRtpStreamStats. The follow-up CL will add integration
tests exercising the full code path.
[1] https://webrtc-review.googlesource.com/c/src/+/136584
[2] https://webrtc-review.googlesource.com/c/src/+/138067
Bug: webrtc:10456
Change-Id: Icd20452cb4b4908203b28ae9d9f52c25693cf91d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138065
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28071}
diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h
index 450b65b..0db4f80 100644
--- a/call/rtp_transport_controller_send_interface.h
+++ b/call/rtp_transport_controller_send_interface.h
@@ -25,6 +25,7 @@
#include "api/transport/bitrate_settings.h"
#include "call/rtp_config.h"
#include "logging/rtc_event_log/rtc_event_log.h"
+#include "modules/rtp_rtcp/include/report_block_data.h"
#include "modules/rtp_rtcp/include/rtcp_statistics.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
@@ -56,6 +57,7 @@
RtcpIntraFrameObserver* intra_frame_callback;
RtcpLossNotificationObserver* rtcp_loss_notification_observer;
RtcpStatisticsCallback* rtcp_stats;
+ ReportBlockDataObserver* report_block_data_observer;
StreamDataCountersCallback* rtp_stats;
BitrateStatisticsObserver* bitrate_observer;
FrameCountObserver* frame_count_observer;
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 59817fb..d67bf08 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -306,6 +306,8 @@
// Simulcast has one module for each layer. Set the CNAME on all modules.
stream.rtp_rtcp->SetCNAME(rtp_config_.c_name.c_str());
stream.rtp_rtcp->RegisterRtcpStatisticsCallback(observers.rtcp_stats);
+ stream.rtp_rtcp->SetReportBlockDataObserver(
+ observers.report_block_data_observer);
stream.rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(
observers.rtp_stats);
stream.rtp_rtcp->SetMaxRtpPacketSize(rtp_config_.max_packet_size);
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index bd23bc6..9622fbc 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -59,6 +59,7 @@
RtcpRttStats* rtcp_rtt_stats,
RtcpIntraFrameObserver* intra_frame_callback,
RtcpStatisticsCallback* rtcp_stats,
+ ReportBlockDataObserver* report_block_data_observer,
StreamDataCountersCallback* rtp_stats,
BitrateStatisticsObserver* bitrate_observer,
FrameCountObserver* frame_count_observer,
@@ -70,6 +71,7 @@
observers.intra_frame_callback = intra_frame_callback;
observers.rtcp_loss_notification_observer = nullptr;
observers.rtcp_stats = rtcp_stats;
+ observers.report_block_data_observer = report_block_data_observer;
observers.rtp_stats = rtp_stats;
observers.bitrate_observer = bitrate_observer;
observers.frame_count_observer = frame_count_observer;
@@ -135,8 +137,9 @@
&clock_, suspended_ssrcs, suspended_payload_states, config_.rtp,
config_.rtcp_report_interval_ms, &transport_,
CreateObservers(&call_stats_, &encoder_feedback_, &stats_proxy_,
- &stats_proxy_, &stats_proxy_, frame_count_observer,
- &stats_proxy_, &stats_proxy_, &send_delay_stats_),
+ &stats_proxy_, &stats_proxy_, &stats_proxy_,
+ frame_count_observer, &stats_proxy_, &stats_proxy_,
+ &send_delay_stats_),
&transport_controller_, &event_log_, &retransmission_rate_limiter_,
absl::make_unique<FecControllerDefault>(&clock_), nullptr,
CryptoOptions{});
diff --git a/call/video_send_stream.h b/call/video_send_stream.h
index 42760bd..929aa88 100644
--- a/call/video_send_stream.h
+++ b/call/video_send_stream.h
@@ -28,6 +28,7 @@
#include "api/video/video_stream_encoder_settings.h"
#include "api/video_codecs/video_encoder_config.h"
#include "call/rtp_config.h"
+#include "modules/rtp_rtcp/include/report_block_data.h"
#include "modules/rtp_rtcp/include/rtcp_statistics.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
@@ -57,6 +58,9 @@
StreamDataCounters rtp_stats;
RtcpPacketTypeCounter rtcp_packet_type_counts;
RtcpStatistics rtcp_stats;
+ // A snapshot of the most recent Report Block with additional data of
+ // interest to statistics. Used to implement RTCRemoteInboundRtpStreamStats.
+ absl::optional<ReportBlockData> report_block_data;
};
struct Stats {
diff --git a/media/BUILD.gn b/media/BUILD.gn
index a5a37e6..5cf4175 100644
--- a/media/BUILD.gn
+++ b/media/BUILD.gn
@@ -85,6 +85,7 @@
"../call:call_interfaces",
"../common_video",
"../modules/audio_processing:audio_processing_statistics",
+ "../modules/rtp_rtcp:rtp_rtcp_format",
"../rtc_base",
"../rtc_base:checks",
"../rtc_base:rtc_base_approved",
diff --git a/media/base/media_channel.h b/media/base/media_channel.h
index 8bdcd2b..3b9a54c 100644
--- a/media/base/media_channel.h
+++ b/media/base/media_channel.h
@@ -37,6 +37,7 @@
#include "media/base/media_constants.h"
#include "media/base/stream_params.h"
#include "modules/audio_processing/include/audio_processing_statistics.h"
+#include "modules/rtp_rtcp/include/report_block_data.h"
#include "rtc_base/async_packet_socket.h"
#include "rtc_base/buffer.h"
#include "rtc_base/copy_on_write_buffer.h"
@@ -404,6 +405,11 @@
absl::optional<int> codec_payload_type;
std::vector<SsrcSenderInfo> local_stats;
std::vector<SsrcReceiverInfo> remote_stats;
+ // A snapshot of the most recent Report Block with additional data of interest
+ // to statistics. Used to implement RTCRemoteInboundRtpStreamStats. Within
+ // this list, the ReportBlockData::RTCPReportBlock::source_ssrc(), which is
+ // the SSRC of the corresponding outbound RTP stream, is unique.
+ std::vector<webrtc::ReportBlockData> report_block_datas;
};
struct MediaReceiverInfo {
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 2e7e86e..5950204 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -2314,6 +2314,10 @@
info.firs_rcvd += stream_stats.rtcp_packet_type_counts.fir_packets;
info.nacks_rcvd += stream_stats.rtcp_packet_type_counts.nack_packets;
info.plis_rcvd += stream_stats.rtcp_packet_type_counts.pli_packets;
+ if (stream_stats.report_block_data.has_value() && !stream_stats.is_rtx &&
+ !stream_stats.is_flexfec) {
+ info.report_block_datas.push_back(stream_stats.report_block_data.value());
+ }
}
if (!stats.substreams.empty()) {
diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h
index a207a11..216f758 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -24,6 +24,7 @@
#include "modules/include/module.h"
#include "modules/rtp_rtcp/include/flexfec_sender.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
+#include "modules/rtp_rtcp/include/report_block_data.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_sender.h"
#include "rtc_base/constructor_magic.h"
@@ -397,9 +398,19 @@
virtual bool StorePackets() const = 0;
// Called on receipt of RTCP report block from remote side.
+ // TODO(https://crbug.com/webrtc/10678): Remove RtcpStatisticsCallback in
+ // favor of ReportBlockDataObserver.
+ // TODO(https://crbug.com/webrtc/10679): Consider whether we want to use only
+ // getters or only callbacks. If we decide on getters, the
+ // ReportBlockDataObserver should also be removed in favor of
+ // GetLatestReportBlockData().
virtual void RegisterRtcpStatisticsCallback(
RtcpStatisticsCallback* callback) = 0;
virtual RtcpStatisticsCallback* GetRtcpStatisticsCallback() = 0;
+ // TODO(https://crbug.com/webrtc/10680): When callbacks are registered at
+ // construction, remove this setter.
+ virtual void SetReportBlockDataObserver(
+ ReportBlockDataObserver* observer) = 0;
// BWE feedback packets.
bool SendFeedbackPacket(const rtcp::TransportFeedback& packet) override = 0;
diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
index 95d3cb2..eaad03d 100644
--- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
+++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
@@ -145,6 +145,7 @@
MOCK_CONST_METHOD0(StorePackets, bool());
MOCK_METHOD1(RegisterRtcpStatisticsCallback, void(RtcpStatisticsCallback*));
MOCK_METHOD0(GetRtcpStatisticsCallback, RtcpStatisticsCallback*());
+ MOCK_METHOD1(SetReportBlockDataObserver, void(ReportBlockDataObserver*));
MOCK_METHOD1(SendFeedbackPacket, bool(const rtcp::TransportFeedback& packet));
MOCK_METHOD1(SetTargetSendBitrate, void(uint32_t bitrate_bps));
MOCK_METHOD1(SetKeyFrameRequestMethod, int32_t(KeyFrameRequestMethod method));
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 2e8e662..1d7f048 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -721,6 +721,11 @@
return rtcp_receiver_.GetRtcpStatisticsCallback();
}
+void ModuleRtpRtcpImpl::SetReportBlockDataObserver(
+ ReportBlockDataObserver* observer) {
+ return rtcp_receiver_.SetReportBlockDataObserver(observer);
+}
+
bool ModuleRtpRtcpImpl::SendFeedbackPacket(
const rtcp::TransportFeedback& packet) {
return rtcp_sender_.SendFeedbackPacket(packet);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index 3fb1126..71a856c 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -234,6 +234,7 @@
void RegisterRtcpStatisticsCallback(
RtcpStatisticsCallback* callback) override;
RtcpStatisticsCallback* GetRtcpStatisticsCallback() override;
+ void SetReportBlockDataObserver(ReportBlockDataObserver* observer) override;
bool SendFeedbackPacket(const rtcp::TransportFeedback& packet) override;
// (APP) Application specific data.
diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc
index 27e6014..0313316 100644
--- a/video/send_statistics_proxy.cc
+++ b/video/send_statistics_proxy.cc
@@ -1136,6 +1136,16 @@
void SendStatisticsProxy::CNameChanged(const char* cname, uint32_t ssrc) {}
+void SendStatisticsProxy::OnReportBlockDataUpdated(
+ ReportBlockData report_block_data) {
+ rtc::CritScope lock(&crit_);
+ VideoSendStream::StreamStats* stats =
+ GetStatsEntry(report_block_data.report_block().source_ssrc);
+ if (!stats)
+ return;
+ stats->report_block_data = std::move(report_block_data);
+}
+
void SendStatisticsProxy::DataCountersUpdated(
const StreamDataCounters& counters,
uint32_t ssrc) {
diff --git a/video/send_statistics_proxy.h b/video/send_statistics_proxy.h
index 3b61f32..51d5b2f 100644
--- a/video/send_statistics_proxy.h
+++ b/video/send_statistics_proxy.h
@@ -18,6 +18,7 @@
#include "api/video/video_stream_encoder_observer.h"
#include "call/video_send_stream.h"
+#include "modules/rtp_rtcp/include/report_block_data.h"
#include "modules/video_coding/include/video_codec_interface.h"
#include "modules/video_coding/include/video_coding_defines.h"
#include "rtc_base/critical_section.h"
@@ -32,6 +33,7 @@
class SendStatisticsProxy : public VideoStreamEncoderObserver,
public RtcpStatisticsCallback,
+ public ReportBlockDataObserver,
public RtcpPacketTypeCounterObserver,
public StreamDataCountersCallback,
public BitrateStatisticsObserver,
@@ -93,6 +95,8 @@
void StatisticsUpdated(const RtcpStatistics& statistics,
uint32_t ssrc) override;
void CNameChanged(const char* cname, uint32_t ssrc) override;
+ // From ReportBlockDataObserver.
+ void OnReportBlockDataUpdated(ReportBlockData report_block_data) override;
// From RtcpPacketTypeCounterObserver.
void RtcpPacketTypesCounterUpdated(
uint32_t ssrc,
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index 0b3d081..1dc6599 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -151,6 +151,7 @@
observers.intra_frame_callback = encoder_feedback;
observers.rtcp_loss_notification_observer = encoder_feedback;
observers.rtcp_stats = stats_proxy;
+ observers.report_block_data_observer = stats_proxy;
observers.rtp_stats = stats_proxy;
observers.bitrate_observer = stats_proxy;
observers.frame_count_observer = stats_proxy;