Display video errors & video blob information #7433
Conversation
Wumpf
added
exclude from changelog
crates/viewer/re_data_ui/src/blob.rs
Outdated
| ui.list_item_flat_noninteractive(PropertyContent::new("Duration").value_text( | ||
| format!( | ||
| "{}", | ||
| re_log_types::Duration::from_millis(video.duration_ms() as _) |
There was a problem hiding this comment.
Can we also show the number of frames in the video?
There was a problem hiding this comment.
I actually had this in for a moment but then removed it again because it requires iterating over all the samples of a video. Which isn't terribly expensive and could be cached, but then I saw that video players only do this upon read and usually carefully talk about samples, not frames 🤔
@jprochazk what do you think?
There was a problem hiding this comment.
putting it in for now, but calling it Sample count instead. I figure the contention with "frames" is that it may refer to full frames/iframes.
There was a problem hiding this comment.
Yeah, I would be careful about this. Some people may think that num_frames / duration = fps, but that's not true, videos may have variable frame rate. At the same time, we don't want to overload users with video codec/container specific stuff that they have to understand, and for all intents and purposes one sample = one frame.
There was a problem hiding this comment.
makes sense, thx! I'll leave a note in the code so the next person doesn't reconsider too quickly
(also keeping sample count for the moment)
| if response.clicked() { | ||
| self.ui().ctx().copy_text(error_text.to_owned()); |
There was a problem hiding this comment.
This is a nice touch, but there is no affordance that tells the user that they can copy the error with a click.
@gavrelina may have a better idea here, but some alternatives:
- Show "Click to copy" in a tooltip on hover
- Make the text selectable instead
- Add a 📋 button next to the message instead
There was a problem hiding this comment.
for context: I copied this from the "small" error text method above. But I do agree!
There was a problem hiding this comment.
our 📋 icon is too ugly for this (also tried adding it to the regular error_label, but it's just too ugly). going with the minimum here for now whcih is selectable + tooltip
…o-error-reporting
What
As part of this work it was necessary to make clearer difference between video loading and video decoding. You'll notice changes in re_renderer's video and the video cache accordingly.
(there's some loose ends here like "don't create a decoder when we're just inspecting a blob" which will be part of #7420)
Generally, decode errors can only occur when showing (!) a video frame, therefore they are shown on screen as part of a the 2D/3D view:
(trying to show frames on native)
Note that you can interact with this 2D view just as if error was logged data:
video.error.interaction.mp4
On the other hand when we can't read the blob itself (potentially without trying to decode it) the errors show up on it like this:
Since this error happens already on loading right now we can't generate frames automatically if you drag&drop in a video like this. But you can still log your frames manually, resulting in this:
In the shown example the blob isn't part of the entity since we're referring to a different entity for the blob. Also, this is actually a native screenshot - the avc parsing error comes before decoding, which is why this comes up prior to the lack of decoder.
If we instead have the failing blob on the entity that also has the frame, we get this view:
When there's no error, we show now a little bit of information for blobs:
(blob selected)
(data result with this blob selected)
Some shortcomings:
rabbit holefeature I don't want to tackle in the near termChecklist
mainbuild: rerun.io/viewernightlybuild: rerun.io/viewerCHANGELOG.mdand the migration guideTo run all checks from
main, comment on the PR with@rerun-bot full-check.