Skip to content

Commit d4daf35

Browse files
committed
improve ffmpeg version parser
1 parent caff610 commit d4daf35

File tree

1 file changed

+36
-21
lines changed
  • crates/store/re_video/src/decode/ffmpeg_h264

1 file changed

+36
-21
lines changed

crates/store/re_video/src/decode/ffmpeg_h264/ffmpeg.rs

+36-21
Original file line numberDiff line numberDiff line change
@@ -587,28 +587,43 @@ fn read_ffmpeg_output(
587587
FfmpegEvent::ParsedVersion(ffmpeg_version) => {
588588
re_log::debug_once!("FFmpeg version is {}", ffmpeg_version.version);
589589

590-
let mut version_parts = ffmpeg_version.version.split(".");
591-
if let (Some(major), Some(minor)) = (
592-
version_parts
593-
.next()
594-
.and_then(|part| part.parse::<u32>().ok()),
595-
version_parts
590+
fn download_advice() -> String {
591+
if let Ok(download_url) = ffmpeg_sidecar::download::ffmpeg_download_url() {
592+
format!("\nYou can download an up to date version for your system at {download_url}.")
593+
} else {
594+
String::new()
595+
}
596+
}
597+
598+
// Version strings can get pretty wild!
599+
// E.g. choco installed ffmpeg on Windows gives me "7.1-essentials_build-www.gyan.dev".
600+
let mut version_parts = ffmpeg_version.version.split('.');
601+
let major = version_parts
602+
.next()
603+
.and_then(|part| part.parse::<u32>().ok());
604+
let minor = version_parts.next().and_then(|part| {
605+
part.split('-')
596606
.next()
597-
.and_then(|part| part.parse::<u32>().ok()),
598-
) {
607+
.and_then(|part| part.parse::<u32>().ok())
608+
});
609+
610+
if let (Some(major), Some(minor)) = (major, minor) {
611+
re_log::debug_once!("Parsed FFmpeg version as {}.{}", major, minor);
612+
599613
if major < FFMPEG_MINIMUM_VERSION_MAJOR
600614
|| (major == FFMPEG_MINIMUM_VERSION_MAJOR
601615
&& minor < FFMPEG_MINIMUM_VERSION_MINOR)
602616
{
603617
re_log::warn_once!(
604-
"FFmpeg version is {}. Only versions >= {FFMPEG_MINIMUM_VERSION_MAJOR}.{FFMPEG_MINIMUM_VERSION_MINOR} are officially supported",
605-
ffmpeg_version.version
618+
"FFmpeg version is {}. Only versions >= {FFMPEG_MINIMUM_VERSION_MAJOR}.{FFMPEG_MINIMUM_VERSION_MINOR} are officially supported.{}",
619+
ffmpeg_version.version, download_advice()
606620
);
607621
}
608622
} else {
609623
re_log::warn_once!(
610-
"Failed to parse FFmpeg version: {}",
611-
ffmpeg_version.raw_log_message
624+
"Failed to parse FFmpeg version: {}{}",
625+
ffmpeg_version.version,
626+
download_advice()
612627
);
613628
}
614629
}
@@ -791,12 +806,13 @@ fn write_avc_chunk_to_nalu_stream(
791806
return Err(Error::BadVideoData("Not enough bytes to".to_owned()));
792807
}
793808

794-
let nal_header = NalHeader(chunk.data[data_start]);
795-
re_log::trace!(
796-
"nal_header: {:?}, {}",
797-
nal_header.unit_type(),
798-
nal_header.ref_idc()
799-
);
809+
// Can be useful for finding issues, but naturally very spammy.
810+
// let nal_header = NalHeader(chunk.data[data_start]);
811+
// re_log::trace!(
812+
// "nal_header: {:?}, {}",
813+
// nal_header.unit_type(),
814+
// nal_header.ref_idc()
815+
// );
800816

801817
let data = &chunk.data[data_start..data_end];
802818

@@ -868,7 +884,7 @@ fn should_ignore_log_msg(msg: &str) -> bool {
868884
false
869885
}
870886

871-
/// Strips out buffer addresses from FFmpeg log messages so that we can use it with the log-once family of methods.
887+
/// Strips out buffer addresses from `FFmpeg` log messages so that we can use it with the log-once family of methods.
872888
fn sanitize_ffmpeg_log_message(msg: &str) -> String {
873889
// Make warn_once work on `[swscaler @ 0x148db8000]` style warnings even if the address is different every time.
874890
// In older versions of FFmpeg this may happen several times in the same message (happend in 5.1, did not happen in 7.1).
@@ -889,9 +905,8 @@ fn sanitize_ffmpeg_log_message(msg: &str) -> String {
889905
msg
890906
}
891907

908+
#[cfg(test)]
892909
mod tests {
893-
use super::sanitize_ffmpeg_log_message;
894-
895910
#[test]
896911
fn test_sanitize_ffmpeg_log_message() {
897912
assert_eq!(

0 commit comments

Comments
 (0)