diff options
author | Eino-Ville Talvala <etalvala@google.com> | 2012-11-07 16:36:50 -0800 |
---|---|---|
committer | Eino-Ville Talvala <etalvala@google.com> | 2012-11-13 10:26:13 -0800 |
commit | 3154036acd2cc809388d08ff856198a8512f05f0 (patch) | |
tree | 0ede894b4f153ee1d4f2b94fda00c61a297c9aa7 | |
parent | 6c94a620035d8e719bf4e5040e39db92700bff6c (diff) | |
download | android_system_media-3154036acd2cc809388d08ff856198a8512f05f0.tar.gz android_system_media-3154036acd2cc809388d08ff856198a8512f05f0.tar.bz2 android_system_media-3154036acd2cc809388d08ff856198a8512f05f0.zip |
Camera: Fix metadata data alignment, other minor bugs.
- When a metadata entry needs to overflow into the data buffer, make
sure the starting offset is aligned to the maximum needed by all the
metadata types.
- Bounds check the data buffer size when adding a new entry
- Add new test for the bounds check
- Print out doubles correctly.
Bug: 7498597
Change-Id: Ic8645a998c096f5b803839ee8076b97862127021
-rw-r--r-- | camera/src/camera_metadata.c | 48 | ||||
-rw-r--r-- | camera/tests/camera_metadata_tests.cpp | 22 |
2 files changed, 58 insertions, 12 deletions
diff --git a/camera/src/camera_metadata.c b/camera/src/camera_metadata.c index 0b6597df..c922c7fc 100644 --- a/camera/src/camera_metadata.c +++ b/camera/src/camera_metadata.c @@ -23,6 +23,19 @@ #define OK 0 #define ERROR 1 #define NOT_FOUND -ENOENT + +#define _Alignas(T) \ + ({struct _AlignasStruct { char c; T field; }; \ + offsetof(struct _AlignasStruct, field); }) + +// Align entry buffers as the compiler would +#define ENTRY_ALIGNMENT _Alignas(camera_metadata_buffer_entry_t) +// Align data buffer to largest supported data type +#define DATA_ALIGNMENT _Alignas(camera_metadata_rational_t) + +#define ALIGN_TO(val, alignment) \ + (typeof(val))(((uint32_t)(val) + ((alignment) - 1)) & ~((alignment) - 1)) + /** * A single metadata entry, storing an array of values of a given type. If the * array is no larger than 4 bytes in size, it is stored in the data.value[] @@ -38,7 +51,7 @@ typedef struct camera_metadata_buffer_entry { } data; uint8_t type; uint8_t reserved[3]; -} __attribute__((packed)) camera_metadata_buffer_entry_t; +} camera_metadata_buffer_entry_t; /** * A packet of metadata. This is a list of entries, each of which may point to @@ -149,13 +162,15 @@ camera_metadata_t *place_camera_metadata(void *dst, metadata->flags = 0; metadata->entry_count = 0; metadata->entry_capacity = entry_capacity; - metadata->entries = (camera_metadata_buffer_entry_t*)(metadata + 1); + camera_metadata_buffer_entry_t* entries_unaligned = + (camera_metadata_buffer_entry_t*)(metadata + 1); + metadata->entries = ALIGN_TO(entries_unaligned, ENTRY_ALIGNMENT); metadata->data_count = 0; metadata->data_capacity = data_capacity; metadata->size = memory_needed; if (metadata->data_capacity != 0) { - metadata->data = - (uint8_t*)(metadata->entries + metadata->entry_capacity); + uint8_t *data_unaligned = (uint8_t*)(metadata->entries + metadata->entry_capacity); + metadata->data = ALIGN_TO(data_unaligned, DATA_ALIGNMENT); } else { metadata->data = NULL; } @@ -170,7 +185,11 @@ void free_camera_metadata(camera_metadata_t *metadata) { size_t calculate_camera_metadata_size(size_t entry_count, size_t data_count) { size_t memory_needed = sizeof(camera_metadata_t); + // Start entry list at aligned boundary + memory_needed = ALIGN_TO(memory_needed, ENTRY_ALIGNMENT); memory_needed += sizeof(camera_metadata_buffer_entry_t[entry_count]); + // Start buffer list at aligned boundary + memory_needed = ALIGN_TO(memory_needed, DATA_ALIGNMENT); memory_needed += sizeof(uint8_t[data_count]); return memory_needed; } @@ -303,7 +322,7 @@ size_t calculate_camera_metadata_entry_data_size(uint8_t type, if (type >= NUM_TYPES) return 0; size_t data_bytes = data_count * camera_metadata_type_size[type]; - return data_bytes <= 4 ? 0 : data_bytes; + return data_bytes <= 4 ? 0 : ALIGN_TO(data_bytes, DATA_ALIGNMENT); } static int add_camera_metadata_entry_raw(camera_metadata_t *dst, @@ -318,7 +337,10 @@ static int add_camera_metadata_entry_raw(camera_metadata_t *dst, size_t data_bytes = calculate_camera_metadata_entry_data_size(type, data_count); + if (data_bytes + dst->data_count > dst->data_capacity) return ERROR; + size_t data_payload_bytes = + data_count * camera_metadata_type_size[type]; camera_metadata_buffer_entry_t *entry = dst->entries + dst->entry_count; entry->tag = tag; entry->type = type; @@ -326,10 +348,11 @@ static int add_camera_metadata_entry_raw(camera_metadata_t *dst, if (data_bytes == 0) { memcpy(entry->data.value, data, - data_count * camera_metadata_type_size[type] ); + data_payload_bytes); } else { entry->data.offset = dst->data_count; - memcpy(dst->data + entry->data.offset, data, data_bytes); + memcpy(dst->data + entry->data.offset, data, + data_payload_bytes); dst->data_count += data_bytes; } dst->entry_count++; @@ -487,6 +510,9 @@ int update_camera_metadata_entry(camera_metadata_t *dst, size_t data_bytes = calculate_camera_metadata_entry_data_size(entry->type, data_count); + size_t data_payload_bytes = + data_count * camera_metadata_type_size[entry->type]; + size_t entry_bytes = calculate_camera_metadata_entry_data_size(entry->type, entry->count); @@ -521,18 +547,18 @@ int update_camera_metadata_entry(camera_metadata_t *dst, // Append new data entry->data.offset = dst->data_count; - memcpy(dst->data + entry->data.offset, data, data_bytes); + memcpy(dst->data + 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_bytes); + memcpy(dst->data + entry->data.offset, data, data_payload_bytes); } if (data_bytes == 0) { // Data fits into entry memcpy(entry->data.value, data, - data_count * camera_metadata_type_size[entry->type]); + data_payload_bytes); } entry->count = data_count; @@ -728,7 +754,7 @@ static void print_data(int fd, const uint8_t *data_ptr, break; case TYPE_DOUBLE: fdprintf(fd, "%0.2f ", - *(float*)(data_ptr + index)); + *(double*)(data_ptr + index)); break; case TYPE_RATIONAL: { int32_t numerator = *(int32_t*)(data_ptr + index); diff --git a/camera/tests/camera_metadata_tests.cpp b/camera/tests/camera_metadata_tests.cpp index 06c59f43..48f3426b 100644 --- a/camera/tests/camera_metadata_tests.cpp +++ b/camera/tests/camera_metadata_tests.cpp @@ -302,7 +302,7 @@ void add_test_metadata(camera_metadata_t *m, int entry_count) { } EXPECT_EQ(data_used, get_camera_metadata_data_count(m)); EXPECT_EQ(entries_used, get_camera_metadata_entry_count(m)); - EXPECT_GT(get_camera_metadata_data_capacity(m), + EXPECT_GE(get_camera_metadata_data_capacity(m), get_camera_metadata_data_count(m)); } @@ -354,6 +354,26 @@ TEST(camera_metadata, add_get_toomany) { free_camera_metadata(m); } +TEST(camera_metadata, add_too_much_data) { + camera_metadata_t *m = NULL; + const size_t entry_capacity = 5; + int result; + size_t data_used = entry_capacity * calculate_camera_metadata_entry_data_size( + get_camera_metadata_tag_type(ANDROID_SENSOR_EXPOSURE_TIME), 1); + m = allocate_camera_metadata(entry_capacity + 1, data_used); + + + add_test_metadata(m, entry_capacity); + + int64_t exposure_time = 12345; + result = add_camera_metadata_entry(m, + ANDROID_SENSOR_EXPOSURE_TIME, + &exposure_time, 1); + EXPECT_EQ(ERROR, result); + + free_camera_metadata(m); +} + TEST(camera_metadata, copy_metadata) { camera_metadata_t *m = NULL; const size_t entry_capacity = 50; |