From 10ab7e6266c200672f8de6018c42c92b39984252 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 12 Nov 2024 14:58:15 +0100 Subject: [PATCH] ffmpeg executable version check with cache --- Cargo.lock | 2 + crates/store/re_video/Cargo.toml | 2 + .../re_video/src/decode/ffmpeg_h264/ffmpeg.rs | 70 +++++++++++-------- .../re_video/src/decode/ffmpeg_h264/mod.rs | 7 ++ .../src/decode/ffmpeg_h264/version.rs | 55 ++++++++++++++- crates/store/re_video/src/decode/mod.rs | 2 +- crates/viewer/re_data_ui/src/video.rs | 14 ++-- 7 files changed, 111 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5e6c3876f4624..2633d9fee8494 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6478,6 +6478,7 @@ dependencies = [ name = "re_video" version = "0.20.0-alpha.4+dev" dependencies = [ + "anyhow", "bit-vec", "cfg_aliases 0.2.1", "criterion", @@ -6487,6 +6488,7 @@ dependencies = [ "indicatif", "itertools 0.13.0", "js-sys", + "once_cell", "parking_lot", "re_build_info", "re_build_tools", diff --git a/crates/store/re_video/Cargo.toml b/crates/store/re_video/Cargo.toml index bfad392b3be19..0ee3af0bd022d 100644 --- a/crates/store/re_video/Cargo.toml +++ b/crates/store/re_video/Cargo.toml @@ -47,10 +47,12 @@ re_build_info.workspace = true re_log.workspace = true re_tracing.workspace = true +anyhow.workspace = true bit-vec.workspace = true crossbeam.workspace = true econtext.workspace = true itertools.workspace = true +once_cell.workspace = true parking_lot.workspace = true re_mp4.workspace = true thiserror.workspace = true 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 121369b5bff1c..15778c041e08b 100644 --- a/crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs +++ b/crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs @@ -29,16 +29,27 @@ use crate::{ #[derive(thiserror::Error, Debug)] pub enum Error { #[error("Couldn't find an installation of the FFmpeg executable.")] - FfmpegNotInstalled { - /// Download URL for the latest version of `FFmpeg` on the current platform. - /// None if the platform is not supported. - // TODO(andreas): as of writing, ffmpeg-sidecar doesn't define a download URL for linux arm. - download_url: Option<&'static str>, - }, + FfmpegNotInstalled, #[error("Failed to start FFmpeg: {0}")] FailedToStartFfmpeg(std::io::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("Failed to get stdin handle")] NoStdin, @@ -74,21 +85,6 @@ pub enum Error { #[error("Failed to parse sequence parameter set.")] SpsParsing, - - #[error("FFmpeg version is {actual_version}. Only versions >= {minimum_version_major}.{minimum_version_minor} are officially supported.")] - UnsupportedFFmpegVersion { - actual_version: FFmpegVersion, - /// Download URL for the latest version of `FFmpeg` on the current platform. - /// None if the platform is not supported. - // TODO(andreas): as of writing, ffmpeg-sidecar doesn't define a download URL for linux arm. - download_url: Option<&'static str>, - - /// Copy of [`FFMPEG_MINIMUM_VERSION_MAJOR`]. - minimum_version_major: u32, - - /// Copy of [`FFMPEG_MINIMUM_VERSION_MINOR`]. - minimum_version_minor: u32, - }, } impl From for crate::decode::Error { @@ -167,12 +163,6 @@ impl FfmpegProcessAndListener { ) -> Result { re_tracing::profile_function!(); - if !ffmpeg_sidecar::command::ffmpeg_is_installed() { - return Err(Error::FfmpegNotInstalled { - download_url: ffmpeg_sidecar::download::ffmpeg_download_url().ok(), - }); - } - let sps_result = H264Sps::parse_from_avcc(&avcc); if let Ok(sps) = &sps_result { re_log::trace!("Successfully parsed SPS for {debug_name}:\n{sps:?}"); @@ -596,8 +586,6 @@ fn read_ffmpeg_output( } FfmpegEvent::ParsedVersion(ffmpeg_version) => { - re_log::debug_once!("FFmpeg version is {}", ffmpeg_version.version); - fn download_advice() -> String { if let Ok(download_url) = ffmpeg_sidecar::download::ffmpeg_download_url() { format!("\nYou can download an up to date version for your system at {download_url}.") @@ -612,7 +600,6 @@ fn read_ffmpeg_output( if !ffmpeg_version.is_compatible() { (on_output.lock().as_ref()?)(Err(Error::UnsupportedFFmpegVersion { actual_version: ffmpeg_version, - download_url: ffmpeg_sidecar::download::ffmpeg_download_url().ok(), minimum_version_major: FFMPEG_MINIMUM_VERSION_MAJOR, minimum_version_minor: FFMPEG_MINIMUM_VERSION_MINOR, } @@ -666,6 +653,29 @@ impl FfmpegCliH264Decoder { ) -> Result { re_tracing::profile_function!(); + // TODO(ab): Pass exectuable path here. + if !ffmpeg_sidecar::command::ffmpeg_is_installed() { + return Err(Error::FfmpegNotInstalled); + } + + // Check the version once ahead of running FFmpeg. + // The error is still handled if it happens while running FFmpeg, but it's a bit unclear if we can get it to start in the first place then. + // TODO(ab): Pass exectuable path here. + match FFmpegVersion::for_executable(None) { + Ok(version) => { + if !version.is_compatible() { + return Err(Error::UnsupportedFFmpegVersion { + actual_version: version, + minimum_version_major: FFMPEG_MINIMUM_VERSION_MAJOR, + minimum_version_minor: FFMPEG_MINIMUM_VERSION_MINOR, + }); + } + } + 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())?; 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 fbda56e045344..a14a1bcf297ce 100644 --- a/crates/store/re_video/src/decode/ffmpeg_h264/mod.rs +++ b/crates/store/re_video/src/decode/ffmpeg_h264/mod.rs @@ -5,3 +5,10 @@ mod version; pub use ffmpeg::{Error, FfmpegCliH264Decoder}; pub use version::{FFmpegVersion, 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. +// TODO(andreas): as of writing, ffmpeg-sidecar doesn't define a download URL for linux arm. +pub fn ffmpeg_download_url() -> Option<&'static str> { + ffmpeg_sidecar::download::ffmpeg_download_url().ok() +} 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 50094437393a2..8ef09b0d50419 100644 --- a/crates/store/re_video/src/decode/ffmpeg_h264/version.rs +++ b/crates/store/re_video/src/decode/ffmpeg_h264/version.rs @@ -1,11 +1,16 @@ +use std::{collections::HashMap, path::PathBuf}; + +use once_cell::sync::Lazy; +use parking_lot::Mutex; + // FFmpeg 5.1 "Riemann" is from 2022-07-22. // It's simply the oldest I tested manually as of writing. We might be able to go lower. // However, we also know that FFmpeg 4.4 is already no longer working. pub const FFMPEG_MINIMUM_VERSION_MAJOR: u32 = 5; pub const FFMPEG_MINIMUM_VERSION_MINOR: u32 = 1; -/// A successfully parsed FFmpeg version. -#[derive(Debug, PartialEq, Eq)] +/// A successfully parsed `FFmpeg` version. +#[derive(Clone, Debug, PartialEq, Eq)] pub struct FFmpegVersion { major: u32, minor: u32, @@ -49,6 +54,52 @@ impl FFmpegVersion { }) } + /// Try to parse the `FFmpeg` version for a given `FFmpeg` executable. + /// + /// If none is passed for the path, it uses `ffmpeg` from PATH. + /// + /// 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)>; + static CACHE: Lazy> = Lazy::new(|| Mutex::new(HashMap::new())); + + re_tracing::profile_function!(); + + // 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}"))? + .modified() + .ok() + } else { + None + }; + + // Check first if we already have the version cached. + let mut cache = CACHE.lock(); + 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()); + } + } + + // Run FFmpeg (or whatever was passed to us) to get the version. + let raw_version = if let Some(path) = path { + 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}"))?; + cache.insert( + cache_key.to_path_buf(), + (modification_time, version.clone()), + ); + + Ok(version) + } + /// Returns true if this version is compatible with Rerun's minimum requirements. pub fn is_compatible(&self) -> bool { self.major > FFMPEG_MINIMUM_VERSION_MAJOR diff --git a/crates/store/re_video/src/decode/mod.rs b/crates/store/re_video/src/decode/mod.rs index 1624ab4375b18..602201b26cf52 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::Error as FfmpegError; +pub use ffmpeg_h264::{ffmpeg_download_url, Error as FfmpegError}; #[cfg(target_arch = "wasm32")] mod webcodecs; diff --git a/crates/viewer/re_data_ui/src/video.rs b/crates/viewer/re_data_ui/src/video.rs index e0697f978d972..4e8f66312a261 100644 --- a/crates/viewer/re_data_ui/src/video.rs +++ b/crates/viewer/re_data_ui/src/video.rs @@ -207,14 +207,12 @@ pub fn show_decoded_frame_info( ) = &err { match err.as_ref() { - re_video::decode::FfmpegError::UnsupportedFFmpegVersion { - download_url: Some(url), - .. - } - | re_video::decode::FfmpegError::FfmpegNotInstalled { - download_url: Some(url), - } => { - ui.markdown_ui(&format!("You can download a build of `FFmpeg` [here]({url}). For Rerun to be able to use it, its binaries need to be reachable from `PATH`.")); + 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`.")); + } } _ => {}