First-order-FEC recoverability calculation

webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc

 /*
  *  Copyright (c) 2017 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 <algorithm>
 
 #include "webrtc/base/array_view.h"
 #include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 #include "webrtc/voice_engine/transport_feedback_packet_loss_tracker.h"
 
 namespace webrtc {
 
 namespace {
 
+template <typename T>
+T FuzzInput(const uint8_t** data, size_t* size) {

[stefan-webrtc, 2017/02/01 13:11:19]

Seems like over complicating things to me that we make this a template. It's
only used with uint16_t?

[sprang_webrtc, 2017/02/01 14:27:19]

Only used in one place, seems like you don't need (to templetize) this?

[elad.alon_webrtc.org, 2017/02/01 15:25:49]

Please see the subsequent CL in the series, where this is:
1. used for uint8_t and for uint16_t.
2. is used in the already-templatized ReadData().

+  RTC_CHECK(*size >= sizeof(T));

[sprang_webrtc, 2017/02/01 14:27:19]

nit: RTC_CHECK_GE

[elad.alon_webrtc.org, 2017/02/01 15:25:49]

Done.

+  T rc = ByteReader<T>::ReadBigEndian(*data);
+  *data += sizeof(T);
+  *size -= sizeof(T);
+  return rc;
+}
+
+size_t FuzzInRange(const uint8_t** data,
+                   size_t* size,
+                   size_t lower,
+                   size_t upper) {
+  // Achieve a close-to-uniform distribution.
+  RTC_CHECK_LE(lower, upper);
+  RTC_CHECK_LT(upper - lower, 1 << (8 * sizeof(uint16_t)));
+  const size_t range = upper - lower;
+  const uint16_t fuzzed = FuzzInput<uint16_t>(data, size);
+  const size_t offset = (static_cast<float>(fuzzed) / 0x10000) * (range + 1);

[minyue-webrtc, 2017/01/31 12:50:19]

nit: you probably need an explicit static_cast<size_t> here to pass some bot, or
maybe even better a std::floor

[elad.alon_webrtc.org, 2017/01/31 13:43:54]

Do you think I should do that even if all bots pass, or only if one complains?
(I wouldn't go with std::floor, as that would mask my logic being wrong, if it
ever indeed has an effect.)

[minyue-webrtc, 2017/01/31 15:09:39]

Forget about my saying of std::floor, it returns a float number which also
requires casting.

Since bots don't complain, this is fine.

+  RTC_CHECK_LE(offset, range);  // (fuzzed <= 0xffff) -> (offset < range + 1)

[minyue-webrtc, 2017/01/31 12:53:27]

BTW, this CHECK is not necessary. Aren't you sure about the calculation in line
40?

[elad.alon_webrtc.org, 2017/01/31 13:43:54]

This check explains why the calculation above was correct, and shows it with a
CHECK. I think it's helpful for code readers.

[minyue-webrtc, 2017/01/31 15:09:39]

It looks redundant. 40 can speaks for itself.

+  return lower + offset;
+}
+
 class TransportFeedbackGenerator {
  public:
   explicit TransportFeedbackGenerator(rtc::ArrayView<const uint8_t> data)
       : data_(data), ended_(false), data_idx_(0) {}
 
   void GetNextTransportFeedback(rtcp::TransportFeedback* feedback) {
     uint16_t base_seq_num = 0;
     if (!ReadData<uint16_t>(&base_seq_num)) {
       return;
     }
@@ skipping 24 matching lines @@
         status_byte >>= 1;
       }
     }
   }
 
   bool ended() const { return ended_; }
 
  private:
   template <typename T>
   bool ReadData(T* value) {
-    RTC_DCHECK(!ended_);
+    RTC_CHECK(!ended_);
     if (data_idx_ + sizeof(T) > data_.size()) {
       ended_ = true;
       return false;
     }
     *value = ByteReader<T>::ReadBigEndian(&data_[data_idx_]);
     data_idx_ += sizeof(T);
     return true;
   }
 
   const rtc::ArrayView<const uint8_t> data_;
   bool ended_;
   size_t data_idx_;
 };
 
 }  // namespace
 
 void FuzzOneInput(const uint8_t* data, size_t size) {
-  if (size < sizeof(uint32_t)) {
+  if (size < 3 * sizeof(uint16_t)) {
     return;
   }
   constexpr size_t kSeqNumHalf = 0x8000u;
-  const size_t window_size_1 = std::min<size_t>(
-      kSeqNumHalf,
-      std::max<uint16_t>(1, ByteReader<uint16_t>::ReadBigEndian(data)));
-  data += sizeof(uint16_t);
-  const size_t window_size_2 = std::min<size_t>(
-      kSeqNumHalf,
-      std::max<uint16_t>(1, ByteReader<uint16_t>::ReadBigEndian(data)));
-  data += sizeof(uint16_t);
-  size -= 2 * sizeof(uint16_t);
+
+  // 0x8000 >= max_window_size >= plr_min_num_packets > rplr_min_num_pairs >= 1
+  // (The distribution isn't uniform, but it's enough; more would be overkill.)
+  const size_t max_window_size = FuzzInRange(&data, &size, 2, kSeqNumHalf);
+  const size_t plr_min_num_packets =
+      FuzzInRange(&data, &size, 2, max_window_size);
+  const size_t rplr_min_num_pairs =
+      FuzzInRange(&data, &size, 1, plr_min_num_packets - 1);
 
   TransportFeedbackPacketLossTracker tracker(
-      std::min(window_size_1, window_size_2),
-      std::max(window_size_1, window_size_2));
+      max_window_size, plr_min_num_packets, rplr_min_num_pairs);
+
   TransportFeedbackGenerator feedback_generator(
       rtc::ArrayView<const uint8_t>(data, size));
+
   while (!feedback_generator.ended()) {
     rtcp::TransportFeedback feedback;
     feedback_generator.GetNextTransportFeedback(&feedback);
     tracker.OnReceivedTransportFeedback(feedback);
     tracker.Validate();
   }
 }
 
 }  // namespace webrtc