Safari doesn't apply frameRate limit that is provided in constaraints for getUserMedia(). Track settings shows that provided frame rate is set, but stats shows that limit is not applied. Also it is not possible to change frame rate via MediaStreamTrack::applyConstraints(). jsfiddle: https://jsfiddle.net/sj1m6eqg/
<rdar://problem/61452794>
Created attachment 396636 [details] Patch
Created attachment 396642 [details] Patch
Comment on attachment 396642 [details] 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:20 > + 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:26 > + let frameRate = await computeFrameRate(stream2, video); Nit: this can be a const. > LayoutTests/fast/mediastream/mediastreamtrack-video-frameRate-increasing.html:26 > + 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.
Thanks for the review. (In reply to Eric Carlson from comment #4) > Comment on attachment 396642 [details] > 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. Will fix. > > LayoutTests/fast/mediastream/mediastreamtrack-video-frameRate-decreasing.html:20 > > + if (!stream1.getVideoTracks()[0].ended) { > > Is this check necessary? If so, doesn't it mean we're testing different > things on different runs? This is to cover the case of iOS which might mute the previous track. I should replace ended by muted though. > > LayoutTests/fast/mediastream/mediastreamtrack-video-frameRate-decreasing.html:26 > > + let frameRate = await computeFrameRate(stream2, video); > > Nit: this can be a const. > > > LayoutTests/fast/mediastream/mediastreamtrack-video-frameRate-increasing.html:26 > > + 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. OK
Created attachment 396758 [details] Patch for landing
Committed r260245: <https://trac.webkit.org/changeset/260245> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396758 [details].
It looks like all the tests added with this change are failing more often than they are passing on macOS release bots: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&suite=layout-tests&test=fast%2Fmediastream%2Fmediastreamtrack-video-frameRate-clone-decreasing.html&test=fast%2Fmediastream%2Fmediastreamtrack-video-frameRate-clone-increasing.html&test=fast%2Fmediastream%2Fmediastreamtrack-video-frameRate-decreasing.html&test=fast%2Fmediastream%2Fmediastreamtrack-video-frameRate-increasing.html
Reverted r260245 for reason: The tests added with this change are frequently failing on macOS bots. Committed r260274: <https://trac.webkit.org/changeset/260274>
It seems that the release bots have issues in producing more than 10fps frame rate for capture video tracks.
Created attachment 396970 [details] Patch
Comment on attachment 396970 [details] Patch Relanding the patch with checking whether the source is producing 30fps.
Committed r260364: <https://trac.webkit.org/changeset/260364> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396970 [details].