Communicate more detail about what specifically is unsupported across FFI

This is in support of https://bugzilla.mozilla.org/show_bug.cgi?id=1696045

Additionally, add more tests and do some cleanup
This commit is contained in:
Jon Bauman 2021-08-07 17:43:13 -07:00
Родитель 5326af6b54
Коммит 6d4ed9b69c
13 изменённых файлов: 258 добавлений и 83 удалений

@ -1 +1 @@
Subproject commit 9e32b4f7b3bce2e57d9c84816c3dc5b64027273a
Subproject commit 7577f3a202c6d7754408ae09b4eca67cc1840736

Просмотреть файл

@ -126,6 +126,9 @@ box_database!(
CleanApertureBox 0x636c_6170, // "clap"
ImageRotation 0x6972_6f74, // "irot"
ImageMirror 0x696d_6972, // "imir"
OperatingPointSelectorProperty 0x6131_6f70, // "a1op"
AV1LayeredImageIndexingProperty 0x6131_6c78, // "a1lx"
LayerSelectorProperty 0x6c73_656c, // "lsel"
SampleTableBox 0x7374_626c, // "stbl"
SampleDescriptionBox 0x7374_7364, // "stsd"
TimeToSampleBox 0x7374_7473, // "stts"

Просмотреть файл

@ -148,6 +148,87 @@ struct HashMap;
#[allow(dead_code)]
struct String;
/// The return value to the C API
/// Any detail that needs to be communicated to the caller must be encoded here
/// since the [`Error`] type's associated data is part of the FFI.
#[repr(C)]
#[derive(PartialEq, Debug)]
pub enum Status {
Ok = 0,
BadArg = 1,
Invalid = 2,
Unsupported = 3,
Eof = 4,
Io = 5,
Oom = 6,
UnsupportedA1lx,
UnsupportedA1op,
UnsupportedClap,
UnsupportedGrid,
UnsupportedIpro,
UnsupportedLsel,
}
/// For convenience of creating an error for an unsupported feature which we
/// want to communicate the specific feature back to the C API caller
impl From<Status> for Error {
fn from(parse_status: Status) -> Self {
let msg = match parse_status {
Status::Ok
| Status::BadArg
| Status::Invalid
| Status::Unsupported
| Status::Eof
| Status::Io
| Status::Oom => {
panic!("Status -> Error is only for Status:UnsupportedXXX errors")
}
Status::UnsupportedA1lx => "AV1 layered image indexing (a1lx) is unsupported",
Status::UnsupportedA1op => "Operating point selection (a1op) is unsupported",
Status::UnsupportedClap => "Clean aperture (clap) transform is unsupported",
Status::UnsupportedGrid => "Grid-based images are unsupported",
Status::UnsupportedIpro => "Item protection (ipro) is unsupported",
Status::UnsupportedLsel => "Layer selection (lsel) is unsupported",
};
Self::UnsupportedDetail(parse_status, msg)
}
}
impl From<Error> for Status {
fn from(error: Error) -> Self {
match error {
Error::NoMoov | Error::InvalidData(_) => Self::Invalid,
Error::Unsupported(_) => Self::Unsupported,
Error::UnsupportedDetail(parse_status, _msg) => parse_status,
Error::UnexpectedEOF => Self::Eof,
Error::Io(_) => {
// Getting std::io::ErrorKind::UnexpectedEof is normal
// but our From trait implementation should have converted
// those to our Error::UnexpectedEOF variant.
Self::Io
}
Error::OutOfMemory => Self::Oom,
}
}
}
impl From<Result<(), Status>> for Status {
fn from(result: Result<(), Status>) -> Self {
match result {
Ok(()) => Status::Ok,
Err(Status::Ok) => unreachable!(),
Err(e) => e,
}
}
}
impl From<fallible_collections::TryReserveError> for Status {
fn from(_: fallible_collections::TryReserveError) -> Self {
Status::Oom
}
}
/// Describes parser failures.
///
/// This enum wraps the standard `io::Error` type, unified with
@ -158,6 +239,10 @@ pub enum Error {
InvalidData(&'static str),
/// Parse error caused by limited parser support rather than invalid data.
Unsupported(&'static str),
/// Similar to [`Self::Unsupported`], but for errors that have a specific
/// [`Status`] variant for communicating the detail across FFI.
/// See the helper [`From<Status> for Error`](enum.Error.html#impl-From<Status>)
UnsupportedDetail(Status, &'static str),
/// Reflect `std::io::ErrorKind::UnexpectedEof` for short data.
UnexpectedEOF,
/// Propagate underlying errors from `std::io`.
@ -1850,9 +1935,15 @@ fn read_avif_meta<T: Read + Offset>(
let item_infos = item_infos.ok_or(Error::InvalidData("iinf missing"))?;
if let Some(item_info) = item_infos.iter().find(|x| x.item_id == primary_item_id) {
if &item_info.item_type.to_be_bytes() != b"av01" {
warn!("primary_item_id type: {}", U32BE(item_info.item_type));
return Err(Error::InvalidData("primary_item_id type is not av01"));
debug!("primary_item_id type: {}", U32BE(item_info.item_type));
match &item_info.item_type.to_be_bytes() {
b"av01" => {}
b"grid" => return Err(Error::from(Status::UnsupportedGrid)),
_ => {
return Err(Error::InvalidData(
"primary_item_id type is neither 'av01' nor 'grid'",
))
}
}
} else {
return Err(Error::InvalidData(
@ -1957,9 +2048,7 @@ fn read_infe<T: Read>(src: &mut BMFFBox<T>, strictness: ParseStrictness) -> Resu
let item_protection_index = be_u16(src)?;
if item_protection_index != 0 {
return Err(Error::Unsupported(
"protected items (infe.item_protection_index != 0) are not supported",
));
return Err(Error::from(Status::UnsupportedIpro));
}
let item_type = be_u32(src)?;
@ -2102,6 +2191,9 @@ fn read_iprp<T: Read>(
| ItemProperty::Rotation(_) => {
if !a.essential {
warn!("{:?} is invalid", property);
// This is a "shall", but it is likely to change, so only
// fail if using strict parsing.
// See https://github.com/mozilla/mp4parse-rust/issues/284
fail_if(
strictness == ParseStrictness::Strict,
"All transformative properties associated with coded and \
@ -2525,6 +2617,11 @@ fn read_ipma<T: Read>(
/// Parse an ItemPropertyContainerBox
///
/// For unsupported properties that we know about, return specific
/// [`Status`] UnsupportedXXXX variants. Unless running in
/// [`ParseStrictness::Permissive`] mode, in which case, unsupported properties
/// will be ignored.
///
/// See ISOBMFF (ISO 14496-12:2020 § 8.11.14.1
fn read_ipco<T: Read>(
src: &mut BMFFBox<T>,
@ -2538,6 +2635,14 @@ fn read_ipco<T: Read>(
if let Some(property) = match b.head.name {
BoxType::AuxiliaryTypeProperty => Some(ItemProperty::AuxiliaryType(read_auxc(&mut b)?)),
BoxType::AV1CodecConfigurationBox => Some(ItemProperty::AV1Config(read_av1c(&mut b)?)),
BoxType::AV1LayeredImageIndexingProperty
if strictness != ParseStrictness::Permissive =>
{
return Err(Error::from(Status::UnsupportedA1lx))
}
BoxType::CleanApertureBox if strictness != ParseStrictness::Permissive => {
return Err(Error::from(Status::UnsupportedClap))
}
BoxType::ColourInformationBox => {
Some(ItemProperty::Colour(read_colr(&mut b, strictness)?))
}
@ -2546,6 +2651,14 @@ fn read_ipco<T: Read>(
BoxType::ImageSpatialExtentsProperty => {
Some(ItemProperty::ImageSpatialExtents(read_ispe(&mut b)?))
}
BoxType::LayerSelectorProperty if strictness != ParseStrictness::Permissive => {
return Err(Error::from(Status::UnsupportedLsel))
}
BoxType::OperatingPointSelectorProperty
if strictness != ParseStrictness::Permissive =>
{
return Err(Error::from(Status::UnsupportedA1op))
}
BoxType::PixelInformationBox => Some(ItemProperty::Channels(read_pixi(&mut b)?)),
other_box_type => {
// Though we don't do anything with other property types, we still store

Просмотреть файл

Просмотреть файл

@ -9,7 +9,6 @@ use mp4::ParseStrictness;
use std::convert::TryInto;
use std::fs::File;
use std::io::{Cursor, Read, Seek, SeekFrom};
use std::path::Path;
static MINI_MP4: &str = "tests/minimal.mp4";
static MINI_MP4_WITH_METADATA: &str = "tests/metadata.mp4";
@ -38,8 +37,8 @@ static IMAGE_AVIF_ALPHA: &str = "tests/valid-alpha.avif";
static IMAGE_AVIF_ALPHA_PREMULTIPLIED: &str = "tests/1x1-black-alpha-50pct-premultiplied.avif";
static IMAGE_AVIF_CORRUPT: &str = "tests/corrupt/bug-1655846.avif";
static IMAGE_AVIF_CORRUPT_2: &str = "tests/corrupt/bug-1661347.avif";
static IMAGE_AVIF_IPMA_BAD_VERSION: &str = "tests/corrupt/bad-ipma-version.avif";
static IMAGE_AVIF_IPMA_BAD_FLAGS: &str = "tests/corrupt/bad-ipma-flags.avif";
static IMAGE_AVIF_IPMA_BAD_VERSION: &str = "tests/bad-ipma-version.avif";
static IMAGE_AVIF_IPMA_BAD_FLAGS: &str = "tests/bad-ipma-flags.avif";
static IMAGE_AVIF_IPMA_DUPLICATE_VERSION_AND_FLAGS: &str =
"tests/corrupt/ipma-duplicate-version-and-flags.avif";
static IMAGE_AVIF_IPMA_DUPLICATE_ITEM_ID: &str = "tests/corrupt/ipma-duplicate-item_id.avif";
@ -48,24 +47,47 @@ static IMAGE_AVIF_IPMA_INVALID_PROPERTY_INDEX: &str =
static IMAGE_AVIF_NO_HDLR: &str = "tests/corrupt/hdlr-not-first.avif";
static IMAGE_AVIF_HDLR_NOT_FIRST: &str = "tests/corrupt/no-hdlr.avif";
static IMAGE_AVIF_HDLR_NOT_PICT: &str = "tests/corrupt/hdlr-not-pict.avif";
static IMAGE_AVIF_NO_MIF1: &str = "tests/corrupt/no-mif1.avif";
static IMAGE_AVIF_NO_MIF1: &str = "tests/no-mif1.avif";
static IMAGE_AVIF_NO_PIXI: &str = "tests/corrupt/no-pixi.avif";
static IMAGE_AVIF_NO_AV1C: &str = "tests/corrupt/no-av1C.avif";
static IMAGE_AVIF_NO_ISPE: &str = "tests/corrupt/no-ispe.avif";
static IMAGE_AVIF_NO_ALPHA_AV1C: &str = "tests/corrupt/no-alpha-av1C.avif";
static IMAGE_AVIF_NO_ALPHA_PIXI: &str = "tests/corrupt/no-pixi-for-alpha.avif";
static IMAGE_AVIF_AV1C_MISSING_ESSENTIAL: &str = "tests/corrupt/av1C-missing-essential.avif";
static IMAGE_AVIF_IMIR_MISSING_ESSENTIAL: &str = "tests/corrupt/imir-missing-essential.avif";
static IMAGE_AVIF_IROT_MISSING_ESSENTIAL: &str = "tests/corrupt/irot-missing-essential.avif";
static IMAGE_AVIF_GRID: &str = "av1-avif/testFiles/Microsoft/Summer_in_Tomsk_720p_5x4_grid.avif";
static IMAGE_AVIF_AV1C_MISSING_ESSENTIAL: &str = "tests/av1C-missing-essential.avif";
static IMAGE_AVIF_IMIR_MISSING_ESSENTIAL: &str = "tests/imir-missing-essential.avif";
static IMAGE_AVIF_IROT_MISSING_ESSENTIAL: &str = "tests/irot-missing-essential.avif";
static AVIF_TEST_DIRS: &[&str] = &["tests", "av1-avif/testFiles"];
// TODO: file bug on https://github.com/AOMediaCodec/av1-avif
static AVIF_A1OP: &str =
"av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_a1op.avif";
static AVIF_A1LX: &str =
"av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_grid_a1lx.avif";
static AVIF_CLAP: &str = "tests/clap-basic-1_3x3-to-1x1.avif";
static AVIF_GRID: &str = "av1-avif/testFiles/Microsoft/Summer_in_Tomsk_720p_5x4_grid.avif";
static AVIF_LSEL: &str =
"av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_lsel.avif";
static AVIF_UNSUPPORTED_IMAGES: &[&str] = &[
AVIF_A1OP,
AVIF_A1LX,
AVIF_CLAP,
AVIF_GRID,
AVIF_LSEL,
"av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_a1lx.avif",
"av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_a1op_lsel.avif",
"av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_grid_lsel.avif",
"av1-avif/testFiles/Xiph/abandoned_filmgrain.avif",
"av1-avif/testFiles/Xiph/fruits_2layer_thumbsize.avif",
"av1-avif/testFiles/Xiph/quebec_3layer_op2.avif",
"av1-avif/testFiles/Xiph/tiger_3layer_1res.avif",
"av1-avif/testFiles/Xiph/tiger_3layer_3res.avif",
];
/// See https://github.com/AOMediaCodec/av1-avif/issues/150
static AV1_AVIF_CORRUPT_IMAGES: &[&str] = &[
"av1-avif/testFiles/Microsoft/Chimera_10bit_cropped_to_1920x1008.avif",
"av1-avif/testFiles/Microsoft/Chimera_10bit_cropped_to_1920x1008_with_HDR_metadata.avif",
"av1-avif/testFiles/Microsoft/Chimera_8bit_cropped_480x256.avif",
"av1-avif/testFiles/Link-U/kimono.crop.avif",
"av1-avif/testFiles/Link-U/kimono.mirror-vertical.rotate270.crop.avif",
"av1-avif/testFiles/Netflix/avis/Chimera-AV1-10bit-480x270.avif",
];
static AVIF_CORRUPT_IMAGES_DIR: &str = "tests/corrupt";
// The 1 frame h263 3gp file can be generated by ffmpeg with command
@ -790,7 +812,7 @@ fn assert_invalid_data<T: std::fmt::Debug>(result: mp4::Result<T>, expected_msg:
Err(Error::InvalidData(msg)) if msg == expected_msg => {}
Err(Error::InvalidData(msg)) if msg != expected_msg => {
panic!(
"Error message mismtatch\nExpected: {}\nFound: {}",
"Error message mismatch\nExpected: {}\nFound: {}",
expected_msg, msg
);
}
@ -943,12 +965,66 @@ fn public_avif_ispe_present() {
assert_avif_shall(IMAGE_AVIF_NO_ISPE, expected_msg);
}
fn assert_unsupported<T: std::fmt::Debug>(result: mp4::Result<T>, expected_status: mp4::Status) {
match result {
Err(Error::UnsupportedDetail(status, _msg)) if status == expected_status => {}
Err(Error::UnsupportedDetail(status, _msg)) if status != expected_status => {
panic!(
"Error message mismatch\nExpected: {:?}\nFound: {:?}",
expected_status, status
);
}
r => panic!(
"Expected Err({:?}), found {:?}",
Error::from(expected_status),
r
),
}
}
#[test]
#[ignore] // Remove when we add support; see https://github.com/mozilla/mp4parse-rust/issues/198
fn public_avif_primary_item_is_grid() {
let input = &mut File::open(IMAGE_AVIF_GRID).expect("Unknown file");
mp4::read_avif(input, ParseStrictness::Normal).expect("read_avif failed");
// Add some additional checks
fn public_avif_a1lx() {
let input = &mut File::open(AVIF_A1LX).expect("Unknown file");
assert_unsupported(
mp4::read_avif(input, ParseStrictness::Normal),
mp4::Status::UnsupportedA1lx,
);
}
#[test]
fn public_avif_a1op() {
let input = &mut File::open(AVIF_A1OP).expect("Unknown file");
assert_unsupported(
mp4::read_avif(input, ParseStrictness::Normal),
mp4::Status::UnsupportedA1op,
);
}
#[test]
fn public_avif_clap() {
let input = &mut File::open(AVIF_CLAP).expect("Unknown file");
assert_unsupported(
mp4::read_avif(input, ParseStrictness::Normal),
mp4::Status::UnsupportedClap,
);
}
#[test]
fn public_avif_grid() {
let input = &mut File::open(AVIF_GRID).expect("Unknown file");
assert_unsupported(
mp4::read_avif(input, ParseStrictness::Normal),
mp4::Status::UnsupportedGrid,
);
}
#[test]
fn public_avif_lsel() {
let input = &mut File::open(AVIF_LSEL).expect("Unknown file");
assert_unsupported(
mp4::read_avif(input, ParseStrictness::Normal),
mp4::Status::UnsupportedLsel,
);
}
#[test]
@ -962,7 +1038,16 @@ fn public_avif_read_samples_strict() {
public_avif_read_samples_impl(ParseStrictness::Strict);
}
fn to_canonical_paths(strs: &[&str]) -> Vec<std::path::PathBuf> {
strs.iter()
.map(std::fs::canonicalize)
.map(Result::unwrap)
.collect()
}
fn public_avif_read_samples_impl(strictness: ParseStrictness) {
let corrupt_images = to_canonical_paths(AV1_AVIF_CORRUPT_IMAGES);
let unsupported_images = to_canonical_paths(AVIF_UNSUPPORTED_IMAGES);
for dir in AVIF_TEST_DIRS {
for entry in walkdir::WalkDir::new(dir) {
let entry = entry.expect("AVIF entry");
@ -971,21 +1056,33 @@ fn public_avif_read_samples_impl(strictness: ParseStrictness) {
eprintln!("Skipping {:?}", path);
continue; // Skip directories, ReadMe.txt, etc.
}
if path.parent().unwrap() == Path::new(AVIF_CORRUPT_IMAGES_DIR) {
eprintln!("Skipping {:?}", path);
continue;
}
if AV1_AVIF_CORRUPT_IMAGES.contains(&path.to_str().unwrap()) {
eprintln!("Skipping invalid image from av1-avif repo: {:?}", path);
continue;
}
if path == Path::new(IMAGE_AVIF_GRID) {
eprintln!("Skipping {:?}", path);
continue; // Remove when public_avif_primary_item_is_grid passes
}
println!("parsing {:?}", path);
let corrupt = path.canonicalize().unwrap().parent().unwrap()
== std::fs::canonicalize(AVIF_CORRUPT_IMAGES_DIR).unwrap()
|| corrupt_images.contains(&path.canonicalize().unwrap());
let unsupported = unsupported_images.contains(&path.canonicalize().unwrap());
println!(
"parsing {}{}{:?}",
if corrupt { "(corrupt) " } else { "" },
if unsupported { "(unsupported) " } else { "" },
path,
);
let input = &mut File::open(path).expect("Unknow file");
mp4::read_avif(input, strictness).expect("read_avif failed");
match mp4::read_avif(input, strictness) {
Ok(_) if unsupported || corrupt => panic!("Expected error parsing {:?}", path),
Ok(_) => eprintln!("Successfully parsed {:?}", path),
Err(e @ Error::Unsupported(_)) | Err(e @ Error::UnsupportedDetail(..))
if unsupported =>
{
eprintln!(
"Expected error parsing unsupported input {:?}: {:?}",
path, e
)
}
Err(e) if corrupt => {
eprintln!("Expected error parsing corrupt input {:?}: {:?}", path, e)
}
Err(e) => panic!("Unexected error parsing {:?}: {:?}", path, e),
}
}
}
}

Просмотреть файл

@ -25,9 +25,16 @@ rename_variants = "QualifiedScreamingSnakeCase"
parse_deps = true
include = ["mp4parse"]
[export]
# `mp4parse::Status` was previously defined as `mp4parse_capi::Mp4parseStatus`,
# but now is referenced from `mp4parse_capi` via `pub use mp4parse::Status as Mp4parseStatus`,
# the name `Status` does not appear in the public C API, so we must force its inclusion
include = ["Status"]
[export.rename]
# We need to declare these types in mp4parse, but we rename them in the generated
# header to match mp4parse_capi naming conventions
"Status" = "Mp4parseStatus"
"ParseStrictness" = "Mp4parseStrictness"
"ImageSpatialExtentsProperty" = "Mp4parseImageSpatialExtents"
"ImageRotation" = "Mp4parseIrot"

Просмотреть файл

@ -29,7 +29,7 @@ fuzz_target!(|data: &[u8]| {
return;
}
mp4parse_avif_get_image_safe(&*context);
let _ = mp4parse_avif_get_image_safe(&*context);
mp4parse_avif_free(context);
}

Просмотреть файл

@ -54,11 +54,11 @@ use mp4parse::unstable::{
use mp4parse::AudioCodecSpecific;
use mp4parse::AvifContext;
use mp4parse::CodecType;
use mp4parse::Error;
use mp4parse::MediaContext;
// Re-exported so consumers don't have to depend on mp4parse as well
pub use mp4parse::ParseStrictness;
use mp4parse::SampleEntry;
pub use mp4parse::Status as Mp4parseStatus;
use mp4parse::TrackType;
use mp4parse::TryBox;
use mp4parse::TryHashMap;
@ -75,18 +75,6 @@ struct HashMap;
#[allow(dead_code)]
struct String;
#[repr(C)]
#[derive(PartialEq, Debug)]
pub enum Mp4parseStatus {
Ok = 0,
BadArg = 1,
Invalid = 2,
Unsupported = 3,
Eof = 4,
Io = 5,
Oom = 6,
}
#[repr(C)]
#[derive(PartialEq, Debug)]
pub enum Mp4parseTrackType {
@ -518,39 +506,6 @@ fn mp4parse_new_common_safe<T: Read, P: ContextParser>(
.map_err(Mp4parseStatus::from)
}
impl From<mp4parse::Error> for Mp4parseStatus {
fn from(error: mp4parse::Error) -> Self {
match error {
Error::NoMoov | Error::InvalidData(_) => Mp4parseStatus::Invalid,
Error::Unsupported(_) => Mp4parseStatus::Unsupported,
Error::UnexpectedEOF => Mp4parseStatus::Eof,
Error::Io(_) => {
// Getting std::io::ErrorKind::UnexpectedEof is normal
// but our From trait implementation should have converted
// those to our Error::UnexpectedEOF variant.
Mp4parseStatus::Io
}
Error::OutOfMemory => Mp4parseStatus::Oom,
}
}
}
impl From<Result<(), Mp4parseStatus>> for Mp4parseStatus {
fn from(result: Result<(), Mp4parseStatus>) -> Self {
match result {
Ok(()) => Mp4parseStatus::Ok,
Err(Mp4parseStatus::Ok) => unreachable!(),
Err(e) => e,
}
}
}
impl From<fallible_collections::TryReserveError> for Mp4parseStatus {
fn from(_: fallible_collections::TryReserveError) -> Self {
Mp4parseStatus::Oom
}
}
/// Free an `Mp4parseParser*` allocated by `mp4parse_new()`.
///
/// # Safety