Make track.getFrameStats() work when cloning or disabling the track.

When cloning a track, the stats counters should have a baseline of
zero. This is automatically true for `deliveredFrames` since frame
delivery counters are handled by MediaStreamVideoTrack already.

But this is not true of the video's source, since the same source can
be shared by multiple tracks cloned at multiple times or having been
enabled or disabled at different points in time.

In order to make track counters work as expected, each track gets its
own `baseline_discarded_frames_` and `baseline_dropped_frames_`
counters.

An alternative would be to move the frame counters to
MediaStreamTrackImpl, but that would require wiring up the
OnFrameDropped event further and I prefer to keep those counters on
the source level in case we want to refactor the threading assumptions
of OnFrameDropped (I would argue we do!).

Bug: chromium:1472978
Change-Id: I941a31273e34f72e07300cb4504b9efb6c425a6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4781127
Reviewed-by: Tony Herre <toprice@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184608}

third_party/blink/renderer/modules/mediastream/media_stream_track_impl.cc

@@ -259,6 +259,14 @@ MediaStreamTrackImpl::MediaStreamTrackImpl(
   DCHECK(component_);
   component_->AddSourceObserver(this);
 
+  // Set discarded/dropped frames baselines to the snapshot at construction.
+  if (component_->GetSourceType() == MediaStreamSource::kTypeVideo) {
+    MediaStreamVideoSource* source =
+        MediaStreamVideoSource::GetVideoSource(component_->Source());
+    video_source_discarded_frames_baseline_ = source->discarded_frames();
+    video_source_dropped_frames_baseline_ = source->dropped_frames();
+  }
+
   // If the source is already non-live at this point, the observer won't have
   // been called. Update the muted state manually.
   muted_ = ready_state_ == MediaStreamSource::kReadyStateMuted;
@@ -324,6 +332,27 @@ void MediaStreamTrackImpl::setEnabled(bool enabled) {
 
   component_->SetEnabled(enabled);
 
+  if (component_->GetSourceType() == MediaStreamSource::kTypeVideo) {
+    MediaStreamVideoSource* video_source =
+        MediaStreamVideoSource::GetVideoSource(component_->Source());
+    CHECK(video_source);
+    if (!enabled) {
+      // Upon disabling, take a snapshot of the current frame counters. This
+      // ensures frames does not increment while we are disabled.
+      discarded_frames_at_last_disable_ =
+          video_source->discarded_frames() -
+          video_source_discarded_frames_baseline_;
+      dropped_frames_at_last_disable_ = video_source->dropped_frames() -
+                                        video_source_dropped_frames_baseline_;
+    } else {
+      // Upon enabling, refresh our baseline to exclude the disabled period.
+      video_source_discarded_frames_baseline_ =
+          video_source->discarded_frames() - discarded_frames_at_last_disable_;
+      video_source_dropped_frames_baseline_ =
+          video_source->dropped_frames() - dropped_frames_at_last_disable_;
+    }
+  }
+
   SendLogMessage(
       String::Format("%s({enabled=%s})", __func__, enabled ? "true" : "false"));
 }
@@ -692,14 +721,27 @@ ScriptPromise MediaStreamTrackImpl::getFrameStats(
 void MediaStreamTrackImpl::OnDeliverableVideoFramesCount(
     Persistent<ScriptPromiseResolver> resolver,
     size_t deliverable_frames) const {
-  MediaTrackFrameStats* track_stats = MediaTrackFrameStats::Create();
   MediaStreamVideoSource* video_source =
       MediaStreamVideoSource::GetVideoSource(component_->Source());
+  CHECK(video_source);
+
+  MediaTrackFrameStats* track_stats = MediaTrackFrameStats::Create();
   track_stats->setDeliveredFrames(deliverable_frames);
-  track_stats->setDiscardedFrames(video_source->discarded_frames());
-  track_stats->setTotalFrames(deliverable_frames +
-                              video_source->discarded_frames() +
-                              video_source->dropped_frames());
+  size_t discarded_frames, dropped_frames;
+  if (enabled()) {
+    // The dropped/discarded counters are relative to our baseline.
+    discarded_frames = video_source->discarded_frames() -
+                       video_source_discarded_frames_baseline_;
+    dropped_frames =
+        video_source->dropped_frames() - video_source_dropped_frames_baseline_;
+  } else {
+    // The track is disabled, so we return the frozen disabled snapshots.
+    discarded_frames = discarded_frames_at_last_disable_;
+    dropped_frames = dropped_frames_at_last_disable_;
+  }
+  track_stats->setDiscardedFrames(discarded_frames);
+  track_stats->setTotalFrames(deliverable_frames + discarded_frames +
+                              dropped_frames);
   resolver->Resolve(track_stats);
 }
 

third_party/blink/renderer/modules/mediastream/media_stream_track_impl.h

@@ -198,6 +198,18 @@ class MODULES_EXPORT MediaStreamTrackImpl : public MediaStreamTrack,
   bool muted_ = false;
   MediaConstraints constraints_;
   absl::optional<bool> suppress_local_audio_playback_setting_;
+  // Video-only.
+  //   It is the source' job to keep track of the true number of discarded or
+  // dropped frames. But tracks, unlike sources, can be cloned or disabled so
+  // we need to keep track of the baseline for when the track was cloned and
+  // a snapshot for when the track was disabled.
+  size_t video_source_discarded_frames_baseline_ = 0u;
+  size_t video_source_dropped_frames_baseline_ = 0u;
+  // To avoid counters increasing while the track is disabled, a snapshot of the
+  // discarded/dropped stats are taken at the time of disabling the track. These
+  // are also used when adjusting the baseline when the track is re-enabled.
+  size_t discarded_frames_at_last_disable_ = 0u;
+  size_t dropped_frames_at_last_disable_ = 0u;
 };
 
 }  // namespace blink

