Skip to content

cameraCapture: Simplify codepaths, try to wait for a real frame #4365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 23, 2025

Conversation

kainino0x
Copy link
Collaborator

@kainino0x kainino0x commented Apr 19, 2025

  • Don't use fallback paths in camera capture; instead, have the test pick the exact path it's using, and parameterize over it. Test two paths: VideoFrame from MediaStreamTrackProcessor, and the old-fashioned HTMLVideoElement.
  • Work around an issue where Chrome (at least on Mac) shows blank frames for a while after initializing the camera.
  • I happened to notice that requesting different width/height from the camera is broken in Chrome in several ways, so added cases for that.
  • Added a TODO for copyExternalImageToTexture from camera

These tests only worked in Chrome, and even then they were buggy (usually running the test on a blank first frame). Now, the HTMLVideoElement tests work in Safari.

Test failures are as follows, on an M1 Mac:

  • Chrome: https://crbug.com/411656657
    • HTMLVideoElement passes some cases, but fails when a requested size is passed to getUserMedia().
    • VideoFrame fails due to incorrect color management (regardless of dstColorSpace).
  • Safari:
    • HTMLVideoElement fails due to incorrect color management (regardless of dstColorSpace).
    • VideoFrame is skipped.
  • Firefox: doesn't yet implement importExternalTexture.

Issue: fixes #4363, cc #4364


Requirements for PR author:

  • All missing test coverage is tracked with "TODO" or .unimplemented().
  • New helpers are /** documented */ and new helper files are found in helper_index.txt.
  • Test behaves as expected in a WebGPU implementation. (If not passing, explain above.)
  • Test have be tested with compatibility mode validation enabled and behave as expected. (If not passing, explain above.)

Requirements for reviewer sign-off:

  • Tests are properly located in the test tree.
  • Test descriptions allow a reader to "read only the test plans and evaluate coverage completeness", and accurately reflect the test code.
  • Tests provide complete coverage (including validation control cases). Missing coverage MUST be covered by TODOs.
  • Helpers and types promote readability and maintainability.

When landing this PR, be sure to make any necessary issue status updates.

- Don't use fallback paths in camera capture; instead, have the test
  pick the exact path it's using, and parameterize over it. Test two
  paths: VideoFrame from MediaStreamTrackProcessor, and the
  old-fashioned HTMLVideoElement.
- Work around an issue where Chrome (at least on Mac) shows blank frames
  for a while after initializing the camera.
- I happened to notice that requesting different width/height from the
  camera is broken in Chrome in several ways, so added cases for that.
- Added a TODO for copyExternalImageToTexture from camera

These tests only worked in Chrome, and even then they were buggy
(usually running the test on a blank first frame). Now, the
HTMLVideoElement tests work in Safari. Firefox doesn't yet implement
importExternalTexture.

Test failures are as follows, on an M1 Mac:
- Chrome: HTMLVideoElement passes some cases, but fails when a requested
  size is passed to getUserMedia().
- Chrome: VideoFrame fails due to incorrect color management
  (regardless of dstColorSpace).
- Safari: HTMLVideoElement fails due to incorrect color management
  (regardless of dstColorSpace).
- Safari: VideoFrame is skipped.

Issue: 4363
@shaoboyan091 shaoboyan091 requested review from shaoboyan091 and removed request for shaoboyan April 21, 2025 01:25
Copy link
Contributor

@shaoboyan091 shaoboyan091 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing! Some questions.

@kainino0x kainino0x requested a review from shaoboyan091 April 23, 2025 05:11
@kainino0x
Copy link
Collaborator Author

Oops, wait, I still haven't fixed the other code path.

@kainino0x
Copy link
Collaborator Author

Done, @shaoboyan091 PTAL. Thanks for keeping me honest in using proper logic instead of timeout hacks.

Copy link
Contributor

@shaoboyan091 shaoboyan091 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@kainino0x kainino0x enabled auto-merge (squash) April 23, 2025 06:37
@kainino0x kainino0x merged commit 26a9777 into gpuweb:main Apr 23, 2025
1 check passed
@kainino0x kainino0x deleted the webcam branch April 23, 2025 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cameraCapture test doesn't reliably get a real frame
2 participants