[webkit-reviews] review granted: [Bug 210186] Safari doesn't apply frameRate limit when request stream from Camera : [Attachment 396642] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 16 07:12:44 PDT 2020
Eric Carlson <eric.carlson at apple.com> has granted review:
Bug 210186: Safari doesn't apply frameRate limit when request stream from
Camera
https://bugs.webkit.org/show_bug.cgi?id=210186
Attachment 396642: Patch
https://bugs.webkit.org/attachment.cgi?id=396642&action=review
--- Comment #4 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 396642
--> https://bugs.webkit.org/attachment.cgi?id=396642
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=396642&action=review
r=me once the bots are happy
> Source/WebCore/platform/mediastream/RealtimeVideoSource.cpp:153
> + auto size = this->size();
Nit: this local variable is only used once now.
>
LayoutTests/fast/mediastream/mediastreamtrack-video-frameRate-decreasing.html:2
0
> + if (!stream1.getVideoTracks()[0].ended) {
Is this check necessary? If so, doesn't it mean we're testing different things
on different runs?
>
LayoutTests/fast/mediastream/mediastreamtrack-video-frameRate-decreasing.html:2
6
> + let frameRate = await computeFrameRate(stream2, video);
Nit: this can be a const.
>
LayoutTests/fast/mediastream/mediastreamtrack-video-frameRate-increasing.html:2
6
> + if (!stream1.getVideoTracks()[0].ended) {
> + const frameRate = await computeFrameRate(stream1, video);
> + assert_greater_than(frameRate, 1, "stream1 frame rate above 1");
> + assert_less_than(frameRate, 20, "stream1 frame rate below 20");
> + }
> +
> + let frameRate = await computeFrameRate(stream2, video);
Ditto the comments above.
More information about the webkit-reviews
mailing list