mirror of
https://github.com/freemkv/libfreemkv.git
synced 2026-10-07 18:13:10 +00:00
fix(disc): SAFETY — spare seamless-branch feature from wrapper-composite demotion (#45)
The fallback composite gate in rank_titles flagged title P as a "play-all wrapper" whenever some other title Q's clip set was a proper subset of P's, Q had video, and Q.size >= 0.5 * P.size. main_feature_order then ranks standalone-before-composite, demoting P below the decoy Q. On a seamless-branch feature this inverts the pick: the real 00800 (2h03, 4 clips = body + intro/credits/seamless segments) is a SUPERSET of the single-clip body decoy 00700 (1h50), and 700 >= 50% of 800's size, so 800 was mis-flagged composite and lost to the shorter 700 — the wrong title gets ripped. Add a duration-first wrapper-shape gate: P is only a composite when the subset Q is EITHER a much shorter cut (Q.duration < 0.85 * P.duration, a concat/play-all padding Q's runtime) OR nearly size-equal (Q.size >= 0.90 * P.size, a bumper/outro wrapper). A near-full-length but markedly smaller body subset (Q runs ~0.89 of P's duration yet holds far fewer bytes, because P's branch segments add bytes not runtime) is a real seamless branch, so P is kept. Uses per-title duration_secs/size_bytes, stays capacity-independent and feature-gate-safe. Preserves the real play-all avoidance: the 00245 bumper-wrapper decoy (Q ~0.96 of both duration and size) stays demoted via the size limb, and a concat play-all (subset < half the runtime) via the duration limb. Add regression tests: seamless_branch_feature_not_demoted_issue_45 and true_play_all_concat_wrapper_still_demoted. All existing selection tests pass.
This commit is contained in:
+34
-4
@@ -217,10 +217,40 @@ authoring an entire synthetic disc image. Running it against an HD-DVD/DVD
|
||||
|
||||
This catches both the wrapper decoy (`00245 ⊋ 00001`) and the oversize
|
||||
play-all — independent of capacity, which the `fits-disc` gate needs and
|
||||
which is `0`/unknown on drives that fail READ CAPACITY. The corpus
|
||||
title-selection gate is the backstop across the disc hoard for the known
|
||||
residual (a real feature with a separate "resume from the middle" branch
|
||||
playlist reusing most of its clips).
|
||||
which is `0`/unknown on drives that fail READ CAPACITY.
|
||||
|
||||
### Wrapper-shape gate (issue #45)
|
||||
|
||||
A proper-subset relation with `Q.size ≥ 0.5·P.size` is NOT by itself proof
|
||||
that `P` is a wrapper. A genuine **seamless-branch feature** `P` (body clip
|
||||
plus intro/credits/alternate-branch segments) is a *superset* of its
|
||||
single-clip body decoy `Q`: `Q` runs almost as long as `P` (e.g. 1h50 vs
|
||||
2h03, ~0.89 of the duration) yet is markedly smaller, because `P`'s branch
|
||||
segments add bytes without adding much played runtime. The old size-only
|
||||
gate mis-flagged such a `P` as a "play-all wrapper" and demoted the real
|
||||
2h03 feature below the shorter 1h50 body — a SAFETY bug (wrong title ripped).
|
||||
|
||||
So the composite flag additionally requires `P` to *have* a wrapper shape,
|
||||
via one of two capacity-independent, per-title (`duration_secs`, `size_bytes`)
|
||||
signals on the subset `Q`:
|
||||
|
||||
- **concat / play-all:** `Q.duration < 0.85·P.duration` — `Q` is a much
|
||||
shorter cut and `P` pads its runtime with other segments (the true
|
||||
play-all runs ~the SUM of its parts, so each part is well under half).
|
||||
- **bumper / outro wrapper:** `Q.size ≥ 0.90·P.size` — `Q` already holds
|
||||
almost all of `P`'s bytes and `P` merely prepends/appends a short
|
||||
non-feature lead/tail (the `00245 ⊋ 00001` decoy: `Q` is ~0.96 of both the
|
||||
duration and the size, so the size limb keeps it demoted).
|
||||
|
||||
Otherwise — `Q` runs almost as long as `P` (≥0.85) yet is well under 0.90 of
|
||||
its size — `P` is the fuller branch set and is kept. `0.85` sits above a real
|
||||
concat's per-part ratio (~0.5) and below a seamless body's (~0.89); `0.90`
|
||||
sits above a bumper wrapper's near-1.0 size share and below a branch body's.
|
||||
|
||||
**Known residual:** a wrapper that pads runtime only modestly AND withholds
|
||||
most of its bytes from the body clip is ambiguous by shape alone. The corpus
|
||||
title-selection gate is the backstop across the disc hoard, and
|
||||
higher-precedence nav/authoring signals override the composite key.
|
||||
|
||||
## aligned_unit_keys_validate
|
||||
|
||||
|
||||
+81
-5
@@ -2142,11 +2142,11 @@ impl Disc {
|
||||
/// Precompute the [`TitleRank`] for every title, in the same index
|
||||
/// order. Composite detection: title `P` is a composite when some OTHER
|
||||
/// title `Q` has a non-empty clip-id set that is a PROPER SUBSET of `P`'s,
|
||||
/// `Q` has plausible video, and `Q` accounts for at least half of `P`'s
|
||||
/// declared size — independent of capacity, which may be `0`/unknown.
|
||||
/// Known residual: a "resume from the middle" branch playlist could be
|
||||
/// mis-flagged; rare, and higher-precedence nav/authoring signals
|
||||
/// override it. See docs/disc-mod.md for detail.
|
||||
/// `Q` has plausible video, `Q` accounts for at least half of `P`'s
|
||||
/// declared size, AND `P` has a WRAPPER shape (issue #45): either `Q` is a
|
||||
/// much shorter cut or `Q` is nearly `P`'s size-equal. Capacity-independent
|
||||
/// (`capacity` may be `0`/unknown). See docs/disc-mod.md for the shape
|
||||
/// gate, its thresholds, and the known residual.
|
||||
pub fn rank_titles(
|
||||
titles: &[DiscTitle],
|
||||
hint: Option<&crate::labels::FeaturePlaylistHint>,
|
||||
@@ -2200,6 +2200,11 @@ impl Disc {
|
||||
&& qj.len() < pi.len()
|
||||
&& q.has_probable_video()
|
||||
&& q.size_bytes as f64 >= 0.5 * p.size_bytes as f64
|
||||
// Seamless-branch guard (issue #45): wrapper only
|
||||
// if Q is a much SHORTER cut (concat) or nearly P's
|
||||
// SIZE-equal (bumper). See docs/disc-mod.md.
|
||||
&& (q.duration_secs < 0.85 * p.duration_secs
|
||||
|| q.size_bytes as f64 >= 0.90 * p.size_bytes as f64)
|
||||
&& qj.is_subset(pi)
|
||||
}
|
||||
});
|
||||
@@ -4552,6 +4557,77 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Issue #45 (SAFETY): a genuine seamless-branch FEATURE (00800, 4 clips,
|
||||
/// 2h03) is a SUPERSET of its single-clip body decoy (00700, 1h50, ≥50 %
|
||||
/// size). The old size-only composite gate mis-demoted 00800 as a "play-all
|
||||
/// wrapper" and ripped the shorter decoy. The duration-shape guard keeps
|
||||
/// 00800: its body subset runs almost as long (1h50 vs 2h03, ~0.89) yet is
|
||||
/// markedly smaller (0.55) — the hallmark of seamless branching, not
|
||||
/// concatenation.
|
||||
#[test]
|
||||
fn seamless_branch_feature_not_demoted_issue_45() {
|
||||
// 00800: real feature = body + intro/credits/seamless branch segments.
|
||||
let feature = bd_title(
|
||||
"00800.mpls",
|
||||
800,
|
||||
7380.0, // 2h03
|
||||
60_000_000_000,
|
||||
&["00800", "00801", "00802", "00803"],
|
||||
);
|
||||
// 00700: single body clip, a PROPER subset, 1h50 (~89 % of 00800's
|
||||
// duration) and 55 % of its size — enough to trip the old size gate.
|
||||
let decoy = bd_title("00700.mpls", 700, 6600.0, 33_000_000_000, &["00800"]);
|
||||
let capacity = 88_000_000_000u64;
|
||||
|
||||
// The body decoy is a proper subset with ≥50 % size — the OLD gate
|
||||
// flagged 00800 composite on that alone. It must no longer be flagged.
|
||||
let ranks = Disc::rank_titles(&[feature.clone(), decoy.clone()], None, None);
|
||||
assert!(
|
||||
!ranks[0].composite,
|
||||
"the seamless-branch feature must not be flagged a wrapper composite"
|
||||
);
|
||||
assert!(!ranks[1].composite, "the shorter body subset is standalone");
|
||||
|
||||
let mut titles = vec![decoy, feature];
|
||||
Disc::sort_titles_by_main_feature(&mut titles, capacity, None, None);
|
||||
assert_eq!(
|
||||
titles[0].playlist_id, 800,
|
||||
"the full seamless-branch feature is selected, not the shorter body decoy"
|
||||
);
|
||||
}
|
||||
|
||||
/// The seamless-branch guard must NOT weaken the real play-all case: a
|
||||
/// concat wrapper (00099, four distinct parts, ~4h13 = their SUM) whose body
|
||||
/// subset is much SHORTER (00800, one part, ~2h02, <half the runtime) is
|
||||
/// still flagged composite and demoted below that standalone part.
|
||||
#[test]
|
||||
fn true_play_all_concat_wrapper_still_demoted() {
|
||||
let playall = bd_title(
|
||||
"00099.mpls",
|
||||
99,
|
||||
15_180.0, // ~4h13 — the SUM of its four parts
|
||||
92_400_000_000,
|
||||
&["00800", "00801", "00802", "00803"],
|
||||
);
|
||||
// The real feature = one part, ~2h02, less than half the wrapper's runtime.
|
||||
let feature = bd_title("00800.mpls", 800, 7320.0, 57_200_000_000, &["00800"]);
|
||||
let capacity = 100_000_000_000u64;
|
||||
|
||||
let ranks = Disc::rank_titles(&[playall.clone(), feature.clone()], None, None);
|
||||
assert!(
|
||||
ranks[0].composite,
|
||||
"a concat play-all whose subset is much shorter is still a composite"
|
||||
);
|
||||
assert!(!ranks[1].composite, "the standalone feature part is not");
|
||||
|
||||
let mut titles = vec![playall, feature];
|
||||
Disc::sort_titles_by_main_feature(&mut titles, capacity, None, None);
|
||||
assert_eq!(
|
||||
titles[0].playlist_id, 800,
|
||||
"the standalone feature wins; the concat play-all is demoted"
|
||||
);
|
||||
}
|
||||
|
||||
/// The disc's own authoring-designated feature wins even when it is NOT the
|
||||
/// longest/largest title — a size-independent signal a decoy cannot forge.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user