Detach LossNotificationController from RtpGenericFrameDescriptor
To allow to use the LossNotificationController with
an updated version of the frame descriptor extension
Bug: webrtc:10342
Change-Id: I5ac44dc5549dfcfc73bf81ad1e8eab8bd5dd136e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166166
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Elad Alon <eladalon@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30369}
diff --git a/modules/video_coding/loss_notification_controller.cc b/modules/video_coding/loss_notification_controller.cc
index 20752f8..77d47c4 100644
--- a/modules/video_coding/loss_notification_controller.cc
+++ b/modules/video_coding/loss_notification_controller.cc
@@ -10,8 +10,12 @@
#include "modules/video_coding/loss_notification_controller.h"
+#include <stdint.h>
+
+#include "api/array_view.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
+#include "rtc_base/numerics/sequence_number_util.h"
namespace webrtc {
namespace {
@@ -45,7 +49,7 @@
void LossNotificationController::OnReceivedPacket(
uint16_t rtp_seq_num,
- const RtpGenericFrameDescriptor& generic_descriptor) {
+ const LossNotificationController::FrameDetails* frame) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
// Ignore repeated or reordered packets.
@@ -63,38 +67,30 @@
last_received_seq_num_ = rtp_seq_num;
- if (generic_descriptor.FirstPacketInSubFrame()) {
- const uint16_t frame_id = generic_descriptor.FrameId();
- const int64_t unwrapped_frame_id = frame_id_unwrapper_.Unwrap(frame_id);
-
+ // |frame| is not nullptr iff the packet is the first packet in the frame.
+ if (frame != nullptr) {
// Ignore repeated or reordered frames.
- // TODO(TODO(bugs.webrtc.org/10336): Handle frame reordering.
- if (last_received_unwrapped_frame_id_ &&
- unwrapped_frame_id <= *last_received_unwrapped_frame_id_) {
- RTC_LOG(LS_WARNING) << "Repeated or reordered frame ID (" << frame_id
- << ").";
+ // TODO(bugs.webrtc.org/10336): Handle frame reordering.
+ if (last_received_frame_id_.has_value() &&
+ frame->frame_id <= last_received_frame_id_.value()) {
+ RTC_LOG(LS_WARNING) << "Repeated or reordered frame ID ("
+ << frame->frame_id << ").";
return;
}
- last_received_unwrapped_frame_id_ = unwrapped_frame_id;
+ last_received_frame_id_ = frame->frame_id;
- const bool intra_frame =
- generic_descriptor.FrameDependenciesDiffs().empty();
- // Generic Frame Descriptor does not current allow us to distinguish
- // whether an intra frame is a key frame.
- // We therefore assume all intra frames are key frames.
- const bool key_frame = intra_frame;
- if (key_frame) {
+ if (frame->is_keyframe) {
// 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();
+ decodable_frame_ids_.clear();
current_frame_potentially_decodable_ = true;
} else {
- const bool all_dependencies_decodable = AllDependenciesDecodable(
- unwrapped_frame_id, generic_descriptor.FrameDependenciesDiffs());
+ const bool all_dependencies_decodable =
+ AllDependenciesDecodable(frame->frame_dependencies);
current_frame_potentially_decodable_ = all_dependencies_decodable;
if (seq_num_gap || !current_frame_potentially_decodable_) {
HandleLoss(rtp_seq_num, current_frame_potentially_decodable_);
@@ -112,9 +108,9 @@
void LossNotificationController::OnAssembledFrame(
uint16_t first_seq_num,
- uint16_t frame_id,
+ int64_t frame_id,
bool discardable,
- rtc::ArrayView<const uint16_t> frame_dependency_diffs) {
+ rtc::ArrayView<const int64_t> frame_dependencies) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
DiscardOldInformation(); // Prevent memory overconsumption.
@@ -123,13 +119,12 @@
return;
}
- const int64_t unwrapped_frame_id = frame_id_unwrapper_.Unwrap(frame_id);
- if (!AllDependenciesDecodable(unwrapped_frame_id, frame_dependency_diffs)) {
+ if (!AllDependenciesDecodable(frame_dependencies)) {
return;
}
last_decodable_non_discardable_.emplace(first_seq_num);
- const auto it = decodable_unwrapped_frame_ids_.insert(unwrapped_frame_id);
+ const auto it = decodable_frame_ids_.insert(frame_id);
RTC_DCHECK(it.second);
}
@@ -137,12 +132,11 @@
constexpr size_t kExpectedKeyFrameIntervalFrames = 3000;
constexpr size_t kMaxSize = 2 * kExpectedKeyFrameIntervalFrames;
constexpr size_t kTargetSize = kExpectedKeyFrameIntervalFrames;
- PareDown(&decodable_unwrapped_frame_ids_, kMaxSize, kTargetSize);
+ PareDown(&decodable_frame_ids_, kMaxSize, kTargetSize);
}
bool LossNotificationController::AllDependenciesDecodable(
- int64_t unwrapped_frame_id,
- rtc::ArrayView<const uint16_t> frame_dependency_diffs) const {
+ rtc::ArrayView<const int64_t> frame_dependencies) const {
RTC_DCHECK_RUN_ON(&sequence_checker_);
// Due to packet reordering, frame buffering and asynchronous decoders, it is
@@ -151,14 +145,9 @@
// * Intra frames are decodable.
// * Inter frames are decodable if all of their references were decodable.
// One possibility that is ignored, is that the packet may be corrupt.
-
- for (uint16_t frame_dependency_diff : frame_dependency_diffs) {
- const int64_t unwrapped_ref_frame_id =
- unwrapped_frame_id - frame_dependency_diff;
-
- const auto ref_frame_it =
- decodable_unwrapped_frame_ids_.find(unwrapped_ref_frame_id);
- if (ref_frame_it == decodable_unwrapped_frame_ids_.end()) {
+ for (int64_t ref_frame_id : frame_dependencies) {
+ const auto ref_frame_it = decodable_frame_ids_.find(ref_frame_id);
+ if (ref_frame_it == decodable_frame_ids_.end()) {
// Reference frame not decodable.
return false;
}
diff --git a/modules/video_coding/loss_notification_controller.h b/modules/video_coding/loss_notification_controller.h
index 6fc5eb8..a7a1fb9 100644
--- a/modules/video_coding/loss_notification_controller.h
+++ b/modules/video_coding/loss_notification_controller.h
@@ -11,39 +11,45 @@
#ifndef MODULES_VIDEO_CODING_LOSS_NOTIFICATION_CONTROLLER_H_
#define MODULES_VIDEO_CODING_LOSS_NOTIFICATION_CONTROLLER_H_
+#include <stdint.h>
+
#include <set>
#include "absl/types/optional.h"
+#include "api/array_view.h"
#include "modules/include/module_common_types.h"
-#include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h"
-#include "rtc_base/numerics/sequence_number_util.h"
#include "rtc_base/synchronization/sequence_checker.h"
namespace webrtc {
class LossNotificationController {
public:
+ struct FrameDetails {
+ bool is_keyframe;
+ int64_t frame_id;
+ rtc::ArrayView<const int64_t> frame_dependencies;
+ };
+
LossNotificationController(KeyFrameRequestSender* key_frame_request_sender,
LossNotificationSender* loss_notification_sender);
~LossNotificationController();
// An RTP packet was received from the network.
- void OnReceivedPacket(uint16_t sequence_number,
- const RtpGenericFrameDescriptor& generic_descriptor);
+ // |frame| is non-null iff the packet is the first packet in the frame.
+ void OnReceivedPacket(uint16_t rtp_seq_num, const FrameDetails* frame);
// A frame was assembled from packets previously received.
// (Should be called even if the frame was composed of a single packet.)
void OnAssembledFrame(uint16_t first_seq_num,
- uint16_t frame_id,
+ int64_t frame_id,
bool discardable,
- rtc::ArrayView<const uint16_t> frame_dependency_diffs);
+ rtc::ArrayView<const int64_t> frame_dependencies);
private:
void DiscardOldInformation();
bool AllDependenciesDecodable(
- int64_t unwrapped_frame_id,
- rtc::ArrayView<const uint16_t> frame_dependency_diffs) const;
+ rtc::ArrayView<const int64_t> frame_dependencies) const;
// When the loss of a packet or the non-decodability of a frame is detected,
// produces a key frame request or a loss notification.
@@ -67,11 +73,8 @@
LossNotificationSender* const loss_notification_sender_
RTC_GUARDED_BY(sequence_checker_);
- SeqNumUnwrapper<uint16_t> frame_id_unwrapper_
- RTC_GUARDED_BY(sequence_checker_);
-
// Tracked to avoid processing repeated frames (buggy/malicious remote).
- absl::optional<int64_t> last_received_unwrapped_frame_id_
+ absl::optional<int64_t> last_received_frame_id_
RTC_GUARDED_BY(sequence_checker_);
// Tracked to avoid processing repeated packets.
@@ -97,8 +100,7 @@
// Track which frames are decodable. Later frames are also decodable if
// all of their dependencies can be found in this container.
// (Naturally, later frames must also be assemblable to be decodable.)
- std::set<int64_t> decodable_unwrapped_frame_ids_
- RTC_GUARDED_BY(sequence_checker_);
+ std::set<int64_t> decodable_frame_ids_ RTC_GUARDED_BY(sequence_checker_);
SequenceChecker sequence_checker_;
};
diff --git a/modules/video_coding/loss_notification_controller_unittest.cc b/modules/video_coding/loss_notification_controller_unittest.cc
index 62ff889..9c4e715 100644
--- a/modules/video_coding/loss_notification_controller_unittest.cc
+++ b/modules/video_coding/loss_notification_controller_unittest.cc
@@ -10,9 +10,12 @@
#include "modules/video_coding/loss_notification_controller.h"
+#include <stdint.h>
+
#include <limits>
#include <string>
#include <tuple>
+#include <utility>
#include <vector>
#include "absl/types/optional.h"
@@ -24,7 +27,10 @@
// The information about an RTP packet that is relevant in these tests.
struct Packet {
uint16_t seq_num;
- RtpGenericFrameDescriptor descriptor;
+ bool first_in_frame;
+ bool is_keyframe;
+ int64_t frame_id;
+ std::vector<int64_t> frame_dependencies;
};
Packet CreatePacket(
@@ -33,21 +39,17 @@
uint16_t seq_num,
uint16_t frame_id,
bool is_key_frame,
- std::vector<uint16_t> ref_frame_ids = std::vector<uint16_t>()) {
- RtpGenericFrameDescriptor frame_descriptor;
- frame_descriptor.SetFirstPacketInSubFrame(first_in_frame);
- frame_descriptor.SetLastPacketInSubFrame(last_in_frame);
+ std::vector<int64_t> ref_frame_ids = std::vector<int64_t>()) {
+ Packet packet;
+ packet.seq_num = seq_num;
+ packet.first_in_frame = first_in_frame;
if (first_in_frame) {
- frame_descriptor.SetFrameId(frame_id);
- if (!is_key_frame) {
- for (uint16_t ref_frame_id : ref_frame_ids) {
- uint16_t fdiff = frame_id - ref_frame_id;
- EXPECT_TRUE(frame_descriptor.AddFrameDependencyDiff(fdiff));
- }
- }
+ packet.is_keyframe = is_key_frame;
+ packet.frame_id = frame_id;
+ RTC_DCHECK(!is_key_frame || ref_frame_ids.empty());
+ packet.frame_dependencies = std::move(ref_frame_ids);
}
-
- return Packet{seq_num, frame_descriptor};
+ return packet;
}
class PacketStreamCreator final {
@@ -55,7 +57,7 @@
PacketStreamCreator() : seq_num_(0), frame_id_(0), next_is_key_frame_(true) {}
Packet NextPacket() {
- std::vector<uint16_t> ref_frame_ids;
+ std::vector<int64_t> ref_frame_ids;
if (!next_is_key_frame_) {
ref_frame_ids.push_back(frame_id_ - 1);
}
@@ -70,7 +72,7 @@
private:
uint16_t seq_num_;
- uint16_t frame_id_;
+ int64_t frame_id_;
bool next_is_key_frame_;
};
} // namespace
@@ -112,25 +114,27 @@
EXPECT_FALSE(LastKeyFrameRequest());
EXPECT_FALSE(LastLossNotification());
- if (packet.descriptor.FirstPacketInSubFrame()) {
+ if (packet.first_in_frame) {
previous_first_packet_in_frame_ = packet;
+ LossNotificationController::FrameDetails frame;
+ frame.is_keyframe = packet.is_keyframe;
+ frame.frame_id = packet.frame_id;
+ frame.frame_dependencies = packet.frame_dependencies;
+ uut_.OnReceivedPacket(packet.seq_num, &frame);
+ } else {
+ uut_.OnReceivedPacket(packet.seq_num, nullptr);
}
-
- uut_.OnReceivedPacket(packet.seq_num, packet.descriptor);
}
void OnAssembledFrame(uint16_t first_seq_num,
- uint16_t frame_id,
+ int64_t frame_id,
bool discardable) {
EXPECT_FALSE(LastKeyFrameRequest());
EXPECT_FALSE(LastLossNotification());
ASSERT_TRUE(previous_first_packet_in_frame_);
- const RtpGenericFrameDescriptor& frame_descriptor =
- previous_first_packet_in_frame_->descriptor;
-
uut_.OnAssembledFrame(first_seq_num, frame_id, discardable,
- frame_descriptor.FrameDependenciesDiffs());
+ previous_first_packet_in_frame_->frame_dependencies);
}
void ExpectKeyFrameRequest() {
@@ -255,19 +259,6 @@
OnReceivedPacket(CreatePacket(first, last, ++seq_num, 1, false, {0}));
}
-// No key frame or loss notifications issued due to an innocuous wrap-around
-// of the frame ID.
-TEST_P(LossNotificationControllerTest, FrameIdWrapAround) {
- uint16_t frame_id = std::numeric_limits<uint16_t>::max();
- OnReceivedPacket(CreatePacket(true, true, 100, frame_id, true));
- OnAssembledFrame(100, frame_id, false);
- ++frame_id;
- const bool first = Bool<0>();
- const bool last = Bool<1>();
- OnReceivedPacket(CreatePacket(first, last, 100, frame_id, false,
- {static_cast<uint16_t>(frame_id - 1)}));
-}
-
TEST_F(LossNotificationControllerTest,
KeyFrameAfterPacketLossProducesNoLossNotifications) {
OnReceivedPacket(CreatePacket(true, true, 100, 1, true));
@@ -334,8 +325,7 @@
const auto key_frame_packet = packet_stream.NextPacket();
OnReceivedPacket(key_frame_packet);
- OnAssembledFrame(key_frame_packet.seq_num,
- key_frame_packet.descriptor.FrameId(), false);
+ OnAssembledFrame(key_frame_packet.seq_num, key_frame_packet.frame_id, false);
const bool gap = Bool<0>();
@@ -355,6 +345,27 @@
OnReceivedPacket(repeated_packet);
}
+TEST_F(LossNotificationControllerTest,
+ RecognizesDependencyAcrossIntraFrameThatIsNotAKeyframe) {
+ int last_seq_num = 1;
+ auto receive = [&](bool is_key_frame, int64_t frame_id,
+ std::vector<int64_t> ref_frame_ids) {
+ ++last_seq_num;
+ OnReceivedPacket(CreatePacket(
+ /*first_in_frame=*/true, /*last_in_frame=*/true, last_seq_num, frame_id,
+ is_key_frame, std::move(ref_frame_ids)));
+ OnAssembledFrame(last_seq_num, frame_id, /*discardable=*/false);
+ };
+ // 11 -- 13
+ // | |
+ // 10 12
+ receive(/*is_key_frame=*/true, /*frame_id=*/10, /*ref_frame_ids=*/{});
+ receive(/*is_key_frame=*/false, /*frame_id=*/11, /*ref_frame_ids=*/{10});
+ receive(/*is_key_frame=*/false, /*frame_id=*/12, /*ref_frame_ids=*/{});
+ receive(/*is_key_frame=*/false, /*frame_id=*/13, /*ref_frame_ids=*/{11, 12});
+ EXPECT_FALSE(LastLossNotification());
+}
+
class LossNotificationControllerTestDecodabilityFlag
: public LossNotificationControllerBaseTest {
protected:
@@ -376,7 +387,7 @@
void ReceivePacket(bool first_packet_in_frame,
bool last_packet_in_frame,
- const std::vector<uint16_t>& ref_frame_ids) {
+ const std::vector<int64_t>& ref_frame_ids) {
if (first_packet_in_frame) {
frame_id_ += 1;
}
@@ -397,10 +408,10 @@
// The tests intentionally never receive this, and can therefore always
// use this as an unsatisfied dependency.
- const uint16_t never_received_frame_id_ = 123;
+ const int64_t never_received_frame_id_ = 123;
uint16_t seq_num_;
- uint16_t frame_id_;
+ int64_t frame_id_;
};
TEST_F(LossNotificationControllerTestDecodabilityFlag,
@@ -408,7 +419,7 @@
ReceiveKeyFrame();
CreateGap();
- const std::vector<uint16_t> ref_frame_ids = {key_frame_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {key_frame_frame_id_};
ReceivePacket(true, true, ref_frame_ids);
const bool expected_decodability_flag = true;
@@ -421,7 +432,7 @@
ReceiveKeyFrame();
CreateGap();
- const std::vector<uint16_t> ref_frame_ids = {never_received_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {never_received_frame_id_};
ReceivePacket(true, true, ref_frame_ids);
const bool expected_decodability_flag = false;
@@ -434,7 +445,7 @@
ReceiveKeyFrame();
CreateGap();
- const std::vector<uint16_t> ref_frame_ids = {key_frame_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {key_frame_frame_id_};
ReceivePacket(true, false, ref_frame_ids);
const bool expected_decodability_flag = true;
@@ -447,7 +458,7 @@
ReceiveKeyFrame();
CreateGap();
- const std::vector<uint16_t> ref_frame_ids = {never_received_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {never_received_frame_id_};
ReceivePacket(true, false, ref_frame_ids);
const bool expected_decodability_flag = false;
@@ -460,7 +471,7 @@
ReceiveKeyFrame();
CreateGap();
- const std::vector<uint16_t> ref_frame_ids = {key_frame_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {key_frame_frame_id_};
ReceivePacket(false, false, ref_frame_ids);
const bool expected_decodability_flag = false;
@@ -473,7 +484,7 @@
ReceiveKeyFrame();
CreateGap();
- const std::vector<uint16_t> ref_frame_ids = {never_received_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {never_received_frame_id_};
ReceivePacket(false, false, ref_frame_ids);
const bool expected_decodability_flag = false;
@@ -488,7 +499,7 @@
// First packet in multi-packet frame. A loss notification is produced
// because of the gap in RTP sequence numbers.
- const std::vector<uint16_t> ref_frame_ids = {key_frame_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {key_frame_frame_id_};
ReceivePacket(true, false, ref_frame_ids);
const bool expected_decodability_flag_first = true;
ExpectLossNotification(key_frame_seq_num_, seq_num_,
@@ -510,7 +521,7 @@
// First packet in multi-packet frame. A loss notification is produced
// because of the gap in RTP sequence numbers. The frame is also recognized
// as having non-decodable dependencies.
- const std::vector<uint16_t> ref_frame_ids = {never_received_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {never_received_frame_id_};
ReceivePacket(true, false, ref_frame_ids);
const bool expected_decodability_flag_first = false;
ExpectLossNotification(key_frame_seq_num_, seq_num_,
@@ -529,7 +540,7 @@
ReceiveKeyFrame();
CreateGap();
- const std::vector<uint16_t> ref_frame_ids = {key_frame_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {key_frame_frame_id_};
ReceivePacket(false, true, ref_frame_ids);
const bool expected_decodability_flag = false;
@@ -542,7 +553,7 @@
ReceiveKeyFrame();
CreateGap();
- const std::vector<uint16_t> ref_frame_ids = {never_received_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {never_received_frame_id_};
ReceivePacket(false, true, ref_frame_ids);
const bool expected_decodability_flag = false;
@@ -557,7 +568,7 @@
// First packet in multi-packet frame. A loss notification is produced
// because of the gap in RTP sequence numbers.
- const std::vector<uint16_t> ref_frame_ids = {key_frame_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {key_frame_frame_id_};
ReceivePacket(true, false, ref_frame_ids);
const bool expected_decodability_flag_first = true;
ExpectLossNotification(key_frame_seq_num_, seq_num_,
@@ -579,7 +590,7 @@
// First packet in multi-packet frame. A loss notification is produced
// because of the gap in RTP sequence numbers. The frame is also recognized
// as having non-decodable dependencies.
- const std::vector<uint16_t> ref_frame_ids = {never_received_frame_id_};
+ const std::vector<int64_t> ref_frame_ids = {never_received_frame_id_};
ReceivePacket(true, false, ref_frame_ids);
const bool expected_decodability_flag_first = false;
ExpectLossNotification(key_frame_seq_num_, seq_num_,
diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc
index 9ae562b..6cff575 100644
--- a/video/rtp_video_stream_receiver.cc
+++ b/video/rtp_video_stream_receiver.cc
@@ -388,7 +388,6 @@
video_header.is_first_packet_in_frame =
packet.generic_descriptor->FirstPacketInSubFrame();
video_header.is_last_packet_in_frame =
- rtp_packet.Marker() ||
packet.generic_descriptor->LastPacketInSubFrame();
if (packet.generic_descriptor->FirstPacketInSubFrame()) {
@@ -396,6 +395,19 @@
packet.generic_descriptor->FrameDependenciesDiffs().empty()
? VideoFrameType::kVideoFrameKey
: VideoFrameType::kVideoFrameDelta;
+
+ auto& descriptor = video_header.generic.emplace();
+ int64_t frame_id =
+ frame_id_unwrapper_.Unwrap(packet.generic_descriptor->FrameId());
+ descriptor.frame_id = frame_id;
+ descriptor.spatial_index = packet.generic_descriptor->SpatialLayer();
+ descriptor.temporal_index = packet.generic_descriptor->TemporalLayer();
+ descriptor.discardable =
+ packet.generic_descriptor->Discardable().value_or(false);
+ for (uint16_t fdiff :
+ packet.generic_descriptor->FrameDependenciesDiffs()) {
+ descriptor.dependencies.push_back(frame_id - fdiff);
+ }
}
video_header.width = packet.generic_descriptor->Width();
@@ -427,8 +439,19 @@
RTC_LOG(LS_WARNING) << "LossNotificationController requires generic "
"frame descriptor, but it is missing.";
} else {
- loss_notification_controller_->OnReceivedPacket(
- rtp_packet.SequenceNumber(), *packet.generic_descriptor);
+ if (video_header.is_first_packet_in_frame) {
+ RTC_DCHECK(video_header.generic);
+ LossNotificationController::FrameDetails frame;
+ frame.is_keyframe =
+ video_header.frame_type == VideoFrameType::kVideoFrameKey;
+ frame.frame_id = video_header.generic->frame_id;
+ frame.frame_dependencies = video_header.generic->dependencies;
+ loss_notification_controller_->OnReceivedPacket(
+ rtp_packet.SequenceNumber(), &frame);
+ } else {
+ loss_notification_controller_->OnReceivedPacket(
+ rtp_packet.SequenceNumber(), nullptr);
+ }
}
}
@@ -610,14 +633,13 @@
RTC_DCHECK_RUN_ON(&network_tc_);
RTC_DCHECK(frame);
- absl::optional<RtpGenericFrameDescriptor> descriptor =
- frame->GetGenericFrameDescriptor();
+ const absl::optional<RTPVideoHeader::GenericDescriptorInfo>& descriptor =
+ frame->GetRtpVideoHeader().generic;
if (loss_notification_controller_ && descriptor) {
loss_notification_controller_->OnAssembledFrame(
- frame->first_seq_num(), descriptor->FrameId(),
- descriptor->Discardable().value_or(false),
- descriptor->FrameDependenciesDiffs());
+ frame->first_seq_num(), descriptor->frame_id, descriptor->discardable,
+ descriptor->dependencies);
}
// If frames arrive before a key frame, they would not be decodable.
diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h
index 7fa3e0b..8b6ffbd 100644
--- a/video/rtp_video_stream_receiver.h
+++ b/video/rtp_video_stream_receiver.h
@@ -277,6 +277,8 @@
video_coding::PacketBuffer packet_buffer_;
UniqueTimestampCounter frame_counter_ RTC_GUARDED_BY(worker_task_checker_);
+ SeqNumUnwrapper<uint16_t> frame_id_unwrapper_
+ RTC_GUARDED_BY(worker_task_checker_);
rtc::CriticalSection reference_finder_lock_;
std::unique_ptr<video_coding::RtpFrameReferenceFinder> reference_finder_
diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc
index 569c515..d5d0be5 100644
--- a/video/rtp_video_stream_receiver_unittest.cc
+++ b/video/rtp_video_stream_receiver_unittest.cc
@@ -975,6 +975,55 @@
rtp_video_stream_receiver_->OnRtpPacket(rtp_packet);
}
+TEST_P(RtpVideoStreamReceiverGenericDescriptorTest, UnwrapsFrameId) {
+ const int version = GetParam();
+ const std::vector<uint8_t> data = {0, 1, 2, 3, 4};
+ const int kPayloadType = 123;
+
+ VideoCodec codec;
+ codec.plType = kPayloadType;
+ rtp_video_stream_receiver_->AddReceiveCodec(codec, {}, /*raw_payload=*/true);
+ rtp_video_stream_receiver_->StartReceive();
+ RtpHeaderExtensionMap extension_map;
+ RegisterRtpGenericFrameDescriptorExtension(&extension_map, version);
+
+ uint16_t rtp_sequence_number = 1;
+ auto inject_packet = [&](uint16_t wrapped_frame_id) {
+ RtpPacketReceived rtp_packet(&extension_map);
+
+ RtpGenericFrameDescriptor generic_descriptor;
+ generic_descriptor.SetFirstPacketInSubFrame(true);
+ generic_descriptor.SetLastPacketInSubFrame(true);
+ generic_descriptor.SetFrameId(wrapped_frame_id);
+ ASSERT_TRUE(SetExtensionRtpGenericFrameDescriptorExtension(
+ generic_descriptor, &rtp_packet, version));
+
+ uint8_t* payload = rtp_packet.SetPayloadSize(data.size());
+ ASSERT_TRUE(payload);
+ memcpy(payload, data.data(), data.size());
+ mock_on_complete_frame_callback_.ClearExpectedBitstream();
+ mock_on_complete_frame_callback_.AppendExpectedBitstream(data.data(),
+ data.size());
+ rtp_packet.SetMarker(true);
+ rtp_packet.SetPayloadType(kPayloadType);
+ rtp_packet.SetSequenceNumber(++rtp_sequence_number);
+ rtp_video_stream_receiver_->OnRtpPacket(rtp_packet);
+ };
+
+ int64_t first_picture_id;
+ EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame)
+ .WillOnce([&](video_coding::EncodedFrame* frame) {
+ first_picture_id = frame->id.picture_id;
+ });
+ inject_packet(/*wrapped_frame_id=*/0xffff);
+
+ EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame)
+ .WillOnce([&](video_coding::EncodedFrame* frame) {
+ EXPECT_EQ(frame->id.picture_id - first_picture_id, 3);
+ });
+ inject_packet(/*wrapped_frame_id=*/0x0002);
+}
+
#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
TEST_F(RtpVideoStreamReceiverTest, RepeatedSecondarySinkDisallowed) {
MockRtpPacketSink secondary_sink;
