From 4e880413ae3f2280f2ba2456b50eb42ea97061f7 Mon Sep 17 00:00:00 2001
From: Miguel Palhas <mpalhas@gmail.com>
Date: Sat, 22 Aug 2026 20:39:25 +0100
Subject: [PATCH] fix(probe): do not hide prober failures or cover art

Review on #59. select_feature skipped every probe error, so a missing
ffprobe or a timeout reported "no video file" for a torrent that has
one; only per-file content failures are skipped now. And an audio file
with embedded artwork reports the cover as a video stream, which made a
soundtrack look importable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 crates/arr-probe/src/error.rs                 |  21 +++++++++++++++
 crates/arr-probe/src/ffprobe.rs               |  16 ++++++++++++
 crates/arr-probe/src/lib.rs                   |  11 +++++---
 crates/arr-probe/src/model.rs                 |   2 +-
 .../tests/fixtures/audio-with-cover.m4a       | Bin 0 -> 5207 bytes
 crates/arr-probe/tests/probe.rs               |  24 ++++++++++++++++++
 scripts/make-probe-fixtures.sh                |   9 +++++++
 7 files changed, 78 insertions(+), 5 deletions(-)
 create mode 100644 crates/arr-probe/tests/fixtures/audio-with-cover.m4a

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<String>,
     pub duration: Option<String>,
     #[serde(default)]
+    pub disposition: Disposition,
+    #[serde(default)]
     pub tags: Tags,
     #[serde(default)]
     pub side_data_list: Vec<SideData>,
