From 6661310597947d760a7eb5f55934be4a70b9a36a Mon Sep 17 00:00:00 2001 From: Alastor Wu Date: Thu, 16 Apr 2026 13:09:07 -0700 Subject: [PATCH 1/3] Parse colr box CICP data for video tracks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add parsing of the ISO BMFF `colr` box (ISO 14496-12:2020 §12.1.5) in `read_video_sample_entry`, storing the result in a new `colour_info` field on `VideoSampleEntry`. Expose the nclx CICP values through the C API via four new fields on `Mp4parseTrackVideoSampleInfo`: `has_colour_info`, `colour_primaries`, `transfer_characteristics`, `matrix_coefficients`, and `full_range_flag`. This enables Firefox's MP4 demuxer to set `mVideoInfo.mTransferFunction` for fMP4 content (Netflix, Prime Video, DRM), fixing HDR10/HLG video appearing near-black on Windows HDR displays. - `read_video_sample_entry` now takes `strictness` (consistent with `read_audio_sample_entry`) and passes it to `read_colr`. - Duplicate `colr` boxes in a sample entry violate ISO 14496-12:2020 §12.1.5; we warn and keep the first rather than failing playback. - `has_colour_info: bool` is the canonical "present" flag; avoids misinterpreting MC=0 (Identity/GBR) as "absent". - `NclxColourInformation` fields made public for Rust callers. - Seven integration tests added. Test MP4 files were generated with ffmpeg using `-movflags +write_colr` and `-x264-params colorprim/transfer/colormatrix`: HDR10 (cp=9,tc=16,mc=9), HLG (tc=18), HDR10 full-range, HLG full-range, RGB/Identity (mc=0, exercises has_colour_info), and no-colr variants. --- mp4parse/src/lib.rs | 35 +++- mp4parse/src/tests.rs | 5 +- mp4parse_capi/src/lib.rs | 20 +++ mp4parse_capi/tests/test_colour_info.rs | 158 ++++++++++++++++++ mp4parse_capi/tests/video_colr_nclx_hdr10.mp4 | Bin 0 -> 1666 bytes .../video_colr_nclx_hdr10_full_range.mp4 | Bin 0 -> 1666 bytes mp4parse_capi/tests/video_colr_nclx_hlg.mp4 | Bin 0 -> 1666 bytes .../tests/video_colr_nclx_hlg_full_range.mp4 | Bin 0 -> 1666 bytes mp4parse_capi/tests/video_colr_nclx_rgb.mp4 | Bin 0 -> 1667 bytes 9 files changed, 208 insertions(+), 10 deletions(-) create mode 100644 mp4parse_capi/tests/test_colour_info.rs create mode 100644 mp4parse_capi/tests/video_colr_nclx_hdr10.mp4 create mode 100644 mp4parse_capi/tests/video_colr_nclx_hdr10_full_range.mp4 create mode 100644 mp4parse_capi/tests/video_colr_nclx_hlg.mp4 create mode 100644 mp4parse_capi/tests/video_colr_nclx_hlg_full_range.mp4 create mode 100644 mp4parse_capi/tests/video_colr_nclx_rgb.mp4 diff --git a/mp4parse/src/lib.rs b/mp4parse/src/lib.rs index 58bdcc36..3a77c473 100644 --- a/mp4parse/src/lib.rs +++ b/mp4parse/src/lib.rs @@ -1175,6 +1175,7 @@ pub struct VideoSampleEntry { pub codec_specific: VideoCodecSpecific, pub protection_info: TryVec, pub pixel_aspect_ratio: Option, + pub colour_info: Option, } /// Represent a Video Partition Codec Configuration 'vpcC' box (aka vp9). The meaning of each @@ -3722,10 +3723,10 @@ fn read_pixi(src: &mut BMFFBox) -> Result { #[repr(C)] #[derive(Debug)] pub struct NclxColourInformation { - colour_primaries: u8, - transfer_characteristics: u8, - matrix_coefficients: u8, - full_range_flag: bool, + pub colour_primaries: u8, + pub transfer_characteristics: u8, + pub matrix_coefficients: u8, + pub full_range_flag: bool, } /// The raw bytes of the ICC profile @@ -5533,7 +5534,10 @@ fn read_hdlr(src: &mut BMFFBox, strictness: ParseStrictness) -> Resu } /// Parse an video description inside an stsd box. -fn read_video_sample_entry(src: &mut BMFFBox) -> Result { +fn read_video_sample_entry( + src: &mut BMFFBox, + strictness: ParseStrictness, +) -> Result { let name = src.get_header().name; let codec_type = match name { BoxType::AVCSampleEntry | BoxType::AVC3SampleEntry => CodecType::H264, @@ -5567,6 +5571,7 @@ fn read_video_sample_entry(src: &mut BMFFBox) -> Result // Skip clap/pasp/etc. for now. let mut codec_specific = None; let mut pixel_aspect_ratio = None; + let mut colour_info = None; let mut protection_info = TryVec::new(); let mut iter = src.box_iter(); while let Some(mut b) = iter.next_box()? { @@ -5683,6 +5688,19 @@ fn read_video_sample_entry(src: &mut BMFFBox) -> Result } debug!("Parsed pasp box: {pasp:?}, PAR {pixel_aspect_ratio:?}"); } + BoxType::ColourInformationBox => { + if colour_info.is_some() { + // ISO 14496-12:2020 §12.1.5 requires at most one ColourInformationBox + // per sample entry. Duplicates are a spec violation, but we tolerate + // them to avoid breaking playback of malformed content. + warn!("Multiple colr boxes in video sample entry, keeping first"); + skip_box_content(&mut b)?; + } else { + let colr = read_colr(&mut b, strictness)?; + debug!("Parsed colr box: {colr:?}"); + colour_info = Some(colr); + } + } _ => { debug!("Unsupported video codec, box {:?} found", b.head.name); skip_box_content(&mut b)?; @@ -5701,6 +5719,7 @@ fn read_video_sample_entry(src: &mut BMFFBox) -> Result codec_specific, protection_info, pixel_aspect_ratio, + colour_info, }) }), ) @@ -5908,9 +5927,9 @@ fn read_stsd( while descriptions.len() < description_count { if let Some(mut b) = iter.next_box()? { let description = match track.track_type { - TrackType::Video => read_video_sample_entry(&mut b), - TrackType::Picture => read_video_sample_entry(&mut b), - TrackType::AuxiliaryVideo => read_video_sample_entry(&mut b), + TrackType::Video => read_video_sample_entry(&mut b, strictness), + TrackType::Picture => read_video_sample_entry(&mut b, strictness), + TrackType::AuxiliaryVideo => read_video_sample_entry(&mut b, strictness), TrackType::Audio => read_audio_sample_entry(&mut b, strictness), TrackType::Metadata => Err(Error::Unsupported("metadata track")), TrackType::Unknown => Err(Error::Unsupported("unknown track type")), diff --git a/mp4parse/src/tests.rs b/mp4parse/src/tests.rs index d7fd74c9..52edf661 100644 --- a/mp4parse/src/tests.rs +++ b/mp4parse/src/tests.rs @@ -1145,7 +1145,8 @@ fn read_stsd_mp4v() { let mut iter = super::BoxIter::new(&mut stream); let mut stream = iter.next_box().unwrap().unwrap(); - let sample_entry = super::read_video_sample_entry(&mut stream).unwrap(); + let sample_entry = + super::read_video_sample_entry(&mut stream, super::ParseStrictness::Normal).unwrap(); match sample_entry { super::SampleEntry::Video(v) => { @@ -1245,7 +1246,7 @@ fn unknown_video_sample_entry() { }); let mut iter = super::BoxIter::new(&mut stream); let mut stream = iter.next_box().unwrap().unwrap(); - match super::read_video_sample_entry(&mut stream) { + match super::read_video_sample_entry(&mut stream, super::ParseStrictness::Normal) { Ok(super::SampleEntry::Unknown) => (), _ => panic!("expected a different error result"), } diff --git a/mp4parse_capi/src/lib.rs b/mp4parse_capi/src/lib.rs index dbdfafaf..1b51d0cf 100644 --- a/mp4parse_capi/src/lib.rs +++ b/mp4parse_capi/src/lib.rs @@ -264,6 +264,18 @@ pub struct Mp4parseTrackVideoSampleInfo { pub image_height: u16, pub extra_data: Mp4parseByteData, pub protected_data: Mp4parseSinfInfo, + /// True when a `colr` box with `colour_type = 'nclx'` was present. When false, + /// the CICP fields below are all zero and must not be interpreted. + pub has_colour_info: bool, + /// CICP colour primaries (ISO 23091-2 § 8.1). Valid only when `has_colour_info`. + pub colour_primaries: u8, + /// CICP transfer characteristics (ISO 23091-2 § 8.2). Valid only when `has_colour_info`. + pub transfer_characteristics: u8, + /// CICP matrix coefficients (ISO 23091-2 § 8.3). Valid only when `has_colour_info`. + /// Note: value 0 is a valid CICP value (Identity/GBR), not an absence indicator. + pub matrix_coefficients: u8, + /// Full range flag from the colr nclx box. Valid only when `has_colour_info`. + pub full_range_flag: bool, } #[repr(C)] @@ -1120,6 +1132,14 @@ fn mp4parse_get_track_video_info_safe( }; } } + if let Some(mp4parse::ColourInformation::Nclx(ref nclx)) = video.colour_info { + sample_info.has_colour_info = true; + sample_info.colour_primaries = nclx.colour_primaries; + sample_info.transfer_characteristics = nclx.transfer_characteristics; + sample_info.matrix_coefficients = nclx.matrix_coefficients; + sample_info.full_range_flag = nclx.full_range_flag; + } + video_sample_infos.push(sample_info)?; } diff --git a/mp4parse_capi/tests/test_colour_info.rs b/mp4parse_capi/tests/test_colour_info.rs new file mode 100644 index 00000000..3d0312d1 --- /dev/null +++ b/mp4parse_capi/tests/test_colour_info.rs @@ -0,0 +1,158 @@ +use mp4parse_capi::*; +use std::io::Read; + +extern "C" fn buf_read(buf: *mut u8, size: usize, userdata: *mut std::os::raw::c_void) -> isize { + let input: &mut std::fs::File = unsafe { &mut *(userdata as *mut _) }; + let buf = unsafe { std::slice::from_raw_parts_mut(buf, size) }; + match input.read(buf) { + Ok(n) => n as isize, + Err(_) => -1, + } +} + +unsafe fn open_parser(path: &str) -> *mut Mp4parseParser { + let mut file = std::fs::File::open(path).expect("file not found"); + let io = Mp4parseIo { + read: Some(buf_read), + userdata: &mut file as *mut _ as *mut std::os::raw::c_void, + }; + let mut parser = std::ptr::null_mut(); + let rv = mp4parse_new(&io, &mut parser); + assert_eq!(rv, Mp4parseStatus::Ok); + assert!(!parser.is_null()); + parser +} + +/// HDR10 (PQ): colour_primaries=9, transfer=16, matrix=9, full_range=false +#[test] +fn video_colr_nclx_hdr10() { + unsafe { + let parser = open_parser("tests/video_colr_nclx_hdr10.mp4"); + + let mut video = Mp4parseTrackVideoInfo::default(); + let rv = mp4parse_get_track_video_info(parser, 0, &mut video); + assert_eq!(rv, Mp4parseStatus::Ok); + + assert_eq!(video.sample_info_count, 1); + let sample = &*video.sample_info; + assert!(sample.has_colour_info); + assert_eq!(sample.colour_primaries, 9); + assert_eq!(sample.transfer_characteristics, 16); + assert_eq!(sample.matrix_coefficients, 9); + assert!(!sample.full_range_flag); + + mp4parse_free(parser); + } +} + +/// HDR10 full-range: colour_primaries=9, transfer=16, matrix=9, full_range=true +#[test] +fn video_colr_nclx_hdr10_full_range() { + unsafe { + let parser = open_parser("tests/video_colr_nclx_hdr10_full_range.mp4"); + + let mut video = Mp4parseTrackVideoInfo::default(); + let rv = mp4parse_get_track_video_info(parser, 0, &mut video); + assert_eq!(rv, Mp4parseStatus::Ok); + + assert_eq!(video.sample_info_count, 1); + let sample = &*video.sample_info; + assert!(sample.has_colour_info); + assert_eq!(sample.colour_primaries, 9); + assert_eq!(sample.transfer_characteristics, 16); + assert_eq!(sample.matrix_coefficients, 9); + assert!(sample.full_range_flag); + + mp4parse_free(parser); + } +} + +/// HLG: colour_primaries=9, transfer=18, matrix=9, full_range=false +#[test] +fn video_colr_nclx_hlg() { + unsafe { + let parser = open_parser("tests/video_colr_nclx_hlg.mp4"); + + let mut video = Mp4parseTrackVideoInfo::default(); + let rv = mp4parse_get_track_video_info(parser, 0, &mut video); + assert_eq!(rv, Mp4parseStatus::Ok); + + assert_eq!(video.sample_info_count, 1); + let sample = &*video.sample_info; + assert!(sample.has_colour_info); + assert_eq!(sample.colour_primaries, 9); + assert_eq!(sample.transfer_characteristics, 18); + assert_eq!(sample.matrix_coefficients, 9); + assert!(!sample.full_range_flag); + + mp4parse_free(parser); + } +} + +/// HLG full-range: colour_primaries=9, transfer=18, matrix=9, full_range=true +#[test] +fn video_colr_nclx_hlg_full_range() { + unsafe { + let parser = open_parser("tests/video_colr_nclx_hlg_full_range.mp4"); + + let mut video = Mp4parseTrackVideoInfo::default(); + let rv = mp4parse_get_track_video_info(parser, 0, &mut video); + assert_eq!(rv, Mp4parseStatus::Ok); + + assert_eq!(video.sample_info_count, 1); + let sample = &*video.sample_info; + assert!(sample.has_colour_info); + assert_eq!(sample.colour_primaries, 9); + assert_eq!(sample.transfer_characteristics, 18); + assert_eq!(sample.matrix_coefficients, 9); + assert!(sample.full_range_flag); + + mp4parse_free(parser); + } +} + +/// RGB (Identity matrix, MC=0): has_colour_info=true but matrix_coefficients=0. +/// Validates that has_colour_info correctly distinguishes "colr box present with MC=0" +/// from "no colr box" (where matrix_coefficients is also 0). +#[test] +fn video_colr_nclx_rgb_identity_matrix() { + unsafe { + let parser = open_parser("tests/video_colr_nclx_rgb.mp4"); + + let mut video = Mp4parseTrackVideoInfo::default(); + let rv = mp4parse_get_track_video_info(parser, 0, &mut video); + assert_eq!(rv, Mp4parseStatus::Ok); + + assert_eq!(video.sample_info_count, 1); + let sample = &*video.sample_info; + assert!(sample.has_colour_info); + assert_eq!(sample.colour_primaries, 1); + assert_eq!(sample.transfer_characteristics, 1); + assert_eq!(sample.matrix_coefficients, 0); // Identity/GBR — valid CICP value, not "absent" + assert!(!sample.full_range_flag); + + mp4parse_free(parser); + } +} + +/// No colr box: has_colour_info=false, CICP fields are zero +#[test] +fn video_no_colr_box() { + unsafe { + let parser = open_parser("tests/white.mp4"); + + let mut video = Mp4parseTrackVideoInfo::default(); + let rv = mp4parse_get_track_video_info(parser, 0, &mut video); + assert_eq!(rv, Mp4parseStatus::Ok); + + assert_eq!(video.sample_info_count, 1); + let sample = &*video.sample_info; + assert!(!sample.has_colour_info); + assert_eq!(sample.colour_primaries, 0); + assert_eq!(sample.transfer_characteristics, 0); + assert_eq!(sample.matrix_coefficients, 0); + assert!(!sample.full_range_flag); + + mp4parse_free(parser); + } +} diff --git a/mp4parse_capi/tests/video_colr_nclx_hdr10.mp4 b/mp4parse_capi/tests/video_colr_nclx_hdr10.mp4 new file mode 100644 index 0000000000000000000000000000000000000000..70cb5a3a68fda3415775695d74b8c2fc9398f381 GIT binary patch literal 1666 zcmZuy&5ImG6tCG$77YOdu9(9LWsKkj=KvDS=Znm6ug*25Sr@h>g{IwqqeGM zcP1y59QH5p;6c4A;=zk2<0?@E@8W_NBZ{KvO>#-b-|Lylj*E|}dcXHsAMaHUV~nS6 zq;p%Bj4d$Y7=hTAeyP`d#@HjNQ3|$S%0xKuPnOP(#~=N8?f1`rJ>L23ohv`^<7fW* zdl>ZAc$>@GDDDHAuJ5h$uCM%!bOVN+4jfireeK%I?KQr2eH)&Mk}z!7dQ_;?@xb%C z?Z6AVP-f2Q&DGU|gM-d~o~XJIRi`$)tF%fdb7cXaTD!ci>?W5Y7Bcj?QEAxaiHeI_ z_CtTu+w{1oL@}}|^oHF{Z|M77UaBxtLv9B#&XnnYzNCzPI%Fx3WL~^sPR2cYIe4nSnx=7o8uE}GP zU{6PNnp)+;cEDX`5J8EGy6%e%LUf|DMJ}dNZsuax zYOShhx7M&ulp3LZHA3z%c;1`BsLU&f+)7m`IdI{cN82=luGA=-A>7F5;c>}sWGGF_ z%NX}c$AScj=+;A}rdmB(#r*yc+bB!XQAy7l)5__mWRg=Am4t`7> z-eTSHgMa?KeYUs!?U&TlTkl>fUi{+05#iQ*TcdO1J8vLvF2ohGiGC534aCVHMTO#C`M*BwzM zg(9EUS14Etu)JS3N$0nDIhoj25_AOxgUEhG~bt3wV*o^eLGboB85+488%c?N-9hjelY*yWk%mM~8<; zH*OuC`2gL)IIdyq?n|E@wb)Yj$IstgI=X-F9>qT{>%vsB7_vv%1q{#yEvyEo-O*S^ zV^6RC_Fcwku#46;WdUq_Pg{IwqqeGM zcP6JOIqYBH!Gn5L#Df=4##N#S-o=0yBZ{KvE#zXx-|Lylj*E|}dcXHsAMaHUV~nS6 zq;p%Bj4d$Y7=hTAeyP`d#@HjNQ3|$S%0xKuPnJ%P#~=N8_4m(zy|eS#JC}dpcb@+1 z?_tne<83Z$qqq-fy1uv0yT0-_(hV4PI&fHh<<+Y%wb%I8wQYDNO2V*R>rtUn#{&>MC)y`k@Sd8xuo4Y?h}K+kh+M+n6nnI!Z(9)d9OG9RiWqKrNw zA|t9@75Y6cGgFr$La5K3QALqkp!MN;A{~I-D?<-k5XtqrQeog<_I;iT>mqIYxh9WE zf;}D8X=;@V+W~i(K?Ef#>bfs72+^_57P-V;W(tp2rddPgrEs*5ymHDE0#4A!#lVPB zBdBM{zDJEs`76oa;d4yP1n& ztF@}4-CDysQEG(t)d;!6;5ly!qcX1`aw}D(BQThHxXJhsPzmk)bpx zFJs&*9ScGjbUPm3qr-$=rw6Dhal^$h>}^0{ozmeN&ozpKhCmIGOYEWkR84jR9sH;| zyve%b2lxKGb-K6w?U&TlTkl>hUijj{5#iQ*Tcfk%+ix+pI4J9SAG)I4&l2!CTKo;d zkhK{7j_E%F)FLvEt?Iy0BC534aCVHMTO#B)>*BwzM zg(9EUS14Etu)JS3N$0nDDVf+-5_AOxgUEhvUG~bt3wV*o^eLGboB6_c488%c?N-9hj(=h+yWk(+i4G5s z-ne;q>H~BKMXP@UJpYH->e zjb$|U^y+WjWsC;9U|mxdz{Urqr<$>4oR-Q(G>d;$9{W;@K#$Tjd}+#?*cHS|Qh;W$ zQ-8)`yFM{BGp+YE(7EZry0V_yDb~Y?2ZT>I^9fvRH|x{ znLVlOA^rm%Jg8SiJb3Zs8f6v1yBP4|ilQiblfAg(@Ab@V#^7VB-tT?Z$9vVo7~`oM z>D<;OV+)KpMj-a3U+OiVG4?=el!EQ&G7%2^J-sZA4iu-`3>wD|G>nndF-GE`I1Bca@U%C2XdyQ{h+lFVNBn;cN9u+EeJn+13 zJMe-ol$mpSb9MFL;GnagC#o())v3+yDy`DVTv@=U)-JCryUC@9g$#XeR2p`9qT-^K z{m|d^Ha#vXQH-n#yfAgiy?pNkYHlAqW#M^Px&2%IG5^ zGNRg5q2J>&Gj%B&6$bug-{+~YF4DH2Yx0;R z*way+rdGMI9dMT!L{OrluKOZ`5S{33kxT4lrto-Wnl)rz3P=0ME2m5$-~@eK42&2> zvM#l7kR`6j8Ie~=0Y@XqCpDr}7MB%AdIX#&VE`-=q3INH6z9UyBDqn`xeipmo4FXa zTB|DBtu?F@rABC9jgUJGp7o|MD)R~=w^CI~4qUkA(Kd~sD>cey2sbjie_XN~8A_A# zGRD2qu^@y&x8w0WI!x$wdVrb|H(U(E-UbxbDIKoyT%$;62-Fa{#2)HT)nu=ugCA0d zH(7Ul@9#fuo$W1u^942a);kxA=Rd!9M7Z_t*67^$_8W{X4$8XThps61vjlvO7Jr2> zWGzO&6Z($;^(pzviRb^J1w5$ta4&rrUvvFt&BZy#S+?PvZUSePnc;qfxb1$4YjPMS6Te2!bw^Z5 zp~$E8B?^`TEbo_1()n#(N+!0I1YJSFAhMqyn1WAm>rLz80{A=Y?5rT<^EG{;z=`H_ zPBaTR_6{HoqSj+C!N2Eqx_-y^0$$`XeM%A>v6rZybi$sw?U_=Td4IxMrx%2LxIc!TS#e@hKAkmNlkXEy^v)@GDDDHAuJ5h$uCM%!bOVO%4jfirdim-L?KQr2?Fu{-C1JQy>rtUn#{&>MC)y`k@Sd8xuo4Y?h}K+ke*M+n6nnI!Z(9)d9OG9RiWqKrNw zA|t9D75Y6cGgFr$La5K3QALqkp!MN;A{~I-Ekh4m5XtqrQeog<@_n8P>mqIYxh9WE zf?XZeX=;@V+W~i(K?Ef#>bfs72+^_57P-V;W(tp2rddPgrEs*5ymHDE0#4A!#lVPB zBdBM{zDJEs`76oNHg@JDH1N ztF@}4om#^>QEG(t)d;!6;2CcUqcX1`aw}D(BQThHxXJ`^P1_k)bpx zFJs&*9ScGjbUPm3rNe|?rw6Dhal^$h>}^0{ozmeN&ozpKhCmIGOYEZlR84jR9sHm= zxXHTXdw>6N>vV7Vn=h!Tx8Au>Joov%L&B|hw?=2jx8GoFaZuLv9&|;ymnGnHwD>E8 zA!{-E9n*gVs87h3k3IhnE#N`DhkNP6_`2&iYc9??&aw^XbQ3tU%nT>w%cMv4W-xY9 zCC)-xQH8}LZipg~_tbRH3y+maE}-LOa`Nl})JyD@+f7_MOA6BlFo1QVluITz!ZFfTW?wy7r@_HXJ-W=pRef)1x_@d zbD~+mv9|$f5VanC5&k`|)Ac*P7w{sF=~FT>HuL%M7<>a>S6T@>JN}Wa?0|oC6dfEK zzIOAiQ}3fY7{@hi-Fg0#!xmf0e*fv)3y1&Qy-V?r$ht6h!@gQ*m{MAx5oXk(mw*cD42(76~_o YTulid$A1SUp`;A8NKp&Fhgx|50v&;PdjJ3c literal 0 HcmV?d00001 diff --git a/mp4parse_capi/tests/video_colr_nclx_rgb.mp4 b/mp4parse_capi/tests/video_colr_nclx_rgb.mp4 new file mode 100644 index 0000000000000000000000000000000000000000..00ea41240f273508e1d2655c00f940a8ae5b5118 GIT binary patch literal 1667 zcmZuy&1)n@6t77%$hwDs5pu{v>8>ESnRNHWF&S*w7#HWDupllXLQ`E`oo=Q-Dpi%q z%$^iQyn5M#f_UA7XHR~BEBYU}3tm(dMNk$*a@z6tdS*6b@R6$bd!JSH-s=ftjHhm_ zb6b~;EivL)ff!1^)Z0E|?2*(c1=}xWA{_XqD-Ta5w|=<#`{%#j?SFRj@*RHn;-7zw zg6=loIFq0fyN_J$RI?gI$PutSD7n3UYX_ynU})RHS)?SQwTUg9~VbPj3Zf> zS~$oO6>>)86;i;_2=YmdD3wL6;z*By^CS#_MItmkB97u*SUMy(s(IG2$`3LZ!&Ylm zMF+Kpb*j_|9jGyKhrzSn3`S*MLF87dO35QU(xYn{K@~O1W(YSjdhlGb8yQNI@-jwQ z=~)oMpwssFAw4Ga+Fd|Ri5o6PVRsh_>y$?|??baYFRhnJE}-LOa`x>3)GO?@+fCeNmK3Hz*y!_|W8&B8bM1>NDHQp% zzDCDVfVE-SBwf7brDST`NH7)@3?lpaBUA7xZoOk&TmXM>ot-y?e7>b$C~%_1oKwvb zp4|teMbvuyW%zf!cE@k~Ucieyrk|3jvDw!r6Yw6q_FI3mbCVz0g#+*pK6>Wfy^}|; zt*)X>%T%jn_TBUSo3FRnO7_RkxBD+nPEIJnW3nzxB?}sm*?m^Euo@;3X{}+jXSM(0 zC&p;8b?chG05(48JvEK3;knc=qIvw&^3<1R1bW=3;VU!V#BLx~k^(f3o%u5k+l{HI z+3CD*fX>er&Xx7dPO% Date: Fri, 17 Apr 2026 11:43:26 -0700 Subject: [PATCH 2/3] Address review feedback on colr box parsing for video tracks - Add ColrBadQuantityBMFF status (distinct from the HEIF-specific ColrBadQuantity) and enforce it via fail_with_status_if when strictness != Permissive for duplicate colr boxes in video tracks. - Introduce ParsedColourInformation enum so read_colr returns Supported/Unsupported instead of always erroring on unknown colour_type values; the AVIF call site converts Unsupported to ColrBadType, while the video call site warns and continues. - Document that VideoSampleEntry::colour_info only surfaces Nclx through the C API. --- mp4parse/src/lib.rs | 62 ++++++++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/mp4parse/src/lib.rs b/mp4parse/src/lib.rs index 3a77c473..b9bf40ce 100644 --- a/mp4parse/src/lib.rs +++ b/mp4parse/src/lib.rs @@ -185,6 +185,7 @@ pub enum Status { BoxBadWideSize, CheckParserStateErr, ColrBadQuantity, + ColrBadQuantityBMFF, ColrBadSize, ColrBadType, ColrReservedNonzero, @@ -465,6 +466,10 @@ impl From for &str { ColourInformationBox (colr) for a given value of colour_type \ per HEIF (ISO/IEC DIS 23008-12) § 6.5.5.1" } + Status::ColrBadQuantityBMFF => { + "Each sample entry shall have at most one ColourInformationBox (colr) \ + per ISOBMFF (ISO 14496-12:2020) § 12.1.5" + } Status::ColrBadSize => { "Unexpected size for colr box" } @@ -1175,6 +1180,8 @@ pub struct VideoSampleEntry { pub codec_specific: VideoCodecSpecific, pub protection_info: TryVec, pub pixel_aspect_ratio: Option, + /// Only `ColourInformation::Nclx` is currently surfaced through the C API; + /// `ColourInformation::Icc` is stored but not exposed to C consumers. pub colour_info: Option, } @@ -3601,7 +3608,13 @@ fn read_ipco( let property = match b.head.name { BoxType::AuxiliaryTypeProperty => ItemProperty::AuxiliaryType(read_auxc(&mut b)?), BoxType::AV1CodecConfigurationBox => ItemProperty::AV1Config(read_av1c(&mut b)?), - BoxType::ColourInformationBox => ItemProperty::Colour(read_colr(&mut b, strictness)?), + BoxType::ColourInformationBox => match read_colr(&mut b, strictness)? { + ParsedColourInformation::Supported(colr) => ItemProperty::Colour(colr), + ParsedColourInformation::Unsupported(colour_type) => { + error!("read_colr colour_type: {colour_type:?}"); + return Status::ColrBadType.into(); + } + }, BoxType::ImageMirror => ItemProperty::Mirroring(read_imir(&mut b)?), BoxType::ImageRotation => ItemProperty::Rotation(read_irot(&mut b)?), BoxType::ImageSpatialExtentsProperty => { @@ -3759,12 +3772,17 @@ impl ColourInformation { } } +enum ParsedColourInformation { + Supported(ColourInformation), + Unsupported(FourCC), +} + /// Parse colour information /// See ISOBMFF (ISO 14496-12:2020) § 12.1.5 fn read_colr( src: &mut BMFFBox, strictness: ParseStrictness, -) -> Result { +) -> Result { let colour_type = be_u32(src)?.to_be_bytes(); match &colour_type { @@ -3791,22 +3809,26 @@ fn read_colr( )?; } - Ok(ColourInformation::Nclx(NclxColourInformation { - colour_primaries, - transfer_characteristics, - matrix_coefficients, - full_range_flag, - })) + Ok(ParsedColourInformation::Supported(ColourInformation::Nclx( + NclxColourInformation { + colour_primaries, + transfer_characteristics, + matrix_coefficients, + full_range_flag, + }, + ))) } - b"rICC" | b"prof" => Ok(ColourInformation::Icc( + b"rICC" | b"prof" => Ok(ParsedColourInformation::Supported(ColourInformation::Icc( IccColourInformation { bytes: src.read_into_try_vec()?, }, FourCC::from(colour_type), - )), + ))), _ => { - error!("read_colr colour_type: {colour_type:?}"); - Status::ColrBadType.into() + let four_cc = FourCC::from(colour_type); + warn!("read_colr: unsupported colour_type {four_cc:?}, skipping"); + skip_box_remain(src)?; + Ok(ParsedColourInformation::Unsupported(four_cc)) } } } @@ -5690,15 +5712,19 @@ fn read_video_sample_entry( } BoxType::ColourInformationBox => { if colour_info.is_some() { - // ISO 14496-12:2020 §12.1.5 requires at most one ColourInformationBox - // per sample entry. Duplicates are a spec violation, but we tolerate - // them to avoid breaking playback of malformed content. warn!("Multiple colr boxes in video sample entry, keeping first"); + fail_with_status_if( + strictness != ParseStrictness::Permissive, + Status::ColrBadQuantityBMFF, + )?; skip_box_content(&mut b)?; } else { - let colr = read_colr(&mut b, strictness)?; - debug!("Parsed colr box: {colr:?}"); - colour_info = Some(colr); + if let ParsedColourInformation::Supported(colr) = + read_colr(&mut b, strictness)? + { + debug!("Parsed colr box: {colr:?}"); + colour_info = Some(colr); + } } } _ => { From 701cf90c6677a43ef75e5ba943165fa9f3aa4a95 Mon Sep 17 00:00:00 2001 From: Alastor Wu Date: Fri, 17 Apr 2026 11:48:03 -0700 Subject: [PATCH 3/3] cargo fmt --- mp4parse/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mp4parse/src/lib.rs b/mp4parse/src/lib.rs index b9bf40ce..7d4a292a 100644 --- a/mp4parse/src/lib.rs +++ b/mp4parse/src/lib.rs @@ -5719,8 +5719,7 @@ fn read_video_sample_entry( )?; skip_box_content(&mut b)?; } else { - if let ParsedColourInformation::Supported(colr) = - read_colr(&mut b, strictness)? + if let ParsedColourInformation::Supported(colr) = read_colr(&mut b, strictness)? { debug!("Parsed colr box: {colr:?}"); colour_info = Some(colr);