LP64 fixes for camera_metadata.
camera_metadata packets are transferred across process
boundaries with Parcel::readBlob / Parcel::writeBlob calls
so we should make sure they have a consistent layout across
32 and 64 bit processes. In this change :
- Replace size_t members with uint32_t members.
- Remove the "void*" user pointer which is no longer required
Change-Id: Ia0eada5d8358be21d725c05d6105705186b3d9c8
diff --git a/camera/include/system/camera_metadata.h b/camera/include/system/camera_metadata.h
index abfff57..9de902b 100644
--- a/camera/include/system/camera_metadata.h
+++ b/camera/include/system/camera_metadata.h
@@ -424,21 +424,6 @@
camera_metadata_entry_t *updated_entry);
/**
- * Set user pointer in buffer. This can be used for linking the metadata buffer
- * with other associated data. This user pointer is not copied with
- * copy_camera_metadata, and is unaffected by append or any other methods.
- */
-ANDROID_API
-int set_camera_metadata_user_pointer(camera_metadata_t *dst, void* user);
-
-/**
- * Retrieve user pointer in buffer. Returns NULL in user if
- * set_camera_metadata_user_pointer has not been called with this buffer.
- */
-ANDROID_API
-int get_camera_metadata_user_pointer(camera_metadata_t *dst, void** user);
-
-/**
* Retrieve human-readable name of section the tag is in. Returns NULL if
* no such tag is defined. Returns NULL for tags in the vendor section, unless
* set_vendor_tag_query_ops() has been used.
diff --git a/camera/src/camera_metadata.c b/camera/src/camera_metadata.c
index 8f78be7..eb914eb 100644
--- a/camera/src/camera_metadata.c
+++ b/camera/src/camera_metadata.c
@@ -29,37 +29,30 @@
#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_data_t)
-
#define ALIGN_TO(val, alignment) \
(((uintptr_t)(val) + ((alignment) - 1)) & ~((alignment) - 1))
-typedef size_t uptrdiff_t;
-
/**
* 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[]
* array; otherwise, it can found in the parent's data array at index
* data.offset.
*/
+#define ENTRY_ALIGNMENT ((size_t) 4)
typedef struct camera_metadata_buffer_entry {
uint32_t tag;
- size_t count;
+ uint32_t count;
union {
- size_t offset;
- uint8_t value[4];
+ uint32_t offset;
+ uint8_t value[4];
} data;
uint8_t type;
uint8_t reserved[3];
} camera_metadata_buffer_entry_t;
+typedef uint32_t metadata_uptrdiff_t;
+typedef uint32_t metadata_size_t;
+
/**
* A packet of metadata. This is a list of entries, each of which may point to
* its values stored at an offset in data.
@@ -94,18 +87,18 @@
* In short, the entries and data are contiguous in memory after the metadata
* header.
*/
+#define METADATA_ALIGNMENT ((size_t) 4)
struct camera_metadata {
- size_t size;
+ metadata_size_t size;
uint32_t version;
uint32_t flags;
- size_t entry_count;
- size_t entry_capacity;
- uptrdiff_t entries_start; // Offset from camera_metadata
- size_t data_count;
- size_t data_capacity;
- uptrdiff_t data_start; // Offset from camera_metadata
- void *user; // User set pointer, not copied with buffer
- uint8_t reserved[0];
+ metadata_size_t entry_count;
+ metadata_size_t entry_capacity;
+ metadata_uptrdiff_t entries_start; // Offset from camera_metadata
+ metadata_size_t data_count;
+ metadata_size_t data_capacity;
+ metadata_uptrdiff_t data_start; // Offset from camera_metadata
+ uint8_t reserved[];
};
/**
@@ -114,6 +107,7 @@
* non-pointer type description in order to figure out the largest alignment
* requirement for data (DATA_ALIGNMENT).
*/
+#define DATA_ALIGNMENT ((size_t) 8)
typedef union camera_metadata_data {
uint8_t u8;
int32_t i32;
@@ -123,6 +117,15 @@
camera_metadata_rational_t r;
} camera_metadata_data_t;
+/**
+ * The preferred alignment of a packet of camera metadata. In general,
+ * this is the lowest common multiple of the constituents of a metadata
+ * package, i.e, of DATA_ALIGNMENT and ENTRY_ALIGNMENT.
+ */
+#define MAX_ALIGNMENT(A, B) (((A) > (B)) ? (A) : (B))
+#define METADATA_PACKET_ALIGNMENT \
+ MAX_ALIGNMENT(MAX_ALIGNMENT(DATA_ALIGNMENT, METADATA_ALIGNMENT), ENTRY_ALIGNMENT);
+
/** Versioning information */
#define CURRENT_METADATA_VERSION 1
@@ -167,20 +170,7 @@
}
size_t get_camera_metadata_alignment() {
- size_t alignments[] = {
- _Alignas(struct camera_metadata),
- _Alignas(struct camera_metadata_buffer_entry),
- _Alignas(union camera_metadata_data)
- };
- size_t max_alignment = alignments[0];
-
- for (size_t i = 1; i < sizeof(alignments)/sizeof(alignments[0]); ++i) {
- if (max_alignment < alignments[i]) {
- max_alignment = alignments[i];
- }
- }
-
- return max_alignment;
+ return METADATA_PACKET_ALIGNMENT;
}
camera_metadata_t *allocate_copy_camera_metadata_checked(
@@ -237,7 +227,6 @@
size_t data_unaligned = (uint8_t*)(get_entries(metadata) +
metadata->entry_capacity) - (uint8_t*)metadata;
metadata->data_start = ALIGN_TO(data_unaligned, DATA_ALIGNMENT);
- metadata->user = NULL;
assert(validate_camera_metadata_structure(metadata, NULL) == OK);
return metadata;
@@ -305,7 +294,6 @@
sizeof(camera_metadata_buffer_entry_t[metadata->entry_count]));
memcpy(get_data(metadata), get_data(src),
sizeof(uint8_t[metadata->data_count]));
- metadata->user = NULL;
assert(validate_camera_metadata_structure(metadata, NULL) == OK);
return metadata;
@@ -321,21 +309,21 @@
// Check that the metadata pointer is well-aligned first.
{
- struct {
+ static const struct {
const char *name;
size_t alignment;
} alignments[] = {
{
.name = "camera_metadata",
- .alignment = _Alignas(struct camera_metadata)
+ .alignment = METADATA_ALIGNMENT
},
{
.name = "camera_metadata_buffer_entry",
- .alignment = _Alignas(struct camera_metadata_buffer_entry)
+ .alignment = ENTRY_ALIGNMENT
},
{
.name = "camera_metadata_data",
- .alignment = _Alignas(union camera_metadata_data)
+ .alignment = DATA_ALIGNMENT
},
};
@@ -357,33 +345,38 @@
*/
if (expected_size != NULL && metadata->size > *expected_size) {
- ALOGE("%s: Metadata size (%zu) should be <= expected size (%zu)",
+ ALOGE("%s: Metadata size (%" PRIu32 ") should be <= expected size (%zu)",
__FUNCTION__, metadata->size, *expected_size);
return ERROR;
}
if (metadata->entry_count > metadata->entry_capacity) {
- ALOGE("%s: Entry count (%zu) should be <= entry capacity (%zu)",
+ ALOGE("%s: Entry count (%" PRIu32 ") should be <= entry capacity "
+ "(%" PRIu32 ")",
__FUNCTION__, metadata->entry_count, metadata->entry_capacity);
return ERROR;
}
- uptrdiff_t entries_end = metadata->entries_start + metadata->entry_capacity;
+ const metadata_uptrdiff_t entries_end =
+ metadata->entries_start + metadata->entry_capacity;
if (entries_end < metadata->entries_start || // overflow check
entries_end > metadata->data_start) {
- ALOGE("%s: Entry start + capacity (%zu) should be <= data start (%zu)",
+ ALOGE("%s: Entry start + capacity (%" PRIu32 ") should be <= data start "
+ "(%" PRIu32 ")",
__FUNCTION__,
(metadata->entries_start + metadata->entry_capacity),
metadata->data_start);
return ERROR;
}
- uptrdiff_t data_end = metadata->data_start + metadata->data_capacity;
+ const metadata_uptrdiff_t data_end =
+ metadata->data_start + metadata->data_capacity;
if (data_end < metadata->data_start || // overflow check
data_end > metadata->size) {
- ALOGE("%s: Data start + capacity (%zu) should be <= total size (%zu)",
+ ALOGE("%s: Data start + capacity (%" PRIu32 ") should be <= total size "
+ "(%" PRIu32 ")",
__FUNCTION__,
(metadata->data_start + metadata->data_capacity),
metadata->size);
@@ -391,7 +384,7 @@
}
// Validate each entry
- size_t entry_count = metadata->entry_count;
+ const metadata_size_t entry_count = metadata->entry_count;
camera_metadata_buffer_entry_t *entries = get_entries(metadata);
for (size_t i = 0; i < entry_count; ++i) {
@@ -444,7 +437,7 @@
data_entry_end > metadata->data_capacity) {
ALOGE("%s: Entry index %zu data ends (%zu) beyond the capacity "
- "%zu", __FUNCTION__, i, data_entry_end,
+ "%" PRIu32, __FUNCTION__, i, data_entry_end,
metadata->data_capacity);
return ERROR;
}
@@ -452,7 +445,7 @@
} else if (entry.count == 0) {
if (entry.data.offset != 0) {
ALOGE("%s: Entry index %zu had 0 items, but offset was non-0 "
- "(%zu), tag name: %s", __FUNCTION__, i, entry.data.offset,
+ "(%" PRIu32 "), tag name: %s", __FUNCTION__, i, entry.data.offset,
get_camera_metadata_tag_name(entry.tag) ?: "unknown");
return ERROR;
}
@@ -783,18 +776,6 @@
return OK;
}
-int set_camera_metadata_user_pointer(camera_metadata_t *dst, void* user) {
- if (dst == NULL) return ERROR;
- dst->user = user;
- return OK;
-}
-
-int get_camera_metadata_user_pointer(camera_metadata_t *dst, void** user) {
- if (dst == NULL) return ERROR;
- *user = dst->user;
- return OK;
-}
-
static const vendor_tag_ops_t *vendor_tag_ops = NULL;
const char *get_camera_metadata_section_name(uint32_t tag) {
@@ -873,8 +854,8 @@
}
unsigned int i;
dprintf(fd,
- "%*sDumping camera metadata array: %zu / %zu entries, "
- "%zu / %zu bytes of extra data.\n", indentation, "",
+ "%*sDumping camera metadata array: %" PRIu32 " / %" PRIu32 " entries, "
+ "%" PRIu32 " / %" PRIu32 " bytes of extra data.\n", indentation, "",
metadata->entry_count, metadata->entry_capacity,
metadata->data_count, metadata->data_capacity);
dprintf(fd, "%*sVersion: %d, Flags: %08x\n",
@@ -898,7 +879,7 @@
} else {
type_name = camera_metadata_type_names[entry->type];
}
- dprintf(fd, "%*s%s.%s (%05x): %s[%zu]\n",
+ dprintf(fd, "%*s%s.%s (%05x): %s[%" PRIu32 "]\n",
indentation + 2, "",
tag_section,
tag_name,
@@ -914,7 +895,7 @@
uint8_t *data_ptr;
if ( type_size * entry->count > 4 ) {
if (entry->data.offset >= metadata->data_count) {
- ALOGE("%s: Malformed entry data offset: %zu (max %zu)",
+ ALOGE("%s: Malformed entry data offset: %" PRIu32 " (max %" PRIu32 ")",
__FUNCTION__,
entry->data.offset,
metadata->data_count);
diff --git a/camera/tests/camera_metadata_tests.cpp b/camera/tests/camera_metadata_tests.cpp
index 98f5c82..916db15 100644
--- a/camera/tests/camera_metadata_tests.cpp
+++ b/camera/tests/camera_metadata_tests.cpp
@@ -1685,55 +1685,6 @@
}
-TEST(camera_metadata, user_pointer) {
- 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);
-
- size_t num_entries = 5;
- size_t data_per_entry =
- calculate_camera_metadata_entry_data_size(TYPE_INT64, 1);
- size_t num_data = num_entries * data_per_entry;
-
- add_test_metadata(m, num_entries);
- EXPECT_EQ(num_entries, get_camera_metadata_entry_count(m));
- EXPECT_EQ(num_data, get_camera_metadata_data_count(m));
-
- void* ptr;
- result = get_camera_metadata_user_pointer(m, &ptr);
- EXPECT_EQ(OK, result);
- EXPECT_NULL(ptr);
-
- int testValue = 10;
- result = set_camera_metadata_user_pointer(m, &testValue);
- EXPECT_EQ(OK, result);
-
- result = get_camera_metadata_user_pointer(m, &ptr);
- EXPECT_EQ(OK, result);
- EXPECT_EQ(&testValue, (int*)ptr);
- EXPECT_EQ(testValue, *(int*)ptr);
-
- size_t buf_size = get_camera_metadata_compact_size(m);
- EXPECT_LT((size_t)0, buf_size);
-
- uint8_t *buf = (uint8_t*)malloc(buf_size);
- EXPECT_NOT_NULL(buf);
-
- camera_metadata_t *m2 = copy_camera_metadata(buf, buf_size, m);
- EXPECT_NOT_NULL(m2);
-
- result = get_camera_metadata_user_pointer(m2, &ptr);
- EXPECT_NULL(ptr);
-
- EXPECT_EQ(OK, validate_camera_metadata_structure(m2, &buf_size));
- free(buf);
- FINISH_USING_CAMERA_METADATA(m);
-}
-
TEST(camera_metadata, memcpy) {
camera_metadata_t *m = NULL;
const size_t entry_capacity = 50;