Clean up aom_image_t metadata allocation functions
aom_img_add_metadata() (a public function) and alloc_read_metadata() (a
static function in av1/decoder/obu.c) both add a new aom_metadata_t to
an aom_metadata_array_t. They differ only in
1) who owns the aom_metadata_array_t (aom_image_t vs. AV1Decoder), and
2) the error reporting method (return value vs. aom_internal_error()).
Make the logic of the two functions as similar as possible. In
particular, aom_img_add_metadata() no longer frees the img->metadata
array on failure. This rewrite also fixes the leak of the old
img->metadata->metadata_array in aom_img_add_metadata() if the realloc()
call fails.
Improve the comments for some aom_image_t metadata allocation functions.
Change-Id: I20fa46934f112b4d0727ad848ac9c8a335183bf9
diff --git a/aom/aom_image.h b/aom/aom_image.h
index bb6973f..03bc73e 100644
--- a/aom/aom_image.h
+++ b/aom/aom_image.h
@@ -360,6 +360,9 @@
* \param[in] data Metadata contents
* \param[in] sz Metadata contents size
* \param[in] insert_flag Metadata insert flag
+ *
+ * \return Returns 0 on success. If img or data is NULL, sz is 0, or memory
+ * allocation fails, it returns -1.
*/
int aom_img_add_metadata(aom_image_t *img, uint32_t type, const uint8_t *data,
size_t sz, aom_metadata_insert_flags_t insert_flag);
@@ -410,6 +413,9 @@
* \param[in] data Metadata data pointer
* \param[in] sz Metadata size
* \param[in] insert_flag Metadata insert flag
+ *
+ * \return Returns the newly allocated aom_metadata struct. If data is NULL,
+ * sz is 0, or memory allocation fails, it returns NULL.
*/
aom_metadata_t *aom_img_metadata_alloc(uint32_t type, const uint8_t *data,
size_t sz,
diff --git a/aom/internal/aom_image_internal.h b/aom/internal/aom_image_internal.h
index 7f2fd18..1b04c9e 100644
--- a/aom/internal/aom_image_internal.h
+++ b/aom/internal/aom_image_internal.h
@@ -32,8 +32,8 @@
/*!\brief Alloc memory for aom_metadata_array struct.
*
* Allocate memory for aom_metadata_array struct.
- * If sz is 0 the aom_metadata_array structs internal buffer list will be NULL,
- * but the aom_metadata_array struct itself will still be allocated.
+ * If sz is 0 the aom_metadata_array struct's internal buffer list will be
+ * NULL, but the aom_metadata_array struct itself will still be allocated.
* Returns a pointer to the allocated struct or NULL on failure.
*
* \param[in] sz Size of internal metadata list buffer
diff --git a/aom/src/aom_image.c b/aom/src/aom_image.c
index cd0b5ed..dfdee87 100644
--- a/aom/src/aom_image.c
+++ b/aom/src/aom_image.c
@@ -350,26 +350,18 @@
}
aom_metadata_t *metadata =
aom_img_metadata_alloc(type, data, sz, insert_flag);
- if (!metadata) goto fail;
- if (!img->metadata->metadata_array) {
- img->metadata->metadata_array =
- (aom_metadata_t **)calloc(1, sizeof(metadata));
- if (!img->metadata->metadata_array || img->metadata->sz != 0) {
- aom_img_metadata_free(metadata);
- goto fail;
- }
- } else {
- img->metadata->metadata_array =
- (aom_metadata_t **)realloc(img->metadata->metadata_array,
- (img->metadata->sz + 1) * sizeof(metadata));
+ if (!metadata) return -1;
+ aom_metadata_t **metadata_array =
+ (aom_metadata_t **)realloc(img->metadata->metadata_array,
+ (img->metadata->sz + 1) * sizeof(metadata));
+ if (!metadata_array) {
+ aom_img_metadata_free(metadata);
+ return -1;
}
+ img->metadata->metadata_array = metadata_array;
img->metadata->metadata_array[img->metadata->sz] = metadata;
img->metadata->sz++;
return 0;
-fail:
- aom_img_metadata_array_free(img->metadata);
- img->metadata = NULL;
- return -1;
}
void aom_img_remove_metadata(aom_image_t *img) {
diff --git a/av1/decoder/obu.c b/av1/decoder/obu.c
index a172e88..2fd06ce 100644
--- a/av1/decoder/obu.c
+++ b/av1/decoder/obu.c
@@ -576,30 +576,30 @@
const uint8_t *data, size_t sz,
aom_metadata_insert_flags_t insert_flag) {
AV1_COMMON *const cm = &pbi->common;
+ if (!pbi->metadata) {
+ pbi->metadata = aom_img_metadata_array_alloc(0);
+ if (!pbi->metadata) {
+ aom_internal_error(&cm->error, AOM_CODEC_MEM_ERROR,
+ "Failed to allocate metadata array");
+ }
+ }
aom_metadata_t *metadata =
aom_img_metadata_alloc(metadata_type, data, sz, insert_flag);
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++;
+ aom_metadata_t **metadata_array =
+ (aom_metadata_t **)realloc(pbi->metadata->metadata_array,
+ (pbi->metadata->sz + 1) * sizeof(metadata));
+ if (!metadata_array) {
+ aom_img_metadata_free(metadata);
+ aom_internal_error(&cm->error, AOM_CODEC_MEM_ERROR,
+ "Error growing metadata array");
}
- pbi->metadata->metadata_array[pbi->metadata->sz - 1] = metadata;
+ pbi->metadata->metadata_array = metadata_array;
+ pbi->metadata->metadata_array[pbi->metadata->sz] = metadata;
+ pbi->metadata->sz++;
}
// On failure, calls aom_internal_error() and does not return.
