Skip to content

Commit

Permalink
Fix decoding errors no longer showing (#7722)
Browse files Browse the repository at this point in the history
### What

* Fixes #7672 

<img width="1598" alt="image"
src="https://github.com/user-attachments/assets/499431c5-5ec0-49ba-9978-e3f3173ae46a">

Through all the refactors it seems we lost our ability to show errors
from the decoder.
What happened is that we'd retrieve an error and then immediately forget
about it due to `reset`.

### 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/7722?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/7722?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/7722)
- [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 Oct 14, 2024
1 parent 84ceefd commit 1d8bff9
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 16 deletions.
35 changes: 20 additions & 15 deletions crates/viewer/re_renderer/src/video/decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use super::{DecodeHardwareAcceleration, DecodingError, VideoFrameTexture};
const DECODING_GRACE_DELAY: Duration = Duration::from_millis(400);

#[allow(unused)] // Unused for certain build flags
#[derive(Debug)]
struct TimedDecodingError {
time_of_first_error: Instant,
latest_error: DecodingError,
Expand Down Expand Up @@ -90,8 +91,10 @@ pub struct VideoDecoder {
current_gop_idx: usize,
current_sample_idx: usize,

error: Option<TimedDecodingError>,
error_on_last_frame_at: bool,
/// Last error that was encountered during decoding.
///
/// Only fully reset after a successful decode.
last_error: Option<TimedDecodingError>,
}

impl VideoDecoder {
Expand Down Expand Up @@ -174,8 +177,7 @@ impl VideoDecoder {
current_gop_idx: usize::MAX,
current_sample_idx: usize::MAX,

error: None,
error_on_last_frame_at: false,
last_error: None,
}
}

Expand All @@ -194,6 +196,7 @@ impl VideoDecoder {
let presentation_timestamp = Time::from_secs(presentation_timestamp_s, self.data.timescale);
let presentation_timestamp = presentation_timestamp.min(self.data.duration); // Don't seek past the end of the video.

let error_on_last_frame_at = self.last_error.is_some();
let result = self.frame_at_internal(render_ctx, presentation_timestamp);

match result {
Expand All @@ -204,7 +207,7 @@ impl VideoDecoder {
.contains(&presentation_timestamp);

let is_pending = !is_active_frame;
if is_pending && self.error_on_last_frame_at {
if is_pending && error_on_last_frame_at {
// If we switched from error to pending, clear the texture.
// This is important to avoid flickering, in particular when switching from
// benign errors like DecodingError::NegativeTimestamp.
Expand All @@ -213,8 +216,6 @@ impl VideoDecoder {
self.video_texture.time_range = Time::MAX..Time::MAX;
}

self.error_on_last_frame_at = false;

let show_spinner = if presentation_timestamp < self.video_texture.time_range.start {
// We're seeking backwards and somehow forgot to reset.
true
Expand All @@ -239,10 +240,7 @@ impl VideoDecoder {
})
}

Err(err) => {
self.error_on_last_frame_at = true;
Err(err)
}
Err(err) => Err(err),
}
}

Expand Down Expand Up @@ -296,7 +294,7 @@ impl VideoDecoder {

// 4. Enqueue GOPs as needed.

// First, check for decoding errors that may have been set asynchronously and reset if it's a new error.
// First, check for decoding errors that may have been set asynchronously and reset.
if let Some(error) = self.chunk_decoder.take_error() {
// For each new (!) error after entering the error state, we reset the decoder.
// This way, it might later recover from the error as we progress in the video.
Expand All @@ -306,7 +304,13 @@ impl VideoDecoder {
self.current_gop_idx = usize::MAX;
self.current_sample_idx = usize::MAX;

self.error = Some(error);
// If we already have an error set, preserve its occurrence time.
// Otherwise, set the error using the time at which it was registered.
if let Some(last_error) = &mut self.last_error {
last_error.latest_error = error.latest_error;
} else {
self.last_error = Some(error);
}
}

// We maintain a buffer of 2 GOPs, so we can always smoothly transition to the next GOP.
Expand Down Expand Up @@ -355,7 +359,7 @@ impl VideoDecoder {
// since we want to keep playing the video fine even if parts of it are broken.
// That said, practically we reset the decoder and thus all frames upon error,
// so it doesn't make a lot of difference.
if let Some(timed_error) = &self.error {
if let Some(timed_error) = &self.last_error {
if DECODING_GRACE_DELAY <= timed_error.time_of_first_error.elapsed() {
// Report the error only if we have been in an error state for a certain amount of time.
// Don't immediately report the error, since we might immediately recover from it.
Expand All @@ -372,6 +376,7 @@ impl VideoDecoder {
Err(err)
}
} else {
self.last_error = None;
Ok(())
}
}
Expand All @@ -398,9 +403,9 @@ impl VideoDecoder {
/// Reset the video decoder and discard all frames.
fn reset(&mut self) -> Result<(), DecodingError> {
self.chunk_decoder.reset()?;
self.error = None;
self.current_gop_idx = usize::MAX;
self.current_sample_idx = usize::MAX;
// Do *not* reset the error state. We want to keep track of the last error.
Ok(())
}
}
Expand Down
10 changes: 9 additions & 1 deletion crates/viewer/re_renderer/src/video/decoder/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,10 @@ fn init_video_decoder(
Closure::wrap(Box::new(move |err: js_sys::Error| {
let err = DecodingError::Decoding(js_error_to_string(&err));

// Many of the errors we get from a decoder are recoverable.
// They may be very frequent, but it's still useful to see them in the debug log for troubleshooting.
re_log::trace!("WebCodec decoder error: {err}");

let mut output = decoder_output.lock();
if let Some(error) = &mut output.error {
error.latest_error = err;
Expand Down Expand Up @@ -355,7 +359,11 @@ fn js_error_to_string(v: &wasm_bindgen::JsValue) -> String {
}

if let Some(v) = v.dyn_ref::<js_sys::Error>() {
return std::string::ToString::to_string(&v.to_string());
// Firefox prefixes most decoding errors with "EncodingError: ", which isn't super helpful.
let error = std::string::ToString::to_string(&v.to_string());
return error
.strip_prefix("EncodingError: ")
.map_or(error.clone(), |s| s.to_owned());
}

format!("{v:#?}")
Expand Down

0 comments on commit 1d8bff9

Please sign in to comment.