Camera: Actually make camera_metadata memcopyable
- Use internal offsets instead of pointers for the entry and data
arrays.
- Add test to verify memcpy works and doesn't alias data.
- Remove support for forward-compatible reserved space handling, given
that it's unlikely to be useful
Bug: 7546079
Change-Id: I439aa27fed8d243b7a04155daf5e58fa1c4c730e
diff --git a/camera/src/Android.mk b/camera/src/Android.mk
index 19a6f5b..d336618 100644
--- a/camera/src/Android.mk
+++ b/camera/src/Android.mk
@@ -17,6 +17,7 @@
LOCAL_CFLAGS += \
-Wall \
-fvisibility=hidden \
+ -std=c99
include $(BUILD_SHARED_LIBRARY)
diff --git a/camera/src/camera_metadata.c b/camera/src/camera_metadata.c
index c922c7f..e04b32c 100644
--- a/camera/src/camera_metadata.c
+++ b/camera/src/camera_metadata.c
@@ -34,7 +34,7 @@
#define DATA_ALIGNMENT _Alignas(camera_metadata_rational_t)
#define ALIGN_TO(val, alignment) \
- (typeof(val))(((uint32_t)(val) + ((alignment) - 1)) & ~((alignment) - 1))
+ (((uint32_t)(val) + ((alignment) - 1)) & ~((alignment) - 1))
/**
* A single metadata entry, storing an array of values of a given type. If the
@@ -93,10 +93,10 @@
uint32_t flags;
size_t entry_count;
size_t entry_capacity;
- camera_metadata_buffer_entry_t *entries;
+ ptrdiff_t entries_start; // Offset from camera_metadata
size_t data_count;
size_t data_capacity;
- uint8_t *data;
+ ptrdiff_t data_start; // Offset from camera_metadata
void *user; // User set pointer, not copied with buffer
uint8_t reserved[0];
};
@@ -134,6 +134,16 @@
[TYPE_RATIONAL] = "rational"
};
+static camera_metadata_buffer_entry_t *get_entries(
+ const camera_metadata_t *metadata) {
+ return (camera_metadata_buffer_entry_t*)
+ ((uint8_t*)metadata + metadata->entries_start);
+}
+
+static uint8_t *get_data(const camera_metadata_t *metadata) {
+ return (uint8_t*)metadata + metadata->data_start;
+}
+
camera_metadata_t *allocate_camera_metadata(size_t entry_capacity,
size_t data_capacity) {
if (entry_capacity == 0) return NULL;
@@ -162,17 +172,17 @@
metadata->flags = 0;
metadata->entry_count = 0;
metadata->entry_capacity = entry_capacity;
- camera_metadata_buffer_entry_t* entries_unaligned =
- (camera_metadata_buffer_entry_t*)(metadata + 1);
- metadata->entries = ALIGN_TO(entries_unaligned, ENTRY_ALIGNMENT);
+ metadata->entries_start =
+ ALIGN_TO(sizeof(camera_metadata_t), ENTRY_ALIGNMENT);
metadata->data_count = 0;
metadata->data_capacity = data_capacity;
metadata->size = memory_needed;
if (metadata->data_capacity != 0) {
- uint8_t *data_unaligned = (uint8_t*)(metadata->entries + metadata->entry_capacity);
- metadata->data = ALIGN_TO(data_unaligned, DATA_ALIGNMENT);
+ size_t data_unaligned = (uint8_t*)(get_entries(metadata) +
+ metadata->entry_capacity) - (uint8_t*)metadata;
+ metadata->data_start = ALIGN_TO(data_unaligned, DATA_ALIGNMENT);
} else {
- metadata->data = NULL;
+ metadata->data_start = 0;
}
metadata->user = NULL;
@@ -203,12 +213,8 @@
size_t get_camera_metadata_compact_size(const camera_metadata_t *metadata) {
if (metadata == NULL) return ERROR;
- ptrdiff_t reserved_size = metadata->size -
- calculate_camera_metadata_size(metadata->entry_capacity,
- metadata->data_capacity);
-
return calculate_camera_metadata_size(metadata->entry_count,
- metadata->data_count) + reserved_size;
+ metadata->data_count);
}
size_t get_camera_metadata_entry_count(const camera_metadata_t *metadata) {
@@ -234,30 +240,16 @@
if (dst == NULL) return NULL;
if (dst_size < memory_needed) return NULL;
- // If copying a newer version of the structure, there may be additional
- // header fields we don't know about but need to copy
- ptrdiff_t reserved_size = src->size -
- calculate_camera_metadata_size(src->entry_capacity,
- src->data_capacity);
+ camera_metadata_t *metadata =
+ place_camera_metadata(dst, dst_size, src->entry_count, src->data_count);
- camera_metadata_t *metadata = (camera_metadata_t*)dst;
- metadata->version = CURRENT_METADATA_VERSION;
metadata->flags = src->flags;
metadata->entry_count = src->entry_count;
- metadata->entry_capacity = src->entry_count;
- metadata->entries = (camera_metadata_buffer_entry_t*)
- ((uint8_t *)(metadata + 1) + reserved_size);
metadata->data_count = src->data_count;
- metadata->data_capacity = src->data_count;
- metadata->data = (uint8_t *)(metadata->entries + metadata->entry_capacity);
- metadata->size = memory_needed;
- if (reserved_size > 0) {
- memcpy(metadata->reserved, src->reserved, reserved_size);
- }
- memcpy(metadata->entries, src->entries,
+ memcpy(get_entries(metadata), get_entries(src),
sizeof(camera_metadata_buffer_entry_t[metadata->entry_count]));
- memcpy(metadata->data, src->data,
+ memcpy(get_data(metadata), get_data(src),
sizeof(uint8_t[metadata->data_count]));
metadata->user = NULL;
@@ -271,17 +263,15 @@
if (dst->entry_capacity < src->entry_count + dst->entry_count) return ERROR;
if (dst->data_capacity < src->data_count + dst->data_count) return ERROR;
- memcpy(dst->entries + dst->entry_count, src->entries,
+ memcpy(get_entries(dst) + dst->entry_count, get_entries(src),
sizeof(camera_metadata_buffer_entry_t[src->entry_count]));
- memcpy(dst->data + dst->data_count, src->data,
+ memcpy(get_data(dst) + dst->data_count, get_data(src),
sizeof(uint8_t[src->data_count]));
if (dst->data_count != 0) {
- unsigned int i;
- for (i = dst->entry_count;
- i < dst->entry_count + src->entry_count;
- i++) {
- camera_metadata_buffer_entry_t *entry = dst->entries + i;
- if ( camera_metadata_type_size[entry->type] * entry->count > 4 ) {
+ camera_metadata_buffer_entry_t *entry = get_entries(dst) + dst->entry_count;
+ for (size_t i = 0; i < src->entry_count; i++, entry++) {
+ if ( calculate_camera_metadata_entry_data_size(entry->type,
+ entry->count) > 0 ) {
entry->data.offset += dst->data_count;
}
}
@@ -341,7 +331,7 @@
size_t data_payload_bytes =
data_count * camera_metadata_type_size[type];
- camera_metadata_buffer_entry_t *entry = dst->entries + dst->entry_count;
+ camera_metadata_buffer_entry_t *entry = get_entries(dst) + dst->entry_count;
entry->tag = tag;
entry->type = type;
entry->count = data_count;
@@ -351,7 +341,7 @@
data_payload_bytes);
} else {
entry->data.offset = dst->data_count;
- memcpy(dst->data + entry->data.offset, data,
+ memcpy(get_data(dst) + entry->data.offset, data,
data_payload_bytes);
dst->data_count += data_bytes;
}
@@ -390,7 +380,7 @@
if (dst == NULL) return ERROR;
if (dst->flags & FLAG_SORTED) return OK;
- qsort(dst->entries, dst->entry_count,
+ qsort(get_entries(dst), dst->entry_count,
sizeof(camera_metadata_buffer_entry_t),
compare_entry_tags);
dst->flags |= FLAG_SORTED;
@@ -404,7 +394,7 @@
if (src == NULL || entry == NULL) return ERROR;
if (index >= src->entry_count) return ERROR;
- camera_metadata_buffer_entry_t *buffer_entry = src->entries + index;
+ camera_metadata_buffer_entry_t *buffer_entry = get_entries(src) + index;
entry->index = index;
entry->tag = buffer_entry->tag;
@@ -412,7 +402,7 @@
entry->count = buffer_entry->count;
if (buffer_entry->count *
camera_metadata_type_size[buffer_entry->type] > 4) {
- entry->data.u8 = src->data + buffer_entry->data.offset;
+ entry->data.u8 = get_data(src) + buffer_entry->data.offset;
} else {
entry->data.u8 = buffer_entry->data.value;
}
@@ -431,16 +421,17 @@
camera_metadata_buffer_entry_t key;
key.tag = tag;
search_entry = bsearch(&key,
- src->entries,
+ get_entries(src),
src->entry_count,
sizeof(camera_metadata_buffer_entry_t),
compare_entry_tags);
if (search_entry == NULL) return NOT_FOUND;
- index = search_entry - src->entries;
+ index = search_entry - get_entries(src);
} else {
// Not sorted, linear search
- for (index = 0; index < src->entry_count; index++) {
- if (src->entries[index].tag == tag) {
+ camera_metadata_buffer_entry_t *search_entry = get_entries(src);
+ for (index = 0; index < src->entry_count; index++, search_entry++) {
+ if (search_entry->tag == tag) {
break;
}
}
@@ -464,19 +455,19 @@
if (dst == NULL) return ERROR;
if (index >= dst->entry_count) return ERROR;
- camera_metadata_buffer_entry_t *entry = dst->entries + index;
+ camera_metadata_buffer_entry_t *entry = get_entries(dst) + index;
size_t data_bytes = calculate_camera_metadata_entry_data_size(entry->type,
entry->count);
if (data_bytes > 0) {
// Shift data buffer to overwrite deleted data
- uint8_t *start = dst->data + entry->data.offset;
+ uint8_t *start = get_data(dst) + entry->data.offset;
uint8_t *end = start + data_bytes;
size_t length = dst->data_count - entry->data.offset - data_bytes;
memmove(start, end, length);
// Update all entry indices to account for shift
- camera_metadata_buffer_entry_t *e = dst->entries;
+ camera_metadata_buffer_entry_t *e = get_entries(dst);
size_t i;
for (i = 0; i < dst->entry_count; i++) {
if (calculate_camera_metadata_entry_data_size(
@@ -505,7 +496,7 @@
if (dst == NULL) return ERROR;
if (index >= dst->entry_count) return ERROR;
- camera_metadata_buffer_entry_t *entry = dst->entries + index;
+ camera_metadata_buffer_entry_t *entry = get_entries(dst) + index;
size_t data_bytes =
calculate_camera_metadata_entry_data_size(entry->type,
@@ -524,14 +515,14 @@
}
if (entry_bytes != 0) {
// Remove old data
- uint8_t *start = dst->data + entry->data.offset;
+ uint8_t *start = get_data(dst) + entry->data.offset;
uint8_t *end = start + entry_bytes;
size_t length = dst->data_count - entry->data.offset - entry_bytes;
memmove(start, end, length);
dst->data_count -= entry_bytes;
// Update all entry indices to account for shift
- camera_metadata_buffer_entry_t *e = dst->entries;
+ camera_metadata_buffer_entry_t *e = get_entries(dst);
size_t i;
for (i = 0; i < dst->entry_count; i++) {
if (calculate_camera_metadata_entry_data_size(
@@ -547,12 +538,12 @@
// Append new data
entry->data.offset = dst->data_count;
- memcpy(dst->data + entry->data.offset, data, data_payload_bytes);
+ memcpy(get_data(dst) + entry->data.offset, data, data_payload_bytes);
dst->data_count += data_bytes;
}
} else if (data_bytes != 0) {
// data size unchanged, reuse same data location
- memcpy(dst->data + entry->data.offset, data, data_payload_bytes);
+ memcpy(get_data(dst) + entry->data.offset, data, data_payload_bytes);
}
if (data_bytes == 0) {
@@ -661,8 +652,8 @@
fdprintf(fd, "%*sVersion: %d, Flags: %08x\n",
indentation + 2, "",
metadata->version, metadata->flags);
- for (i=0; i < metadata->entry_count; i++) {
- camera_metadata_buffer_entry_t *entry = metadata->entries + i;
+ camera_metadata_buffer_entry_t *entry = get_entries(metadata);
+ for (i=0; i < metadata->entry_count; i++, entry++) {
const char *tag_name, *tag_section;
tag_section = get_camera_metadata_section_name(entry->tag);
@@ -701,7 +692,7 @@
metadata->data_count);
continue;
}
- data_ptr = metadata->data + entry->data.offset;
+ data_ptr = get_data(metadata) + entry->data.offset;
} else {
data_ptr = entry->data.value;
}
diff --git a/camera/tests/camera_metadata_tests.cpp b/camera/tests/camera_metadata_tests.cpp
index 48f3426..99bfb45 100644
--- a/camera/tests/camera_metadata_tests.cpp
+++ b/camera/tests/camera_metadata_tests.cpp
@@ -1616,3 +1616,99 @@
free(buf);
free_camera_metadata(m);
}
+
+TEST(camera_metadata, memcpy) {
+ camera_metadata_t *m = NULL;
+ const size_t entry_capacity = 50;
+ const size_t data_capacity = 450;
+
+ int result;
+
+ m = allocate_camera_metadata(entry_capacity, data_capacity);
+
+ add_test_metadata(m, 5);
+
+ uint8_t *dst = new uint8_t[get_camera_metadata_size(m)];
+
+ memcpy(dst, m, get_camera_metadata_size(m));
+
+ camera_metadata_t *m2 = reinterpret_cast<camera_metadata_t*>(dst);
+
+ ASSERT_EQ(get_camera_metadata_size(m),
+ get_camera_metadata_size(m2));
+ EXPECT_EQ(get_camera_metadata_compact_size(m),
+ get_camera_metadata_compact_size(m2));
+ ASSERT_EQ(get_camera_metadata_entry_count(m),
+ get_camera_metadata_entry_count(m2));
+ EXPECT_EQ(get_camera_metadata_entry_capacity(m),
+ get_camera_metadata_entry_capacity(m2));
+ EXPECT_EQ(get_camera_metadata_data_count(m),
+ get_camera_metadata_data_count(m2));
+ EXPECT_EQ(get_camera_metadata_data_capacity(m),
+ get_camera_metadata_data_capacity(m2));
+
+ camera_metadata_entry_t e1, e2;
+ for (size_t i = 0; i < get_camera_metadata_entry_count(m); i++) {
+ result = get_camera_metadata_entry(m, i, &e1);
+ ASSERT_EQ(OK, result);
+ result = get_camera_metadata_entry(m2, i, &e2);
+ ASSERT_EQ(OK, result);
+
+ EXPECT_EQ(e1.index, e2.index);
+ EXPECT_EQ(e1.tag, e2.tag);
+ ASSERT_EQ(e1.type, e2.type);
+ ASSERT_EQ(e1.count, e2.count);
+
+ ASSERT_TRUE(!memcmp(e1.data.u8, e2.data.u8,
+ camera_metadata_type_size[e1.type] * e1.count));
+ }
+
+ // Make sure updating one metadata buffer doesn't change the other
+
+ int64_t double_exposure_time[] = { 100, 200 };
+
+ result = update_camera_metadata_entry(m, 0,
+ double_exposure_time,
+ sizeof(double_exposure_time)/sizeof(int64_t), NULL);
+ EXPECT_EQ(OK, result);
+
+ result = get_camera_metadata_entry(m, 0, &e1);
+ ASSERT_EQ(OK, result);
+ result = get_camera_metadata_entry(m2, 0, &e2);
+ ASSERT_EQ(OK, result);
+
+ EXPECT_EQ(e1.index, e2.index);
+ EXPECT_EQ(e1.tag, e2.tag);
+ ASSERT_EQ(e1.type, e2.type);
+ ASSERT_EQ((size_t)2, e1.count);
+ ASSERT_EQ((size_t)1, e2.count);
+ EXPECT_EQ(100, e1.data.i64[0]);
+ EXPECT_EQ(200, e1.data.i64[1]);
+ EXPECT_EQ(100, e2.data.i64[0]);
+
+ // And in the reverse direction as well
+
+ double_exposure_time[0] = 300;
+ result = update_camera_metadata_entry(m2, 0,
+ double_exposure_time,
+ sizeof(double_exposure_time)/sizeof(int64_t), NULL);
+ EXPECT_EQ(OK, result);
+
+ result = get_camera_metadata_entry(m, 0, &e1);
+ ASSERT_EQ(OK, result);
+ result = get_camera_metadata_entry(m2, 0, &e2);
+ ASSERT_EQ(OK, result);
+
+ EXPECT_EQ(e1.index, e2.index);
+ EXPECT_EQ(e1.tag, e2.tag);
+ ASSERT_EQ(e1.type, e2.type);
+ ASSERT_EQ((size_t)2, e1.count);
+ ASSERT_EQ((size_t)2, e2.count);
+ EXPECT_EQ(100, e1.data.i64[0]);
+ EXPECT_EQ(200, e1.data.i64[1]);
+ EXPECT_EQ(300, e2.data.i64[0]);
+ EXPECT_EQ(200, e2.data.i64[1]);
+
+ delete dst;
+ free_camera_metadata(m);
+}