third_party/blink/web_tests/external/wpt/mediacapture-extensions/MediaStreamTrack-getFrameStats.https.html

@@ -94,28 +94,175 @@
 
   // Wait for media to flow before disabling the `track`.
   const startTimeMs = performance.now();
-  await getFrameStatsUntil(track, stats => stats.totalFrames > 10);
+  const initialStats = await getFrameStatsUntil(track, stats =>
+      stats.deliveredFrames > 0 && stats.discardedFrames > 0 &&
+      stats.totalFrames > 10);
   const elapsedTimeMs = performance.now() - startTimeMs;
   track.enabled = false;
+  // Upon disabling, the counters are not reset.
   const disabledSnapshot = await track.getFrameStats();
+  assert_greater_than_equal(disabledSnapshot.deliveredFrames,
+                            initialStats.deliveredFrames);
+  assert_greater_than_equal(disabledSnapshot.discardedFrames,
+                            initialStats.discardedFrames);
+  assert_greater_than_equal(disabledSnapshot.totalFrames,
+                            initialStats.totalFrames);
 
   // Wait enough time that frames should have been produced.
   // `elapsedTimeMs` should be enough to produce ~10 frames.
   await new Promise(r => t.step_timeout(r, elapsedTimeMs));
 
   // Frame metrics should be frozen.
-  // Prefer `assert_true` over `assert_equals` because if the test fails exact
-  // number of frames could be different on different runs due to timing,
-  // resulting in flaky test expectations.
   const stats = await track.getFrameStats();
-  assert_true(stats.deliveredFrames == disabledSnapshot.deliveredFrames,
-              'The deliveredFrames counter is not frozen');
-  assert_true(stats.discardedFrames == disabledSnapshot.discardedFrames,
-              'The discardedFrames counter is not frozen');
-  assert_true(stats.totalFrames == disabledSnapshot.totalFrames,
-              'The totalFrames counter is not frozen');
+  assert_equals(stats.deliveredFrames, disabledSnapshot.deliveredFrames);
+  assert_equals(stats.discardedFrames, disabledSnapshot.discardedFrames);
+  assert_equals(stats.totalFrames, disabledSnapshot.totalFrames);
 }, `Stats are frozen while disabled`);
 
