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 7f46b7cc24c6..e14b6dc27716 100644 --- a/crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs +++ b/crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs @@ -26,10 +26,12 @@ use crate::{ PixelFormat, Time, }; +use super::version::FFmpegVersionParseError; + #[derive(thiserror::Error, Debug)] pub enum Error { #[error("Couldn't find an installation of the FFmpeg executable.")] - FfmpegNotInstalled, + FFmpegNotInstalled, #[error("Failed to start FFmpeg: {0}")] FailedToStartFfmpeg(std::io::Error), @@ -37,18 +39,14 @@ pub enum Error { #[error("FFmpeg version is {actual_version}. Only versions >= {minimum_version_major}.{minimum_version_minor} are officially supported.")] UnsupportedFFmpegVersion { actual_version: FFmpegVersion, - - /// Copy of [`FFMPEG_MINIMUM_VERSION_MAJOR`]. minimum_version_major: u32, - - /// Copy of [`FFMPEG_MINIMUM_VERSION_MINOR`]. minimum_version_minor: u32, }, // TODO(andreas): This error can have a variety of reasons and is as such redundant to some of the others. // It works with an inner error because some of the error sources are behind an anyhow::Error inside of ffmpeg-sidecar. - #[error("Failed to determine FFmpeg version: {0}")] - FailedToDetermineFFmpegVersion(anyhow::Error), + #[error(transparent)] + FailedToDetermineFFmpegVersion(FFmpegVersionParseError), #[error("Failed to get stdin handle")] NoStdin, @@ -95,7 +93,7 @@ impl From for crate::decode::Error { /// ffmpeg does not tell us the timestamp/duration of a given frame, so we need to remember it. #[derive(Clone)] -struct FfmpegFrameInfo { +struct FFmpegFrameInfo { /// The start of a new [`crate::demux::GroupOfPictures`]? /// /// This probably means this is a _keyframe_, and that and entire frame @@ -106,7 +104,7 @@ struct FfmpegFrameInfo { decode_timestamp: Time, } -enum FfmpegFrameData { +enum FFmpegFrameData { Chunk(Chunk), EndOfStream, } @@ -141,14 +139,14 @@ impl std::io::Write for StdinWithShutdown { } } -struct FfmpegProcessAndListener { +struct FFmpegProcessAndListener { ffmpeg: FfmpegChild, /// For sending frame timestamps to the ffmpeg listener thread. - frame_info_tx: Sender, + frame_info_tx: Sender, /// For sending chunks to the ffmpeg write thread. - frame_data_tx: Sender, + frame_data_tx: Sender, listen_thread: Option>, write_thread: Option>, @@ -160,7 +158,7 @@ struct FfmpegProcessAndListener { on_output: Arc>>>, } -impl FfmpegProcessAndListener { +impl FFmpegProcessAndListener { fn new( debug_name: &str, on_output: Arc, @@ -301,7 +299,7 @@ impl FfmpegProcessAndListener { } } -impl Drop for FfmpegProcessAndListener { +impl Drop for FFmpegProcessAndListener { fn drop(&mut self) { re_tracing::profile_function!(); @@ -313,7 +311,7 @@ impl Drop for FfmpegProcessAndListener { } // Notify (potentially wake up) the stdin write thread to stop it (it might be sleeping). - self.frame_data_tx.send(FfmpegFrameData::EndOfStream).ok(); + self.frame_data_tx.send(FFmpegFrameData::EndOfStream).ok(); // Kill stdin for the write thread. This helps cancelling ongoing stream write operations. self.stdin_shutdown .store(true, std::sync::atomic::Ordering::Release); @@ -361,7 +359,7 @@ impl Drop for FfmpegProcessAndListener { fn write_ffmpeg_input( ffmpeg_stdin: &mut dyn std::io::Write, - frame_data_rx: &Receiver, + frame_data_rx: &Receiver, on_output: &Mutex>>, avcc: &re_mp4::Avc1Box, ) { @@ -369,8 +367,8 @@ fn write_ffmpeg_input( while let Ok(data) = frame_data_rx.recv() { let chunk = match data { - FfmpegFrameData::Chunk(chunk) => chunk, - FfmpegFrameData::EndOfStream => break, + FFmpegFrameData::Chunk(chunk) => chunk, + FFmpegFrameData::EndOfStream => break, }; if let Err(err) = write_avc_chunk_to_nalu_stream(avcc, ffmpeg_stdin, &chunk, &mut state) { @@ -396,7 +394,7 @@ fn write_ffmpeg_input( fn read_ffmpeg_output( debug_name: &str, ffmpeg_iterator: ffmpeg_sidecar::iter::FfmpegIterator, - frame_info_rx: &Receiver, + frame_info_rx: &Receiver, pixel_format: &PixelFormat, on_output: &Mutex>>, ) -> Option<()> { @@ -644,14 +642,14 @@ fn read_ffmpeg_output( } /// Decode H.264 video via ffmpeg over CLI -pub struct FfmpegCliH264Decoder { +pub struct FFmpegCliH264Decoder { debug_name: String, - ffmpeg: FfmpegProcessAndListener, + ffmpeg: FFmpegProcessAndListener, avcc: re_mp4::Avc1Box, on_output: Arc, } -impl FfmpegCliH264Decoder { +impl FFmpegCliH264Decoder { pub fn new( debug_name: String, avcc: re_mp4::Avc1Box, @@ -677,13 +675,18 @@ impl FfmpegCliH264Decoder { }); } } + Err(FFmpegVersionParseError::ParseVersion { raw_version }) => { + // This happens quite often, don't fail playing video over it! + re_log::warn_once!("Failed to parse FFmpeg version: {raw_version}"); + } + Err(err) => { return Err(Error::FailedToDetermineFFmpegVersion(err)); } } let on_output = Arc::new(on_output); - let ffmpeg = FfmpegProcessAndListener::new(&debug_name, on_output.clone(), avcc.clone())?; + let ffmpeg = FFmpegProcessAndListener::new(&debug_name, on_output.clone(), avcc.clone())?; Ok(Self { debug_name, @@ -694,19 +697,19 @@ impl FfmpegCliH264Decoder { } } -impl AsyncDecoder for FfmpegCliH264Decoder { +impl AsyncDecoder for FFmpegCliH264Decoder { fn submit_chunk(&mut self, chunk: Chunk) -> crate::decode::Result<()> { re_tracing::profile_function!(); // We send the information about this chunk first. // Chunks are defined to always yield a single frame. - let frame_info = FfmpegFrameInfo { + let frame_info = FFmpegFrameInfo { is_sync: chunk.is_sync, presentation_timestamp: chunk.presentation_timestamp, decode_timestamp: chunk.decode_timestamp, duration: chunk.duration, }; - let chunk = FfmpegFrameData::Chunk(chunk); + let chunk = FFmpegFrameData::Chunk(chunk); if self.ffmpeg.frame_info_tx.send(frame_info).is_err() || self.ffmpeg.frame_data_tx.send(chunk).is_err() @@ -729,7 +732,7 @@ impl AsyncDecoder for FfmpegCliH264Decoder { fn reset(&mut self) -> crate::decode::Result<()> { re_log::debug!("Resetting ffmpeg decoder {}", self.debug_name); - self.ffmpeg = FfmpegProcessAndListener::new( + self.ffmpeg = FFmpegProcessAndListener::new( &self.debug_name, self.on_output.clone(), self.avcc.clone(), diff --git a/crates/store/re_video/src/decode/ffmpeg_h264/mod.rs b/crates/store/re_video/src/decode/ffmpeg_h264/mod.rs index a14a1bcf297c..cfff9fea69f8 100644 --- a/crates/store/re_video/src/decode/ffmpeg_h264/mod.rs +++ b/crates/store/re_video/src/decode/ffmpeg_h264/mod.rs @@ -3,8 +3,11 @@ mod nalu; mod sps; mod version; -pub use ffmpeg::{Error, FfmpegCliH264Decoder}; -pub use version::{FFmpegVersion, FFMPEG_MINIMUM_VERSION_MAJOR, FFMPEG_MINIMUM_VERSION_MINOR}; +pub use ffmpeg::{Error, FFmpegCliH264Decoder}; +pub use version::{ + FFmpegVersion, FFmpegVersionParseError, FFMPEG_MINIMUM_VERSION_MAJOR, + FFMPEG_MINIMUM_VERSION_MINOR, +}; /// Download URL for the latest version of `FFmpeg` on the current platform. /// None if the platform is not supported. diff --git a/crates/store/re_video/src/decode/ffmpeg_h264/version.rs b/crates/store/re_video/src/decode/ffmpeg_h264/version.rs index 520ec248e4f2..271fa44cb161 100644 --- a/crates/store/re_video/src/decode/ffmpeg_h264/version.rs +++ b/crates/store/re_video/src/decode/ffmpeg_h264/version.rs @@ -23,6 +23,18 @@ impl std::fmt::Display for FFmpegVersion { } } +#[derive(thiserror::Error, Debug, Clone)] +pub enum FFmpegVersionParseError { + #[error("Failed to retrieve file modification time of FFmpeg executable: {0}")] + RetrieveFileModificationTime(String), + + #[error("Failed to determine FFmpeg version: {0}")] + RunFFmpeg(String), + + #[error("Failed to parse FFmpeg version: {raw_version}")] + ParseVersion { raw_version: String }, +} + impl FFmpegVersion { pub fn parse(raw_version: &str) -> Option { // Version strings can get pretty wild! @@ -58,9 +70,18 @@ impl FFmpegVersion { /// /// If none is passed for the path, it uses `ffmpeg` from PATH. /// + /// Error indicates issues running `FFmpeg`. Ok(None) indicates that we weren't able to parse the + /// version string. Since version strings can get pretty wild, we don't want to fail in this case. + /// /// Internally caches the result per path together with its modification time to re-run/parse the version only if the file has changed. - pub fn for_executable(path: Option<&std::path::Path>) -> anyhow::Result { - type VersionMap = HashMap, FFmpegVersion)>; + pub fn for_executable(path: Option<&std::path::Path>) -> Result { + type VersionMap = HashMap< + PathBuf, + ( + Option, + Result, + ), + >; static CACHE: Lazy> = Lazy::new(|| Mutex::new(HashMap::new())); re_tracing::profile_function!(); @@ -68,7 +89,9 @@ impl FFmpegVersion { // Retrieve file modification time first. let modification_time = if let Some(path) = path { path.metadata() - .map_err(|err| anyhow::anyhow!("Failed to read file: {err}"))? + .map_err(|err| { + FFmpegVersionParseError::RetrieveFileModificationTime(err.to_string()) + })? .modified() .ok() } else { @@ -80,7 +103,7 @@ impl FFmpegVersion { let cache_key = path.unwrap_or(std::path::Path::new("ffmpeg")); if let Some(cached) = cache.get(cache_key) { if modification_time == cached.0 { - return Ok(cached.1.clone()); + return cached.1.clone(); } } @@ -89,15 +112,23 @@ impl FFmpegVersion { ffmpeg_sidecar::version::ffmpeg_version_with_path(path) } else { ffmpeg_sidecar::version::ffmpeg_version() - }?; - let version = Self::parse(&raw_version) - .ok_or_else(|| anyhow::anyhow!("Failed to parse FFmpeg version: {raw_version}"))?; + } + .map_err(|err| FFmpegVersionParseError::RunFFmpeg(err.to_string()))?; + + let version = if let Some(version) = Self::parse(&raw_version) { + Ok(version) + } else { + Err(FFmpegVersionParseError::ParseVersion { + raw_version: raw_version.clone(), + }) + }; + cache.insert( cache_key.to_path_buf(), (modification_time, version.clone()), ); - Ok(version) + version } /// Returns true if this version is compatible with Rerun's minimum requirements. diff --git a/crates/store/re_video/src/decode/mod.rs b/crates/store/re_video/src/decode/mod.rs index 59fcfd2e7775..79cd4d7c5444 100644 --- a/crates/store/re_video/src/decode/mod.rs +++ b/crates/store/re_video/src/decode/mod.rs @@ -86,7 +86,7 @@ mod av1; mod ffmpeg_h264; #[cfg(with_ffmpeg)] -pub use ffmpeg_h264::{ffmpeg_download_url, Error as FfmpegError}; +pub use ffmpeg_h264::{ffmpeg_download_url, Error as FFmpegError, FFmpegVersionParseError}; #[cfg(target_arch = "wasm32")] mod webcodecs; @@ -120,7 +120,7 @@ pub enum Error { #[cfg(with_ffmpeg)] #[error(transparent)] - Ffmpeg(std::sync::Arc), + Ffmpeg(std::sync::Arc), #[error("Unsupported bits per component: {0}")] BadBitsPerComponent(usize), @@ -193,7 +193,7 @@ pub fn new_decoder( #[cfg(with_ffmpeg)] re_mp4::StsdBoxContent::Avc1(avc1_box) => { re_log::trace!("Decoding H.264…"); - Ok(Box::new(ffmpeg_h264::FfmpegCliH264Decoder::new( + Ok(Box::new(ffmpeg_h264::FFmpegCliH264Decoder::new( debug_name.to_owned(), avc1_box.clone(), on_output, diff --git a/crates/viewer/re_data_ui/src/video.rs b/crates/viewer/re_data_ui/src/video.rs index 65c58be06457..7098c650b5f5 100644 --- a/crates/viewer/re_data_ui/src/video.rs +++ b/crates/viewer/re_data_ui/src/video.rs @@ -209,9 +209,9 @@ pub fn show_decoded_frame_info( ) = &err { match err.as_ref() { - re_video::decode::FfmpegError::UnsupportedFFmpegVersion { .. } - | re_video::decode::FfmpegError::FailedToDetermineFFmpegVersion(_) - | re_video::decode::FfmpegError::FfmpegNotInstalled => { + re_video::decode::FFmpegError::UnsupportedFFmpegVersion { .. } + | re_video::decode::FFmpegError::FailedToDetermineFFmpegVersion(_) + | re_video::decode::FFmpegError::FfmpegNotInstalled => { if let Some(download_url) = re_video::decode::ffmpeg_download_url() { ui.markdown_ui(&format!("You can download a build of `FFmpeg` [here]({download_url}). For Rerun to be able to use it, its binaries need to be reachable from `PATH`.")); }