De-flake fromPixels test failure #6626
Conversation
| @@ -56,6 +56,11 @@ describeWebGPU('fromPixels', () => { | |||
| if ('requestVideoFrameCallback' in video) { | |||
| // tslint:disable-next-line:no-any | |||
| await new Promise(go => (video as any).requestVideoFrameCallback(go)); | |||
| } else { | |||
| // On mobile safari the ready state is ready immediately. | |||
| if (video.readyState < HTMLVideoElement.prototype.HAVE_CURRENT_DATA) { | |||
There was a problem hiding this comment.
WebGPU uses video.currentTime for check: /~https://github.com/gpuweb/cts/blob/0dd7405d1bfa950a71572aeb77aa38aacef6653d/src/webgpu/web_platform/util.ts#L48.
WebGL uses a simpler solution as currently we have: /~https://github.com/KhronosGroup/WebGL/blob/main/sdk/tests/js/webgl-test-utils.js#L3182.
Do you like to keep sync with WebGPU solution?
There was a problem hiding this comment.
Digging through the history of WebGL CTS, it seems awaiting video.play() + optional rVFC works fine across browsers and is preferred over video.currentTime, so I decided to drop the change here.
There was a problem hiding this comment.
thanks, does this change affects WebGL tests?
Reviewable status:
complete! 2 of 1 approvals obtained (waiting on @gyagp, @lina128, and @Linchenn)
There was a problem hiding this comment.
Looks like this new API is not supported by Firefox and older Chrome/Safari yet, can we have a fallback? https://web.dev/requestvideoframecallback-rvfc/
Reviewable status:
There was a problem hiding this comment.
Currently rVFC is only used in from_pixels_webgpu_test.ts, and the WebGPU backend is always incompatible with older Chrome/Safari. In this sense, rVFC is assumed to be supported by Chrome/Safari. It also sounds a fallback is redundant in Firefox because of its extra guarantee provided by await video.play() right above (KhronosGroup/WebGL#3319).
WebGL tests seem unaffected by this. All are passing and I didn't grep any relevant case under the backend.
Reviewable status:
`HTMLVideoElement.requestVideoFrameCallback()` provides a realiable way to guarantee the readiness of a video frame, whereas the browser may not update the ready state until a few frames later. In such cases, false alarms will be raised by the validation in `fromPixels()`. Fixes tensorflow#6577
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:
HTMLVideoElement.requestVideoFrameCallback()provides a realiable way to guarantee the readiness of a video frame, whereas the browser may not update the ready state until a few frames later. In such cases, false alarms will be raised by the validation infromPixels().Fixes #6577
To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.
This change is