Fix: Obtain aom_image metadata on decode
Fixes reported issues oss-fuzz:19118, oss-fuzz:19157 and decoder
conformance test vector failure.
Summary:
- Added aom_img_get_metadata() function so user can obtain metadata
associated with an aom_image.
- Added aom_img_num_metadata() function so user can get the number of
metadata blocks associated with an aom_image.
- Added image metadata array to AV1Decoder so it can be obtained during
decoding process.
- Implemented read_metadata_itut_t35() to parse ITU-T T.35 metadata from
stream to metadata array.
- Added necessary tests for these new functions.
- Only ITU-T T.35 metadata decoding is implemented in this CL.
BUG=aomedia:2507
Change-Id: I33f32b3e59d25630bd7f5c3eb1791fc8ffee384c
diff --git a/aom/aom_image.h b/aom/aom_image.h
index 863277e..14603a6 100644
--- a/aom/aom_image.h
+++ b/aom/aom_image.h
@@ -30,7 +30,7 @@
* types, removing or reassigning enums, adding/removing/rearranging
* fields to structures
*/
-#define AOM_IMAGE_ABI_VERSION (6) /**<\hideinitializer*/
+#define AOM_IMAGE_ABI_VERSION (7) /**<\hideinitializer*/
#define AOM_IMG_FMT_PLANAR 0x100 /**< Image is a planar format. */
#define AOM_IMG_FMT_UV_FLIP 0x200 /**< V plane precedes U in memory. */
@@ -351,6 +351,33 @@
int aom_img_add_metadata(aom_image_t *img, uint32_t type, const uint8_t *data,
size_t sz);
+/*!\brief Return a metadata payload stored within the image metadata array.
+ *
+ * Gets the metadata (aom_metadata_t) at the indicated index in the image
+ * metadata array.
+ *
+ * \param[in] img Pointer to image descriptor to get metadata from
+ * \param[in] index Metadata index to get from metadata array
+ *
+ * \return Returns a const pointer to the selected metadata, if img and/or index
+ * is invalid, it returns NULL.
+ */
+const aom_metadata_t *aom_img_get_metadata(const aom_image_t *img,
+ size_t index);
+
+/*!\brief Return the number of metadata blocks within the image.
+ *
+ * Gets the number of metadata blocks contained within the provided image
+ * metadata array.
+ *
+ * \param[in] img Pointer to image descriptor to get metadata number
+ * from.
+ *
+ * \return Returns the size of the metadata array. If img or metadata is NULL,
+ * it returns 0.
+ */
+size_t aom_img_num_metadata(const aom_image_t *img);
+
/*!\brief Remove metadata from image.
*
* Removes all metadata in image metadata list and sets metadata list pointer
@@ -362,12 +389,9 @@
/*!\brief Allocate memory for aom_metadata struct.
*
- * Allocates memory for aom_metadata struct and sets its type. Optionally
- * allocates storage for the metadata payload and copies the payload data
- * into the aom_metadata struct:
- * - When sz is > 0 and data is NULL, allocates metadata payload buffer of sz.
- * - When sz is > 0 and data is non-NULL, a metadata payload buffer of sz
- * is allocated and sz bytes are copied from data into the payload buffer.
+ * Allocates storage for the metadata payload, sets its type and copies the
+ * payload data into the aom_metadata struct. A metadata payload buffer of size
+ * sz is allocated and sz bytes are copied from data into the payload buffer.
*
* \param[in] type Metadata type
* \param[in] data Metadata data pointer
diff --git a/aom/exports_com b/aom/exports_com
index a192cf9..68dbfe0 100644
--- a/aom/exports_com
+++ b/aom/exports_com
@@ -15,10 +15,12 @@
text aom_img_alloc_with_border
text aom_img_flip
text aom_img_free
+text aom_img_get_metadata
text aom_img_metadata_array_free
text aom_img_metadata_array_alloc
text aom_img_metadata_free
text aom_img_metadata_alloc
+text aom_img_num_metadata
text aom_img_plane_height
text aom_img_plane_width
text aom_img_remove_metadata
diff --git a/aom/src/aom_image.c b/aom/src/aom_image.c
index 5e2edb4..c8676cf 100644
--- a/aom/src/aom_image.c
+++ b/aom/src/aom_image.c
@@ -290,21 +290,17 @@
aom_metadata_t *aom_img_metadata_alloc(uint32_t type, const uint8_t *data,
size_t sz) {
- aom_metadata_t *metadata =
- (aom_metadata_t *)calloc(1, sizeof(aom_metadata_t));
+ if (!data || sz == 0) return NULL;
+ aom_metadata_t *metadata = (aom_metadata_t *)malloc(sizeof(aom_metadata_t));
if (!metadata) return NULL;
metadata->type = type;
- if (sz > 0) {
- metadata->payload = (uint8_t *)calloc(sz, sizeof(uint8_t));
- if (!metadata->payload) {
- free(metadata);
- return NULL;
- }
- if (data) {
- memcpy(metadata->payload, data, sz);
- metadata->sz = sz;
- }
+ metadata->payload = (uint8_t *)malloc(sz);
+ if (!metadata->payload) {
+ free(metadata);
+ return NULL;
}
+ memcpy(metadata->payload, data, sz);
+ metadata->sz = sz;
return metadata;
}
@@ -379,3 +375,18 @@
img->metadata = NULL;
}
}
+
+const aom_metadata_t *aom_img_get_metadata(const aom_image_t *img,
+ size_t index) {
+ if (!img) return NULL;
+ const aom_metadata_array_t *array = img->metadata;
+ if (array && index < array->sz) {
+ return array->metadata_array[index];
+ }
+ return NULL;
+}
+
+size_t aom_img_num_metadata(const aom_image_t *img) {
+ if (!img || !img->metadata) return 0;
+ return img->metadata->sz;
+}
diff --git a/aom_scale/generic/yv12config.c b/aom_scale/generic/yv12config.c
index bb7cae0..b007b7b 100644
--- a/aom_scale/generic/yv12config.c
+++ b/aom_scale/generic/yv12config.c
@@ -301,7 +301,7 @@
if (!ybf || !arr || !arr->metadata_array) return -1;
aom_remove_metadata_from_frame_buffer(ybf);
ybf->metadata = aom_img_metadata_array_alloc(arr->sz);
- if (!ybf->metadata) return 0;
+ if (!ybf->metadata) return -1;
for (size_t i = 0; i < ybf->metadata->sz; i++) {
ybf->metadata->metadata_array[i] = aom_img_metadata_alloc(
arr->metadata_array[i]->type, arr->metadata_array[i]->payload,
diff --git a/av1/av1_dx_iface.c b/av1/av1_dx_iface.c
index 6b06ad4..4a0a217 100644
--- a/av1/av1_dx_iface.c
+++ b/av1/av1_dx_iface.c
@@ -152,6 +152,7 @@
aom_free(ctx->frame_workers);
aom_free(ctx->buffer_pool);
+ aom_img_free(&ctx->img);
aom_free(ctx);
return AOM_CODEC_OK;
}
@@ -761,6 +762,15 @@
return grain_img;
}
+// Copies and clears the metadata from AV1Decoder.
+static void move_decoder_metadata_to_img(AV1Decoder *pbi, aom_image_t *img) {
+ if (pbi->metadata && img) {
+ assert(!img->metadata);
+ img->metadata = pbi->metadata;
+ pbi->metadata = NULL;
+ }
+}
+
static aom_image_t *decoder_get_frame(aom_codec_alg_priv_t *ctx,
aom_codec_iter_t *iter) {
aom_image_t *img = NULL;
@@ -800,12 +810,15 @@
RefCntBuffer *const output_frame_buf = pbi->output_frames[*index];
ctx->last_show_frame = output_frame_buf;
if (ctx->need_resync) return NULL;
+ aom_img_remove_metadata(&ctx->img);
yuvconfig2image(&ctx->img, sd, frame_worker_data->user_priv);
+ move_decoder_metadata_to_img(pbi, &ctx->img);
if (!pbi->ext_tile_debug && cm->large_scale_tile) {
*index += 1; // Advance the iterator to point to the next image
-
+ aom_img_remove_metadata(&ctx->img);
yuvconfig2image(&ctx->img, &pbi->tile_list_outbuf, NULL);
+ move_decoder_metadata_to_img(pbi, &ctx->img);
img = &ctx->img;
return img;
}
diff --git a/av1/av1_iface_common.h b/av1/av1_iface_common.h
index f87fd1b..9b5ffcb 100644
--- a/av1/av1_iface_common.h
+++ b/av1/av1_iface_common.h
@@ -11,6 +11,8 @@
#ifndef AOM_AV1_AV1_IFACE_COMMON_H_
#define AOM_AV1_AV1_IFACE_COMMON_H_
+#include <assert.h>
+
#include "aom_ports/mem.h"
#include "aom_scale/yv12config.h"
@@ -74,7 +76,8 @@
img->img_data_owner = 0;
img->self_allocd = 0;
img->sz = yv12->frame_size;
- img->metadata = yv12->metadata;
+ assert(!yv12->metadata);
+ img->metadata = NULL;
}
static aom_codec_err_t image2yuvconfig(const aom_image_t *img,
diff --git a/av1/decoder/decoder.c b/av1/decoder/decoder.c
index c2b84d4..574b30c 100644
--- a/av1/decoder/decoder.c
+++ b/av1/decoder/decoder.c
@@ -247,7 +247,7 @@
aom_accounting_clear(&pbi->accounting);
#endif
av1_free_mc_tmp_buf(&pbi->td);
-
+ aom_img_metadata_array_free(pbi->metadata);
aom_free(pbi);
}
diff --git a/av1/decoder/decoder.h b/av1/decoder/decoder.h
index 6df582d..25f2ac5 100644
--- a/av1/decoder/decoder.h
+++ b/av1/decoder/decoder.h
@@ -243,6 +243,7 @@
#endif
AV1DecRowMTInfo frame_row_mt_info;
+ aom_metadata_array_t *metadata;
} AV1Decoder;
// Returns 0 on success. Sets pbi->common.error.error_code to a nonzero error
diff --git a/av1/decoder/obu.c b/av1/decoder/obu.c
index c725db1..56ac72c 100644
--- a/av1/decoder/obu.c
+++ b/av1/decoder/obu.c
@@ -548,32 +548,60 @@
return tile_list_payload_size;
}
-// Reads the country code as specified in Recommendation ITU-T T.35. On
-// success, returns the number of bytes read from 'data'. On failure, calls
+// Returns the last nonzero byte index in 'data'. If there is no nonzero byte in
+// 'data', returns -1.
+static int get_last_nonzero_byte_index(const uint8_t *data, size_t sz) {
+ // Scan backward and return on the first nonzero byte.
+ int i = (int)sz - 1;
+ while (i >= 0 && data[i] == 0) {
+ --i;
+ }
+ return i;
+}
+
+// On success, returns the number of bytes read from 'data'. On failure, calls
// aom_internal_error() and does not return.
-//
-// Note: This function does not read itu_t_t35_payload_bytes because the exact
-// syntax of itu_t_t35_payload_bytes is not defined in the spec.
-static size_t read_metadata_itut_t35(AV1_COMMON *const cm, const uint8_t *data,
+static size_t read_metadata_itut_t35(AV1Decoder *const pbi, const uint8_t *data,
size_t sz) {
- size_t i = 0;
- // itu_t_t35_country_code f(8)
- if (i >= sz) {
+ AV1_COMMON *const cm = &pbi->common;
+ if (sz == 0) {
aom_internal_error(&cm->error, AOM_CODEC_CORRUPT_FRAME,
"itu_t_t35_country_code is missing");
}
- const int itu_t_t35_country_code = data[i];
- ++i;
- if (itu_t_t35_country_code == 0xFF) {
- // itu_t_t35_country_code_extension_byte f(8)
- if (i >= sz) {
- aom_internal_error(&cm->error, AOM_CODEC_CORRUPT_FRAME,
- "itu_t_t35_country_code_extension_byte is missing");
- }
- ++i;
+ if (*data == 0xFF && sz < 2) {
+ aom_internal_error(&cm->error, AOM_CODEC_CORRUPT_FRAME,
+ "itu_t_t35_country_code_extension_byte is missing");
}
- // itu_t_t35_payload_bytes
- return i;
+ int bytes_read = get_last_nonzero_byte_index(data, sz);
+ if (bytes_read < 0) {
+ aom_internal_error(&cm->error, AOM_CODEC_CORRUPT_FRAME,
+ "No trailing bits found on metadata");
+ }
+ aom_metadata_t *metadata = aom_img_metadata_alloc(OBU_METADATA_TYPE_ITUT_T35,
+ data, (size_t)bytes_read);
+ if (!metadata) {
+ aom_internal_error(&cm->error, AOM_CODEC_MEM_ERROR,
+ "Error allocating metadata");
+ }
+ if (!pbi->metadata) {
+ pbi->metadata = aom_img_metadata_array_alloc(1);
+ if (!pbi->metadata) {
+ aom_internal_error(&cm->error, AOM_CODEC_MEM_ERROR,
+ "Failed to allocate metadata array");
+ }
+ } else {
+ aom_metadata_t **metadata_array =
+ (aom_metadata_t **)realloc(pbi->metadata->metadata_array,
+ (pbi->metadata->sz + 1) * sizeof(metadata));
+ if (!metadata_array) {
+ aom_internal_error(&cm->error, AOM_CODEC_MEM_ERROR,
+ "Error allocating metadata");
+ }
+ pbi->metadata->metadata_array = metadata_array;
+ pbi->metadata->sz++;
+ }
+ pbi->metadata->metadata_array[pbi->metadata->sz - 1] = metadata;
+ return (size_t)bytes_read;
}
static void read_metadata_hdr_cll(struct aom_read_bit_buffer *rb) {
@@ -709,7 +737,7 @@
if (metadata_type == OBU_METADATA_TYPE_ITUT_T35) {
size_t bytes_read =
type_length +
- read_metadata_itut_t35(cm, data + type_length, sz - type_length);
+ read_metadata_itut_t35(pbi, data + type_length, sz - type_length);
// Ignore itu_t_t35_payload_bytes and check trailing bits. Section 6.7.2
// of the spec says:
// itu_t_t35_payload_bytes shall be bytes containing data registered as
diff --git a/test/metadata_test.cc b/test/metadata_test.cc
index bd8a1dd..6aeebdd 100644
--- a/test/metadata_test.cc
+++ b/test/metadata_test.cc
@@ -24,7 +24,9 @@
namespace {
const size_t kExampleDataSize = 10;
-const uint8_t kExampleData[kExampleDataSize] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+// 0xB5 stands for the itut t35 metadata country code for the Unites States
+const uint8_t kExampleData[kExampleDataSize] = { 0xB5, 0x01, 0x02, 0x03, 0x04,
+ 0x05, 0x06, 0x07, 0x08, 0x09 };
#if CONFIG_AV1_ENCODER
@@ -46,6 +48,21 @@
if (current_frame) {
if (current_frame->metadata) aom_img_remove_metadata(current_frame);
ASSERT_EQ(aom_img_add_metadata(current_frame, OBU_METADATA_TYPE_ITUT_T35,
+ kExampleData, 0),
+ -1);
+ ASSERT_EQ(aom_img_add_metadata(current_frame, OBU_METADATA_TYPE_ITUT_T35,
+ NULL, kExampleDataSize),
+ -1);
+ ASSERT_EQ(aom_img_add_metadata(current_frame, OBU_METADATA_TYPE_ITUT_T35,
+ NULL, 0),
+ -1);
+ ASSERT_EQ(aom_img_add_metadata(current_frame, OBU_METADATA_TYPE_ITUT_T35,
+ kExampleData, kExampleDataSize),
+ 0);
+
+ // The duplicate statement is deliberate to test multiple metadata objects
+ // per image.
+ ASSERT_EQ(aom_img_add_metadata(current_frame, OBU_METADATA_TYPE_ITUT_T35,
kExampleData, kExampleDataSize),
0);
}
@@ -54,23 +71,42 @@
virtual void FramePktHook(const aom_codec_cx_pkt_t *pkt) {
if (pkt->kind == AOM_CODEC_CX_FRAME_PKT) {
const size_t metadata_obu_size = 14;
- const uint8_t metadata_obu[metadata_obu_size] = {
- 42, 12, 4, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 128
- };
+ const uint8_t metadata_obu[metadata_obu_size] = { 0x2A, 0x0C, 0x04, 0xB5,
+ 0x01, 0x02, 0x03, 0x04,
+ 0x05, 0x06, 0x07, 0x08,
+ 0x09, 0x80 };
const size_t bitstream_size = pkt->data.frame.sz;
const uint8_t *bitstream =
static_cast<const uint8_t *>(pkt->data.frame.buf);
- // look for expected metadata in bitstream
- bool found_metadata = false;
- for (size_t i = 0; i < bitstream_size; ++i) {
- if (memcmp(bitstream + i, metadata_obu, metadata_obu_size) == 0) {
- found_metadata = true;
- break;
+ // look for valid metadatas in bitstream
+ size_t valid_metadatas_found = 0;
+ if (bitstream_size >= metadata_obu_size) {
+ for (size_t i = 0; i <= bitstream_size - metadata_obu_size; ++i) {
+ if (memcmp(bitstream + i, metadata_obu, metadata_obu_size) == 0) {
+ valid_metadatas_found++;
+ }
}
}
- EXPECT_TRUE(found_metadata);
+ ASSERT_EQ(valid_metadatas_found, 2u);
}
}
+
+ virtual void DecompressedFrameHook(const aom_image_t &img,
+ aom_codec_pts_t /*pts*/) {
+ ASSERT_TRUE(img.metadata != nullptr);
+
+ ASSERT_EQ(img.metadata->sz, 2u);
+
+ ASSERT_EQ(kExampleDataSize, img.metadata->metadata_array[0]->sz);
+ EXPECT_EQ(memcmp(kExampleData, img.metadata->metadata_array[0]->payload,
+ kExampleDataSize),
+ 0);
+
+ ASSERT_EQ(kExampleDataSize, img.metadata->metadata_array[1]->sz);
+ EXPECT_EQ(memcmp(kExampleData, img.metadata->metadata_array[1]->payload,
+ kExampleDataSize),
+ 0);
+ }
};
TEST_P(MetadataEncodeTest, TestMetadataEncoding) {
@@ -181,3 +217,54 @@
aom_remove_metadata_from_frame_buffer(&yvBuf);
aom_remove_metadata_from_frame_buffer(NULL);
}
+
+TEST(MetadataTest, GetMetadataFromImage) {
+ aom_image_t image;
+ image.metadata = NULL;
+
+ ASSERT_EQ(aom_img_add_metadata(&image, OBU_METADATA_TYPE_ITUT_T35,
+ kExampleData, kExampleDataSize),
+ 0);
+
+ EXPECT_TRUE(aom_img_get_metadata(NULL, 0) == NULL);
+ EXPECT_TRUE(aom_img_get_metadata(&image, 1u) == NULL);
+ EXPECT_TRUE(aom_img_get_metadata(&image, 10u) == NULL);
+
+ const aom_metadata_t *metadata = aom_img_get_metadata(&image, 0);
+ ASSERT_TRUE(metadata != NULL);
+ ASSERT_EQ(metadata->sz, kExampleDataSize);
+ EXPECT_EQ(memcmp(kExampleData, metadata->payload, kExampleDataSize), 0);
+
+ aom_img_metadata_array_free(image.metadata);
+}
+
+TEST(MetadataTest, ReadMetadatasFromImage) {
+ aom_image_t image;
+ image.metadata = NULL;
+
+ uint32_t types[3];
+ types[0] = OBU_METADATA_TYPE_ITUT_T35;
+ types[1] = OBU_METADATA_TYPE_HDR_CLL;
+ types[2] = OBU_METADATA_TYPE_HDR_MDCV;
+
+ ASSERT_EQ(
+ aom_img_add_metadata(&image, types[0], kExampleData, kExampleDataSize),
+ 0);
+ ASSERT_EQ(
+ aom_img_add_metadata(&image, types[1], kExampleData, kExampleDataSize),
+ 0);
+ ASSERT_EQ(
+ aom_img_add_metadata(&image, types[2], kExampleData, kExampleDataSize),
+ 0);
+
+ size_t number_metadata = aom_img_num_metadata(&image);
+ ASSERT_EQ(number_metadata, 3u);
+ for (size_t i = 0; i < number_metadata; ++i) {
+ const aom_metadata_t *metadata = aom_img_get_metadata(&image, i);
+ ASSERT_TRUE(metadata != NULL);
+ ASSERT_EQ(metadata->type, types[i]);
+ ASSERT_EQ(metadata->sz, kExampleDataSize);
+ EXPECT_EQ(memcmp(kExampleData, metadata->payload, kExampleDataSize), 0);
+ }
+ aom_img_metadata_array_free(image.metadata);
+}
