Skip to content

Commit

Permalink
[Track Stats] Remove support for non-gUM/gDM tracks.
Browse files Browse the repository at this point in the history
In a defensive stance (in terms of implementer burdon), the track.stats
API was only defined for getUserMedia/getDisplayMedia-based sources.
- This is a shame, because video processing is gaining popularity and it
  would be great to be able to know about frame counters for these
  non-device tracks as well.

Based on implementation experience, this does seem unnecessary and I
filed w3c/mediacapture-extensions#102, but
for now this API should only be applicable to "device"-backed tracks.

It's possible there could be more implementation burdon if it turns out
that other sources are able to drop frames early (meaning, before the
VideoTrackAdapter sees the frame), because if so we need to wire up the
OnFrameDropped callback to all source implementations. But for now my
understanding is that "early" OnFrameDropped is only applicable on
real-time devices and that supporting all types of tracks would have
been no extra effort.

But until we can confirm this and convince the WG, let's align with
the spec and throw an exception on non-device sources.

# Skipping unrelated slow bot
NOTRY=True

Bug: chromium:1472978
Change-Id: I5cc40c07303a9e87d76105fb3dbf03b7259dd72f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4946414
Commit-Queue: Henrik Boström <hbos@chromium.org>
Commit-Queue: Harald Alvestrand <hta@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1211445}
  • Loading branch information
henbos authored and Chromium LUCI CQ committed Oct 18, 2023
1 parent e0895f9 commit 891c2d3
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -674,11 +674,33 @@ MediaStreamTrackVideoStats* MediaStreamTrackImpl::stats(
DOMExceptionCode::kNotSupportedError,
"MediaStreamTrack.stats is not supported on audio tracks.");
return nullptr;
case MediaStreamSource::kTypeVideo:
case MediaStreamSource::kTypeVideo: {
absl::optional<const MediaStreamDevice> source_device = device();
if (!source_device.has_value() ||
source_device->type == mojom::blink::MediaStreamType::NO_SERVICE) {
// If the track is backed by a getUserMedia or getDisplayMedia device,
// a service will be set. Other sources may have default initialized
// devices, but these have type NO_SERVICE.
// TODO(https://github.com/w3c/mediacapture-extensions/issues/102): This
// is an unnecessary restriction - if the W3C Working Group can be
// convinced otherwise, simply don't throw this exception. Some sources
// may need to wire up the OnFrameDropped callback in order for
// totalFrames to include "early" frame drops, but this is probably N/A
// for most (if not all) sources that are not backed by a gUM/gDM device
// since non-device sources aren't real-time in which case FPS can be
// reduced by not generating the frame in the first place, so then there
// is no need to drop it.
exception_state.ThrowDOMException(
DOMExceptionCode::kNotSupportedError,
"MediaStreamTrack.stats on video tracks is only supported if the "
"source is a getUserMedia() or getDisplayMedia() source.");
return nullptr;
}
if (!video_stats_) {
video_stats_ = MakeGarbageCollected<MediaStreamTrackVideoStats>(this);
}
return video_stats_;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
<!doctype html>
<meta charset=utf-8>
<meta name="timeout" content="long">
<button id="button">User gesture</button>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script>
'use strict';

Expand Down Expand Up @@ -323,11 +326,28 @@
const [track] = stream.getTracks();
t.add_cleanup(() => track.stop());

try {
track.stats.toJSON();
assert_not_reached();
} catch (e) {
assert_equals(e.name, 'NotSupportedError');
}
}, `getFrameStats throws on audio tracks`);
assert_throws_dom('NotSupportedError', () => track.stats);
}, `track.stats throws on audio tracks`);

promise_test(async t => {
const canvas = document.createElement('canvas');
const stream = canvas.captureStream(10);
const [track] = stream.getTracks();
t.add_cleanup(() => track.stop());

assert_throws_dom('NotSupportedError', () => track.stats);
}, `track.stats throws on non-device tracks, such as canvas`);

promise_test(async t => {
// getDisplayMedia() requires inducing a user gesture.
const p = new Promise(r => button.onclick = r);
await test_driver.click(button);
await p;

const stream = await navigator.mediaDevices.getDisplayMedia({video:true});
const [track] = stream.getTracks();
t.add_cleanup(() => track.stop());

await getFrameStatsUntil(track, stats => stats.totalFrames > 0)
}, `track.stats is supported on getDisplayMedia tracks`);
</script>

0 comments on commit 891c2d3

Please sign in to comment.