From d61018cad62013f614a39947f322908b9c32b5db Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 5 Nov 2024 13:06:01 +0100 Subject: [PATCH] Better error & download url if ffmpeg is missing (#7991) ### What * part of #7607 image Future work: * download for you in the viewer to a location we manage * have location be an application setting ### 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/7991?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/7991?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/7991) - [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`. --- crates/store/re_video/src/decode/ffmpeg.rs | 42 +++++++++++++++------- crates/store/re_video/src/decode/mod.rs | 11 ++++++ crates/viewer/re_data_ui/src/video.rs | 9 +++++ 3 files changed, 50 insertions(+), 12 deletions(-) diff --git a/crates/store/re_video/src/decode/ffmpeg.rs b/crates/store/re_video/src/decode/ffmpeg.rs index 8fdfe466e256..95a9e628bef6 100644 --- a/crates/store/re_video/src/decode/ffmpeg.rs +++ b/crates/store/re_video/src/decode/ffmpeg.rs @@ -19,7 +19,15 @@ use super::{AsyncDecoder, Chunk, Frame, OutputCallback}; #[derive(thiserror::Error, Debug)] pub enum Error { - #[error("Failed to start ffmppeg: {0}")] + #[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>, + }, + + #[error("Failed to start FFmpeg: {0}")] FailedToStartFfmpeg(std::io::Error), #[error("Failed to get stdin handle")] @@ -31,34 +39,38 @@ pub enum Error { #[error("No frame info received, this is a likely a bug in Rerun")] NoFrameInfo, - #[error("Failed to write data to ffmpeg: {0}")] + #[error("Failed to write data to FFmpeg: {0}")] FailedToWriteToFfmpeg(std::io::Error), #[error("Bad video data: {0}")] BadVideoData(String), - #[error("FFMPEG error: {0}")] + #[error("FFmpeg error: {0}")] Ffmpeg(String), - #[error("FFMPEG fatal error: {0}")] + #[error("FFmpeg fatal error: {0}")] FfmpegFatal(String), - #[error("FFMPEG IPC error: {0}")] + #[error("FFmpeg IPC error: {0}")] FfmpegSidecar(String), - #[error("FFMPEG exited unexpectedly with code {0:?}")] + #[error("FFmpeg exited unexpectedly with code {0:?}")] FfmpegUnexpectedExit(Option), - #[error("FFMPEG output a non-image chunk when we expected only images.")] + #[error("FFmpeg output a non-image chunk when we expected only images.")] UnexpectedFfmpegOutputChunk, - #[error("Failed to send video frame info to the ffmpeg read thread.")] + #[error("Failed to send video frame info to the FFmpeg read thread.")] BrokenFrameInfoChannel, } impl From for super::Error { fn from(err: Error) -> Self { - Self::Ffmpeg(std::sync::Arc::new(err)) + if let Error::FfmpegNotInstalled { download_url } = err { + Self::FfmpegNotInstalled { download_url } + } else { + Self::Ffmpeg(std::sync::Arc::new(err)) + } } } @@ -99,6 +111,12 @@ 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 mut ffmpeg = FfmpegCommand::new() .hide_banner() // "Reduce the latency introduced by buffering during initial input streams analysis." @@ -303,7 +321,7 @@ fn read_ffmpeg_output( FfmpegEvent::Log(LogLevel::Unknown, msg) => { if msg.contains("system signals, hard exiting") { // That was probably us, killing the process. - re_log::debug!("ffmpeg process for {debug_name} was killed"); + re_log::debug!("FFmpeg process for {debug_name} was killed"); return; } if !should_ignore_log_msg(&msg) { @@ -462,12 +480,12 @@ fn read_ffmpeg_output( } FfmpegEvent::ParsedVersion(ffmpeg_version) => { - re_log::debug_once!("ffmpeg version is: {}", ffmpeg_version.version); + re_log::debug_once!("FFmpeg version is: {}", ffmpeg_version.version); } FfmpegEvent::ParsedConfiguration(ffmpeg_configuration) => { re_log::debug_once!( - "ffmpeg configuration: {:?}", + "FFmpeg configuration: {:?}", ffmpeg_configuration.configuration ); } diff --git a/crates/store/re_video/src/decode/mod.rs b/crates/store/re_video/src/decode/mod.rs index 6b50ac7a87bb..09d1f02cd7ee 100644 --- a/crates/store/re_video/src/decode/mod.rs +++ b/crates/store/re_video/src/decode/mod.rs @@ -81,8 +81,10 @@ mod async_decoder_wrapper; #[cfg(with_dav1d)] mod av1; + #[cfg(with_ffmpeg)] mod ffmpeg; + #[cfg(target_arch = "wasm32")] mod webcodecs; @@ -117,6 +119,15 @@ pub enum Error { #[error(transparent)] Ffmpeg(std::sync::Arc), + // We need to check for this one and don't want to infect more crates with the feature requirement. + #[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>, + }, + #[error("Unsupported bits per component: {0}")] BadBitsPerComponent(usize), } diff --git a/crates/viewer/re_data_ui/src/video.rs b/crates/viewer/re_data_ui/src/video.rs index 2cd4c69a4eea..621b20535c0f 100644 --- a/crates/viewer/re_data_ui/src/video.rs +++ b/crates/viewer/re_data_ui/src/video.rs @@ -154,6 +154,15 @@ pub fn show_video_blob_info( Err(err) => { ui.error_label_long(&err.to_string()); + + if let re_renderer::video::VideoPlayerError::Decoding( + re_video::decode::Error::FfmpegNotInstalled { + download_url: Some(url), + }, + ) = err + { + 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`.")); + } } } }