@@ -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<Item = impl Into<PathBuf>>,
@@ -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 0000000000000000000000000000000000000000..8c032801d0ba003547634e753f056a9cca105d6d
GIT binary patch
literal 5207
zcmaht2|Scr`|sFO$w(h{b!ky3N+OaaTcJf<bjL_BltJSfOW8*KaV@PHskkYHvQ1%#
zVoIXr)?A87x~5Ca$5dmR2s7{hoOjHv?)UwE-}7@k=Y7ueKIfcgpBF(8t)R%G;X5~O
z)j<#$1ma*SB@B!W2!bdDMFa*Sh|1G2(t*gg$c_Wi0cIvf#zw~L*CR4lvKP7VI5xf^
znZ9pxj9Om~jyFyYm6Ye}7wMJUTpV<vp!Bp>)lQ8cw<P`?F^n_Mc=PzF?pk`~9yb3Y
zN1Qz^Z2dek^QCWU8nr$$UtDt{03SK$ylT~`Lhn=aleA?O3lA8fN}j1o2CH^!Y#B+=
z?C)KYq4y4FJCsrJh5JD?IaQl8->afbmqRW3&|*@LJQ-G?g>MLwwYj%q)qa!dQ#fUV
z)Dp%^w0jcepuy$7Z^rgD>R(;FcfoP9Bkv8vvb&W+Pehw7_wjGLb>4q#eiGy_OJIr1
z(P@@OOzYy!f5wdLRASF0;_q$yi<L0w`Y=HdJw>S>FZ>JVdOFHubQrvmUXbeDjebm|
z3DGy?Dd&V(;lzW{4Bfnzw1PB8b<Z^lH{44*b=SGy!zsbj<fCOMM_k`P_KnIF@NXMZ
zpRG0KFo-;TX5psxOtqxfhWByvfY9TOT-22%qJ6&G@%9|m{Py^iTk_eEPD-Ix--};M
z^dm0@K3N#?4ku%f-Wzmq5FNh@t{B2u=w0?~Mr<*+AXzITL+?t<dhcCxM22dQvyy6o
z)YQUwROBsYjm4DSE%#`x{*lXTq_-`dXy8!xzjT`1DkHyW&3){c?3ZY-$OhcVPvcd6
z4C}-pr^K1?lcPB0p^ngzQ#b70m6?7DN}ldH`uM|`mf87eDGlZS-YbkD6tjC;9}@fZ
z2RK%-?U(CZyy)qx(slAN?K0S?UpML}Dq>`IteW5{j@CTwPd3{Y)bhA>UO)<|xk_zr
zI8FhW0SW{Mi0izw+oFMY%OhcMhiZeMVbVRPE-R-&nYc0g9n@1(1kh7JtC;m=AH&0Q
zA#bQvo9=3H5^vvZzsC_z^&}WCr>fT?Q$VA?M$OPfM(7vxjWDLuykuhP^*L5ndq6~g
z^x5!7j|~D&deY^|-btADQ><y?avC4?-B<Cx{Ginm{Mn@D75fp{!GY$Y-ne}BZ}|?j
zPBo98?n>>>Qpv-~8KhSD6{FqL;1%Z3B+IH$Ow%;;2@F;WJ@q`(X`PX<;6%NR=6MYU
zv;uU3n>Hew+>Q|=Pd`>mihiv72$z*8ocPm@POO*Ls3!tlWh{5H*^N^AfU*ilT1q`A
z+M3HVbg#6eWi0PKK9F|o4@YCy?0H0JuJT5jxC|AD4+~>r>)H8tTs-KvT8X^!4OFjK
zUc}=pVh*-#B26q{p*CN9dzfky+|C92o@djSRgt3298)cIL)LD%FLqadhls&lxW(uz
zu9!8wuR?HgsES2aFXV5xA8zX&Cf=ji8LxJwV-nBWF@c(jP$BwMXrATC<q2-hGk^Op
zLsGgdg-Y!x-Z`jDOzCygzYq0O23+dXG|ETwI+>e2y%~*<qP*HqMA>`YAgAn|Y`URS
zcEvdY-aEC7K?A0rMspj;?S7V{+ViROD*JtP{L6LYS=5jm#xd6{wPMWa{toTZMh%nG
zZ2s%6s=zQ9)5~ifx41h6#yYq(-+|s#&S9e+`QrTNLxOaoHYG!wjRo^W&%~a%HkVq4
zo;s0tABaqEiUzU#DIOfNx|3<dyvWKp<m8BipFCr`fWBw#z(z-PyX9ML=DR(>xn|_#
zVE1Cwq8)4zO%QuGk6dMy@Nd_B_bMMQenH)Ssm^nkLxD4u0+gRo0wQlG5S!h@RMF@2
z0|v)Clg}oY=Kh;)9d*A&?o+6d{PiS4r{-me%x+Y01O{{id7y~TI`OZXr|qg;m6mvi
z>faU`y3+Sz(F|q&j54N&$Waj`_u}0?3%D+aPKhQOr>i@ajs4xqTrxRH&_Bx9Y_xU+
zWr$yTSLl+9*?sa+<eDR`Kbda5Irm|k=b~h#rCwOtNiD&$-;_u|XwdNH_sKC|xaUQ&
z_cg@wEJB>y7O#HOpFD;oz2oQqPI@84jr9!Lm8m^;YK~iP6lCc69BeW=Pe13dHvApD
zO*zX3^+Xw>!@>ck$LSwgOHWQuv|m`(IMKs(<&wwibLZG{Ut@8X0k_YgO`#uSVjk_Q
zD;K$aaa4N|;Mk&>fq$-cYWMF`r&ObG`}bOe;}CEwt~Bj6kUtIC*qm~tS$WX|N>pDR
zGdD3^=HjX5wg~2kRH!GI^<q07^G#TrwVx6D*Zu)=6K^;#-8pBSb5;`2Ck)|`adg@y
zALQIi9F>v~F3sl$!^&zbJ3T*3Fn-Ov?%pq(BXLnI-7U!Mjm(M3w+ZJqju-aD33`gA
z+{a609XSsEm38K`{=}1(A9C<f8l|OqdphMp8{T1$%ujpvQ_DN<AA1XOF7#25$h>HL
z@kDH|?@^w`=Ld#V`mGk~<(leZ!{DIgXYRe7vo->8JaHPFGN;^m-I`Xu;%HZ8B|K&8
zXr@0`&Py%b;YB<M<BSTwR`ewn#!PclkFm-3+A7Ifnb!(VcaRq{we8yVR?h9T)p>@C
z2TDpWNU=i2Opw3gI(kZWS@QTx@aK+c!kpJXnlIZD{H3mOtMX;tDD@tFYPo_KGc0Ei
zov?;=Pe3GBm~gnjt*WmrbxSQ+wB{zE$<g0b;~9)95YaH<&CnjbFl`FCC>T+M)!`vX
zP~{aKBQ1$H4>%p&zi@?wH8Eez1?ha+wXx#Pvm=wQyTY=omsJ{P2Kei}(R*Fk9O|0%
zHKIl6^O03d(`>;+x1C9wmK<exH-#JPA-9Q}z%lA6^tk4-@*GG%F#fRQ2ju#W7!gm`
z_Wt?=55gIg2vt3|o5#}}M9w-moYxlUSLG}UdJ|OM(T<VA&MIh##9SBS{Qlf&Q{m1|
z=c?_6Pgi+-EfZM&6L?48ixXRDiDTb8YuH%Qwp}^e@TX?bpoLoc;vJGaA<2JZX!eL{
z^Q#!X;C5K<n(g+a0Ck_WAAjsNoR6=RgJ=CN0s3iV3aoF?tR@OKZcCUDG<0)_g^ors
zZfvYde+_Fwp3zX8D3sh$(g9QI^(Cc>n}__=4LfypBqCwSDaA?-7yrnbi(>!-zzPB3
zp^63b%0d$u*1%G=k}6i+5ZM=hcqV?<xtxqcnb$0TCXqNbee*5?nZRM8N->9NG=EVM
z6ek+?NEq>uTbD5Xs(#$xOWbyO<vHNlm}`MDkx!dJ`(sL(4|xLn{?Xp53vZ7+Y+0nS
zn7L-Jue{rmR57M{0f@jJ*mnrLKZMi^-lSk2MYyU&cHN+!d_lRrA9g6fp35D9OwdV*
z1m3Zk^<QS9txp^4-?<#X-ZDI;cwViWQ|aswdhzi0*^kgWaI}1C>UI?=q(-%CY3_>3
zw#$!l4X+U7uKnD&1-rkb`x6@pZsb^Uxo2mazJ|t}f^mIi`W?_lNjts|;(U;(OcTtr
zJvh1AdIdfM8y)s26|D3uPS|dg?&vZMFI2(*hzk9t&`x1&);NzlF>K1uD@@(Y1Nygk
zZgH)N@Wl3dkq_ofy@A%V9ba8@M|jzbYU@kFt1Ah@wsFc{;}b#8hjvuHFW422j2omf
z@<cXRp5(<my0hBay2k9|$zY{8v(T=k@*hL8m~$M?U^20)12bRzPGZIFvGE^P=kZRw
zEWH%9W3|O!79I4oBpsFQ%G)p{fm-@O3}acinaSiQJ|GN%P$n?9mMk(zo=dN@g%`&H
zxfFB}&5h<+A)B~dX&_fbT&``3whFyo;9|P#9>yt+<DGtLlVz;@G%Fu7@3qS+EzOo!
zcO5+6y=u_X_Dt*~wl&bX#FFvh5rRJdT!G8}6hT}C_sE5-j><#>NlVrgWn+WJabWv0
z^r^FA=Jl6A;6!bpAnT-E9VocPFbz>VLwqL*;L~U^2;k2R`xrl2b5E$T-&J4JR-k;m
zXzooE5FRW!f6UY45p<)o1u5PPv=kV!M?h5L^!=aRhB`zpH2;BIa{tmYgF*Mg!-aEP
zs>^D3M%1_v=?;=+g0&m18w1sp6FX?Mx?6C(vx{K&!wSvvuuQYX0e{e4cYy5hZPx2y
zWVACr3In=fl>lY0=X3jb!$gzvd)w<qu2_LC2O_afuZS62{Tr{<Q0eg%6?~=QUo(73
zL>Jv4-%~BQb=CFqpQa%CU`@xgCJ<8yzie&+c@q4FkT}*rUi<i)U1Q;NXQ%gfB&L9U
zAMECe7~6cN6W7xWbJytE2R+Zl`^=k)bTlv4Q@c=%DGnrb;U;)9^WLS@7xAGU39#>*
zSvRR4*E3|fQJIJx80f7j3WT2|^AU0kyc5G^WliKWeRP_Sl__{eZa=$4=<84bJ3kqa
zPkEqBB%q-0{)Kw|l7K6G@9kCUV%#yGaK`RdNk(&tZ1$6IoUKd7%`x@}?PS!*y7Y+m
zcQtga((i^fyT|uwCRE7sG;O*MX`A1lCN3O%hjYj*n&S$7NGd0akBAy+K6U7BR!h>B
zuDyd}X_~nk{p=mbO~>3d5`C|(_+|g}-1<Xj9D;rq;53g_ha-r5a2SOW4Sv~%hD9GD
z0sIY=IUu;bX$TI1e_!Fd2SEPEv+Y;T|2C6B5ZRQ-hyx*Dkr)|*?J4^WqeL3q_bcEq
z;y?EJAN#HhBt=qz2(7?SYUHda9CjZ>ZuZPFqA*hM0kEbMM*44eC#HEeV{aNBB85iG
z%0Q@;sKe4Z#2(<8wEe(gQfMILk+~5Td^iY9RH<R#30+B&(6vYrfp8rHywbd=h)^BL
zTm~G~KNQT#)JUq-7?6h$7QPW46asvf01i=w6JRC?W`R@^yf7l0l$4|k*yI4#uw4X6
z$Yuf@@_-_*rsUO+D1aFbm;%wEMq+A$Oz_zof$EX$0!^R=P7(kK0N~DW?3G|#Bv^|D
zb16W&6&TyWFaTpc81SRXS}^p%STj5H05%2#Vn~M=@<TooFd*M*Fd%K!EQUOA9n#=C
zQViuoUc*`Z?V5Byxc2Qna1PfXh6Ap{eWi4`7d$)Uk)8|AAzg~4cY^D1KsuyJ&kLX7
zfHL9ww{$p{u5Sc$sa$vlsXU0KdqBJi40$lrsgVH`fFVFzz_lgRgW)9j5J3qI1qj*^
zhoa!W7S;bC2=c#IzO`O>6e;omU_2BS2r&lBzIAjE91#b?!=Xee;4FhB(X{erBn2Kr
z3w#EHFGIobYgo|VU&9dGf&fZ1hzOkGEYEz%gT6-JA*wD;&Q1vQKM;|CCPo%~_azPO
zM(hzKISBF?D0~Zwii%1~O5neO0ya~cGiMGU0Hmy}Ja69Ixhg6uc)U8=jO;`tnFDS-
z3ko;@(gz7vP*nW~1wrutK|${$cqPOZv6aOwM`ZAbEFMAMAoFB#-~r5SkWAbv?)~p2
K&pzSMuKx#$WQoxL

literal 0
HcmV?d00001

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"