diff --git a/crates/arr-probe/src/error.rs b/crates/arr-probe/src/error.rs index 306a847..d1bc4c8 100644 --- a/crates/arr-probe/src/error.rs +++ b/crates/arr-probe/src/error.rs @@ -95,3 +95,24 @@ pub enum Error { #[error("no video file among the candidates")] NoCandidates, } + +impl Error { + /// Whether this failure is a fact about one file rather than about the + /// prober. + /// + /// Feature selection walks a torrent full of `.nfo`, `.jpg` and `.txt` + /// files, so a file that is not importable media is expected and gets + /// skipped. A missing binary, a timeout or unparseable output is not about + /// the file at all, and swallowing it would report "no video file" for a + /// torrent that has one. + #[must_use] + pub const fn is_about_the_file(&self) -> bool { + matches!( + self, + Self::Rejected { .. } + | Self::NotVideo { .. } + | Self::NoVideoStream { .. } + | Self::NoDuration { .. } + ) + } +} diff --git a/crates/arr-probe/src/ffprobe.rs b/crates/arr-probe/src/ffprobe.rs index b8c80d9..8de0b46 100644 --- a/crates/arr-probe/src/ffprobe.rs +++ b/crates/arr-probe/src/ffprobe.rs @@ -35,6 +35,8 @@ pub(crate) struct Stream { pub color_transfer: Option, pub duration: Option, #[serde(default)] + pub disposition: Disposition, + #[serde(default)] pub tags: Tags, #[serde(default)] pub side_data_list: Vec, @@ -44,6 +46,20 @@ impl Stream { pub fn is_kind(&self, kind: &str) -> bool { self.codec_type.as_deref() == Some(kind) } + + /// A real video stream, not the cover art bolted onto an audio file. + pub fn is_video(&self) -> bool { + self.is_kind("video") && self.disposition.attached_pic == 0 + } +} + +/// Stream flags. Only `attached_pic` is read: an MP3 or FLAC with embedded +/// artwork reports it as a video stream, and taking that for video would make +/// an audio file look importable. +#[derive(Debug, Default, Deserialize)] +pub(crate) struct Disposition { + #[serde(default)] + pub attached_pic: u8, } /// Stream metadata. Matroska writes `HANDLER_NAME`, MP4 writes `handler_name`, diff --git a/crates/arr-probe/src/lib.rs b/crates/arr-probe/src/lib.rs index 3b1aac3..42cd143 100644 --- a/crates/arr-probe/src/lib.rs +++ b/crates/arr-probe/src/lib.rs @@ -123,13 +123,15 @@ impl Prober { /// marked [`RuntimeMatch::Mismatched`]; deciding what to do about that is /// the import's call, not this crate's. /// - /// Paths `ffprobe` cannot read are skipped rather than failing the - /// selection — a torrent is full of `.nfo` and `.jpg` files. + /// Paths that are not importable media are skipped rather than failing the + /// selection — a torrent is full of `.nfo` and `.jpg` files. A failure of + /// the prober itself is not skipped: see [`Error::is_about_the_file`]. /// /// # Errors /// /// [`Error::NoCandidates`] when no path in the set is a readable video - /// file. + /// file, and anything [`Prober::probe`] raises that is about the prober + /// rather than about one file. pub async fn select_feature( &self, paths: impl IntoIterator>, @@ -140,9 +142,10 @@ impl Prober { let path = path.into(); match self.probe(path.clone()).await { Ok(file) => candidates.push(file), - Err(error) => { + Err(error) if error.is_about_the_file() => { tracing::debug!(path = %path.display(), %error, "not a video file, skipping"); } + Err(error) => return Err(error), } } diff --git a/crates/arr-probe/src/model.rs b/crates/arr-probe/src/model.rs index ba237c8..ad6097b 100644 --- a/crates/arr-probe/src/model.rs +++ b/crates/arr-probe/src/model.rs @@ -50,7 +50,7 @@ impl ProbedFile { let video = output .streams .iter() - .find(|stream| stream.is_kind("video")) + .find(|stream| stream.is_video()) .ok_or_else(|| Error::NoVideoStream { path: path.clone() })?; let duration = duration(output).ok_or_else(|| Error::NoDuration { path: path.clone() })?; diff --git a/crates/arr-probe/tests/fixtures/audio-with-cover.m4a b/crates/arr-probe/tests/fixtures/audio-with-cover.m4a new file mode 100644 index 0000000..8c03280 Binary files /dev/null and b/crates/arr-probe/tests/fixtures/audio-with-cover.m4a differ diff --git a/crates/arr-probe/tests/probe.rs b/crates/arr-probe/tests/probe.rs index b31caae..3d09732 100644 --- a/crates/arr-probe/tests/probe.rs +++ b/crates/arr-probe/tests/probe.rs @@ -128,6 +128,17 @@ async fn a_text_file_is_not_media() { assert!(matches!(error, Error::NotVideo { .. }), "{error:?}"); } +/// Cover art is a video stream as far as ffprobe is concerned. +#[tokio::test] +async fn an_audio_file_with_artwork_is_not_video() { + let error = Prober::new() + .probe(fixture("audio-with-cover.m4a")) + .await + .expect_err("an attached picture is not a video stream"); + + assert!(matches!(error, Error::NoVideoStream { .. }), "{error:?}"); +} + #[tokio::test] async fn a_missing_file_is_an_error() { let error = Prober::new() @@ -208,6 +219,19 @@ async fn nothing_within_tolerance_still_returns_the_largest_file() { ); } +/// A prober that cannot run is not the same fact as a torrent without video, +/// and must not be reported as one. +#[tokio::test] +async fn a_broken_prober_fails_the_selection() { + let error = Prober::new() + .with_binary("ffprobe-that-does-not-exist") + .select_feature([fixture("movie-1080p.mkv")], None) + .await + .expect_err("nothing could be probed"); + + assert!(matches!(error, Error::Spawn { .. }), "{error:?}"); +} + #[tokio::test] async fn a_torrent_with_no_video_file_selects_nothing() { let error = Prober::new() diff --git a/scripts/make-probe-fixtures.sh b/scripts/make-probe-fixtures.sh index 3d29f88..08af9e3 100755 --- a/scripts/make-probe-fixtures.sh +++ b/scripts/make-probe-fixtures.sh @@ -144,6 +144,15 @@ PY python3 "$work/dovi.py" "$work/base.mp4" "$out/dv-profile5.mp4" 5 0 python3 "$work/dovi.py" "$work/base.mp4" "$out/dv-profile8-1.mp4" 8 1 +# An audio file with embedded artwork. ffprobe reports the cover as a video +# stream, so this is what stops a soundtrack passing for a feature. +ffmpeg -hide_banner -loglevel error -y \ + -f lavfi -i "color=c=red:s=64x64:d=1" -frames:v 1 "$work/cover.jpg" +ffmpeg -hide_banner -loglevel error -y \ + -f lavfi -i "sine=frequency=440:duration=2" -i "$work/cover.jpg" \ + -map 0:a -map 1:v -c:a aac -b:a 8k -c:v mjpeg -disposition:v attached_pic \ + "$out/audio-with-cover.m4a" + # Not media at all: the thing a multi-file torrent is full of. printf 'Release notes. Not a video file.\n' > "$out/notes.nfo"