gralloc: Refactor gralloc4.0 for some concerns in code review
There were some concerns raised in the intern code review process. Since
Qualcomm has merged their Gralloc4.0 changes into their codebase, the
patch created another standlane change to address the concerns within
the code review.
Bug: 141596968
Test: VtsHalGraphicsMapperV4_0TargetTest
Change-Id: Ie687557089064a6d02c7505f2024db6a23e003c8
(cherry picked from commit ce25d55c476a20e6cc130d3ba9500cd055b63392)
Change-Id: Ic397ae3874e68a7b5ba9315f31fbc098db21a800
diff --git a/gralloc/Android.mk b/gralloc/Android.mk
index 4132c58..ebb3cd4 100644
--- a/gralloc/Android.mk
+++ b/gralloc/Android.mk
@@ -32,6 +32,7 @@
LOCAL_C_INCLUDES := $(common_includes) $(kernel_includes)
LOCAL_HEADER_LIBRARIES := display_headers
LOCAL_SHARED_LIBRARIES := $(common_libs) libqdMetaData libdl \
+ android.hardware.graphics.common@1.2 \
android.hardware.graphics.mapper@2.0 \
android.hardware.graphics.mapper@2.1 \
android.hardware.graphics.mapper@3.0 \
diff --git a/gralloc/QtiAllocator.cpp b/gralloc/QtiAllocator.cpp
index 1a04237..35145dc 100644
--- a/gralloc/QtiAllocator.cpp
+++ b/gralloc/QtiAllocator.cpp
@@ -42,23 +42,12 @@
#include "gr_utils.h"
static void get_properties(gralloc::GrallocProperties *props) {
- char property[PROPERTY_VALUE_MAX];
- property_get("vendor.gralloc.use_system_heap_for_sensors", property, "1");
- if (!(strncmp(property, "0", PROPERTY_VALUE_MAX))) {
- props->use_system_heap_for_sensors = false;
- }
+ props->use_system_heap_for_sensors =
+ property_get_bool("vendor.gralloc.use_system_heap_for_sensors", 1);
- property_get("vendor.gralloc.disable_ubwc", property, "0");
- if (!(strncmp(property, "1", PROPERTY_VALUE_MAX)) ||
- !(strncmp(property, "true", PROPERTY_VALUE_MAX))) {
- props->ubwc_disable = true;
- }
+ props->ubwc_disable = property_get_bool("vendor.gralloc.disable_ubwc", 0);
- property_get("vendor.gralloc.disable_ahardware_buffer", property, "0");
- if (!(strncmp(property, "1", PROPERTY_VALUE_MAX)) ||
- !(strncmp(property, "true", PROPERTY_VALUE_MAX))) {
- props->ahardware_buffer_disable = true;
- }
+ props->ahardware_buffer_disable = property_get_bool("vendor.gralloc.disable_ahardware_buffer", 0);
}
namespace vendor {
diff --git a/gralloc/QtiMapper4.cpp b/gralloc/QtiMapper4.cpp
index 83591b0..fe12dd0 100644
--- a/gralloc/QtiMapper4.cpp
+++ b/gralloc/QtiMapper4.cpp
@@ -384,6 +384,7 @@
hidl_cb(err, reserved_region, reserved_size);
return Void();
}
+
Error QtiMapper::DumpBufferMetadata(const private_handle_t *buffer, BufferDump *outBufferDump) {
outBufferDump->metadataDump.resize(metadata_type_descriptions_.size());
for (int i = 0; i < static_cast<int>(metadata_type_descriptions_.size()); i++) {
diff --git a/gralloc/QtiMapper4.h b/gralloc/QtiMapper4.h
index caf8d12..7fa27b1 100644
--- a/gralloc/QtiMapper4.h
+++ b/gralloc/QtiMapper4.h
@@ -129,7 +129,7 @@
std::memcpy(&out[index], &magic_version, sizeof(magic_version));
index += sizeof(magic_version);
- out[index] = static_cast<uint8_t> (name_size);
+ std::memcpy(&out[index], &name_size, sizeof(name_size));
index += sizeof(name_size);
std::memcpy(&out[index], bd_info.name.c_str(), bd_info.name.size());
@@ -168,7 +168,8 @@
std::memcpy(&magic_version, &in[index], sizeof(magic_version));
index += sizeof(magic_version);
- uint64_t name_size = in[index];
+ uint64_t name_size;
+ std::memcpy(&name_size, &in[index], sizeof(name_size));
index += sizeof(name_size);
// The second check validates that the size and magic version are correct
diff --git a/gralloc/gr_buf_descriptor.h b/gralloc/gr_buf_descriptor.h
index 32a208a..bd50259 100644
--- a/gralloc/gr_buf_descriptor.h
+++ b/gralloc/gr_buf_descriptor.h
@@ -59,6 +59,7 @@
void SetName(std::string name) { name_ = name; }
void SetReservedSize(uint64_t reserved_size) { reserved_size_ = reserved_size; }
+
uint64_t GetUsage() const { return usage_; }
int GetWidth() const { return width_; }
@@ -72,6 +73,7 @@
uint64_t GetId() const { return id_; }
uint64_t GetReservedSize() const { return reserved_size_; }
+
std::string GetName() const { return name_; }
private:
diff --git a/gralloc/gr_buf_mgr.cpp b/gralloc/gr_buf_mgr.cpp
index e66162c..96a9463 100644
--- a/gralloc/gr_buf_mgr.cpp
+++ b/gralloc/gr_buf_mgr.cpp
@@ -592,9 +592,8 @@
std::vector<PlaneLayoutComponent> components;
grallocToStandardPlaneLayoutComponentType(plane_layout[i].component, &plane_info[i].components,
handle->format);
- plane_info[i].horizontalSubsampling =
- static_cast<int64_t>(pow(2, plane_layout[i].h_subsampling));
- plane_info[i].verticalSubsampling = static_cast<int64_t>(pow(2, plane_layout[i].v_subsampling));
+ plane_info[i].horizontalSubsampling = (1ull << plane_layout[i].h_subsampling);
+ plane_info[i].verticalSubsampling = (1ull << plane_layout[i].v_subsampling);
plane_info[i].offsetInBytes = static_cast<int64_t>(plane_layout[i].offset);
plane_info[i].sampleIncrementInBits = static_cast<int64_t>(plane_layout[i].step * 8);
plane_info[i].strideInBytes = static_cast<int64_t>(plane_layout[i].stride_bytes);
@@ -808,6 +807,7 @@
return err;
}
+
Error BufferManager::FlushBuffer(const private_handle_t *handle) {
std::lock_guard<std::mutex> lock(buffer_lock_);
auto status = Error::NONE;
@@ -957,8 +957,9 @@
return Error::BAD_BUFFER;
}
auto metadata = reinterpret_cast<MetaData_t *>(hnd->base_metadata);
- descriptor.GetName().copy(metadata->name, descriptor.GetName().size() + 1);
- metadata->name[descriptor.GetName().size()] = '\0';
+ auto nameLength = std::min(descriptor.GetName().size(), size_t(MAX_NAME_LEN - 1));
+ nameLength = descriptor.GetName().copy(metadata->name, nameLength);
+ metadata->name[nameLength] = '\0';
metadata->reservedRegion.size = static_cast<uint32_t>(descriptor.GetReservedSize());
@@ -1016,6 +1017,7 @@
}
return Error::NONE;
}
+
Error BufferManager::GetReservedRegion(private_handle_t *handle, void **reserved_region,
uint64_t *reserved_region_size) {
std::lock_guard<std::mutex> lock(buffer_lock_);
@@ -1177,12 +1179,13 @@
break;
}
case (int64_t)StandardMetadataType::SMPTE2094_40: {
- std::vector<uint8_t> dynamic_metadata_payload;
if (metadata->color.dynamicMetaDataValid &&
metadata->color.dynamicMetaDataLen <= HDR_DYNAMIC_META_DATA_SZ) {
+ std::vector<uint8_t> dynamic_metadata_payload;
dynamic_metadata_payload.resize(metadata->color.dynamicMetaDataLen);
- memcpy(dynamic_metadata_payload.data(), &metadata->color.dynamicMetaDataPayload,
- metadata->color.dynamicMetaDataLen);
+ dynamic_metadata_payload.assign(
+ metadata->color.dynamicMetaDataPayload,
+ metadata->color.dynamicMetaDataPayload + metadata->color.dynamicMetaDataLen);
android::gralloc4::encodeSmpte2094_40(dynamic_metadata_payload, out);
} else {
android::gralloc4::encodeSmpte2094_40(std::nullopt, out);
@@ -1366,13 +1369,13 @@
std::optional<std::vector<uint8_t>> dynamic_metadata_payload;
android::gralloc4::decodeSmpte2094_40(in, &dynamic_metadata_payload);
if (dynamic_metadata_payload != std::nullopt) {
- if (dynamic_metadata_payload->size() <= HDR_DYNAMIC_META_DATA_SZ &&
- dynamic_metadata_payload->size() > 0) {
- metadata->color.dynamicMetaDataLen = static_cast<uint32_t>(dynamic_metadata_payload->size());
- memcpy(&metadata->color.dynamicMetaDataPayload, &dynamic_metadata_payload,
- metadata->color.dynamicMetaDataLen);
- metadata->color.dynamicMetaDataValid = true;
- }
+ if (dynamic_metadata_payload->size() > HDR_DYNAMIC_META_DATA_SZ)
+ return Error::BAD_VALUE;
+
+ metadata->color.dynamicMetaDataLen = static_cast<uint32_t>(dynamic_metadata_payload->size());
+ std::copy(dynamic_metadata_payload->begin(), dynamic_metadata_payload->end(),
+ metadata->color.dynamicMetaDataPayload);
+ metadata->color.dynamicMetaDataValid = true;
} else {
// Reset metadata by passing in std::nullopt
metadata->color.dynamicMetaDataValid = false;