Make six silent mux failures observable
All six confirmed against the code. The governing rule this cluster serves: a
lossy or degraded outcome is never silent, because a corrupt rip the user does
not know about is the worst failure available.
**A 3D MKV re-mux silently lost one eye.** The BlockGroup read path had arms for
BLOCK / BLOCK_DURATION / REFERENCE_BLOCK only, so BLOCK_ADDITIONS fell into the
skip arm — while the writer does emit BlockAdditions > BlockMore > BlockAdditional
for the MVC dependent view. Reconstruction was judged out of scope and the
reasoning is recorded: PesFrame has no side-payload field and the header parser
never reads BlockAdditionMapping, so there is no dependent-view track to route the
AU to. Instead the loss is now LOUD — counted in bytes and events, warned once,
and surfaced through MkvStream's errors()/lost_bytes(), which the driver already
samples into MuxOutcome. One detail in the finding was wrong and is corrected: the
re-mux does NOT still advertise the mvcC mapping, because the header parser
ignores that element, so the output is a plain 2D H.264 track.
**An all-titles rip silently skipped real titles.** The header-buffer-cap
overflow returned Error::MkvInvalid, and is_skippable_title_stub matches exactly
E_MKV_INVALID | E_CSS_KEY_MISSING — verified here — so a 512 MiB-of-frames title
was classified as an empty nav/menu PGC stub and dropped. It now has its own
E9051 / MuxHeaderBufferExceeded { bytes }, outside the skippable set.
**The public pre-mux report contradicted the file.** Mp4Sink::finish() drops an
audio track it cannot describe, which I chose last round over failing an export
whose video is fine — but mp4_fit_report still listed that stream as included, so
the application's plan and the actual output disagreed. Fixed at both levels:
Mp4SkipReason is now non_exhaustive with NoSamples and UndescribableAudio,
Mp4Sink::final_report() describes the FILE rather than the plan, and for the
boxed dyn Stream path a defaulted Stream::undelivered_streams() carries the
information out to MuxOutcome::undelivered_streams with a driver-side warn.
**MP4 track ids could collide.** ids were assigned before the retain that drops
sample-less tracks, while next_id came from the post-retain count, so [1,3]
yielded next_id 3. Now max(track_id) + 1, saturating.
**Stream selection silently skipped its codec_privates prune** when the lists were
not the same length — but codec_privates is consumed POSITIONALLY and trailing
extras are documented as benign, so the length-equality guard was itself the bug.
The prune now runs unconditionally by index.
**The m2ts_mux scaffolding armed params_written on both the absent and the
unparseable codec_private arms** — the same defect already fixed in tsmux.rs.
Split into params_attempted (a latch, since retrying identical bytes cannot help)
and params_emitted, with a warn on each failure arm and an accessor so the
eventual wiring and its test can observe it.
Each fix verified red by mutating back to the prior behaviour: errors() 0 vs 1,
E6008 vs E9051, final_report [0,1] vs [0], next_track_id 3 vs [1,3], and the
selection prune resolving index 1 to the wrong track's record.
API surface deliberately widened: MuxOutcome gains a public field and Mp4Sink
becomes public. Nothing in-repo breaks. Note a behaviour change on the mkv://
input path — a 3D re-mux now reports non-zero loss, so a consumer treating
errors > 0 as disc damage will trip on it. That is intended: the outcome IS
degraded.
This commit is contained in:
+73
-10
@@ -51,7 +51,9 @@ impl StreamSelection {
|
||||
/// PID passes the corresponding [`PidFilter`]; drop the rest. Declared order
|
||||
/// is preserved. The parallel `codec_privates` vec is pruned in lockstep
|
||||
/// when it is populated (it is empty on a freshly-scanned title, non-empty
|
||||
/// only if a caller pre-filled it).
|
||||
/// only if a caller pre-filled it) — by index, whatever its length, since it
|
||||
/// is consumed positionally and a partial prune would attach the wrong
|
||||
/// codec-private to a retained track.
|
||||
///
|
||||
/// Errors [`Error::SelectionPidUnknown`] if a filter lists a PID that does
|
||||
/// not exist in `title.streams` — a caller bug (e.g. a stale scan). Fail
|
||||
@@ -94,8 +96,6 @@ impl StreamSelection {
|
||||
}
|
||||
}
|
||||
|
||||
let codec_privates_aligned = title.codec_privates.len() == title.streams.len();
|
||||
|
||||
// Retain by index so we can prune the parallel codec_privates in lockstep.
|
||||
let keep: Vec<bool> = title
|
||||
.streams
|
||||
@@ -109,14 +109,37 @@ impl StreamSelection {
|
||||
i += 1;
|
||||
k
|
||||
});
|
||||
if codec_privates_aligned {
|
||||
let mut j = 0;
|
||||
title.codec_privates.retain(|_| {
|
||||
let k = keep[j];
|
||||
j += 1;
|
||||
k
|
||||
});
|
||||
// Prune `codec_privates` by the SAME index decision, whatever its length.
|
||||
//
|
||||
// This used to run only when `codec_privates.len() == streams.len()`, and
|
||||
// do nothing otherwise. `codec_privates` is consumed positionally
|
||||
// (`codec_privates[i]` describes `streams[i]` — see
|
||||
// `TsMuxer::set_codec_private` / `Mp4Sink::create`), and `m2ts.rs::create`
|
||||
// documents a longer-than-streams vec as benign ("ignore any trailing
|
||||
// entries that exceed the track count"). So a title carrying one trailing
|
||||
// extra entry skipped the prune entirely and every retained stream after
|
||||
// the first dropped one silently got the PREVIOUS stream's
|
||||
// codec_private — wrong SPS/PPS on the track, no error, no log.
|
||||
//
|
||||
// Indices at or past `keep`'s length can only be entries that already
|
||||
// exceeded the stream count, i.e. describe no stream; drop them rather
|
||||
// than leave them dangling behind the pruned list.
|
||||
let extra = title.codec_privates.len().saturating_sub(keep.len());
|
||||
if extra > 0 {
|
||||
tracing::debug!(
|
||||
target: "mux",
|
||||
codec_privates = title.codec_privates.len(),
|
||||
streams = keep.len(),
|
||||
"stream selection: dropping {extra} codec_private entry/entries that describe \
|
||||
no declared stream"
|
||||
);
|
||||
}
|
||||
let mut j = 0;
|
||||
title.codec_privates.retain(|_| {
|
||||
let k = keep.get(j).copied().unwrap_or(false);
|
||||
j += 1;
|
||||
k
|
||||
});
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -307,6 +330,46 @@ mod tests {
|
||||
"codec_privates pruned to match the retained streams, in order"
|
||||
);
|
||||
}
|
||||
|
||||
/// A `codec_privates` vec that is NOT exactly stream-length must still be
|
||||
/// pruned in lockstep. `m2ts.rs::create` documents a longer-than-streams vec
|
||||
/// as a benign shape ("ignore any trailing entries that exceed the track
|
||||
/// count"), and the vec is consumed positionally, so skipping the prune left
|
||||
/// `codec_privates[i]` describing a stream that is no longer at index `i`.
|
||||
///
|
||||
/// Regression: with one trailing extra entry the prune was skipped entirely
|
||||
/// and index 1 — the retained `fra` audio — resolved to `eng`'s record, so
|
||||
/// the muxer attached the wrong codec-private to the track. No error, no log.
|
||||
#[test]
|
||||
fn apply_prunes_codec_privates_even_when_length_does_not_match_streams() {
|
||||
let mut t = title();
|
||||
t.streams.truncate(4); // video + eng + spa + fra audio
|
||||
t.codec_privates = vec![
|
||||
Some(vec![0xAA]), // video 0x1011
|
||||
Some(vec![0x11]), // audio 0x1100 eng
|
||||
Some(vec![0x22]), // audio 0x1101 spa
|
||||
Some(vec![0x33]), // audio 0x1102 fra
|
||||
Some(vec![0xEE]), // trailing extra — describes no declared stream
|
||||
];
|
||||
let sel = StreamSelection {
|
||||
audio: PidFilter::Only(vec![0x1102]),
|
||||
subtitle: PidFilter::All,
|
||||
};
|
||||
sel.apply(&mut t).unwrap();
|
||||
|
||||
assert_eq!(pids(&t), vec![0x1011, 0x1102], "video + fra audio");
|
||||
assert_eq!(
|
||||
t.codec_privates,
|
||||
vec![Some(vec![0xAA]), Some(vec![0x33])],
|
||||
"the retained fra track must keep ITS OWN codec_private, and the \
|
||||
trailing entry that describes no stream must not survive the prune"
|
||||
);
|
||||
assert_eq!(
|
||||
t.codec_privates.len(),
|
||||
t.streams.len(),
|
||||
"the two positional vecs must be aligned after apply()"
|
||||
);
|
||||
}
|
||||
/// A PID listed in the WRONG class's filter must fail loud, not validate and
|
||||
/// then quietly vanish. Validation used to scan both audio and subtitle
|
||||
/// streams, so an audio filter naming a subtitle PID passed — and `keeps`
|
||||
|
||||
Reference in New Issue
Block a user