Call AvifInfoIdentify() in AvifInfoGetFeatures() Having to call AvifInfoIdentify() before AvifInfoGetFeatures() is counter-intuitive. It was justified by the stream API, to avoid reading the bytes of the 'ftyp' box twice. It is not necessary for the raw pointer API. Matching the requirements of AvifInfoGetFeatures() and AvifInfoGetFeaturesStream() is less important than an easy-to-use, straightforward standalone single call to identify the file type and get the AVIF features. Change-Id: Ib726df5446318ceedbc5b0918e2c3e1b154252b0
diff --git a/avifinfo.c b/avifinfo.c index 977ff82..758130e 100644 --- a/avifinfo.c +++ b/avifinfo.c
@@ -768,6 +768,11 @@ AvifInfoStatus AvifInfoGetFeatures(const uint8_t* data, size_t data_size, AvifInfoFeatures* features) { + const AvifInfoStatus status = AvifInfoIdentify(data, data_size); + if (status != kAvifInfoOk) { + if (features != NULL) memset(features, 0, sizeof(*features)); + return status; + } AvifInfoInternalForward stream; stream.data = data; stream.data_size = data_size;
diff --git a/avifinfo.h b/avifinfo.h index 302bfef..fe8f9ab 100644 --- a/avifinfo.h +++ b/avifinfo.h
@@ -55,15 +55,19 @@ // Use this API if a raw byte array of fixed size is available as input. // Parses the 'data' and returns kAvifInfoOk if it is identified as an AVIF. +// 'data' can be partial but must point to the beginning of the AVIF file. // The file type can be identified in the first 12 bytes of most AVIF files. AvifInfoStatus AvifInfoIdentify(const uint8_t* data, size_t data_size); -// Parses the identified AVIF 'data' and extracts its 'features'. +// Parses the 'data' and returns kAvifInfoOk and its 'features' if it is +// identified as an AVIF file. // 'data' can be partial but must point to the beginning of the AVIF file. // The 'features' can be parsed in the first 450 bytes of most AVIF files. // 'features' are set to 0 unless kAvifInfoOk is returned. -// AvifInfoGetFeatures() should only be called if AvifInfoIdentify() returned -// kAvifInfoOk with the same input bytes. +// There is no need to call AvifInfoIdentify() before AvifInfoGetFeatures(). +// AvifInfoGetFeatures() parses the file further than AvifInfoIdentify() so it +// is possible that AvifInfoGetFeatures() returns errors while +// AvifInfoIdentify() returns kAvifInfoOk on the same given input bytes. AvifInfoStatus AvifInfoGetFeatures(const uint8_t* data, size_t data_size, AvifInfoFeatures* features);
diff --git a/tests/avifinfo_fuzz.cc b/tests/avifinfo_fuzz.cc index c289ceb..a58f1bb 100644 --- a/tests/avifinfo_fuzz.cc +++ b/tests/avifinfo_fuzz.cc
@@ -92,6 +92,9 @@ std::abort(); } } else if (status_features == kAvifInfoOk) { + if (status_identity != kAvifInfoOk) { + std::abort(); + } if (features.width == 0u || features.height == 0u || features.bit_depth == 0u || features.num_channels == 0u || !features.has_gainmap != !features.gainmap_item_id || @@ -123,9 +126,16 @@ const AvifInfoStatus status_features_stream = AvifInfoGetFeaturesStream( &stream_features, StreamRead, StreamSkip, &features_stream); // Both API should have exactly the same behavior, errors included. - if (status_identity_stream != status_identity || - status_features_stream != status_features || - !Equals(features_stream, features)) { + if (status_identity_stream != status_identity) { + std::abort(); + } + // AvifInfoGetFeaturesStream() should only be called if + // AvifInfoIdentifyStream() was a success. Call it anyway to make sure it + // does not crash, but only check the returned status and features when it + // makes sense. + if (status_identity_stream == kAvifInfoOk && + (status_features_stream != status_features || + !Equals(features_stream, features))) { std::abort(); } @@ -136,18 +146,20 @@ const AvifInfoStatus status_features_reused_stream = AvifInfoGetFeaturesStream(&stream_reused, StreamRead, StreamSkip, &features_stream_reused); - // AvifInfoGetFeaturesStream() should only be called on the stream already - // given to AvifInfoIdentifyStream() if AvifInfoIdentifyStream() was a - // success. Call it anyway to make sure it does not crash, but only check - // the returned status and features when it makes sense. - if (status_features_reused_stream == kAvifInfoOk) { - if (status_features != kAvifInfoOk) { + // AvifInfoGetFeaturesStream() should only be called if + // AvifInfoIdentifyStream() was a success. Call it anyway to make sure it + // does not crash, but only check the returned status and features when it + // makes sense. + if (status_identity_stream == kAvifInfoOk) { + if (status_features_reused_stream != status_features_stream) { std::abort(); } - // Offset any location-dependent feature as described in avifinfo.h. - features_stream_reused.primary_item_id_location += - size - stream_identity.data_size; - if (!Equals(features_stream_reused, features)) { + if (status_features_reused_stream == kAvifInfoOk) { + // Offset any location-dependent feature as described in avifinfo.h. + features_stream_reused.primary_item_id_location += + size - stream_identity.data_size; + } + if (!Equals(features_stream_reused, features_stream)) { std::abort(); } }