Correct six stale comments and doc claims
All six describe code that does something different from what they say, which is the class of defect that gets a maintainer to write a bug on purpose. docs/clpi.md presented the CLPI stream-PID entry as byte-aligned 2/2/2/4/4-byte fields with a 32-bit fine-entry count. It is one 80-bit packed block — reserved(10) + EP_stream_type(4) + num_EP_coarse(16) + num_EP_fine(18) + EP_map_start_address(32) — and num_EP_fine is 18 bits. Anyone parsing to the doc's offsets would read garbage. Replaced with the real bit layout. docs/udf.md said read_directory()'s recursion cap is 3; MAX_DIR_DEPTH is 8. TROUBLESHOOTING.md called Pass 1 `recovery::copy`. The engine's `sweep` is documented as "Pass 1 of a multipass rip"; `copy` is the dispatch verb that chooses between sweep and patch. This inconsistency was mine, introduced in the 1.6.0 doc rewrite. docs/drive-access.md already said `sweep` and was right — a round-2 finding claimed the opposite on the grounds that `recovery::sweep` appears nowhere else in this crate, which it cannot, being in another crate. io/pipeline.rs cited `disc::patch` as WRITE_THROUGH_DEPTH's caller; that moved to freemkv-engine in 1.6.0 and no `patch` exists here. truehd.rs's doc on mlp_major_sync_crc_ok said the trailer is compared big-endian while the body compares u16::from_le_bytes — and a big-endian compare was the bug the function was fixed for, so the comment described the defect rather than the code. sector/decrypting.rs claimed the decorator owns "the only mutable state (its call-count cap and spent flag)". DecryptingSectorSource has no such fields and no KeyFetch field at all in this revision.
This commit is contained in:
+1
-1
@@ -141,7 +141,7 @@ If your machine has a free SATA port, use it.
|
|||||||
|
|
||||||
freemkv uses a three-layer recovery model. See [`docs/rip-recovery.md`](docs/rip-recovery.md) for full details.
|
freemkv uses a three-layer recovery model. See [`docs/rip-recovery.md`](docs/rip-recovery.md) for full details.
|
||||||
|
|
||||||
- **Pass 1 (`freemkv_engine::recovery::copy`):** Fast sweep with 64 KB reads. On failure, zero-fills the block and skips forward. Writes a ddrescue-format mapfile for later retry.
|
- **Pass 1 (`freemkv_engine::recovery::sweep`):** Fast sweep with 64 KB reads. On failure, zero-fills the block and skips forward. Writes a ddrescue-format mapfile for later retry.
|
||||||
- **Pass 2+ (`freemkv_engine::recovery::patch`):** Targeted re-reads of bad ranges with a long 60-second timeout per CDB. The drive firmware performs its own ECC and laser power retries within that window.
|
- **Pass 2+ (`freemkv_engine::recovery::patch`):** Targeted re-reads of bad ranges with a long 60-second timeout per CDB. The drive firmware performs its own ECC and laser power retries within that window.
|
||||||
- **In-stream (DiscStream):** Adaptive batch halving -- reduces request size on failure to isolate bad sectors within a larger block.
|
- **In-stream (DiscStream):** Adaptive batch halving -- reduces request size on failure to isolate bad sectors within a larger block.
|
||||||
|
|
||||||
|
|||||||
+15
-5
@@ -69,13 +69,23 @@ Each stream PID entry header (14 bytes):
|
|||||||
```
|
```
|
||||||
Offset Size Field
|
Offset Size Field
|
||||||
------ ---- -----
|
------ ---- -----
|
||||||
0 2 Stream PID
|
2 2 stream_PID (byte-aligned)
|
||||||
2 2 Reserved + EP stream type
|
4 10 Bit-packed block, 80 bits total (see below)
|
||||||
4 2 Number of coarse entries
|
|
||||||
6 4 Number of fine entries (note: 32-bit, can be large)
|
|
||||||
10 4 EP map start offset (relative to EP map start)
|
|
||||||
```
|
```
|
||||||
|
|
||||||
|
The stream PID entry is **not** byte-aligned past `stream_PID`. Bytes 4..14 are one
|
||||||
|
80-bit packed field, read as a `u64` plus a trailing `u16`:
|
||||||
|
|
||||||
|
Bits Width Field
|
||||||
|
---- ----- -----
|
||||||
|
0-9 10 reserved
|
||||||
|
10-13 4 EP_stream_type
|
||||||
|
14-29 16 num_EP_coarse
|
||||||
|
30-47 18 num_EP_fine
|
||||||
|
48-79 32 EP_map_start_address (relative to the EP map start)
|
||||||
|
|
||||||
|
Note `num_EP_fine` is **18 bits**, not 32. See `parse_cpi` in `src/clpi.rs`.
|
||||||
|
|
||||||
libfreemkv parses only the first stream (primary video), which is sufficient for sector-level seeking.
|
libfreemkv parses only the first stream (primary video), which is sufficient for sector-level seeking.
|
||||||
|
|
||||||
### Two-Level Index
|
### Two-Level Index
|
||||||
|
|||||||
+1
-1
@@ -114,7 +114,7 @@ The `read_filesystem()` function in `src/udf.rs` follows the pointer chain above
|
|||||||
2. Scans sectors 32-63 for the Partition Descriptor and Logical Volume Descriptor.
|
2. Scans sectors 32-63 for the Partition Descriptor and Logical Volume Descriptor.
|
||||||
3. If two partition maps exist and the second is Type 2, reads the metadata file ICB at partition_start to find metadata_start.
|
3. If two partition maps exist and the second is Type 2, reads the metadata file ICB at partition_start to find metadata_start.
|
||||||
4. Reads the FSD at metadata_start, extracts the root directory ICB LBA.
|
4. Reads the FSD at metadata_start, extracts the root directory ICB LBA.
|
||||||
5. Calls `read_directory()` recursively (max depth 3) to build the full file tree.
|
5. Calls `read_directory()` recursively (max depth `MAX_DIR_DEPTH` = 8) to build the full file tree.
|
||||||
|
|
||||||
Each directory read involves two sector reads: one for the ICB, then one or more for the directory data. File sizes are read from info_length in each file's ICB.
|
Each directory read involves two sector reads: one for the ICB, then one or more for the directory data. File sizes are read from info_length in each file's ICB.
|
||||||
|
|
||||||
|
|||||||
+2
-1
@@ -184,7 +184,8 @@ pub const WRITE_PIPELINE_DEPTH: usize = 16;
|
|||||||
/// Channel depth for write-through pipelines. Each `send` fully
|
/// Channel depth for write-through pipelines. Each `send` fully
|
||||||
/// drains before the next can enqueue. Use this when the producer
|
/// drains before the next can enqueue. Use this when the producer
|
||||||
/// must observe consumer side-effects (e.g. mapfile state) before
|
/// must observe consumer side-effects (e.g. mapfile state) before
|
||||||
/// emitting the next item. Currently used by `disc::patch`.
|
/// emitting the next item. Used by `freemkv_engine::recovery::patch` — the
|
||||||
|
/// recovery strategy moved to that crate in 1.6.0, so there is no `patch` here.
|
||||||
pub const WRITE_THROUGH_DEPTH: usize = 1;
|
pub const WRITE_THROUGH_DEPTH: usize = 1;
|
||||||
|
|
||||||
/// Outcome of [`Sink::apply`]: either keep feeding items
|
/// Outcome of [`Sink::apply`]: either keep feeding items
|
||||||
|
|||||||
@@ -255,7 +255,8 @@ fn mlp_major_sync_header_size(ms: &[u8]) -> Option<usize> {
|
|||||||
/// 0x002D). The stored trailer is the last 2 header bytes; because
|
/// 0x002D). The stored trailer is the last 2 header bytes; because
|
||||||
/// MLP's checksum is byte-reversed relative to a standard CRC, a standard CRC of
|
/// MLP's checksum is byte-reversed relative to a standard CRC, a standard CRC of
|
||||||
/// the header body XOR the little-endian word before the trailer must equal the
|
/// the header body XOR the little-endian word before the trailer must equal the
|
||||||
/// trailer read big-endian.
|
/// trailer read LITTLE-endian. (Comparing it big-endian was the bug this function
|
||||||
|
/// was fixed for; the body and the inline note below are authoritative.)
|
||||||
fn mlp_major_sync_crc_ok(ms: &[u8], mshdr: usize) -> bool {
|
fn mlp_major_sync_crc_ok(ms: &[u8], mshdr: usize) -> bool {
|
||||||
if mshdr < 4 || ms.len() < mshdr {
|
if mshdr < 4 || ms.len() < mshdr {
|
||||||
return false;
|
return false;
|
||||||
|
|||||||
@@ -50,8 +50,7 @@ pub type KeyFetchFn = std::sync::Arc<dyn Fn(&[Vec<u8>]) -> Vec<[u8; 16]> + Send
|
|||||||
/// `len()` and never assumes a fixed N (32 is all we've seen, but the contract
|
/// `len()` and never assumes a fixed N (32 is all we've seen, but the contract
|
||||||
/// is "whatever the source returns, ≥ 1, is all of them").
|
/// is "whatever the source returns, ≥ 1, is all of them").
|
||||||
///
|
///
|
||||||
/// A **stateless, shared** pair of `Arc<Fn>` — the decorator owns the only
|
/// A **stateless, shared** pair of `Arc<Fn>`, so one `KeyFetch` is built
|
||||||
/// mutable state (its call-count cap and spent flag), so one `KeyFetch` is built
|
|
||||||
/// once and cloned cheaply (two `Arc` bumps) into every read path. `Send + Sync`
|
/// once and cloned cheaply (two `Arc` bumps) into every read path. `Send + Sync`
|
||||||
/// so it can ride the mux highway's producer thread.
|
/// so it can ride the mux highway's producer thread.
|
||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
|
|||||||
Reference in New Issue
Block a user