Skip to content

Commit

Permalink
Fix error spam after decode error & make video error icon take a fixe…
Browse files Browse the repository at this point in the history
…d amount of space (rerun-io#7503)

### What

* Fixes rerun-io#7465

.. sort of. It's still rather strange that Firefox doesn't decode this.
It runs into an error during decode which sounds like it incorrectly
advertised support. I considered recognizing this error and handling it
as a decoding not supported, but I don't want to hide the underlying js
error too much to allow for further investigation down the line. What's
shown now is decent enough:

<img width="2246" alt="image"
src="https://github.com/user-attachments/assets/55195a2c-9a14-41e3-bbfe-0bfb960ee6f8">


### 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/7503?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/7503?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/7503)
- [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`.
  • Loading branch information
Wumpf authored Sep 24, 2024
1 parent 9a84603 commit 79da203
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 12 deletions.
3 changes: 2 additions & 1 deletion crates/viewer/re_renderer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,14 @@ getrandom = { workspace = true, features = [
js-sys = { workspace = true }
wasm-bindgen-futures.workspace = true
web-sys = { workspace = true, features = [
"DomException",
"EncodedVideoChunk",
"EncodedVideoChunkInit",
"EncodedVideoChunkType",
"VideoFrame",
"VideoDecoder",
"VideoDecoderConfig",
"VideoDecoderInit",
"VideoFrame",
] }
wasm-bindgen = { workspace = true }

Expand Down
9 changes: 9 additions & 0 deletions crates/viewer/re_renderer/src/video/decoder/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@ impl Drop for VideoDecoder {
fn drop(&mut self) {
re_log::debug!("Dropping VideoDecoder");
if let Err(err) = self.decoder.close() {
if let Some(dom_exception) = err.dyn_ref::<web_sys::DomException>() {
if dom_exception.code() == web_sys::DomException::INVALID_STATE_ERR
&& self.decode_error.lock().is_some()
{
// Invalid state error after a decode error may happen, ignore it!
return;
}
}

re_log::warn!(
"Error when closing video decoder: {}",
js_error_to_string(&err)
Expand Down
2 changes: 2 additions & 0 deletions crates/viewer/re_space_view_spatial/src/ui_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ fn ui_from_scene(
} else if bounds != updated_bounds {
bounds_property.save_blueprint_component(ctx, &updated_bounds);
}
// Update stored bounds on the state, so visualizers see an up-to-date value.
view_state.visual_bounds_2d = Some(bounds);

RectTransform::from_to(letterboxed_bounds, response.rect)
}
Expand Down
43 changes: 32 additions & 11 deletions crates/viewer/re_space_view_spatial/src/visualizers/videos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use re_viewer_context::{

use crate::{
contexts::SpatialSceneEntityContext,
ui::SpatialSpaceViewState,
view_kind::SpatialSpaceViewKind,
visualizers::{entity_iterator, filter_visualizable_2d_entities},
PickableRectSourceData, PickableTexturedRect, SpatialSpaceView2D,
Expand Down Expand Up @@ -179,7 +180,7 @@ impl VideoFrameReferenceVisualizer {
match video.as_ref().map(|v| v.as_ref()) {
None => {
self.show_video_error(
render_ctx,
ctx,
spatial_ctx,
world_from_entity,
format!("No video asset at {video_reference:?}"),
Expand All @@ -200,7 +201,7 @@ impl VideoFrameReferenceVisualizer {
}
FrameDecodingResult::Error(err) => {
self.show_video_error(
render_ctx,
ctx,
spatial_ctx,
world_from_entity,
err.to_string(),
Expand Down Expand Up @@ -237,7 +238,7 @@ impl VideoFrameReferenceVisualizer {
}
Some(Err(err)) => {
self.show_video_error(
render_ctx,
ctx,
spatial_ctx,
world_from_entity,
err.to_string(),
Expand All @@ -259,13 +260,17 @@ impl VideoFrameReferenceVisualizer {

fn show_video_error(
&mut self,
render_ctx: &re_renderer::RenderContext,
ctx: &re_viewer_context::QueryContext<'_>,
spatial_ctx: &SpatialSceneEntityContext<'_>,
world_from_entity: glam::Affine3A,
error_string: String,
video_size: glam::Vec2,
entity_path: &EntityPath,
) {
let Some(render_ctx) = ctx.viewer_ctx.render_ctx else {
return;
};

let video_error_texture_result = render_ctx
.texture_manager_2d
.get_or_try_create_with::<image::ImageError>(
Expand Down Expand Up @@ -295,14 +300,33 @@ impl VideoFrameReferenceVisualizer {
};

// Center the icon in the middle of the video rectangle.
// Don't ignore translation - if the user moved the video frame, we move the error message long.
// Don't ignore translation - if the user moved the video frame, we move the error message along.
// But do ignore any rotation/scale on this, gets complicated to center and weird generally.
let video_error_rect_size = glam::vec2(
let mut video_error_rect_size = glam::vec2(
video_error_texture.width() as _,
video_error_texture.height() as _,
);
// If we're in a 2D view, make the error rect take a fixed amount of view space.
// This makes it look a lot nicer for very small & very large videos.
if let Some(state) = ctx
.view_state
.as_any()
.downcast_ref::<SpatialSpaceViewState>()
{
if let Some(bounds) = state.visual_bounds_2d {
// Aim for 1/8 of the larger visual bounds axis.
let max_extent = bounds.x_range.abs_len().max(bounds.y_range.abs_len()) as f32;
if max_extent > 0.0 {
let video_error_rect_aspect = video_error_rect_size.x / video_error_rect_size.y;
let extent_x = max_extent / 8.0;
let extent_y = extent_x / video_error_rect_aspect;
video_error_rect_size = glam::vec2(extent_x, extent_y);
}
}
}

let center = glam::Vec3::from(world_from_entity.translation).truncate() + video_size * 0.5;
let top_left_corner_position = center - video_error_rect_size;
let top_left_corner_position = center - video_error_rect_size * 0.5;

// Add a label that annotates a rectangle that is a bit bigger than the error icon.
// This makes the label track the icon better than putting it at a point.
Expand All @@ -311,10 +335,7 @@ impl VideoFrameReferenceVisualizer {
top_left_corner_position.x - video_error_rect_size.x,
top_left_corner_position.y,
),
egui::vec2(
video_error_rect_size.x * 3.0,
video_error_rect_size.y + 10.0,
),
egui::vec2(video_error_rect_size.x * 3.0, video_error_rect_size.y),
);
self.data.ui_labels.push(UiLabel {
text: error_string,
Expand Down

0 comments on commit 79da203

Please sign in to comment.