From d679762b8de6ebba6460249c243dfd600189611d Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 13 Nov 2024 11:06:12 +0100 Subject: [PATCH 1/8] Calculate and show frame number --- crates/store/re_video/src/decode/av1.rs | 1 + .../re_video/src/decode/ffmpeg_h264/ffmpeg.rs | 17 +++++++++++- crates/store/re_video/src/decode/mod.rs | 26 +++++++++++++++++++ crates/store/re_video/src/decode/webcodecs.rs | 5 ++-- crates/store/re_video/src/demux/mod.rs | 12 +++++++++ crates/store/re_video/src/demux/mp4.rs | 25 ++++++++++++++++++ crates/viewer/re_data_ui/src/video.rs | 22 +++++++++++++--- crates/viewer/re_renderer/src/video/player.rs | 12 +++++++-- 8 files changed, 112 insertions(+), 8 deletions(-) diff --git a/crates/store/re_video/src/decode/av1.rs b/crates/store/re_video/src/decode/av1.rs index 584011922858..08e17a07504a 100644 --- a/crates/store/re_video/src/decode/av1.rs +++ b/crates/store/re_video/src/decode/av1.rs @@ -255,6 +255,7 @@ fn create_frame(debug_name: &str, picture: &dav1d::Picture) -> Result { info: FrameInfo { is_sync: None, // TODO(emilk) sample_idx: None, // TODO(emilk), + frame_nr: None, // TODO(emilk), presentation_timestamp: Time(picture.timestamp().unwrap_or(0)), duration: Time(picture.duration()), latest_decode_timestamp: None, diff --git a/crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs b/crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs index 7cd4402f4023..00e3641af674 100644 --- a/crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs +++ b/crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs @@ -103,9 +103,22 @@ struct FFmpegFrameInfo { /// can be decoded from only this one sample (though I'm not 100% sure). is_sync: bool, - /// Which sample is this in the video? + /// Which sample in the video is this from? + /// + /// In MP4, one sample is one frame, but we may be reordering samples when decoding. + /// + /// This is the order of which the samples appear in the container, + /// which is usually ordered by [`Self::decode_timestamp`]. sample_idx: usize, + /// Which frame is this? + /// + /// This is on the assumption that each sample produces a single frame, + /// which is true for MP4. + /// + /// This is the index of frames ordered by [`Self::presentation_timestamp`]. + frame_nr: usize, + presentation_timestamp: Time, duration: Time, decode_timestamp: Time, @@ -321,6 +334,7 @@ impl FFmpegProcessAndListener { let frame_info = FFmpegFrameInfo { is_sync: chunk.is_sync, sample_idx: chunk.sample_idx, + frame_nr: chunk.frame_nr, presentation_timestamp: chunk.presentation_timestamp, decode_timestamp: chunk.decode_timestamp, duration: chunk.duration, @@ -549,6 +563,7 @@ impl FrameBuffer { info: FrameInfo { is_sync: Some(frame_info.is_sync), sample_idx: Some(frame_info.sample_idx), + frame_nr: Some(frame_info.frame_nr), presentation_timestamp: frame_info.presentation_timestamp, duration: frame_info.duration, latest_decode_timestamp: Some(frame_info.decode_timestamp), diff --git a/crates/store/re_video/src/decode/mod.rs b/crates/store/re_video/src/decode/mod.rs index 1d1206dc3a27..7d81382ecc9f 100644 --- a/crates/store/re_video/src/decode/mod.rs +++ b/crates/store/re_video/src/decode/mod.rs @@ -161,6 +161,8 @@ pub fn new_decoder( ) -> Result> { #![allow(unused_variables, clippy::needless_return)] // With some feature flags + re_tracing::profile_function!(); + re_log::trace!( "Looking for decoder for {}", video.human_readable_codec_string() @@ -226,8 +228,19 @@ pub struct Chunk { pub data: Vec, /// Which sample (frame) did this chunk come from? + /// + /// This is the order of which the samples appear in the container, + /// which is usually ordered by [`Self::decode_timestamp`]. pub sample_idx: usize, + /// Which frame does this chunk belong to? + /// + /// This is on the assumption that each sample produces a single frame, + /// which is true for MP4. + /// + /// This is the index of samples ordered by [`Self::presentation_timestamp`]. + pub frame_nr: usize, + /// Decode timestamp of this sample. /// Chunks are expected to be submitted in the order of decode timestamp. /// @@ -271,9 +284,22 @@ pub struct FrameInfo { /// /// In MP4, one sample is one frame, but we may be reordering samples when decoding. /// + /// This is the order of which the samples appear in the container, + /// which is usually ordered by [`Self::decode_timestamp`]. + /// /// None = unknown. pub sample_idx: Option, + /// Which frame is this? + /// + /// This is on the assumption that each sample produces a single frame, + /// which is true for MP4. + /// + /// This is the index of frames ordered by [`Self::presentation_timestamp`]. + /// + /// None = unknown. + pub frame_nr: Option, + /// The presentation timestamp of the frame. pub presentation_timestamp: Time, diff --git a/crates/store/re_video/src/decode/webcodecs.rs b/crates/store/re_video/src/decode/webcodecs.rs index a7f9959e2ca5..a9fbb231bd46 100644 --- a/crates/store/re_video/src/decode/webcodecs.rs +++ b/crates/store/re_video/src/decode/webcodecs.rs @@ -204,8 +204,9 @@ fn init_video_decoder( on_output(Ok(Frame { content: WebVideoFrame(frame), info: FrameInfo { - sample_idx: None, - is_sync: None, + is_sync: None, // TODO(emilk) + sample_idx: None, // TODO(emilk) + frame_nr: None, // TODO(emilk) presentation_timestamp, duration, latest_decode_timestamp: None, diff --git a/crates/store/re_video/src/demux/mod.rs b/crates/store/re_video/src/demux/mod.rs index a1a74e63aa0e..dcafa88f0b59 100644 --- a/crates/store/re_video/src/demux/mod.rs +++ b/crates/store/re_video/src/demux/mod.rs @@ -466,8 +466,19 @@ pub struct Sample { pub is_sync: bool, /// Which sample is this in the video? + /// + /// This is the order of which the samples appear in the container, + /// which is usually ordered by [`Self::decode_timestamp`]. pub sample_idx: usize, + /// Which frame does this sample belong to? + /// + /// This is on the assumption that each sample produces a single frame, + /// which is true for MP4. + /// + /// This is the index of samples ordered by [`Self::presentation_timestamp`]. + pub frame_nr: usize, + /// Time at which this sample appears in the decoded bitstream, in time units. /// /// Samples should be decoded in this order. @@ -508,6 +519,7 @@ impl Sample { Some(Chunk { data, sample_idx: self.sample_idx, + frame_nr: self.frame_nr, decode_timestamp: self.decode_timestamp, presentation_timestamp: self.presentation_timestamp, duration: self.duration, diff --git a/crates/store/re_video/src/demux/mp4.rs b/crates/store/re_video/src/demux/mp4.rs index d97ca441ed87..2fb6aeaaab01 100644 --- a/crates/store/re_video/src/demux/mp4.rs +++ b/crates/store/re_video/src/demux/mp4.rs @@ -66,6 +66,7 @@ impl VideoData { samples.push(Sample { is_sync: sample.is_sync, sample_idx, + frame_nr: 0, // filled in after the loop decode_timestamp, presentation_timestamp, duration, @@ -75,6 +76,7 @@ impl VideoData { } } + // Append the last GOP if there are any samples left: if !samples.is_empty() { let start = samples[gop_sample_start_index].decode_timestamp; let sample_range = gop_sample_start_index as u32..samples.len() as u32; @@ -84,6 +86,29 @@ impl VideoData { }); } + { + re_tracing::profile_scope!("Sanity-check samples"); + let mut samples_are_in_decode_order = true; + for window in samples.windows(2) { + samples_are_in_decode_order &= + window[0].decode_timestamp <= window[1].decode_timestamp; + } + if !samples_are_in_decode_order { + re_log::debug!( + "Video samples are NOT in decode order. This is weird an inefficient." + ); + } + } + + { + re_tracing::profile_scope!("Calculate frame numbers"); + let mut samples_sorted_by_pts = samples.iter_mut().collect::>(); + samples_sorted_by_pts.sort_by_key(|s| s.presentation_timestamp); + for (frame_nr, sample) in samples_sorted_by_pts.into_iter().enumerate() { + sample.frame_nr = frame_nr; + } + } + let samples_statistics = SamplesStatistics::new(&samples); Ok(Self { diff --git a/crates/viewer/re_data_ui/src/video.rs b/crates/viewer/re_data_ui/src/video.rs index ed93e6c0e730..ae367f8ebec7 100644 --- a/crates/viewer/re_data_ui/src/video.rs +++ b/crates/viewer/re_data_ui/src/video.rs @@ -129,13 +129,16 @@ fn samples_table_ui(ui: &mut egui::Ui, video_data: &VideoData) { .auto_shrink([false, true]) .vscroll(true) .max_scroll_height(800.0) - .columns(Column::auto(), 7) + .columns(Column::auto(), 8) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .header(DesignTokens::table_header_height(), |mut header| { DesignTokens::setup_table_header(&mut header); header.col(|ui| { ui.strong("Sample"); }); + header.col(|ui| { + ui.strong("Frame"); + }); header.col(|ui| { ui.strong("GOP"); }); @@ -166,16 +169,21 @@ fn samples_table_ui(ui: &mut egui::Ui, video_data: &VideoData) { let sample = &video_data.samples[sample_idx]; let re_video::Sample { is_sync, - sample_idx: _, + sample_idx: sample_idx_in_sample, + frame_nr, decode_timestamp, presentation_timestamp, duration, byte_offset: _, byte_length, } = *sample; + debug_assert_eq!(sample_idx, sample_idx_in_sample); row.col(|ui| { - ui.monospace(sample_idx.to_string()); + ui.monospace(re_format::format_uint(sample_idx)); + }); + row.col(|ui| { + ui.monospace(re_format::format_uint(frame_nr)); }); row.col(|ui| { if let Some(gop_index) = video_data @@ -336,6 +344,7 @@ fn frame_info_ui(ui: &mut egui::Ui, frame_info: &FrameInfo, video_data: &re_vide let FrameInfo { is_sync, sample_idx, + frame_nr, presentation_timestamp, duration, latest_decode_timestamp, @@ -381,6 +390,13 @@ fn frame_info_ui(ui: &mut egui::Ui, frame_info: &FrameInfo, video_data: &re_vide ); } + if let Some(frame_nr) = frame_nr { + ui.list_item_flat_noninteractive(PropertyContent::new("Frame").value_fn(move |ui, _| { + ui.monospace(re_format::format_uint(frame_nr)); + })) + .on_hover_text("The frame number, as ordered by presentation time"); + } + if let Some(dts) = latest_decode_timestamp { ui.list_item_flat_noninteractive( PropertyContent::new("DTS").value_fn(value_fn_for_time(dts, video_data)), diff --git a/crates/viewer/re_renderer/src/video/player.rs b/crates/viewer/re_renderer/src/video/player.rs index e72c6b8a0f93..cb4f4c96c20b 100644 --- a/crates/viewer/re_renderer/src/video/player.rs +++ b/crates/viewer/re_renderer/src/video/player.rs @@ -266,8 +266,14 @@ impl VideoPlayer { // Therefore, it's important to compare presentation timestamps instead of sample indices. // (comparing decode timestamps should be equivalent to comparing sample indices) let current_pts = self.data.samples[self.current_sample_idx].presentation_timestamp; - let requested_pts = self.data.samples[requested_sample_idx].presentation_timestamp; - if requested_pts < current_pts { + let requested_sample = &self.data.samples[requested_sample_idx]; + + re_log::trace!( + "Seeking to sample {requested_sample_idx} (frame_nr {})", + requested_sample.frame_nr + ); + + if requested_sample.presentation_timestamp < current_pts { self.reset()?; self.enqueue_gop(requested_gop_idx, video_data)?; self.enqueue_gop(requested_gop_idx + 1, video_data)?; @@ -325,6 +331,8 @@ impl VideoPlayer { let samples = &self.data.samples[gop.sample_range_usize()]; + re_log::trace!("Enqueueing GOP {gop_idx} ({} samples)", samples.len()); + for sample in samples { let chunk = sample.get(video_data).ok_or(VideoPlayerError::BadData)?; self.chunk_decoder.decode(chunk)?; From 0501fd93599921e469e35faf4950b51f2848f3a6 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 13 Nov 2024 11:10:39 +0100 Subject: [PATCH 2/8] Make the video sample table resizable --- crates/viewer/re_data_ui/src/video.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/viewer/re_data_ui/src/video.rs b/crates/viewer/re_data_ui/src/video.rs index ae367f8ebec7..8e441ff36138 100644 --- a/crates/viewer/re_data_ui/src/video.rs +++ b/crates/viewer/re_data_ui/src/video.rs @@ -117,8 +117,14 @@ fn video_data_ui(ui: &mut egui::Ui, ui_layout: UiLayout, video_data: &VideoData) }); ui.list_item_collapsible_noninteractive_label("Video samples", false, |ui| { + egui::Resize::default() + .with_stroke(true) + .resizable([false, true]) + .max_height(611.0) // Odd value so the user can see half-hidden rows + .show(ui, |ui| { samples_table_ui(ui, video_data); }); + }); } } @@ -128,7 +134,7 @@ fn samples_table_ui(ui: &mut egui::Ui, video_data: &VideoData) { egui_extras::TableBuilder::new(ui) .auto_shrink([false, true]) .vscroll(true) - .max_scroll_height(800.0) + .max_scroll_height(611.0) // Odd value so the user can see half-hidden rows .columns(Column::auto(), 8) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .header(DesignTokens::table_header_height(), |mut header| { From 8a36acaa938f3d4df47becd8f80773c77bc9cabd Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 13 Nov 2024 11:11:06 +0100 Subject: [PATCH 3/8] Shrink video/image preview in selection panel --- crates/viewer/re_data_ui/src/image.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/viewer/re_data_ui/src/image.rs b/crates/viewer/re_data_ui/src/image.rs index caacd42acd80..7ba2fdc22747 100644 --- a/crates/viewer/re_data_ui/src/image.rs +++ b/crates/viewer/re_data_ui/src/image.rs @@ -85,10 +85,13 @@ pub fn texture_preview_ui( ) .inner } else { + // TODO(emilk): we should limit the HEIGHT primarily, + // since if the image uses up too much vertical space, + // it is really annoying in the selection panel. let size_range = if ui_layout == UiLayout::Tooltip { egui::Rangef::new(64.0, 128.0) } else { - egui::Rangef::new(240.0, 640.0) + egui::Rangef::new(240.0, 320.0) }; let preview_size = Vec2::splat( size_range From 0239c217d58ac89b5330211310a89bed4248e244 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 13 Nov 2024 11:17:11 +0100 Subject: [PATCH 4/8] cargo fmt --- crates/viewer/re_data_ui/src/video.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/viewer/re_data_ui/src/video.rs b/crates/viewer/re_data_ui/src/video.rs index 8e441ff36138..655a421fb395 100644 --- a/crates/viewer/re_data_ui/src/video.rs +++ b/crates/viewer/re_data_ui/src/video.rs @@ -122,8 +122,8 @@ fn video_data_ui(ui: &mut egui::Ui, ui_layout: UiLayout, video_data: &VideoData) .resizable([false, true]) .max_height(611.0) // Odd value so the user can see half-hidden rows .show(ui, |ui| { - samples_table_ui(ui, video_data); - }); + samples_table_ui(ui, video_data); + }); }); } } From 66675d90250352ca8a3198d53635d6d33006004f Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 13 Nov 2024 11:19:10 +0100 Subject: [PATCH 5/8] Stronger warning when DTS:es are not sorted --- crates/store/re_video/src/demux/mp4.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/store/re_video/src/demux/mp4.rs b/crates/store/re_video/src/demux/mp4.rs index 2fb6aeaaab01..ee44ff0d9775 100644 --- a/crates/store/re_video/src/demux/mp4.rs +++ b/crates/store/re_video/src/demux/mp4.rs @@ -94,8 +94,8 @@ impl VideoData { window[0].decode_timestamp <= window[1].decode_timestamp; } if !samples_are_in_decode_order { - re_log::debug!( - "Video samples are NOT in decode order. This is weird an inefficient." + re_log::warn!( + "Video samples are NOT in decode order. This implies either invalid video data or a bug in parsing the mp4." ); } } From e698ebaaed4a05e290a97bcc7de5824088e3bcdc Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 13 Nov 2024 11:25:13 +0100 Subject: [PATCH 6/8] Show frame-nr in collapsing header title --- crates/viewer/re_data_ui/src/video.rs | 45 ++++++++++++++++++--------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/crates/viewer/re_data_ui/src/video.rs b/crates/viewer/re_data_ui/src/video.rs index 655a421fb395..326805ec6f86 100644 --- a/crates/viewer/re_data_ui/src/video.rs +++ b/crates/viewer/re_data_ui/src/video.rs @@ -4,7 +4,10 @@ use re_renderer::{ video::VideoFrameTexture, }; use re_types::components::VideoTimestamp; -use re_ui::{list_item::PropertyContent, DesignTokens, UiExt}; +use re_ui::{ + list_item::{self, PropertyContent}, + DesignTokens, UiExt, +}; use re_video::{decode::FrameInfo, demux::SamplesStatistics, VideoData}; use re_viewer_context::UiLayout; @@ -93,7 +96,7 @@ fn video_data_ui(ui: &mut egui::Ui, ui_layout: UiLayout, video_data: &VideoData) ); if ui_layout != UiLayout::Tooltip { - ui.list_item_collapsible_noninteractive_label("MP4 tracks", true, |ui| { + ui.list_item_collapsible_noninteractive_label("MP4 tracks", false, |ui| { for (track_id, track_kind) in &video_data.mp4_tracks { let track_kind_string = match track_kind { Some(re_video::TrackKind::Audio) => "audio", @@ -274,19 +277,31 @@ pub fn show_decoded_frame_info( frame_info, source_pixel_format, }) => { - re_ui::list_item::list_item_scope(ui, "decoded_frame_ui", |ui| { - let default_open = false; - if let Some(frame_info) = frame_info { - ui.list_item_collapsible_noninteractive_label( - "Current decoded frame", - default_open, - |ui| { - frame_info_ui(ui, &frame_info, video.data()); - source_image_data_format_ui(ui, &source_pixel_format); - }, - ); - } - }); + if let Some(frame_info) = frame_info { + re_ui::list_item::list_item_scope(ui, "decoded_frame_ui", |ui| { + let id = ui.id().with("decoded_frame_collapsible"); + let default_open = false; + let label = if let Some(frame_nr) = frame_info.frame_nr { + format!("Decoded frame #{}", re_format::format_uint(frame_nr)) + } else { + "Current decoded frame".to_owned() + }; + ui.list_item() + .interactive(false) + .show_hierarchical_with_children( + ui, + id, + default_open, + list_item::LabelContent::new(label), + |ui| { + list_item::list_item_scope(ui, id, |ui| { + frame_info_ui(ui, &frame_info, video.data()); + source_image_data_format_ui(ui, &source_pixel_format); + }); + }, + ) + }); + } let response = crate::image::texture_preview_ui( render_ctx, From e8e475c3f0196be77ee03ad1800d202f240e9322 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 13 Nov 2024 11:26:13 +0100 Subject: [PATCH 7/8] Fix test --- crates/store/re_video/src/demux/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/store/re_video/src/demux/mod.rs b/crates/store/re_video/src/demux/mod.rs index dcafa88f0b59..be4a04448bf9 100644 --- a/crates/store/re_video/src/demux/mod.rs +++ b/crates/store/re_video/src/demux/mod.rs @@ -674,6 +674,7 @@ mod tests { .map(|(sample_idx, (pts, dts))| Sample { is_sync: false, sample_idx, + frame_nr: 0, // unused decode_timestamp: Time(dts), presentation_timestamp: Time(pts), duration: Time(1), From 8d60ffec434becd8252d094093934e718e5c2973 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 13 Nov 2024 12:06:34 +0100 Subject: [PATCH 8/8] Fix doclink --- crates/store/re_video/src/decode/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/store/re_video/src/decode/mod.rs b/crates/store/re_video/src/decode/mod.rs index 7d81382ecc9f..6ab96d536188 100644 --- a/crates/store/re_video/src/decode/mod.rs +++ b/crates/store/re_video/src/decode/mod.rs @@ -285,7 +285,7 @@ pub struct FrameInfo { /// In MP4, one sample is one frame, but we may be reordering samples when decoding. /// /// This is the order of which the samples appear in the container, - /// which is usually ordered by [`Self::decode_timestamp`]. + /// which is usually ordered by [`Self::latest_decode_timestamp`]. /// /// None = unknown. pub sample_idx: Option,