Skip to content

Commit

Permalink
Make old ffmpeg version a hard error (#8094)
Browse files Browse the repository at this point in the history
### What

... and make ffmpeg version parsing more robust


### 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/8094?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/8094?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/8094)
- [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`.
  • Loading branch information
Wumpf authored Nov 12, 2024
1 parent 925fec3 commit 93b611e
Show file tree
Hide file tree
Showing 8 changed files with 313 additions and 84 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6487,6 +6487,7 @@ dependencies = [
"indicatif",
"itertools 0.13.0",
"js-sys",
"once_cell",
"parking_lot",
"re_build_info",
"re_build_tools",
Expand Down
1 change: 1 addition & 0 deletions crates/store/re_video/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ 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
Expand Down
140 changes: 74 additions & 66 deletions crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,35 @@ use crate::{
ffmpeg_h264::{
nalu::{NalHeader, NalUnitType, NAL_START_CODE},
sps::H264Sps,
FFmpegVersion, FFMPEG_MINIMUM_VERSION_MAJOR, FFMPEG_MINIMUM_VERSION_MINOR,
},
AsyncDecoder, Chunk, Frame, FrameContent, FrameInfo, OutputCallback,
},
PixelFormat, Time,
};

// 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.
const FFMPEG_MINIMUM_VERSION_MAJOR: u32 = 5;
const FFMPEG_MINIMUM_VERSION_MINOR: u32 = 1;
use super::version::FFmpegVersionParseError;

#[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,
minimum_version_major: u32,
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(transparent)]
FailedToDetermineFFmpegVersion(FFmpegVersionParseError),

#[error("Failed to get stdin handle")]
NoStdin,

Expand Down Expand Up @@ -82,17 +87,13 @@ pub enum Error {

impl From<Error> for crate::decode::Error {
fn from(err: Error) -> Self {
if let Error::FfmpegNotInstalled { download_url } = err {
Self::FfmpegNotInstalled { download_url }
} else {
Self::Ffmpeg(std::sync::Arc::new(err))
}
Self::Ffmpeg(std::sync::Arc::new(err))
}
}

/// 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
Expand All @@ -103,7 +104,7 @@ struct FfmpegFrameInfo {
decode_timestamp: Time,
}

enum FfmpegFrameData {
enum FFmpegFrameData {
Chunk(Chunk),
EndOfStream,
}
Expand Down Expand Up @@ -138,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<FfmpegFrameInfo>,
frame_info_tx: Sender<FFmpegFrameInfo>,

/// For sending chunks to the ffmpeg write thread.
frame_data_tx: Sender<FfmpegFrameData>,
frame_data_tx: Sender<FFmpegFrameData>,

listen_thread: Option<std::thread::JoinHandle<()>>,
write_thread: Option<std::thread::JoinHandle<()>>,
Expand All @@ -157,20 +158,14 @@ struct FfmpegProcessAndListener {
on_output: Arc<Mutex<Option<Arc<OutputCallback>>>>,
}

impl FfmpegProcessAndListener {
impl FFmpegProcessAndListener {
fn new(
debug_name: &str,
on_output: Arc<OutputCallback>,
avcc: re_mp4::Avc1Box,
) -> Result<Self, Error> {
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:?}");
Expand Down Expand Up @@ -304,7 +299,7 @@ impl FfmpegProcessAndListener {
}
}

impl Drop for FfmpegProcessAndListener {
impl Drop for FFmpegProcessAndListener {
fn drop(&mut self) {
re_tracing::profile_function!();

Expand All @@ -316,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);
Expand Down Expand Up @@ -364,16 +359,16 @@ impl Drop for FfmpegProcessAndListener {

fn write_ffmpeg_input(
ffmpeg_stdin: &mut dyn std::io::Write,
frame_data_rx: &Receiver<FfmpegFrameData>,
frame_data_rx: &Receiver<FFmpegFrameData>,
on_output: &Mutex<Option<Arc<OutputCallback>>>,
avcc: &re_mp4::Avc1Box,
) {
let mut state = NaluStreamState::default();

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) {
Expand All @@ -399,7 +394,7 @@ fn write_ffmpeg_input(
fn read_ffmpeg_output(
debug_name: &str,
ffmpeg_iterator: ffmpeg_sidecar::iter::FfmpegIterator,
frame_info_rx: &Receiver<FfmpegFrameInfo>,
frame_info_rx: &Receiver<FFmpegFrameInfo>,
pixel_format: &PixelFormat,
on_output: &Mutex<Option<Arc<OutputCallback>>>,
) -> Option<()> {
Expand Down Expand Up @@ -595,8 +590,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}.")
Expand All @@ -605,29 +598,16 @@ fn read_ffmpeg_output(
}
}

// Version strings can get pretty wild!
// E.g. choco installed ffmpeg on Windows gives me "7.1-essentials_build-www.gyan.dev".
let mut version_parts = ffmpeg_version.version.split('.');
let major = version_parts
.next()
.and_then(|part| part.parse::<u32>().ok());
let minor = version_parts.next().and_then(|part| {
part.split('-')
.next()
.and_then(|part| part.parse::<u32>().ok())
});

if let (Some(major), Some(minor)) = (major, minor) {
re_log::debug_once!("Parsed FFmpeg version as {}.{}", major, minor);

if major < FFMPEG_MINIMUM_VERSION_MAJOR
|| (major == FFMPEG_MINIMUM_VERSION_MAJOR
&& minor < FFMPEG_MINIMUM_VERSION_MINOR)
{
re_log::warn_once!(
"FFmpeg version is {}. Only versions >= {FFMPEG_MINIMUM_VERSION_MAJOR}.{FFMPEG_MINIMUM_VERSION_MINOR} are officially supported.{}",
ffmpeg_version.version, download_advice()
);
if let Some(ffmpeg_version) = FFmpegVersion::parse(&ffmpeg_version.version) {
re_log::debug_once!("Parsed FFmpeg version: {ffmpeg_version}");

if !ffmpeg_version.is_compatible() {
(on_output.lock().as_ref()?)(Err(Error::UnsupportedFFmpegVersion {
actual_version: ffmpeg_version,
minimum_version_major: FFMPEG_MINIMUM_VERSION_MAJOR,
minimum_version_minor: FFMPEG_MINIMUM_VERSION_MINOR,
}
.into()));
}
} else {
re_log::warn_once!(
Expand Down Expand Up @@ -662,23 +642,51 @@ 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<OutputCallback>,
}

impl FfmpegCliH264Decoder {
impl FFmpegCliH264Decoder {
pub fn new(
debug_name: String,
avcc: re_mp4::Avc1Box,
on_output: impl Fn(crate::decode::Result<Frame>) + Send + Sync + 'static,
) -> Result<Self, Error> {
re_tracing::profile_function!();

// TODO(ab): Pass executable 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 executable 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(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,
Expand All @@ -689,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()
Expand All @@ -724,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(),
Expand Down
14 changes: 13 additions & 1 deletion crates/store/re_video/src/decode/ffmpeg_h264/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
mod ffmpeg;
mod nalu;
mod sps;
mod version;

pub use ffmpeg::{Error, FfmpegCliH264Decoder};
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.
// 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()
}
Loading

0 comments on commit 93b611e

Please sign in to comment.