diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d7041b..df67b66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -877,12 +877,13 @@ consumers are the in-tree toolchain crates. SD-DVD.** rc.5.1 added a 20 ms `DefaultDecodedFieldDuration` field element to the 576i/480i track header on the theory that Windows derives fps from it. Captured evidence showed that element made Windows Explorer report 12.5 fps - (half) and MediaInfo flip the track to "Frame rate mode: Variable", while - MakeMKV's rip of the same disc omits it. The element is therefore no longer - written (`MkvTrack::video` now passes `field_duration_ns == 0`); the track - keeps `FlagInterlaced=1` + `FieldOrder=TFF` and the full-frame 40 ms - `DefaultDuration` (`1/DefaultDuration` = 25 fps), matching MakeMKV. How a given - player or shell handler chooses to display interlaced fps is not guaranteed. + (half) and MediaInfo flip the track to "Frame rate mode: Variable". The + element is optional in RFC 9559 and nothing requires it for interlaced SD, so + it is no longer written (`MkvTrack::video` now passes `field_duration_ns == 0`); + the track keeps `FlagInterlaced=1` + `FieldOrder=TFF` and the full-frame 40 ms + `DefaultDuration` (`1/DefaultDuration` = 25 fps), which is the frame rate the + source actually carries. How a given player or shell handler chooses to display + interlaced fps is not guaranteed. - **Correct AC-3 audio track selected on DVDs with non-standard sub-stream ordering.** freemkv assigned each declared audio stream a physical sub-stream by ordinal (`0x80+n`), assuming the IFO's first stream lives at `0x80`. On diff --git a/src/clpi.rs b/src/clpi.rs index a5c3235..cc4630a 100644 --- a/src/clpi.rs +++ b/src/clpi.rs @@ -236,8 +236,8 @@ pub fn parse(data: &[u8]) -> Result { } /// Parse the ProgramInfo section: per-stream (pid, coding_type, -/// language, codec sub-fields). Layout per the BD CLPI spec -/// clpi_parse.c: +/// language, codec sub-fields). Layout per the Blu-ray Disc Read-Only Format +/// Part 3 CLIPINF (CLPI) specification: /// /// ```text /// ProgramInfo: diff --git a/src/session.rs b/src/session.rs index 0f86f44..94d0d06 100644 --- a/src/session.rs +++ b/src/session.rs @@ -244,7 +244,15 @@ impl DiscSession { /// set), runs [`Disc::scan`], stores the result, and returns a borrow. pub fn scan(&mut self, opts: ScanOptions) -> Result<&Disc> { let opts = forward_key_material(&mut self.spec, opts); - let disc = Disc::scan(self.drive.as_mut().expect("drive present for scan"), &opts)?; + // `stage_drive_as_reader` is PUBLIC and moves the drive into the reader + // slot, so this slot can legitimately be empty when a caller reaches + // here. A library must not panic from public API, and "no shipped + // consumer calls it in that order" is not the same as "cannot happen" — + // the public surface permits it, so it must be an error. + let drive = self.drive.as_mut().ok_or_else(|| Error::DeviceNotReady { + path: self.device.clone(), + })?; + let disc = Disc::scan(drive, &opts)?; self.disc = Some(disc); Ok(self.disc.as_ref().expect("disc just stored")) } @@ -272,12 +280,13 @@ impl DiscSession { let disc = self.disc.as_mut().expect("disc present (checked above)"); resolve_keys_for(reader.as_mut(), disc, sources) } else { + // Same reachability as `scan` above: the drive may have been staged + // into the reader slot by the public `stage_drive_as_reader`. + let drive = self.drive.as_mut().ok_or_else(|| Error::DeviceNotReady { + path: self.device.clone(), + })?; let disc = self.disc.as_mut().expect("disc present (checked above)"); - resolve_keys_for( - self.drive.as_mut().expect("drive present for key sampling"), - disc, - sources, - ) + resolve_keys_for(drive, disc, sources) }; self.key_fetch = resolved.key_fetch; Ok(resolved.trace)