Translate loss notifications and pass to encoder
Translate LossNotification RTCP messages (sequence number to
timestamp and additional information), then send the translted
message onwards to the encoder.
Bug: webrtc:10501
Change-Id: If2fd943f75c36cf813a83120318d8eefc8c595d2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/131950
Commit-Queue: Elad Alon <eladalon@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27545}diff --git a/api/video/video_stream_encoder_interface.h b/api/video/video_stream_encoder_interface.h
index 5463118..4e3c470 100644
--- a/api/video/video_stream_encoder_interface.h
+++ b/api/video/video_stream_encoder_interface.h
@@ -78,6 +78,10 @@
// Request a key frame. Used for signalling from the remote receiver.
virtual void SendKeyFrame() = 0;
+ // Inform the encoder that a loss has occurred.
+ virtual void OnLossNotification(
+ const VideoEncoder::LossNotification& loss_notification) = 0;
+
// Set the currently estimated network properties. A |target_bitrate|
// of zero pauses the encoder.
// |link_allocation| is the bandwidth available for this video stream on the
diff --git a/modules/video_coding/loss_notification_controller.cc b/modules/video_coding/loss_notification_controller.cc
index 3bab787..9270ca4 100644
--- a/modules/video_coding/loss_notification_controller.cc
+++ b/modules/video_coding/loss_notification_controller.cc
@@ -90,6 +90,10 @@
const bool key_frame = intra_frame;
if (key_frame) {
// Subsequent frames may not rely on frames before the key frame.
+ // Note that upon receiving a key frame, we do not issue a loss
+ // notification on RTP sequence number gap, unless that gap spanned
+ // the key frame itself. This is because any loss which occurred before
+ // the key frame is no longer relevant.
decodable_unwrapped_frame_ids_.clear();
current_frame_potentially_decodable_ = true;
} else {
diff --git a/video/encoder_rtcp_feedback.cc b/video/encoder_rtcp_feedback.cc
index 5a0920f..19a8f64 100644
--- a/video/encoder_rtcp_feedback.cc
+++ b/video/encoder_rtcp_feedback.cc
@@ -11,6 +11,7 @@
#include "video/encoder_rtcp_feedback.h"
#include "absl/types/optional.h"
+#include "api/video_codecs/video_encoder.h"
#include "rtc_base/checks.h"
#include "rtc_base/experiments/keyframe_interval_settings.h"
@@ -25,6 +26,7 @@
VideoStreamEncoderInterface* encoder)
: clock_(clock),
ssrcs_(ssrcs),
+ rtp_video_sender_(nullptr),
video_stream_encoder_(encoder),
time_last_intra_request_ms_(-1),
min_keyframe_send_interval_ms_(
@@ -34,6 +36,13 @@
RTC_DCHECK(!ssrcs.empty());
}
+void EncoderRtcpFeedback::SetRtpVideoSender(
+ const RtpVideoSenderInterface* rtp_video_sender) {
+ RTC_DCHECK(rtp_video_sender);
+ RTC_DCHECK(!rtp_video_sender_);
+ rtp_video_sender_ = rtp_video_sender;
+}
+
bool EncoderRtcpFeedback::HasSsrc(uint32_t ssrc) {
for (uint32_t registered_ssrc : ssrcs_) {
if (registered_ssrc == ssrc) {
@@ -73,7 +82,76 @@
uint16_t seq_num_of_last_decodable,
uint16_t seq_num_of_last_received,
bool decodability_flag) {
- // TODO(eladalon): Handle.
+ RTC_DCHECK(rtp_video_sender_) << "Object initialization incomplete.";
+
+ const std::vector<uint16_t> seq_nums = {seq_num_of_last_decodable,
+ seq_num_of_last_received};
+ const std::vector<RtpSequenceNumberMap::Info> infos =
+ rtp_video_sender_->GetSentRtpPacketInfos(ssrc, seq_nums);
+ if (infos.empty()) {
+ return;
+ }
+ RTC_DCHECK_EQ(infos.size(), 2u);
+
+ const RtpSequenceNumberMap::Info& last_decodable = infos[0];
+ const RtpSequenceNumberMap::Info& last_received = infos[1];
+
+ VideoEncoder::LossNotification loss_notification;
+ loss_notification.timestamp_of_last_decodable = last_decodable.timestamp;
+ loss_notification.timestamp_of_last_received = last_received.timestamp;
+
+ // Deduce decodability of the last received frame and of its dependencies.
+ if (last_received.is_first && last_received.is_last) {
+ // The frame consists of a single packet, and that packet has evidently
+ // been received in full; the frame is therefore assemblable.
+ // In this case, the decodability of the dependencies is communicated by
+ // the decodability flag, and the frame itself is decodable if and only
+ // if they are decodable.
+ loss_notification.dependencies_of_last_received_decodable =
+ decodability_flag;
+ loss_notification.last_received_decodable = decodability_flag;
+ } else if (last_received.is_first && !last_received.is_last) {
+ // In this case, the decodability flag communicates the decodability of
+ // the dependencies. If any is undecodable, we also know that the frame
+ // itself will not be decodable; if all are decodable, the frame's own
+ // decodability will remain unknown, as not all of its packets have
+ // been received.
+ loss_notification.dependencies_of_last_received_decodable =
+ decodability_flag;
+ loss_notification.last_received_decodable =
+ !decodability_flag ? absl::make_optional(false) : absl::nullopt;
+ } else if (!last_received.is_first && last_received.is_last) {
+ if (decodability_flag) {
+ // The frame has been received in full, and found to be decodable.
+ // (Messages of this type are not sent by WebRTC at the moment, but are
+ // theoretically possible, for example for serving as acks.)
+ loss_notification.dependencies_of_last_received_decodable = true;
+ loss_notification.last_received_decodable = true;
+ } else {
+ // It is impossible to tell whether some dependencies were undecodable,
+ // or whether the frame was unassemblable, but in either case, the frame
+ // itself was undecodable.
+ loss_notification.dependencies_of_last_received_decodable = absl::nullopt;
+ loss_notification.last_received_decodable = false;
+ }
+ } else { // !last_received.is_first && !last_received.is_last
+ if (decodability_flag) {
+ // The frame has not yet been received in full, but no gaps have
+ // been encountered so far, and the dependencies were all decodable.
+ // (Messages of this type are not sent by WebRTC at the moment, but are
+ // theoretically possible, for example for serving as acks.)
+ loss_notification.dependencies_of_last_received_decodable = true;
+ loss_notification.last_received_decodable = absl::nullopt;
+ } else {
+ // It is impossible to tell whether some dependencies were undecodable,
+ // or whether the frame was unassemblable, but in either case, the frame
+ // itself was undecodable.
+ loss_notification.dependencies_of_last_received_decodable = absl::nullopt;
+ loss_notification.last_received_decodable = false;
+ }
+ }
+
+ video_stream_encoder_->OnLossNotification(loss_notification);
}
} // namespace webrtc
diff --git a/video/encoder_rtcp_feedback.h b/video/encoder_rtcp_feedback.h
index 4fa2335..8f10442 100644
--- a/video/encoder_rtcp_feedback.h
+++ b/video/encoder_rtcp_feedback.h
@@ -14,6 +14,7 @@
#include "api/media_transport_interface.h"
#include "api/video/video_stream_encoder_interface.h"
+#include "call/rtp_video_sender_interface.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "rtc_base/critical_section.h"
#include "system_wrappers/include/clock.h"
@@ -35,6 +36,8 @@
VideoStreamEncoderInterface* encoder);
~EncoderRtcpFeedback() override = default;
+ void SetRtpVideoSender(const RtpVideoSenderInterface* rtp_video_sender);
+
void OnReceivedIntraFrameRequest(uint32_t ssrc) override;
// Implements MediaTransportKeyFrameRequestCallback
@@ -51,6 +54,7 @@
Clock* const clock_;
const std::vector<uint32_t> ssrcs_;
+ const RtpVideoSenderInterface* rtp_video_sender_;
VideoStreamEncoderInterface* const video_stream_encoder_;
rtc::CriticalSection crit_;
diff --git a/video/test/mock_video_stream_encoder.h b/video/test/mock_video_stream_encoder.h
index cdfb4e6..c27c6e9 100644
--- a/video/test/mock_video_stream_encoder.h
+++ b/video/test/mock_video_stream_encoder.h
@@ -23,6 +23,7 @@
MOCK_METHOD2(SetSink, void(EncoderSink*, bool));
MOCK_METHOD1(SetStartBitrate, void(int));
MOCK_METHOD0(SendKeyFrame, void());
+ MOCK_METHOD1(OnLossNotification, void(const VideoEncoder::LossNotification&));
MOCK_METHOD4(OnBitrateUpdated, void(DataRate, DataRate, uint8_t, int64_t));
MOCK_METHOD1(OnFrame, void(const VideoFrame&));
MOCK_METHOD1(SetBitrateAllocationObserver,
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index af074c4..81d269d 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -249,6 +249,8 @@
RTC_LOG(LS_INFO) << "VideoSendStreamInternal: " << config_->ToString();
weak_ptr_ = weak_ptr_factory_.GetWeakPtr();
+ encoder_feedback_.SetRtpVideoSender(rtp_video_sender_);
+
if (media_transport_) {
// The configured ssrc is interpreted as a channel id, so there must be
// exactly one.
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index ac17124..1cd8c92 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -1403,6 +1403,20 @@
}
}
+void VideoStreamEncoder::OnLossNotification(
+ const VideoEncoder::LossNotification& loss_notification) {
+ if (!encoder_queue_.IsCurrent()) {
+ encoder_queue_.PostTask(
+ [this, loss_notification] { OnLossNotification(loss_notification); });
+ return;
+ }
+
+ RTC_DCHECK_RUN_ON(&encoder_queue_);
+ if (encoder_) {
+ encoder_->OnLossNotification(loss_notification);
+ }
+}
+
EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage(
const EncodedImage& encoded_image,
const CodecSpecificInfo* codec_specific_info,
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h
index 61585f7..45ef430 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -81,6 +81,9 @@
void SendKeyFrame() override;
+ void OnLossNotification(
+ const VideoEncoder::LossNotification& loss_notification) override;
+
void OnBitrateUpdated(DataRate target_bitrate,
DataRate target_headroom,
uint8_t fraction_lost,