+promise_test(async t => {
+  const stream = await navigator.mediaDevices.getUserMedia({
+          video:{frameRate:{ideal:20}}
+      });
+  const [track] = stream.getTracks();
+  t.add_cleanup(() => track.stop());
+
+  // Assert test prerequisite is met: frames will be discarded if the track is
+  // opened with a higher frame rate than we apply after it is opened.
+  assert_greater_than(track.getSettings().frameRate, 10);
+  await track.applyConstraints({frameRate:{ideal:10}});
+
+  // Wait for media to flow before disabling the `track`.
+  const initialStats = await getFrameStatsUntil(track, stats =>
+      stats.deliveredFrames > 10 && stats.discardedFrames > 10);
+  track.enabled = false;
+
+  // Re-enable the track. The stats counters should be greater than or equal to
+  // what they were previously.
+  track.enabled = true;
+  const stats = await track.getFrameStats();
+  assert_greater_than_equal(stats.deliveredFrames,
+                            initialStats.deliveredFrames);
+  assert_greater_than_equal(stats.discardedFrames,
+                            initialStats.discardedFrames);
+  assert_greater_than_equal(stats.totalFrames,
+                            initialStats.totalFrames);
+}, `Disabling and re-enabling does not reset the counters`);
+
+promise_test(async t => {
+  const stream = await navigator.mediaDevices.getUserMedia({
+          video:{frameRate:{ideal:20}}
+      });
+  const [originalTrack] = stream.getTracks();
+  t.add_cleanup(() => originalTrack.stop());
+
+  // Assert test prerequisite is met: frames will be discarded if the track is
+  // opened with a higher frame rate than we apply after it is opened.
+  assert_greater_than(originalTrack.getSettings().frameRate, 10);
+  await originalTrack.applyConstraints({frameRate:{ideal:10}});
+
+  // Wait for media to flow before disabling the `track`.
+  await getFrameStatsUntil(originalTrack, stats =>
+      stats.deliveredFrames > 0 && stats.discardedFrames > 0);
+  originalTrack.enabled = false;
+  const originalTrackInitialStats = await originalTrack.getFrameStats();
+
+  // Clone the track, its counters should be zero initially.
+  // This is not racy because the cloned track is also disabled.
+  const clonedTrack = originalTrack.clone();
+  t.add_cleanup(() => clonedTrack.stop());
+  const clonedTrackStats = await clonedTrack.getFrameStats();
+  assert_equals(clonedTrackStats.deliveredFrames, 0);
+  assert_equals(clonedTrackStats.discardedFrames, 0);
+  assert_equals(clonedTrackStats.totalFrames, 0);
+
+  // Enabled the cloned track and wait for media to flow.
+  clonedTrack.enabled = true;
+  await getFrameStatsUntil(clonedTrack, stats =>
+      stats.deliveredFrames > 0 && stats.discardedFrames > 0);
+
+  // This does not affect the original track's stats, which are still frozen due
+  // to the original track being disabled.
+  const originalTrackSecondStats = await originalTrack.getFrameStats();
+  assert_equals(originalTrackSecondStats.deliveredFrames,
+                originalTrackInitialStats.deliveredFrames);
+  assert_equals(originalTrackSecondStats.discardedFrames,
+                originalTrackInitialStats.discardedFrames);
+  assert_equals(originalTrackSecondStats.totalFrames,
+                originalTrackInitialStats.totalFrames);
+}, `New stats baselines when a track is cloned from a disabled track`);
+
+promise_test(async t => {
+  const stream = await navigator.mediaDevices.getUserMedia({
+          video:{frameRate:{ideal:20}}
+      });
+  const [originalTrack] = stream.getTracks();
+  t.add_cleanup(() => originalTrack.stop());
+
+  // Assert test prerequisite is met: frames will be discarded if the track is
+  // opened with a higher frame rate than we apply after it is opened.
+  assert_greater_than(originalTrack.getSettings().frameRate, 10);
+  await originalTrack.applyConstraints({frameRate:{ideal:10}});
+
+  // Wait for media to flow.
+  await getFrameStatsUntil(originalTrack, stats =>
+      stats.deliveredFrames > 0 && stats.discardedFrames > 0);
+
+  // Clone the track. While its counters should initially be zero, it would be
+  // racy to assert that they are exactly zero because media is flowing.
+  const clonedTrack = originalTrack.clone();
+  t.add_cleanup(() => clonedTrack.stop());
+
+  // Ensure that as media continues to flow, the cloned track will necessarily
+  // have less frames than the original track on all accounts since its counters
+  // will have started from zero.
+  const clonedTrackStats = await getFrameStatsUntil(clonedTrack, stats =>
+      stats.deliveredFrames > 0 && stats.discardedFrames > 0);
+  const originalTrackStats = await originalTrack.getFrameStats();
+  assert_less_than(clonedTrackStats.deliveredFrames,
+                   originalTrackStats.deliveredFrames);
+  assert_less_than(clonedTrackStats.discardedFrames,
+                   originalTrackStats.discardedFrames);
+  assert_less_than(clonedTrackStats.totalFrames,
+                   originalTrackStats.totalFrames);
+}, `New stats baselines when a track is cloned from an enabled track`);
+
+promise_test(async t => {
+  const stream = await navigator.mediaDevices.getUserMedia({
+          video:{frameRate:{ideal:20}}
+      });
+  const [originalTrack] = stream.getTracks();
+  t.add_cleanup(() => originalTrack.stop());
+
+  // Assert test prerequisite is met: frames will be discarded if the track is
+  // opened with a higher frame rate than we apply after it is opened.
+  assert_greater_than(originalTrack.getSettings().frameRate, 10);
+  await originalTrack.applyConstraints({frameRate:{ideal:10}});
+
+  // Wait for media to flow.
+  await getFrameStatsUntil(originalTrack, stats =>
+      stats.deliveredFrames > 0 && stats.discardedFrames > 0);
+
+  // Clone and wait for media to flow.
+  const cloneA = originalTrack.clone();
+  t.add_cleanup(() => cloneA.stop());
+  await getFrameStatsUntil(cloneA, stats =>
+      stats.deliveredFrames > 0 && stats.discardedFrames > 0);
+
+  // Clone the clone and wait for media to flow.
+  const cloneB = cloneA.clone();
+  t.add_cleanup(() => cloneB.stop());
+  await getFrameStatsUntil(cloneB, stats =>
+      stats.deliveredFrames > 0 && stats.discardedFrames > 0);
+
+  // Because every clone reset its counters and every waits for media before
+  // cloning, this must be true: originalStats > cloneAStats > cloneBStats.
+  const originalStats = await originalTrack.getFrameStats();
+  const cloneAStats = await cloneA.getFrameStats();
+  const cloneBStats = await cloneB.getFrameStats();
+  assert_greater_than(originalStats.totalFrames, cloneAStats.totalFrames);
+  assert_greater_than(cloneAStats.totalFrames, cloneBStats.totalFrames);
+}, `New stats baselines for the clone of a clone`);
+
 promise_test(async t => {
   const stream = await navigator.mediaDevices.getUserMedia({audio:true});
   const [track] = stream.getTracks();