Updated SmoothnessHelper to count max consecutive "bad" windows.

The SmoothnessHelper now measures how many consecutive windows have
a proportion of dropped frames that's over a threshold.  It outputs
a training example with maximum such number found over a playback.

Additionally, it now assumes that it will be created when playback
starts, and will be destroyed when it ends, rather than taking an
explicit "is playing" signal.  If the feature vector changes, then
the SmoothnessHelper should be destroyed / reconstructed too.

Change-Id: I8ceef55f33d6d8164f4a6e2d0e1c77fdbbce53e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2004071
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732673}

media/blink/smoothness_helper.cc

@@ -13,6 +13,9 @@
 namespace {
 static constexpr base::TimeDelta kSegmentSize =
     base::TimeDelta::FromSeconds(60);
+
+// Max proportion of dropped frames in a window before we call it "not smooth".
+static constexpr float kMaxDroppedFramesPerWindow = 0.2;
 }
 
 namespace media {
@@ -21,120 +24,148 @@ using learning::FeatureVector;
 using learning::LearningTaskController;
 using learning::TargetValue;
 
-class SmoothnessHelperImpl : public SmoothnessHelper {
+// Monitor smoothness during a playback, and call back on each window.
+class SmoothnessWindowMonitor {
  public:
-  SmoothnessHelperImpl(std::unique_ptr<LearningTaskController> controller,
-                       const FeatureVector& features,
-                       Client* player)
-      : controller_(std::move(controller)),
-        features_(features),
-        player_(player) {}
-
-  // This will ignore the last segment, if any, which is fine since it's not
-  // a complete segment.
-  ~SmoothnessHelperImpl() override = default;
-
-  void NotifyPlayState(bool playing) override {
-    if (playing) {
-      if (segment_decoded_frames_)
-        return;
-
-      // We're starting a new playback, so record the baseline frame counts.
-      segment_dropped_frames_ = player_->DroppedFrameCount();
-      segment_decoded_frames_ = player_->DecodedFrameCount();
-      worst_segment_during_playback_ = TargetValue(0);
-
-      DCHECK(!id_);
-
-      // Don't bother to start the observation until the timer fires, since we
-      // don't wanto to record short playbacks.
-
-      update_timer_.Start(FROM_HERE, kSegmentSize,
-                          base::BindRepeating(&SmoothnessHelperImpl::OnTimer,
-                                              base::Unretained(this)));
-    } else {
-      if (!segment_decoded_frames_)
-        return;
-
-      // If we started an observation, then complete it.  Otherwise, the segment
-      // wasn't long enough.  Note that we also don't update the worst NNR
-      // rate here, so that we don't include very short partial segments that
-      // might be artificially high.  Note that this might be a bad idea; if
-      // the site detects bad playback and adapts before we've measured one
-      // segment, then we'll never record those NNRs.  We might want to allow
-      // the final segment to be smaller than |kSegmentSize|, as long as it's
-      // not too small.
-      if (id_)
-        controller_->CompleteObservation(*id_, worst_segment_during_playback_);
-
-      // End the segment and the playback.
-      segment_decoded_frames_.reset();
-      segment_dropped_frames_.reset();
-      update_timer_.Stop();
-      id_.reset();
-    }
+  using WindowCB = base::RepeatingCallback<void(int64_t dropped_frames,
+                                                int64_t decoded_frames)>;
+  SmoothnessWindowMonitor(SmoothnessHelper::Client* player, WindowCB cb)
+      : player_(player), cb_(std::move(cb)) {
+    segment_dropped_frames_ = player_->DroppedFrameCount();
+    segment_decoded_frames_ = player_->DecodedFrameCount();
+
+    update_timer_.Start(FROM_HERE, kSegmentSize,
+                        base::BindRepeating(&SmoothnessWindowMonitor::OnTimer,
+                                            base::Unretained(this)));
   }
 
+  ~SmoothnessWindowMonitor() = default;
+
   // Split playback into segments of length |kSegmentSize|, and update the
   // default value of the current playback.
   void OnTimer() {
-    DCHECK(segment_decoded_frames_);
-
     auto new_dropped_frames = player_->DroppedFrameCount();
-    auto dropped_frames = new_dropped_frames - *segment_dropped_frames_;
+    auto dropped_frames = new_dropped_frames - segment_dropped_frames_;
     segment_dropped_frames_ = new_dropped_frames;
 
     auto new_decoded_frames = player_->DecodedFrameCount();
-    auto decoded_frames = new_decoded_frames - *segment_decoded_frames_;
+    auto decoded_frames = new_decoded_frames - segment_decoded_frames_;
     segment_decoded_frames_ = new_decoded_frames;
 
     if (!decoded_frames)
       return;
 
-    // The target value is just the percentage of dropped frames.
-    auto target = TargetValue(((double)dropped_frames) / decoded_frames);
+    cb_.Run(dropped_frames, decoded_frames);
+  }
+
+ private:
+  SmoothnessHelper::Client* player_ = nullptr;
+  WindowCB cb_;
+  base::RepeatingTimer update_timer_;
+  // Current dropped, decoded frames at the start of the segment.
+  int64_t segment_decoded_frames_;
+  int64_t segment_dropped_frames_;
+};
 
-    // See if this is worse than any previous segment.
-    if (target > worst_segment_during_playback_)
-      worst_segment_during_playback_ = target;
+class SmoothnessHelperImpl : public SmoothnessHelper {
+ public:
+  SmoothnessHelperImpl(
+      std::unique_ptr<LearningTaskController> consecutive_controller,
+      const FeatureVector& features,
+      Client* player)
+      : consecutive_bad_(std::move(consecutive_controller)),
+        features_(features),
+        player_(player) {
+    monitor_ = std::make_unique<SmoothnessWindowMonitor>(
+        player_, base::BindRepeating(&SmoothnessHelperImpl::OnWindow,
+                                     base::Unretained(this)));
+  }
+
+  // This will ignore the last segment, if any, which is fine since it's not
+  // a complete segment.  However, any in-progress observation will be completed
+  // with the default value if we've gotten enough data to set one.
+  ~SmoothnessHelperImpl() override = default;
 
-    // Start an observation for this playback, or update the default.
-    if (!id_) {
-      id_ = base::UnguessableToken::Create();
-      controller_->BeginObservation(*id_, features_,
-                                    worst_segment_during_playback_);
+  // Split playback into segments of length |kSegmentSize|, and update the
+  // default value of the current playback.
+  void OnWindow(int64_t dropped_frames, int64_t decoded_frames) {
+    // Compute the percentage of dropped frames for this window.
+    double pct = (static_cast<double>(dropped_frames)) / decoded_frames;
+
+    // Once we get one full window, default to 0 for the consecutive windows
+    // prediction task.
+    if (!consecutive_bad_.is_started())
+      consecutive_bad_.UpdateObservation(features_, TargetValue(0));
+
+    // If this is a bad window, extend the run of consecutive bad windows, and
+    // update the target value if this is a new longest run.
+    if (pct >= kMaxDroppedFramesPerWindow) {
+      consecutive_bad_windows_++;
+      if (consecutive_bad_windows_ > max_consecutive_bad_windows_) {
+        max_consecutive_bad_windows_ = consecutive_bad_windows_;
+        consecutive_bad_.UpdateObservation(
+            features_, TargetValue(max_consecutive_bad_windows_));
+      }
     } else {
-      controller_->UpdateDefaultTarget(*id_, worst_segment_during_playback_);
+      consecutive_bad_windows_ = 0;
+      // Don't update the target value, since any previous target value is still
+      // the max consecutive windows.
     }
   }
 
-  // Current dropped, decoded frames at the start of the segment, if any.
-  base::Optional<int64_t> segment_decoded_frames_;
-  base::Optional<int64_t> segment_dropped_frames_;
+  // Helper for different learning tasks.
+  struct Task {
+    Task(std::unique_ptr<LearningTaskController> controller)
+        : controller_(std::move(controller)) {}
+
+    ~Task() = default;
+
+    // Return true if and only if we've started an observation.
+    bool is_started() const { return !!id_; }
+
+    void UpdateObservation(const FeatureVector& features,
+                           TargetValue current_target) {
+      target_value_ = current_target;
+      if (!is_started()) {
+        id_ = base::UnguessableToken::Create();
+        controller_->BeginObservation(*id_, features, target_value_);
+      } else {
+        controller_->UpdateDefaultTarget(*id_, target_value_);
+      }
+    }
 
-  // Of all the segments in this playback, this is the worst NNR ratio.
-  TargetValue worst_segment_during_playback_;
+    const TargetValue& target_value() const { return target_value_; }
 
-  std::unique_ptr<LearningTaskController> controller_;
+   private:
+    // If an observation is in progress, then this is the id.
+    base::Optional<base::UnguessableToken> id_;
+    std::unique_ptr<LearningTaskController> controller_;
+    TargetValue target_value_;
 
-  FeatureVector features_;
+    DISALLOW_COPY_AND_ASSIGN(Task);
+  };
 
-  base::RepeatingTimer update_timer_;
+  // Struct to hold all of the "at least |n| consecutive bad windows" data.
+  struct Task consecutive_bad_;
+
+  int consecutive_bad_windows_ = 0;
+  int max_consecutive_bad_windows_ = 0;
+
+  FeatureVector features_;
 
   // WebMediaPlayer which will tell us about the decoded / dropped frame counts.
   Client* player_;
 
-  // If an observation is in progress, then this is the id.
-  base::Optional<base::UnguessableToken> id_;
+  std::unique_ptr<SmoothnessWindowMonitor> monitor_;
 };
 
 // static
 std::unique_ptr<SmoothnessHelper> SmoothnessHelper::Create(
-    std::unique_ptr<LearningTaskController> controller,
+    std::unique_ptr<LearningTaskController> consecutive_controller,
     const FeatureVector& features,
     Client* player) {
-  return std::make_unique<SmoothnessHelperImpl>(std::move(controller), features,
-                                                player);
+  return std::make_unique<SmoothnessHelperImpl>(
+      std::move(consecutive_controller), features, player);
 }
 
 // static

media/blink/smoothness_helper.h

@@ -40,18 +40,10 @@ class MEDIA_BLINK_EXPORT SmoothnessHelper {
   // we create.  They should be features that could be captured at the time a
   // prediction would be needed.
   static std::unique_ptr<SmoothnessHelper> Create(
-      std::unique_ptr<learning::LearningTaskController> controller,
+      std::unique_ptr<learning::LearningTaskController> consecutive_controller,
       const learning::FeatureVector& features,
       Client* player);
 
-  // Notify us when we start or stop playing.
-  // TODO(liberato): There is an open question whether we'd like one call of
-  // the form "ThePlayerIsInTheRightStateToRecordSmoothness(bool)", or whether
-  // we'd like multiple calls to record the state of the player, like
-  // "SetIsPlaying", "SetIsBackgrounded", etc.  The difference is whether the
-  // decision to record is made by the player or by us.
-  virtual void NotifyPlayState(bool playing) = 0;
-
   // We split playbacks up into |kSegmentSize| units, and record the worst
   // dropped frame ratio over all segments of a playback.  A playback is not
   // recorded if it doesn't contain at least one full segment.

media/blink/smoothness_helper_unittest.cc

@@ -73,10 +73,11 @@ class SmoothnessHelperTest : public testing::Test {
 
  public:
   void SetUp() override {
-    auto ltc = std::make_unique<MockLearningTaskController>();
-    ltc_ = ltc.get();
+    auto consecutive_ltc = std::make_unique<MockLearningTaskController>();
+    consecutive_ltc_ = consecutive_ltc.get();
     features_.push_back(FeatureValue(123));
-    helper_ = SmoothnessHelper::Create(std::move(ltc), features_, &client_);
+    helper_ = SmoothnessHelper::Create(std::move(consecutive_ltc), features_,
+                                       &client_);
     segment_size_ = SmoothnessHelper::SegmentSizeForTesting();
   }
 
@@ -99,101 +100,62 @@ class SmoothnessHelperTest : public testing::Test {
   // Helper under test
   std::unique_ptr<SmoothnessHelper> helper_;
 
-  MockLearningTaskController* ltc_ = nullptr;
+  MockLearningTaskController* consecutive_ltc_;
+
   MockClient client_;
   FeatureVector features_;
 
   base::TimeDelta segment_size_;
 };
 
-TEST_F(SmoothnessHelperTest, PauseWithoutPlayDoesNothing) {
-  EXPECT_CALL(*ltc_, BeginObservation(_, _, _)).Times(0);
-  helper_->NotifyPlayState(false);
-  base::RunLoop().RunUntilIdle();
-}
-
-TEST_F(SmoothnessHelperTest, PlayThenImmediatePauseCancelsObservation) {
-  // If not enough time has elapsed, play then pause shouldn't record anything.
-  // Note that Begin then Cancel would be okay too, but it's hard to set
-  // expectations for either case.  So, we just pick the one that it actually
-  // does in this case.
-  EXPECT_CALL(*ltc_, BeginObservation(_, _, _)).Times(0);
-  helper_->NotifyPlayState(true);
-  helper_->NotifyPlayState(false);
-  base::RunLoop().RunUntilIdle();
-}
-
-TEST_F(SmoothnessHelperTest, PlayRecordsWorstSegment) {
-  // Record three segments, and see if it chooses the worst.
+TEST_F(SmoothnessHelperTest, MaxBadWindowsRecordsTrue) {
+  // Record three bad segments, and verify that it records 'true'.
   SetFrameCounters(0, 0);
-  helper_->NotifyPlayState(true);
   base::RunLoop().RunUntilIdle();
+  int dropped_frames = 0;
+  int total_frames = 0;
 
-  // First segment has no dropped frames..
-  EXPECT_CALL(*ltc_, BeginObservation(_, _, OPT_TARGET(Eq(0.0)))).Times(1);
-  SetFrameCounters(0, 1000);
+  // First segment has no dropped frames.  Should record 0.
+  EXPECT_CALL(*consecutive_ltc_, BeginObservation(_, _, OPT_TARGET(0.0)))
+      .Times(1);
+  SetFrameCounters(dropped_frames += 0, total_frames += 1000);
   FastForwardBy(segment_size_);
   base::RunLoop().RunUntilIdle();
+  testing::Mock::VerifyAndClearExpectations(consecutive_ltc_);
 
-  // Second segment has quite a lot of dropped frames.
-  EXPECT_CALL(*ltc_, UpdateDefaultTarget(_, OPT_TARGET(Gt(0.99)))).Times(1);
-  SetFrameCounters(999, 2000);
+  // Second segment has a lot of dropped frames, so the target should increase.
+  EXPECT_CALL(*consecutive_ltc_, UpdateDefaultTarget(_, OPT_TARGET(1.0)))
+      .Times(1);
+  SetFrameCounters(dropped_frames += 999, total_frames += 1000);
   FastForwardBy(segment_size_);
   base::RunLoop().RunUntilIdle();
+  testing::Mock::VerifyAndClearExpectations(consecutive_ltc_);
 
-  // Third segment has no dropped frames, so the default shouldn't change.
-  EXPECT_CALL(*ltc_, UpdateDefaultTarget(_, OPT_TARGET(Gt(0.99)))).Times(1);
-  SetFrameCounters(999, 3000);
+  // Third segment looks nice, so nothing should update.
+  EXPECT_CALL(*consecutive_ltc_, UpdateDefaultTarget(_, OPT_TARGET(_)))
+      .Times(0);
+  SetFrameCounters(dropped_frames += 0, total_frames += 1000);
   FastForwardBy(segment_size_);
   base::RunLoop().RunUntilIdle();
-
-  EXPECT_CALL(*ltc_, CompleteObservation(_, COMPLETION_TARGET(Gt(0.99))))
-      .Times(1);
-  helper_->NotifyPlayState(false);
-  base::RunLoop().RunUntilIdle();
-}
-
-TEST_F(SmoothnessHelperTest, PlayIgnoresTrailingPartialSegments) {
-  helper_->NotifyPlayState(true);
-  base::RunLoop().RunUntilIdle();
-
-  // First segment has no dropped frames.
-  EXPECT_CALL(*ltc_, BeginObservation(_, _, OPT_TARGET(Eq(0.0)))).Times(1);
-  SetFrameCounters(0, 1000);
+  testing::Mock::VerifyAndClearExpectations(consecutive_ltc_);
+
+  // Fourth segment has dropped frames, but the default shouldn't change.
+  // It's okay if it changes to the same value, but we just memorize that it
+  // won't change at all.
+  EXPECT_CALL(*consecutive_ltc_, UpdateDefaultTarget(_, OPT_TARGET(_)))
+      .Times(0);
+  SetFrameCounters(dropped_frames += 999, total_frames += 1000);
   FastForwardBy(segment_size_);
   base::RunLoop().RunUntilIdle();
+  testing::Mock::VerifyAndClearExpectations(consecutive_ltc_);
 
-  // Second segment has a lot of dropped frames, but isn't a full segment.
-  SetFrameCounters(1000, 2000);
-  FastForwardBy(segment_size_ / 2);
-  base::RunLoop().RunUntilIdle();
-
-  // On completion, we the observation should have no dropped frames.
-  EXPECT_CALL(*ltc_, CompleteObservation(_, COMPLETION_TARGET(Lt(0.1))))
+  // The last segment is also bad, and should increase the max.
+  EXPECT_CALL(*consecutive_ltc_, UpdateDefaultTarget(_, OPT_TARGET(2.0)))
       .Times(1);
-  helper_->NotifyPlayState(false);
-  base::RunLoop().RunUntilIdle();
-}
-
-TEST_F(SmoothnessHelperTest, DestructionRecordsObservations) {
-  // Destroying |helper_| should not send any observation; the last default
-  // value should be used.
-  helper_->NotifyPlayState(true);
-  base::RunLoop().RunUntilIdle();
-
-  EXPECT_CALL(*ltc_, BeginObservation(_, _, _)).Times(AnyNumber());
-  EXPECT_CALL(*ltc_, UpdateDefaultTarget(_, _)).Times(AnyNumber());
-  EXPECT_CALL(*ltc_, CancelObservation(_)).Times(0);
-  EXPECT_CALL(*ltc_, CompleteObservation(_, _)).Times(0);
-
-  // Fast forward so that we're sure that there is something to record.
-  SetFrameCounters(0, 1000);
-  FastForwardBy(segment_size_);
-  SetFrameCounters(0, 2000);
+  SetFrameCounters(dropped_frames += 999, total_frames += 1000);
   FastForwardBy(segment_size_);
-  helper_.reset();
-
   base::RunLoop().RunUntilIdle();
+  testing::Mock::VerifyAndClearExpectations(consecutive_ltc_);
 }
 
 }  // namespace media

media/learning/common/value.cc

@@ -47,6 +47,10 @@ bool Value::operator>(const Value& rhs) const {
   return value_ > rhs.value_;
 }
 
+bool Value::operator>=(const Value& rhs) const {
+  return value_ >= rhs.value_;
+}
+
 std::ostream& operator<<(std::ostream& out, const Value& value) {
   return out << value.value_;
 }

media/learning/common/value.h

@@ -48,6 +48,7 @@ class COMPONENT_EXPORT(LEARNING_COMMON) Value {
   bool operator!=(const Value& rhs) const;
   bool operator<(const Value& rhs) const;
   bool operator>(const Value& rhs) const;
+  bool operator>=(const Value& rhs) const;
 
   double value() const { return value_; }
 

media/learning/common/value_unittest.cc

@@ -39,11 +39,14 @@ TEST_F(LearnerValueTest, IntsCompareCorrectly) {
   Value v2(i1);
   Value v3(i2);
   EXPECT_TRUE(v1 == v2);
+  EXPECT_TRUE(v1 >= v2);
   EXPECT_TRUE(v1 != v3);
   EXPECT_TRUE(v1 < v3);
+  EXPECT_FALSE(v1 >= v3);
   EXPECT_FALSE(v3 < v1);
   EXPECT_FALSE(v3 < v3);
   EXPECT_FALSE(v1 < v1);
+  EXPECT_TRUE(v1 >= v1);
   EXPECT_TRUE(v3 > v1);
   EXPECT_FALSE(v1 > v3);
   EXPECT_FALSE(v1 > v1);