diff --git a/mp4parse/src/lib.rs b/mp4parse/src/lib.rs index 6a404556..b1bae2dd 100644 --- a/mp4parse/src/lib.rs +++ b/mp4parse/src/lib.rs @@ -467,8 +467,8 @@ impl From for &str { 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" + "Each sample entry should have at most one ColourInformationBox (colr) \ + for a given value of colour_type per ISOBMFF (ISO 14496-12:2020) § 12.1.5" } Status::ColrBadSize => { "Unexpected size for colr box" @@ -3636,13 +3636,16 @@ 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 => 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::ColourInformationBox => { + let colour_type = be_u32(&mut b)?.to_be_bytes(); + match read_colr(&mut b, colour_type, 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 => { @@ -3837,12 +3840,12 @@ enum ParsedColourInformation { /// Parse colour information /// See ISOBMFF (ISO 14496-12:2020) § 12.1.5 +/// The caller is expected to have already read the colour_type field. fn read_colr( src: &mut BMFFBox, + colour_type: [u8; 4], strictness: ParseStrictness, ) -> Result { - let colour_type = be_u32(src)?.to_be_bytes(); - match &colour_type { b"nclx" => { const NUM_RESERVED_BITS: u8 = 7; @@ -5652,6 +5655,7 @@ fn read_video_sample_entry( let mut codec_specific = None; let mut pixel_aspect_ratio = None; let mut colour_info = None; + let mut colr_types_seen = TryVec::::new(); let mut hdr_mastering_display = None; let mut hdr_content_light_level = None; let mut protection_info = TryVec::new(); @@ -5771,22 +5775,32 @@ fn read_video_sample_entry( debug!("Parsed pasp box: {pasp:?}, PAR {pixel_aspect_ratio:?}"); } BoxType::ColourInformationBox => { - if colour_info.is_some() { - // ISO/IEC 14496-12:2015 § 12.1.5.1 permits one or more colr boxes - // in a VisualSampleEntry and assigns them no normative behaviour; - // a reader may keep the first (most accurate) and ignore the rest. - // Only reject under Strict. - warn!("Multiple colr boxes in video sample entry, keeping first"); + // ISO/IEC 14496-12:2015 § 12.1.5.1 permits one or more colr boxes + // in a VisualSampleEntry (at most one for a given colour_type) and + // assigns them no normative behaviour; a reader may keep the first + // (most accurate) and ignore the rest. Only reject a repeated + // colour_type, and only under Strict. + let colour_type = FourCC::from(be_u32(&mut b)?.to_be_bytes()); + if colr_types_seen.contains(&colour_type) { + warn!( + "Multiple {colour_type:?} colr boxes in video sample entry, keeping first" + ); fail_with_status_if( strictness == ParseStrictness::Strict, Status::ColrBadQuantityBMFF, )?; - skip_box_content(&mut b)?; - } else if let ParsedColourInformation::Supported(colr) = - read_colr(&mut b, strictness)? - { - debug!("Parsed colr box: {colr:?}"); - colour_info = Some(colr); + skip_box_remain(&mut b)?; + } else { + colr_types_seen.push(colour_type.clone())?; + if colour_info.is_some() { + debug!("Already have colour info, skipping {colour_type:?} colr box"); + skip_box_remain(&mut b)?; + } else if let ParsedColourInformation::Supported(colr) = + read_colr(&mut b, colour_type.value, strictness)? + { + debug!("Parsed colr box: {colr:?}"); + colour_info = Some(colr); + } } } BoxType::MasteringDisplayColourVolumeBox => { diff --git a/mp4parse/tests/public.rs b/mp4parse/tests/public.rs index 98b4678d..632deabe 100644 --- a/mp4parse/tests/public.rs +++ b/mp4parse/tests/public.rs @@ -237,6 +237,15 @@ static VIDEO_MP4V_MP4: &str = "tests/bbb_sunflower_QCIF_30fps_mp4v_noaudio_1f.mp // ffmpeg -f lavfi -i color=c=white:s=640x480 -c:v libx264 -frames:v 1 -pix_fmt yuv420p -vf "setsar=16/9" h264_white_frame_sar_16_9.mp4 static VIDEO_H264_PASP_MP4: &str = "tests/h264_white_frame_sar_16_9.mp4"; +// An h264 video sample entry with two nclx colr boxes (HLG transfer=18 followed +// by a BT.2020 transfer=14 fallback), the backward-compatible HLG signaling +// convention. Copy of mp4parse_capi/tests/video_colr_nclx_two_colr.mp4. +static VIDEO_TWO_NCLX_COLR_MP4: &str = "tests/video_colr_nclx_two_colr.mp4"; +// As above, but with the second colr box's colour_type patched from nclx to +// rICC, giving one colr box of each colour_type as intended by +// ISOBMFF (ISO 14496-12:2020) § 12.1.5. +static VIDEO_NCLX_AND_ICC_COLR_MP4: &str = "tests/video_colr_nclx_and_icc.mp4"; + // Adapted from https://github.com/GuillaumeGomez/audio-video-metadata/blob/9dff40f565af71d5502e03a2e78ae63df95cfd40/src/metadata.rs#L53 #[test] fn public_api() { @@ -890,6 +899,59 @@ fn public_mp4_ctts_overflow() { ); } +fn assert_video_nclx_colour_info( + result: mp4::Result, + expected_transfer_characteristics: u8, +) { + let context = result.expect("read_mp4 failed"); + let track = context.tracks.first().expect("expected a track"); + let stsd = track.stsd.as_ref().expect("expected an stsd"); + let v = match stsd.descriptions.first().expect("expected a SampleEntry") { + mp4::SampleEntry::Video(v) => v, + _ => panic!("expected a VideoSampleEntry"), + }; + match &v.colour_info { + Some(mp4::ColourInformation::Nclx(nclx)) => { + assert_eq!( + nclx.transfer_characteristics, + expected_transfer_characteristics + ); + } + other => panic!("expected nclx colour info, got {:?}", other), + } +} + +/// Two colr boxes with the same colour_type in a video sample entry are +/// accepted (keeping the first) except under Strict parsing, which yields +/// ColrBadQuantityBMFF; see ISOBMFF (ISO 14496-12:2020) § 12.1.5. +#[test] +fn public_video_two_nclx_colr_boxes() { + for strictness in [ParseStrictness::Permissive, ParseStrictness::Normal] { + let input = &mut File::open(VIDEO_TWO_NCLX_COLR_MP4).expect("Unknown file"); + assert_video_nclx_colour_info(mp4::read_mp4(input, strictness), 18); + } + let input = &mut File::open(VIDEO_TWO_NCLX_COLR_MP4).expect("Unknown file"); + assert_eq!( + Status::from(mp4::read_mp4(input, ParseStrictness::Strict)), + Status::ColrBadQuantityBMFF + ); +} + +/// One colr box of each colour_type (nclx and rICC) in a video sample entry +/// is valid per ISOBMFF (ISO 14496-12:2020) § 12.1.5 and accepted at all +/// strictness levels, surfacing the first box. +#[test] +fn public_video_nclx_and_icc_colr_boxes() { + for strictness in [ + ParseStrictness::Permissive, + ParseStrictness::Normal, + ParseStrictness::Strict, + ] { + let input = &mut File::open(VIDEO_NCLX_AND_ICC_COLR_MP4).expect("Unknown file"); + assert_video_nclx_colour_info(mp4::read_mp4(input, strictness), 18); + } +} + #[test] fn public_avif_primary_item() { let input = &mut File::open(IMAGE_AVIF).expect("Unknown file"); diff --git a/mp4parse/tests/video_colr_nclx_and_icc.mp4 b/mp4parse/tests/video_colr_nclx_and_icc.mp4 new file mode 100644 index 00000000..90b033af Binary files /dev/null and b/mp4parse/tests/video_colr_nclx_and_icc.mp4 differ diff --git a/mp4parse/tests/video_colr_nclx_two_colr.mp4 b/mp4parse/tests/video_colr_nclx_two_colr.mp4 new file mode 100644 index 00000000..23d1f715 Binary files /dev/null and b/mp4parse/tests/video_colr_nclx_two_colr.mp4 differ