mirror of
https://github.com/freemkv/libfreemkv.git
synced 2026-10-07 18:13:10 +00:00
mpls/ifo: fix STN gate off-by-one and pin the PGC palette chroma order
mpls: the STN table gate required item.len() > STN_OFFSET + 16, one byte more than the 16-byte header actually spans, skipping a first play item whose STN header ends exactly at the item boundary. Use >=, with a boundary test. ifo: the subtitle palette layout [padding, Y, Cr, Cb] (Cr BEFORE Cb) was correct but under-documented; harden the comment against a future byte-swap and add a test that cross-checks the parse against the DvdSub ycbcr_to_rgb consumer so the two read Cr/Cb the same way.
This commit is contained in:
+27
-1
@@ -942,7 +942,11 @@ fn parse_pgc(data: &[u8], pgc_offset: usize, chapters: u16) -> Result<DvdTitle>
|
||||
times
|
||||
};
|
||||
|
||||
// Extract subtitle palette at PGC offset 0xA4: 16 colors × 4 bytes [padding, Y, Cr, Cb]
|
||||
// Extract subtitle palette at PGC offset 0xA4: 16 colors × 4 bytes
|
||||
// [padding, Y, Cr, Cb]. Chroma order is Cr (byte 2) BEFORE Cb (byte 3),
|
||||
// fixed by the DVD-Video PGC CLUT format — NOT Cb before Cr. The consumer
|
||||
// `mux::codec::dvdsub::ycbcr_to_rgb` reads it the same way; keep them in
|
||||
// lockstep or subtitle colours channel-swap.
|
||||
let palette = if pgc_offset + 0xA4 + 64 <= data.len() {
|
||||
let mut colors = Vec::with_capacity(16);
|
||||
for i in 0..16 {
|
||||
@@ -1728,6 +1732,28 @@ mod tests {
|
||||
assert!(title2.palette.is_none());
|
||||
}
|
||||
|
||||
/// The palette byte order is [padding, Y, Cr, Cb] — chroma is Cr BEFORE Cb
|
||||
/// — and the DvdSub consumer reads it the same way. Pin both together with
|
||||
/// distinct Cr/Cb so a future byte-swap in either place fails here.
|
||||
#[test]
|
||||
fn pgc_palette_chroma_order_is_cr_then_cb_matching_the_consumer() {
|
||||
let mut pgc = vec![0u8; 0xEA];
|
||||
pgc[0x03] = 0; // no cells
|
||||
// Color 0: Y=0x40, Cr=0x11, Cb=0x22 — distinct so an order swap shows.
|
||||
pgc[0xA4] = 0x00; // padding
|
||||
pgc[0xA4 + 1] = 0x40; // Y
|
||||
pgc[0xA4 + 2] = 0x11; // Cr
|
||||
pgc[0xA4 + 3] = 0x22; // Cb
|
||||
let title = parse_pgc(&pgc, 0, 1).unwrap();
|
||||
let pal = title.palette.expect("non-empty palette");
|
||||
assert_eq!(pal[0], [0x00, 0x40, 0x11, 0x22], "stored as [pad, Y, Cr, Cb]");
|
||||
// The consumer interprets index 2 as Cr and index 3 as Cb (see
|
||||
// dvdsub::ycbcr_to_rgb). Cross-check the same entry both ways.
|
||||
let rgb = crate::mux::codec::dvdsub::ycbcr_to_rgb(&pal[0]);
|
||||
let expected = crate::mux::codec::dvdsub::ycbcr_to_rgb(&[0x00, 0x40, 0x11, 0x22]);
|
||||
assert_eq!(rgb, expected, "ifo and dvdsub must read Cr/Cb the same way");
|
||||
}
|
||||
|
||||
/// parse_pgc palette layout: each color is [padding, Y, Cr, Cb] and the
|
||||
/// "non-empty" test ignores the padding byte (index 0). A palette whose
|
||||
/// ONLY non-zero bytes are padding must still be treated as empty (None).
|
||||
|
||||
+21
-1
@@ -154,7 +154,11 @@ pub fn parse(data: &[u8]) -> Result<Playlist> {
|
||||
// UO_mask_table(8) + misc flags(1) + still_mode(1) + still_time(2), then
|
||||
// STN_table starting at STN_OFFSET.
|
||||
const STN_OFFSET: usize = 32;
|
||||
if item_idx == 0 && item.len() > STN_OFFSET + 16 {
|
||||
// `>=`, not `>`: the 16-byte STN header spans item[STN_OFFSET..STN_OFFSET+16],
|
||||
// so item.len() == STN_OFFSET + 16 already holds every header byte. The
|
||||
// strict `>` demanded one extra byte and skipped a play item whose STN
|
||||
// header ends exactly at the item boundary (a stream-less STN table).
|
||||
if item_idx == 0 && item.len() >= STN_OFFSET + 16 {
|
||||
// STN header: length(2) + reserved(2) + counts(8) + reserved(4) = 16 bytes
|
||||
let n_video = item[STN_OFFSET + 4] as usize;
|
||||
let n_audio = item[STN_OFFSET + 5] as usize;
|
||||
@@ -735,6 +739,22 @@ mod tests {
|
||||
assert!(!s.secondary);
|
||||
}
|
||||
|
||||
// The STN gate is inclusive of the boundary: a first play item whose STN
|
||||
// table is JUST the 16-byte header (no stream entries) has item.len() ==
|
||||
// STN_OFFSET(32) + 16 == 48 exactly, and must be parsed, not skipped by a
|
||||
// strict `>` that demands one extra byte.
|
||||
#[test]
|
||||
fn stn_header_ending_exactly_at_the_item_boundary_is_parsed() {
|
||||
let data = build_mpls(&[(b"00001", 1, 0, 9000000)], (0, 0, 0, 0, 0, 0, 0, 0), &[]);
|
||||
// The first play item is exactly the header length (32 + 16).
|
||||
let playlist = parse(&data).expect("a boundary-length STN header must parse");
|
||||
assert_eq!(playlist.play_items.len(), 1);
|
||||
assert!(
|
||||
playlist.streams.is_empty(),
|
||||
"no stream entries were declared at the boundary"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_invalid_magic() {
|
||||
let mut data = build_mpls(&[(b"00001", 1, 0, 9000000)], (0, 0, 0, 0, 0, 0, 0, 0), &[]);
|
||||
|
||||
Reference in New Issue
Block a user