Skip to content
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

Add VideoFrame to ExternalImageSource enum #6170

Merged
merged 5 commits into from
Aug 27, 2024

Conversation

jprochazk
Copy link
Contributor

@jprochazk jprochazk commented Aug 27, 2024

Connections

Description
Adds missing VideoFrame type as a possible external image source.

The enum variant is gated behind cfg(web_sys_unstable_apis), as web_sys::VideoFrame is unavailable without it.

Testing
Tested manually by using it in place of this hack: https://github.com/rerun-io/rerun/pull/7261/files#diff-213f45076b49c7bbe3d7a1f0c7383b3b9e0edb4c02c21bab32709573155a2e63R403-R414
It would be nice to have a Wasm-only example that renders a media stream using VideoDecoder, but I'm not sure how adding such an example would work.

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@jprochazk
Copy link
Contributor Author

jprochazk commented Aug 27, 2024

I'm not sure what the policy is for unstable APIs, but this requires --cfg web_sys_unstable_apis. I gated the new enum variant behind that cfg, so that wgpu-types continues to build without it. Using VideoFrame at all requires using that cfg anyway, so hopefully it's fine!

@jprochazk jprochazk marked this pull request as ready for review August 27, 2024 17:38
@jprochazk jprochazk requested a review from a team as a code owner August 27, 2024 17:38
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Perfect, thanks!

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) August 27, 2024 17:51
@cwfitzgerald cwfitzgerald merged commit 5deaef3 into gfx-rs:trunk Aug 27, 2024
25 checks passed
@emilk emilk mentioned this pull request Aug 28, 2024
6 tasks
@emilk
Copy link
Contributor

emilk commented Aug 28, 2024

I thought web_sys_unstable_apis was removed in #5325 🤔

@jprochazk
Copy link
Contributor Author

It was removed, but this PR doesn't invalidate that one, because wgpu continues to build without requiring the cfg to be set. But if you want web_sys::VideoFrame to be accessible (in any crate, not just wgpu), you need to set --cfg web_sys_unstable_apis. So to satisfy both constraints, the enum variant added here is gated behind the same cfg.

Wumpf added a commit to rerun-io/rerun that referenced this pull request Sep 10, 2024
### What

* Fixes #7328

This is obviously a brittle hack, but allows us to move on without
having to wait on a wgpu release containing
gfx-rs/wgpu#6170 :)

Tested using `pixi run rerun-web /Users/andreas/Desktop/sample\
videos/Big_Buck_Bunny_720_10s_30MB_av1.mp4` (insert video of your
choice) and then running
`http://localhost:9090/?url=ws%3A%2F%2Flocalhost%3A9877&renderer=webgl`
in Chrome (on Mac. shouldn't matter much as long as the browser has
webcodec support and draws video)

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7381?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7381?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7381)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
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.

Add ExternalImageSource::VideoFrame
3 participants