Forward LossNotification from RTCPReceiver to EncoderRtcpFeedback
TBR=sprang@webrtc.org
Bug: webrtc:10501
Change-Id: I09a571a65ba8515b027ee32d1f46e5cc7f699704
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/131325
Reviewed-by: Elad Alon <eladalon@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Elad Alon <eladalon@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27513}diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h
index d564035..450b65b 100644
--- a/call/rtp_transport_controller_send_interface.h
+++ b/call/rtp_transport_controller_send_interface.h
@@ -54,6 +54,7 @@
struct RtpSenderObservers {
RtcpRttStats* rtcp_rtt_stats;
RtcpIntraFrameObserver* intra_frame_callback;
+ RtcpLossNotificationObserver* rtcp_loss_notification_observer;
RtcpStatisticsCallback* rtcp_stats;
StreamDataCountersCallback* rtp_stats;
BitrateStatisticsObserver* bitrate_observer;
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 98cdd9a..94a8fad 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -63,6 +63,7 @@
int rtcp_report_interval_ms,
Transport* send_transport,
RtcpIntraFrameObserver* intra_frame_callback,
+ RtcpLossNotificationObserver* rtcp_loss_notification_observer,
RtcpBandwidthObserver* bandwidth_callback,
RtpTransportControllerSendInterface* transport,
RtcpRttStats* rtt_stats,
@@ -84,6 +85,8 @@
configuration.receiver_only = false;
configuration.outgoing_transport = send_transport;
configuration.intra_frame_callback = intra_frame_callback;
+ configuration.rtcp_loss_notification_observer =
+ rtcp_loss_notification_observer;
configuration.bandwidth_callback = bandwidth_callback;
configuration.transport_feedback_callback =
transport->transport_feedback_observer();
@@ -228,24 +231,26 @@
flexfec_sender_(
MaybeCreateFlexfecSender(clock, rtp_config, suspended_ssrcs_)),
fec_controller_(std::move(fec_controller)),
- rtp_streams_(CreateRtpStreamSenders(clock,
- rtp_config,
- rtcp_report_interval_ms,
- send_transport,
- observers.intra_frame_callback,
- transport->GetBandwidthObserver(),
- transport,
- observers.rtcp_rtt_stats,
- flexfec_sender_.get(),
- observers.bitrate_observer,
- observers.rtcp_type_observer,
- observers.send_delay_observer,
- observers.send_packet_observer,
- event_log,
- retransmission_limiter,
- this,
- frame_encryptor,
- crypto_options)),
+ rtp_streams_(
+ CreateRtpStreamSenders(clock,
+ rtp_config,
+ rtcp_report_interval_ms,
+ send_transport,
+ observers.intra_frame_callback,
+ observers.rtcp_loss_notification_observer,
+ transport->GetBandwidthObserver(),
+ transport,
+ observers.rtcp_rtt_stats,
+ flexfec_sender_.get(),
+ observers.bitrate_observer,
+ observers.rtcp_type_observer,
+ observers.send_delay_observer,
+ observers.send_packet_observer,
+ event_log,
+ retransmission_limiter,
+ this,
+ frame_encryptor,
+ crypto_options)),
rtp_config_(rtp_config),
transport_(transport),
transport_overhead_bytes_per_packet_(0),
diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h
index 9ae6ddb..1c589f4 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -64,9 +64,12 @@
// out on the network.
Transport* outgoing_transport = nullptr;
- // Called when the receiver request a intra frame.
+ // Called when the receiver requests an intra frame.
RtcpIntraFrameObserver* intra_frame_callback = nullptr;
+ // Called when the receiver sends a loss notification.
+ RtcpLossNotificationObserver* rtcp_loss_notification_observer = nullptr;
+
// Called when we receive a changed estimate from the receiver of out
// stream.
RtcpBandwidthObserver* bandwidth_callback = nullptr;
diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
index cbeb77f..41ab1cf 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -186,6 +186,18 @@
virtual void OnReceivedIntraFrameRequest(uint32_t ssrc) = 0;
};
+// Observer for incoming LossNotification RTCP messages.
+// See the documentation of LossNotification for details.
+class RtcpLossNotificationObserver {
+ public:
+ virtual ~RtcpLossNotificationObserver() = default;
+
+ virtual void OnReceivedLossNotification(uint32_t ssrc,
+ uint16_t seq_num_of_last_decodable,
+ uint16_t seq_num_of_last_received,
+ bool decodability_flag) = 0;
+};
+
class RtcpBandwidthObserver {
public:
// REMB or TMMBR
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc
index e3b1d13..0c1c326 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -131,6 +131,7 @@
RtcpPacketTypeCounterObserver* packet_type_counter_observer,
RtcpBandwidthObserver* rtcp_bandwidth_observer,
RtcpIntraFrameObserver* rtcp_intra_frame_observer,
+ RtcpLossNotificationObserver* rtcp_loss_notification_observer,
TransportFeedbackObserver* transport_feedback_observer,
VideoBitrateAllocationObserver* bitrate_allocation_observer,
int report_interval_ms,
@@ -140,6 +141,7 @@
rtp_rtcp_(owner),
rtcp_bandwidth_observer_(rtcp_bandwidth_observer),
rtcp_intra_frame_observer_(rtcp_intra_frame_observer),
+ rtcp_loss_notification_observer_(rtcp_loss_notification_observer),
transport_feedback_observer_(transport_feedback_observer),
bitrate_allocation_observer_(bitrate_allocation_observer),
report_interval_ms_(report_interval_ms),
@@ -1019,6 +1021,18 @@
rtcp_intra_frame_observer_->OnReceivedIntraFrameRequest(local_ssrc);
}
}
+ if (rtcp_loss_notification_observer_ &&
+ (packet_information.packet_type_flags & kRtcpLossNotification)) {
+ rtcp::LossNotification* loss_notification =
+ packet_information.loss_notification.get();
+ RTC_DCHECK(loss_notification);
+ if (loss_notification->media_ssrc() == local_ssrc) {
+ rtcp_loss_notification_observer_->OnReceivedLossNotification(
+ loss_notification->media_ssrc(), loss_notification->last_decoded(),
+ loss_notification->last_received(),
+ loss_notification->decodability_flag());
+ }
+ }
if (rtcp_bandwidth_observer_) {
RTC_DCHECK(!receiver_only_);
if (packet_information.packet_type_flags & kRtcpRemb) {
@@ -1028,16 +1042,6 @@
rtcp_bandwidth_observer_->OnReceivedEstimatedBitrate(
packet_information.receiver_estimated_max_bitrate_bps);
}
- if (packet_information.packet_type_flags & kRtcpLossNotification) {
- rtcp::LossNotification* loss_notification =
- packet_information.loss_notification.get();
- RTC_DCHECK(loss_notification);
- RTC_LOG(LS_VERBOSE) << "Incoming Loss Notification: ("
- << loss_notification->last_decoded() << ", "
- << loss_notification->last_received() << ", "
- << loss_notification->decodability_flag() << ").";
- // TODO(eladalon): Notify observer.
- }
if ((packet_information.packet_type_flags & kRtcpSr) ||
(packet_information.packet_type_flags & kRtcpRr)) {
int64_t now_ms = clock_->TimeInMilliseconds();
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h
index fe6ec4c..afc3f96 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/modules/rtp_rtcp/source/rtcp_receiver.h
@@ -55,6 +55,7 @@
RtcpPacketTypeCounterObserver* packet_type_counter_observer,
RtcpBandwidthObserver* rtcp_bandwidth_observer,
RtcpIntraFrameObserver* rtcp_intra_frame_observer,
+ RtcpLossNotificationObserver* rtcp_loss_notification_observer,
TransportFeedbackObserver* transport_feedback_observer,
VideoBitrateAllocationObserver* bitrate_allocation_observer,
int report_interval_ms,
@@ -215,6 +216,7 @@
rtc::CriticalSection feedbacks_lock_;
RtcpBandwidthObserver* const rtcp_bandwidth_observer_;
RtcpIntraFrameObserver* const rtcp_intra_frame_observer_;
+ RtcpLossNotificationObserver* const rtcp_loss_notification_observer_;
TransportFeedbackObserver* const transport_feedback_observer_;
VideoBitrateAllocationObserver* const bitrate_allocation_observer_;
const int report_interval_ms_;
diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
index 10351da..e1483a4 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
@@ -66,6 +66,16 @@
MOCK_METHOD1(OnReceivedIntraFrameRequest, void(uint32_t));
};
+class MockRtcpLossNotificationObserver : public RtcpLossNotificationObserver {
+ public:
+ ~MockRtcpLossNotificationObserver() override = default;
+ MOCK_METHOD4(OnReceivedLossNotification,
+ void(uint32_t ssrc,
+ uint16_t seq_num_of_last_decodable,
+ uint16_t seq_num_of_last_received,
+ bool decodability_flag));
+};
+
class MockRtcpCallbackImpl : public RtcpStatisticsCallback {
public:
MOCK_METHOD2(StatisticsUpdated, void(const RtcpStatistics&, uint32_t));
@@ -119,6 +129,7 @@
&packet_type_counter_observer_,
&bandwidth_observer_,
&intra_frame_observer_,
+ &rtcp_loss_notification_observer_,
&transport_feedback_observer_,
&bitrate_allocation_observer_,
kRtcpIntervalMs,
@@ -145,6 +156,7 @@
NiceMock<MockRtcpPacketTypeCounterObserver> packet_type_counter_observer_;
StrictMock<MockRtcpBandwidthObserver> bandwidth_observer_;
StrictMock<MockRtcpIntraFrameObserver> intra_frame_observer_;
+ StrictMock<MockRtcpLossNotificationObserver> rtcp_loss_notification_observer_;
StrictMock<MockTransportFeedbackObserver> transport_feedback_observer_;
StrictMock<MockVideoBitrateAllocationObserver> bitrate_allocation_observer_;
StrictMock<MockModuleRtpRtcp> rtp_rtcp_impl_;
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 9a0fc7c..f716718 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -74,6 +74,7 @@
configuration.rtcp_packet_type_counter_observer,
configuration.bandwidth_callback,
configuration.intra_frame_callback,
+ configuration.rtcp_loss_notification_observer,
configuration.transport_feedback_callback,
configuration.bitrate_allocation_observer,
configuration.rtcp_report_interval_ms > 0
diff --git a/test/fuzzers/rtcp_receiver_fuzzer.cc b/test/fuzzers/rtcp_receiver_fuzzer.cc
index f729e55..f686121 100644
--- a/test/fuzzers/rtcp_receiver_fuzzer.cc
+++ b/test/fuzzers/rtcp_receiver_fuzzer.cc
@@ -40,7 +40,7 @@
SimulatedClock clock(1234);
RTCPReceiver receiver(&clock, false, nullptr, nullptr, nullptr, nullptr,
- nullptr, kRtcpIntervalMs, &rtp_rtcp_module);
+ nullptr, nullptr, kRtcpIntervalMs, &rtp_rtcp_module);
receiver.IncomingPacket(data, size);
}
diff --git a/video/encoder_rtcp_feedback.cc b/video/encoder_rtcp_feedback.cc
index 2c54701..5a0920f 100644
--- a/video/encoder_rtcp_feedback.cc
+++ b/video/encoder_rtcp_feedback.cc
@@ -68,4 +68,12 @@
video_stream_encoder_->SendKeyFrame();
}
+void EncoderRtcpFeedback::OnReceivedLossNotification(
+ uint32_t ssrc,
+ uint16_t seq_num_of_last_decodable,
+ uint16_t seq_num_of_last_received,
+ bool decodability_flag) {
+ // TODO(eladalon): Handle.
+}
+
} // namespace webrtc
diff --git a/video/encoder_rtcp_feedback.h b/video/encoder_rtcp_feedback.h
index 1a58a67..4fa2335 100644
--- a/video/encoder_rtcp_feedback.h
+++ b/video/encoder_rtcp_feedback.h
@@ -27,16 +27,25 @@
// TODO(bugs.webrtc.org/9719): Should be eliminated when RtpMediaTransport is
// implemented.
class EncoderRtcpFeedback : public RtcpIntraFrameObserver,
+ public RtcpLossNotificationObserver,
public MediaTransportKeyFrameRequestCallback {
public:
EncoderRtcpFeedback(Clock* clock,
const std::vector<uint32_t>& ssrcs,
VideoStreamEncoderInterface* encoder);
+ ~EncoderRtcpFeedback() override = default;
+
void OnReceivedIntraFrameRequest(uint32_t ssrc) override;
// Implements MediaTransportKeyFrameRequestCallback
void OnKeyFrameRequested(uint64_t channel_id) override;
+ // Implements RtcpLossNotificationObserver.
+ void OnReceivedLossNotification(uint32_t ssrc,
+ uint16_t seq_num_of_last_decodable,
+ uint16_t seq_num_of_last_received,
+ bool decodability_flag) override;
+
private:
bool HasSsrc(uint32_t ssrc);
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index 1acf0b3..f2a7756 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -149,6 +149,7 @@
RtpSenderObservers observers;
observers.rtcp_rtt_stats = call_stats;
observers.intra_frame_callback = encoder_feedback;
+ observers.rtcp_loss_notification_observer = encoder_feedback;
observers.rtcp_stats = stats_proxy;
observers.rtp_stats = stats_proxy;
observers.bitrate_observer = stats_proxy;
