Generalize FEC header formatting. (pt. 4)

webrtc/modules/rtp_rtcp/source/fec_header_reader_writer_unittest.cc

Index: webrtc/modules/rtp_rtcp/source/fec_header_reader_writer_unittest.cc
diff --git a/webrtc/modules/rtp_rtcp/source/fec_header_reader_writer_unittest.cc b/webrtc/modules/rtp_rtcp/source/fec_header_reader_writer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..887ecaa31f48ce5397743c44e3ce88813c387f61
--- /dev/null
+++ b/webrtc/modules/rtp_rtcp/source/fec_header_reader_writer_unittest.cc
@@ -0,0 +1,213 @@
+/*
+ *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include <string.h>
+
+#include <memory>
+#include <utility>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/base/basictypes.h"
+#include "webrtc/base/checks.h"
+#include "webrtc/base/random.h"
+#include "webrtc/base/scoped_ref_ptr.h"
+#include "webrtc/modules/rtp_rtcp/source/fec_test_helper.h"
+#include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h"
+#include "webrtc/modules/rtp_rtcp/source/forward_error_correction_internal.h"
+#include "webrtc/modules/rtp_rtcp/source/ulpfec_header_reader_writer.h"
+
+namespace webrtc {
+
+namespace {
+// Transport header size in bytes. Assume UDP/IPv4 as a reasonable minimum.
+constexpr size_t kTransportOverhead = 28;
+
+constexpr size_t kUlpfecHeaderSizeLBitClear = 14;
+constexpr size_t kUlpfecHeaderSizeLBitSet = 18;
+constexpr size_t kUlpfecPacketMaskOffset = 12;
+// These are pulled in from forward_error_correction_internal.h

[danilchap, 2016/08/25 17:49:15]

Might be better to remove those constants or forward:
constexpr size_t kUlpFecPacketMaskSizeLBitClear =
internal::kUlpFecPacketMaskSizeLBitClear;  // 2.
or may be forward with renaming how they used locally.
constexpr size_t kShortMask = internal::kUlpFecPacketMaskSizeLBitClear;  // 2.
constexpr size_t kLongMask = internal::kUlpFecPacketMaskSizeLBitSet;  // 6.

[brandtr, 2016/08/26 11:22:22]

Removed them.

+// constexpr size_t kUlpfecPacketMaskSizeLBitClear = 2;
+// constexpr size_t kUlpfecPacketMaskSizeLBitSet = 6;
+
+constexpr size_t kFlexfecPacketMaskSize2K = 14;

[danilchap, 2016/08/25 17:49:15]

what does 2K means?
is it kFlexfexMaxPacketMaskSize?

[brandtr, 2016/08/26 11:22:22]

Yep. Added and renamed these in the next CL.

+
+constexpr uint32_t kMediaSsrc = 1254983;
+}  // namespace
+
+using Packet = ForwardErrorCorrection::Packet;

[danilchap, 2016/08/25 17:49:15]

prefer full names:
using Packet = ::webrtc::ForwardErrorCorrection::Packet;

[brandtr, 2016/08/26 11:22:22]

Done.

+using PacketList = ForwardErrorCorrection::PacketList;
+using ReceivedFecPacket = ForwardErrorCorrection::ReceivedFecPacket;
+
+class FecPacketizationTest : public ::testing::Test {
+ protected:
+  FecPacketizationTest(std::unique_ptr<FecHeaderReader> fec_header_reader,
+                       std::unique_ptr<FecHeaderWriter> fec_header_writer,
+                       size_t min_packet_size,
+                       size_t max_packet_size)
+      : fec_header_reader_(std::move(fec_header_reader)),
+        fec_header_writer_(std::move(fec_header_writer)),
+        random_(0xabcdef123456),
+        media_packet_generator_(min_packet_size,
+                                max_packet_size,
+                                kMediaSsrc,
+                                &random_) {}
+
+  // Set the corresponding bits of the ULPFEC formatted packet mask.
+  void ClearBit(size_t index);
+  void SetBit(size_t index);
+
+  // Randomly generate ULPFEC formatted packet mask of specified size.
+  void GenerateUlpfecFormattedPacketMask(size_t packet_mask_size);
+
+  void ReadWriteAndVerifyHeaders(size_t ulpfec_formatted_packet_mask_size,
+                                 size_t expected_fec_header_size,
+                                 size_t expected_packet_mask_size);
+
+  virtual void MaybeSetFecSsrcToMediaSsrc(uint32_t media_ssrc,
+                                          ReceivedFecPacket* fec_packet) = 0;
+
+  virtual size_t ExpectedPacketMaskOffset() = 0;
+
+  std::unique_ptr<FecHeaderReader> fec_header_reader_;
+  std::unique_ptr<FecHeaderWriter> fec_header_writer_;
+
+  Random random_;
+  test::fec::MediaPacketGenerator media_packet_generator_;
+  PacketList* media_packets_;
+  uint8_t packet_mask_[kFlexfecPacketMaskSize2K];  // Only filled with ULPFEC
+                                                   // packet mask (max 6
+                                                   // bytes), but needs to be
+                                                   // 14 bytes in order to do
+                                                   // memcmp with parsed
+                                                   // FlexFEC mask.
+};
+
+class UlpfecPacketizationTest : public FecPacketizationTest {

[danilchap, 2016/08/25 17:49:15]

any reason why this is in same file instead of own test file?
ulpfec_header_reader_writer_unittests.cc

Might also help to rename it to UlpfecHeaderReaderWriterTest to stress what
classes are under test.

[brandtr, 2016/08/26 11:22:22]

Move and renamed.

+ protected:
+  UlpfecPacketizationTest()
+      : FecPacketizationTest(
+            std::unique_ptr<FecHeaderReader>(new UlpfecHeaderReader()),
+            std::unique_ptr<FecHeaderWriter>(new UlpfecHeaderWriter()),
+            kRtpHeaderSize,
+            IP_PACKET_SIZE - kRtpHeaderSize - kTransportOverhead -
+                kUlpfecHeaderSizeLBitSet) {}
+
+  void MaybeSetFecSsrcToMediaSsrc(uint32_t media_ssrc,
+                                  ReceivedFecPacket* fec_packet) override;
+
+  size_t ExpectedPacketMaskOffset() override;
+};
+
+void FecPacketizationTest::ClearBit(size_t index) {
+  packet_mask_[index / 8] &= ~(1 << (7 - index % 8));
+}
+
+void FecPacketizationTest::SetBit(size_t index) {
+  packet_mask_[index / 8] |= (1 << (7 - index % 8));
+}
+
+void FecPacketizationTest::GenerateUlpfecFormattedPacketMask(
+    size_t packet_mask_size) {
+  RTC_DCHECK_LE(packet_mask_size, 6u);

[danilchap, 2016/08/25 17:49:15]

kUlpfecMaxPacketMaskSize instead of 6

[brandtr, 2016/08/26 11:22:22]

Good catch, thanks.

+  memset(packet_mask_, 0u, 14);

[danilchap, 2016/08/25 17:49:15]

kFlexfecPacketMaskSize2K instead of 14

[brandtr, 2016/08/26 11:22:22]

Done.

+  for (size_t i = 0; i < packet_mask_size; ++i) {
+    packet_mask_[i] = random_.Rand<uint8_t>();
+  }
+}
+
+void FecPacketizationTest::ReadWriteAndVerifyHeaders(
+    size_t ulpfec_formatted_packet_mask_size,
+    size_t expected_fec_header_size,
+    size_t expected_packet_mask_size) {
+  // Generate media.
+  constexpr int kNumMediaPackets = 1;
+  PacketList media_packets =
+      media_packet_generator_.ConstructMediaPackets(kNumMediaPackets);
+  uint16_t media_seq_num =
+      media_packet_generator_.GetFecSeqNum() - kNumMediaPackets;

[danilchap, 2016/08/25 17:49:15]

it is not obvious why it is correct.
may be instead:
uint16_t media_seq_num = kMyTestMediaSeqNum; (or random_.Rand<uint16_t>();)
PacketList media_packets =
    media_packet_generator_.ConstructMediaPackets(kNumMediaPackets,
media_seq_num);

[brandtr, 2016/08/26 11:22:22]

Done. (See next patchset.)

+
+  // Set up FEC packet containers.
+  ReceivedFecPacket received_fec_packet;
+  received_fec_packet.pkt = rtc::scoped_refptr<Packet>(new Packet());
+  Packet* sent_fec_packet = received_fec_packet.pkt.get();
+  sent_fec_packet->length = media_packets.front()->length;
+  for (size_t i = 0; i < sent_fec_packet->length; ++i) {
+    sent_fec_packet->data[i] = random_.Rand<uint8_t>();
+  }
+
+  // Write FEC header.
+  fec_header_writer_->FinalizeFecHeader(media_packets, packet_mask_,
+                                        ulpfec_formatted_packet_mask_size,
+                                        sent_fec_packet);
+
+  // Read FEC header.
+  MaybeSetFecSsrcToMediaSsrc(kMediaSsrc, &received_fec_packet);
+  EXPECT_TRUE(fec_header_reader_->ReadFecHeader(&received_fec_packet));
+
+  // Verify that written and read FEC headers are consistent.
+  EXPECT_EQ(expected_fec_header_size, received_fec_packet.fec_header_size);
+  EXPECT_EQ(received_fec_packet.pkt->length - expected_fec_header_size,
+            received_fec_packet.protection_length);
+  auto stream_packet_mask_info =
+      received_fec_packet.packet_mask_infos[kMediaSsrc];
+  EXPECT_EQ(media_seq_num, stream_packet_mask_info.seq_num_base);
+  size_t offset = stream_packet_mask_info.offset;
+  EXPECT_EQ(ExpectedPacketMaskOffset(), offset);
+  size_t size = stream_packet_mask_info.size;
+  ASSERT_EQ(expected_packet_mask_size, size);
+  EXPECT_EQ(0,
+            memcmp(packet_mask_, &received_fec_packet.pkt->data[offset], size));
+  EXPECT_EQ(0, memcmp(&sent_fec_packet->data[expected_fec_header_size],
+                      &received_fec_packet.pkt->data[expected_fec_header_size],
+                      sent_fec_packet->length - expected_fec_header_size));
+}
+
+void UlpfecPacketizationTest::MaybeSetFecSsrcToMediaSsrc(
+    uint32_t media_ssrc,
+    ReceivedFecPacket* fec_packet) {
+  fec_packet->rtp_ssrc = media_ssrc;
+}
+
+size_t UlpfecPacketizationTest::ExpectedPacketMaskOffset() {
+  return kUlpfecPacketMaskOffset;
+}
+
+TEST_F(UlpfecPacketizationTest, CalculateSmallHeaderSize) {
+  constexpr size_t kShortMask = kUlpfecPacketMaskSizeLBitClear;
+  GenerateUlpfecFormattedPacketMask(kShortMask);
+  EXPECT_EQ(
+      kUlpfecHeaderSizeLBitClear,
+      fec_header_writer_->FecHeaderSize(
+          fec_header_writer_->MinPacketMaskSize(packet_mask_, kShortMask)));
+}
+
+TEST_F(UlpfecPacketizationTest, CalculateLargeHeaderSize) {
+  constexpr size_t kLongMask = kUlpfecPacketMaskSizeLBitSet;
+  GenerateUlpfecFormattedPacketMask(kLongMask);
+  EXPECT_EQ(kUlpfecHeaderSizeLBitSet, fec_header_writer_->FecHeaderSize(
+                                          fec_header_writer_->MinPacketMaskSize(
+                                              packet_mask_, kLongMask)));
+}
+
+TEST_F(UlpfecPacketizationTest, WriteAndReadHeaderSmall) {
+  constexpr size_t kShortMask = kUlpfecPacketMaskSizeLBitClear;
+  GenerateUlpfecFormattedPacketMask(kShortMask);
+  ReadWriteAndVerifyHeaders(kShortMask, kUlpfecHeaderSizeLBitClear,
+                            kUlpfecPacketMaskSizeLBitClear);
+}
+
+TEST_F(UlpfecPacketizationTest, WriteAndReadHeaderLarge) {
+  constexpr size_t kLongMask = kUlpfecPacketMaskSizeLBitSet;
+  GenerateUlpfecFormattedPacketMask(kLongMask);
+  ReadWriteAndVerifyHeaders(kLongMask, kUlpfecHeaderSizeLBitSet,
+                            kUlpfecPacketMaskSizeLBitSet);
+}
+
+}  // namespace webrtc