From 3aa890496899b57418a8c7807d8cefa654c2f9b3 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 25 Nov 2024 11:44:57 +0100 Subject: [PATCH 1/4] Remove hack for setting `ExternalImageSource::VideoFrame` --- Cargo.lock | 1 - Cargo.toml | 1 - crates/viewer/re_renderer/Cargo.toml | 1 - .../re_renderer/src/video/chunk_decoder.rs | 39 ++++--------------- crates/viewer/re_renderer/src/video/mod.rs | 2 +- 5 files changed, 8 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0107c880c426..6bb3f1967a08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5870,7 +5870,6 @@ dependencies = [ "web-time", "wgpu", "wgpu-core", - "wgpu-types", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index ab4a9697a76f..3357e8ffea90 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -325,7 +325,6 @@ wgpu = { version = "23.0", default-features = false, features = [ "fragile-send-sync-non-atomic-wasm", ] } wgpu-core = "23.0" -wgpu-types = "23.0" xshell = "0.2" zip = { version = "0.6", default-features = false } # We're stuck on 0.6 because https://crates.io/crates/protoc-prebuilt is still using 0.6 diff --git a/crates/viewer/re_renderer/Cargo.toml b/crates/viewer/re_renderer/Cargo.toml index 0a69c43b13da..da706aa443e7 100644 --- a/crates/viewer/re_renderer/Cargo.toml +++ b/crates/viewer/re_renderer/Cargo.toml @@ -81,7 +81,6 @@ type-map.workspace = true web-time.workspace = true wgpu.workspace = true wgpu-core.workspace = true # Needed for error handling when wgpu-core implemented backend is used. -wgpu-types.workspace = true # optional arrow2 = { workspace = true, optional = true } diff --git a/crates/viewer/re_renderer/src/video/chunk_decoder.rs b/crates/viewer/re_renderer/src/video/chunk_decoder.rs index 978229a57938..ab164ba1d2fb 100644 --- a/crates/viewer/re_renderer/src/video/chunk_decoder.rs +++ b/crates/viewer/re_renderer/src/video/chunk_decoder.rs @@ -185,38 +185,13 @@ fn copy_web_video_frame_to_texture( depth_or_array_layers: 1, }; let frame: &web_sys::VideoFrame = frame; - - let source = { - // TODO(jan): The wgpu version we're using doesn't support `VideoFrame` yet. - // This got fixed in https://github.com/gfx-rs/wgpu/pull/6170 but hasn't shipped yet. - // So instead, we just pretend this is a `HtmlVideoElement` instead. - // SAFETY: Depends on the fact that `wgpu` passes the object through as-is, - // and doesn't actually inspect it in any way. The browser then does its own - // typecheck that doesn't care what kind of image source wgpu gave it. - #[allow(unsafe_code)] - let frame = unsafe { - std::mem::transmute::( - frame.clone().expect("Failed to clone the video frame"), - ) - }; - // Fake width & height to work around wgpu validating this as if it was a `HtmlVideoElement`. - // Since it thinks this is a `HtmlVideoElement`, it will want to call `videoWidth` and `videoHeight` - // on it to validate the size. - // We simply redirect `displayWidth`/`displayHeight` to `videoWidth`/`videoHeight` to make it work! - let display_width = js_sys::Reflect::get(&frame, &"displayWidth".into()) - .expect("Failed to get displayWidth property from VideoFrame."); - js_sys::Reflect::set(&frame, &"videoWidth".into(), &display_width) - .expect("Failed to set videoWidth property."); - let display_height = js_sys::Reflect::get(&frame, &"displayHeight".into()) - .expect("Failed to get displayHeight property from VideoFrame."); - js_sys::Reflect::set(&frame, &"videoHeight".into(), &display_height) - .expect("Failed to set videoHeight property."); - - wgpu_types::ImageCopyExternalImage { - source: wgpu_types::ExternalImageSource::HTMLVideoElement(frame), - origin: wgpu_types::Origin2d { x: 0, y: 0 }, - flip_y: false, - } + let source = wgpu::ImageCopyExternalImage { + // Careful: `web_sys::VideoFrame` has a custom `clone` method: + // https://developer.mozilla.org/en-US/docs/Web/API/VideoFrame/clone + // We instead just want to clone the js value! + source: wgpu::ExternalImageSource::VideoFrame(Clone::clone(frame)), + origin: wgpu::Origin2d { x: 0, y: 0 }, + flip_y: false, }; let dest = wgpu::ImageCopyTextureTagged { texture: &target_texture.texture, diff --git a/crates/viewer/re_renderer/src/video/mod.rs b/crates/viewer/re_renderer/src/video/mod.rs index 96a4142660b7..e2a70621628f 100644 --- a/crates/viewer/re_renderer/src/video/mod.rs +++ b/crates/viewer/re_renderer/src/video/mod.rs @@ -18,7 +18,7 @@ pub enum VideoPlayerError { #[error("The decoder is lagging behind")] EmptyBuffer, - #[error("Video seems to be empty, no segments have beem found.")] + #[error("Video seems to be empty, no segments have been found.")] EmptyVideo, /// e.g. unsupported codec From a036963eb3efb67fba03b2da2017bf49479e18d3 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 25 Nov 2024 16:30:17 +0100 Subject: [PATCH 2/4] Be clearer on who's doing the copy (JsValue) Co-authored-by: Emil Ernerfeldt --- crates/viewer/re_renderer/src/video/chunk_decoder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/viewer/re_renderer/src/video/chunk_decoder.rs b/crates/viewer/re_renderer/src/video/chunk_decoder.rs index ab164ba1d2fb..62cb3002bc13 100644 --- a/crates/viewer/re_renderer/src/video/chunk_decoder.rs +++ b/crates/viewer/re_renderer/src/video/chunk_decoder.rs @@ -189,7 +189,7 @@ fn copy_web_video_frame_to_texture( // Careful: `web_sys::VideoFrame` has a custom `clone` method: // https://developer.mozilla.org/en-US/docs/Web/API/VideoFrame/clone // We instead just want to clone the js value! - source: wgpu::ExternalImageSource::VideoFrame(Clone::clone(frame)), + source: wgpu::ExternalImageSource::VideoFrame(JsValue::clone(frame)), origin: wgpu::Origin2d { x: 0, y: 0 }, flip_y: false, }; From 17c88583a6df597622810e78e9c9ab5c4bc40579 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 25 Nov 2024 16:30:30 +0100 Subject: [PATCH 3/4] Better error message for empty video Co-authored-by: Emil Ernerfeldt --- crates/viewer/re_renderer/src/video/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/viewer/re_renderer/src/video/mod.rs b/crates/viewer/re_renderer/src/video/mod.rs index e2a70621628f..a3c1dd0983e1 100644 --- a/crates/viewer/re_renderer/src/video/mod.rs +++ b/crates/viewer/re_renderer/src/video/mod.rs @@ -18,7 +18,7 @@ pub enum VideoPlayerError { #[error("The decoder is lagging behind")] EmptyBuffer, - #[error("Video seems to be empty, no segments have been found.")] + #[error("Video is empty.")] EmptyVideo, /// e.g. unsupported codec From 0168654a61f08c3978e137a6e39cc1842a76a4c1 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 26 Nov 2024 16:08:16 +0100 Subject: [PATCH 4/4] undo JsValue::clone - that is _not_ what we want since we want to keep the `VideoFrame` wrapping --- crates/viewer/re_renderer/src/video/chunk_decoder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/viewer/re_renderer/src/video/chunk_decoder.rs b/crates/viewer/re_renderer/src/video/chunk_decoder.rs index 62cb3002bc13..a5b5bf49b40b 100644 --- a/crates/viewer/re_renderer/src/video/chunk_decoder.rs +++ b/crates/viewer/re_renderer/src/video/chunk_decoder.rs @@ -188,8 +188,8 @@ fn copy_web_video_frame_to_texture( let source = wgpu::ImageCopyExternalImage { // Careful: `web_sys::VideoFrame` has a custom `clone` method: // https://developer.mozilla.org/en-US/docs/Web/API/VideoFrame/clone - // We instead just want to clone the js value! - source: wgpu::ExternalImageSource::VideoFrame(JsValue::clone(frame)), + // We instead just want to clone the js value wrapped in VideoFrame! + source: wgpu::ExternalImageSource::VideoFrame(Clone::clone(frame)), origin: wgpu::Origin2d { x: 0, y: 0 }, flip_y: false, };