zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor LossNotificationController to not use VCMPacket

Browse Source 
Bug: None
Change-Id: I15e1b3405c6538dd22aaeb125751c158c069478a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/152384
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29193}
This commit is contained in:
Niels Möller
2019-09-13 14:18:58 +02:00
committed by Commit Bot
parent 7bf7a427bf
commit a740142398
4 changed files with 41 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
modules/video_coding/loss_notification_controller.cc
View File

@ -43,19 +43,15 @@ LossNotificationController::LossNotificationController(
LossNotificationController::~LossNotificationController() = default;
void LossNotificationController::OnReceivedPacket(const VCMPacket& packet) {
void LossNotificationController::OnReceivedPacket(
uint16_t rtp_seq_num,
const RtpGenericFrameDescriptor& generic_descriptor) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
if (!packet.generic_descriptor) {
RTC_LOG(LS_WARNING) << "Generic frame descriptor missing. Buggy remote? "
"Misconfigured local?";
return;
}
// Ignore repeated or reordered packets.
// TODO(bugs.webrtc.org/10336): Handle packet reordering.
if (last_received_seq_num_ &&
!AheadOf(packet.seqNum, *last_received_seq_num_)) {
!AheadOf(rtp_seq_num, *last_received_seq_num_)) {
return;
}
@ -63,12 +59,12 @@ void LossNotificationController::OnReceivedPacket(const VCMPacket& packet) {
const bool seq_num_gap =
last_received_seq_num_ &&
packet.seqNum != static_cast<uint16_t>(*last_received_seq_num_ + 1u);
rtp_seq_num != static_cast<uint16_t>(*last_received_seq_num_ + 1u);
last_received_seq_num_ = packet.seqNum;
last_received_seq_num_ = rtp_seq_num;
if (packet.generic_descriptor->FirstPacketInSubFrame()) {
const uint16_t frame_id = packet.generic_descriptor->FrameId();
if (generic_descriptor.FirstPacketInSubFrame()) {
const uint16_t frame_id = generic_descriptor.FrameId();
const int64_t unwrapped_frame_id = frame_id_unwrapper_.Unwrap(frame_id);
// Ignore repeated or reordered frames.
@ -83,7 +79,7 @@ void LossNotificationController::OnReceivedPacket(const VCMPacket& packet) {
last_received_unwrapped_frame_id_ = unwrapped_frame_id;
const bool intra_frame =
packet.generic_descriptor->FrameDependenciesDiffs().empty();
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.
@ -98,11 +94,10 @@ void LossNotificationController::OnReceivedPacket(const VCMPacket& packet) {
current_frame_potentially_decodable_ = true;
} else {
const bool all_dependencies_decodable = AllDependenciesDecodable(
unwrapped_frame_id,
packet.generic_descriptor->FrameDependenciesDiffs());
unwrapped_frame_id, generic_descriptor.FrameDependenciesDiffs());
current_frame_potentially_decodable_ = all_dependencies_decodable;
if (seq_num_gap || !current_frame_potentially_decodable_) {
HandleLoss(packet.seqNum, current_frame_potentially_decodable_);
HandleLoss(rtp_seq_num, current_frame_potentially_decodable_);
}
}
} else if (seq_num_gap || !current_frame_potentially_decodable_) {
@ -111,7 +106,7 @@ void LossNotificationController::OnReceivedPacket(const VCMPacket& packet) {
// even if only one of its packets is lost. We do this because the bigger
// the frame, the more likely it is to be non-discardable, and therefore
// the more robust we wish to be to loss of the feedback messages.
HandleLoss(packet.seqNum, false);
HandleLoss(rtp_seq_num, false);
}
}

5
modules/video_coding/loss_notification_controller.h
View File

@ -15,7 +15,7 @@
#include "absl/types/optional.h"
#include "modules/include/module_common_types.h"
#include "modules/video_coding/packet.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"
@ -28,7 +28,8 @@ class LossNotificationController {
~LossNotificationController();
// An RTP packet was received from the network.
void OnReceivedPacket(const VCMPacket& packet);
void OnReceivedPacket(uint16_t sequence_number,
const RtpGenericFrameDescriptor& generic_descriptor);
// A frame was assembled from packets previously received.
// (Should be called even if the frame was composed of a single packet.)

46
modules/video_coding/loss_notification_controller_unittest.cc
View File

@ -20,7 +20,14 @@
namespace webrtc {
namespace {
VCMPacket CreatePacket(
// The information about an RTP packet that is relevant in these tests.
struct Packet {
uint16_t seq_num;
RtpGenericFrameDescriptor descriptor;
};
Packet CreatePacket(
bool first_in_frame,
bool last_in_frame,
uint16_t seq_num,
@ -40,24 +47,21 @@ VCMPacket CreatePacket(
}
}
VCMPacket packet;
packet.seqNum = seq_num;
packet.generic_descriptor = frame_descriptor;
return packet;
return Packet{seq_num, frame_descriptor};
}
class PacketStreamCreator final {
public:
PacketStreamCreator() : seq_num_(0), frame_id_(0), next_is_key_frame_(true) {}
VCMPacket NextPacket() {
Packet NextPacket() {
std::vector<uint16_t> ref_frame_ids;
if (!next_is_key_frame_) {
ref_frame_ids.push_back(frame_id_ - 1);
}
VCMPacket packet = CreatePacket(true, true, seq_num_++, frame_id_++,
next_is_key_frame_, ref_frame_ids);
Packet packet = CreatePacket(true, true, seq_num_++, frame_id_++,
next_is_key_frame_, ref_frame_ids);
next_is_key_frame_ = false;
@ -104,16 +108,15 @@ class LossNotificationControllerBaseTest : public ::testing::Test,
decodability_flag);
}
void OnReceivedPacket(const VCMPacket& packet) {
void OnReceivedPacket(const Packet& packet) {
EXPECT_FALSE(LastKeyFrameRequest());
EXPECT_FALSE(LastLossNotification());
if (packet.generic_descriptor &&
packet.generic_descriptor->FirstPacketInSubFrame()) {
if (packet.descriptor.FirstPacketInSubFrame()) {
previous_first_packet_in_frame_ = packet;
}
uut_.OnReceivedPacket(packet);
uut_.OnReceivedPacket(packet.seq_num, packet.descriptor);
}
void OnAssembledFrame(uint16_t first_seq_num,
@ -124,7 +127,7 @@ class LossNotificationControllerBaseTest : public ::testing::Test,
ASSERT_TRUE(previous_first_packet_in_frame_);
const RtpGenericFrameDescriptor& frame_descriptor =
previous_first_packet_in_frame_->generic_descriptor.value();
previous_first_packet_in_frame_->descriptor;
uut_.OnAssembledFrame(first_seq_num, frame_id, discardable,
frame_descriptor.FrameDependenciesDiffs());
@ -197,7 +200,7 @@ class LossNotificationControllerBaseTest : public ::testing::Test,
// of a subsequent frame, OnAssembledFrame is not called, and so this is
// note read. Therefore, it's not a problem if it is not cleared when
// the frame changes.)
absl::optional<VCMPacket> previous_first_packet_in_frame_;
absl::optional<Packet> previous_first_packet_in_frame_;
};
class LossNotificationControllerTest
@ -331,8 +334,8 @@ TEST_P(LossNotificationControllerTest, RepeatedPacketsAreIgnored) {
const auto key_frame_packet = packet_stream.NextPacket();
OnReceivedPacket(key_frame_packet);
OnAssembledFrame(key_frame_packet.seqNum,
key_frame_packet.generic_descriptor->FrameId(), false);
OnAssembledFrame(key_frame_packet.seq_num,
key_frame_packet.descriptor.FrameId(), false);
const bool gap = Bool<0>();
@ -346,21 +349,12 @@ TEST_P(LossNotificationControllerTest, RepeatedPacketsAreIgnored) {
if (gap) {
// Loss notification issued because of the gap. This is not the focus of
// the test.
ExpectLossNotification(key_frame_packet.seqNum, repeated_packet.seqNum,
ExpectLossNotification(key_frame_packet.seq_num, repeated_packet.seq_num,
false);
}
OnReceivedPacket(repeated_packet);
}
// Frames without the generic frame descriptor cannot be properly handled,
// but must not induce a crash.
TEST_F(LossNotificationControllerTest,
IgnoreFramesWithoutGenericFrameDescriptor) {
auto packet = CreatePacket(true, true, 1, 0, true);
packet.generic_descriptor.reset();
OnReceivedPacket(packet);
}
class LossNotificationControllerTestDecodabilityFlag
: public LossNotificationControllerBaseTest {
protected: