From 574a6852953ca2b38ed346348a3e88f66fcf90fe Mon Sep 17 00:00:00 2001 From: Fan Xu Date: Fri, 2 Nov 2018 13:22:42 -0700 Subject: Move BufferNode to libbufferhubservice BufferNode is a server-side class and should be located in the service folder. Also, making the old bufferhubd service depending on libbufferhubservice could solve the dependency problem. Test: "atest buffer_node-test", "atest buffer_hub-test" passed. Bug: 118893702 Change-Id: I5fad37d3c0475d6cd4f4e0ed17f911b6777a6868 --- services/bufferhub/BufferNode.cpp | 100 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 services/bufferhub/BufferNode.cpp (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp new file mode 100644 index 0000000000..62583a666d --- /dev/null +++ b/services/bufferhub/BufferNode.cpp @@ -0,0 +1,100 @@ +#include + +#include +#include +#include + +namespace android { +namespace frameworks { +namespace bufferhub { +namespace V1_0 { +namespace implementation { + +void BufferNode::InitializeMetadata() { + // Using placement new here to reuse shared memory instead of new allocation + // Initialize the atomic variables to zero. + dvr::BufferHubDefs::MetadataHeader* metadata_header = metadata_.metadata_header(); + buffer_state_ = new (&metadata_header->buffer_state) std::atomic(0); + fence_state_ = new (&metadata_header->fence_state) std::atomic(0); + active_clients_bit_mask_ = + new (&metadata_header->active_clients_bit_mask) std::atomic(0); +} + +// Allocates a new BufferNode. +BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, + uint64_t usage, size_t user_metadata_size) { + uint32_t out_stride = 0; + // graphicBufferId is not used in GraphicBufferAllocator::allocate + // TODO(b/112338294) After move to the service folder, stop using the + // hardcoded service name "bufferhub". + int ret = GraphicBufferAllocator::get().allocate(width, height, format, layer_count, usage, + const_cast( + &buffer_handle_), + &out_stride, + /*graphicBufferId=*/0, + /*requestor=*/"bufferhub"); + + if (ret != OK || buffer_handle_ == nullptr) { + ALOGE("BufferNode::BufferNode: Failed to allocate buffer: %s", strerror(-ret)); + return; + } + + buffer_desc_.width = width; + buffer_desc_.height = height; + buffer_desc_.layers = layer_count; + buffer_desc_.format = format; + buffer_desc_.usage = usage; + buffer_desc_.stride = out_stride; + + metadata_ = BufferHubMetadata::Create(user_metadata_size); + if (!metadata_.IsValid()) { + ALOGE("BufferNode::BufferNode: Failed to allocate metadata."); + return; + } + InitializeMetadata(); +} + +// Free the handle +BufferNode::~BufferNode() { + if (buffer_handle_ != nullptr) { + status_t ret = GraphicBufferAllocator::get().free(buffer_handle_); + if (ret != OK) { + ALOGE("BufferNode::~BufferNode: Failed to free handle; Got error: %d", ret); + } + } +} + +uint64_t BufferNode::GetActiveClientsBitMask() const { + return active_clients_bit_mask_->load(std::memory_order_acquire); +} + +uint64_t BufferNode::AddNewActiveClientsBitToMask() { + uint64_t current_active_clients_bit_mask = GetActiveClientsBitMask(); + uint64_t client_state_mask = 0ULL; + uint64_t updated_active_clients_bit_mask = 0ULL; + do { + client_state_mask = dvr::BufferHubDefs::FindNextAvailableClientStateMask( + current_active_clients_bit_mask); + if (client_state_mask == 0ULL) { + ALOGE("BufferNode::AddNewActiveClientsBitToMask: reached the maximum " + "mumber of channels per buffer node: 32."); + errno = E2BIG; + return 0ULL; + } + updated_active_clients_bit_mask = current_active_clients_bit_mask | client_state_mask; + } while (!(active_clients_bit_mask_->compare_exchange_weak(current_active_clients_bit_mask, + updated_active_clients_bit_mask, + std::memory_order_acq_rel, + std::memory_order_acquire))); + return client_state_mask; +} + +void BufferNode::RemoveClientsBitFromMask(const uint64_t& value) { + active_clients_bit_mask_->fetch_and(~value); +} + +} // namespace implementation +} // namespace V1_0 +} // namespace bufferhub +} // namespace frameworks +} // namespace android -- cgit v1.2.3-59-g8ed1b From 19a3e2be83f4d50f85b819b8048078968e8acecc Mon Sep 17 00:00:00 2001 From: Fan Xu Date: Mon, 12 Nov 2018 13:45:33 -0800 Subject: Use __FUNCTION__ in BufferNode.cpp for log Test: build passed Bug: none Change-Id: I7e841a39739ad235c590f7ac174e9bd7f4e7c6e0 --- services/bufferhub/BufferNode.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index 62583a666d..53dd702792 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -35,7 +35,7 @@ BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, ui /*requestor=*/"bufferhub"); if (ret != OK || buffer_handle_ == nullptr) { - ALOGE("BufferNode::BufferNode: Failed to allocate buffer: %s", strerror(-ret)); + ALOGE("%s: Failed to allocate buffer: %s", __FUNCTION__, strerror(-ret)); return; } @@ -48,7 +48,7 @@ BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, ui metadata_ = BufferHubMetadata::Create(user_metadata_size); if (!metadata_.IsValid()) { - ALOGE("BufferNode::BufferNode: Failed to allocate metadata."); + ALOGE("%s: Failed to allocate metadata.", __FUNCTION__); return; } InitializeMetadata(); @@ -59,7 +59,7 @@ BufferNode::~BufferNode() { if (buffer_handle_ != nullptr) { status_t ret = GraphicBufferAllocator::get().free(buffer_handle_); if (ret != OK) { - ALOGE("BufferNode::~BufferNode: Failed to free handle; Got error: %d", ret); + ALOGE("%s: Failed to free handle; Got error: %d", __FUNCTION__, ret); } } } @@ -76,8 +76,7 @@ uint64_t BufferNode::AddNewActiveClientsBitToMask() { client_state_mask = dvr::BufferHubDefs::FindNextAvailableClientStateMask( current_active_clients_bit_mask); if (client_state_mask == 0ULL) { - ALOGE("BufferNode::AddNewActiveClientsBitToMask: reached the maximum " - "mumber of channels per buffer node: 32."); + ALOGE("%s: reached the maximum number of channels per buffer node: 32.", __FUNCTION__); errno = E2BIG; return 0ULL; } -- cgit v1.2.3-59-g8ed1b From ffde786ffce1afcd7ee574e7b45026d651ac1779 Mon Sep 17 00:00:00 2001 From: Fan Xu Date: Thu, 8 Nov 2018 16:29:13 -0800 Subject: Add a service-unique ID to BufferNode All the BufferNode allocated by bufferhub hwservice now will have a unique ID generated and managed by the UniqueIdGenerator. The idGenerator is placed in global static namespace because 1) therefore it is process-unique 2) BufferNode could still access it without a reference to the service instance. Added UniqueIdGenerator_test. Test: UniqueIdGenerator_test, BufferNode_test, buffer_hub-test, BufferHubBuffer_test passed. Fix: 118844348 Change-Id: I9c9adebab3af7b9de71dbe8728d3f24ed231338d --- services/bufferhub/Android.bp | 1 + services/bufferhub/BufferHubService.cpp | 2 +- services/bufferhub/BufferNode.cpp | 15 +++++- services/bufferhub/UniqueIdGenerator.cpp | 55 +++++++++++++++++++++ .../bufferhub/include/bufferhub/BufferHubService.h | 3 ++ services/bufferhub/include/bufferhub/BufferNode.h | 11 ++++- .../include/bufferhub/UniqueIdGenerator.h | 57 ++++++++++++++++++++++ services/bufferhub/tests/Android.bp | 16 ++++++ .../bufferhub/tests/UniqueIdGenerator_test.cpp | 45 +++++++++++++++++ 9 files changed, 201 insertions(+), 4 deletions(-) create mode 100644 services/bufferhub/UniqueIdGenerator.cpp create mode 100644 services/bufferhub/include/bufferhub/UniqueIdGenerator.h create mode 100644 services/bufferhub/tests/UniqueIdGenerator_test.cpp (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/services/bufferhub/Android.bp b/services/bufferhub/Android.bp index 8b4333342d..28a7501e0a 100644 --- a/services/bufferhub/Android.bp +++ b/services/bufferhub/Android.bp @@ -25,6 +25,7 @@ cc_library_shared { "BufferClient.cpp", "BufferHubService.cpp", "BufferNode.cpp", + "UniqueIdGenerator.cpp", ], header_libs: [ "libbufferhub_headers", diff --git a/services/bufferhub/BufferHubService.cpp b/services/bufferhub/BufferHubService.cpp index 3bfd9cbe7e..fc5ad1dbbf 100644 --- a/services/bufferhub/BufferHubService.cpp +++ b/services/bufferhub/BufferHubService.cpp @@ -35,7 +35,7 @@ Return BufferHubService::allocateBuffer(const HardwareBufferDescription& d std::shared_ptr node = std::make_shared(desc.width, desc.height, desc.layers, desc.format, - desc.usage, userMetadataSize); + desc.usage, userMetadataSize, nodeIdGenerator.getId()); if (node == nullptr || !node->IsValid()) { ALOGE("%s: creating BufferNode failed.", __FUNCTION__); _hidl_cb(/*bufferClient=*/nullptr, /*status=*/BufferHubStatus::ALLOCATION_FAILED); diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index 53dd702792..715d0a1f67 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -1,5 +1,6 @@ #include +#include #include #include #include @@ -22,7 +23,8 @@ void BufferNode::InitializeMetadata() { // Allocates a new BufferNode. BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, - uint64_t usage, size_t user_metadata_size) { + uint64_t usage, size_t user_metadata_size, uint32_t id) + : mId(id) { uint32_t out_stride = 0; // graphicBufferId is not used in GraphicBufferAllocator::allocate // TODO(b/112338294) After move to the service folder, stop using the @@ -54,14 +56,23 @@ BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, ui InitializeMetadata(); } -// Free the handle BufferNode::~BufferNode() { + // Free the handle if (buffer_handle_ != nullptr) { status_t ret = GraphicBufferAllocator::get().free(buffer_handle_); if (ret != OK) { ALOGE("%s: Failed to free handle; Got error: %d", __FUNCTION__, ret); } } + + // Free the id, if valid + if (id() != UniqueIdGenerator::kInvalidId) { + if (nodeIdGenerator.freeId(id())) { + ALOGI("%s: id #%u is freed.", __FUNCTION__, id()); + } else { + ALOGE("%s: Cannot free nonexistent id #%u", __FUNCTION__, id()); + } + } } uint64_t BufferNode::GetActiveClientsBitMask() const { diff --git a/services/bufferhub/UniqueIdGenerator.cpp b/services/bufferhub/UniqueIdGenerator.cpp new file mode 100644 index 0000000000..362a026f6c --- /dev/null +++ b/services/bufferhub/UniqueIdGenerator.cpp @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +namespace android { +namespace frameworks { +namespace bufferhub { +namespace V1_0 { +namespace implementation { + +constexpr uint32_t UniqueIdGenerator::kInvalidId; + +uint32_t UniqueIdGenerator::getId() { + std::lock_guard lock(mIdsInUseMutex); + + do { + if (++mLastId >= std::numeric_limits::max()) { + mLastId = kInvalidId + 1; + } + } while (mIdsInUse.find(mLastId) != mIdsInUse.end()); + + mIdsInUse.insert(mLastId); + return mLastId; +} + +bool UniqueIdGenerator::freeId(uint32_t id) { + std::lock_guard lock(mIdsInUseMutex); + auto iter = mIdsInUse.find(id); + if (iter != mIdsInUse.end()) { + mIdsInUse.erase(iter); + return true; + } + + return false; +} + +} // namespace implementation +} // namespace V1_0 +} // namespace bufferhub +} // namespace frameworks +} // namespace android diff --git a/services/bufferhub/include/bufferhub/BufferHubService.h b/services/bufferhub/include/bufferhub/BufferHubService.h index dbbee8fccf..5441750e9a 100644 --- a/services/bufferhub/include/bufferhub/BufferHubService.h +++ b/services/bufferhub/include/bufferhub/BufferHubService.h @@ -22,6 +22,7 @@ #include #include +#include #include namespace android { @@ -34,6 +35,8 @@ using hardware::hidl_handle; using hardware::Return; using hardware::graphics::common::V1_2::HardwareBufferDescription; +static UniqueIdGenerator nodeIdGenerator; + class BufferHubService : public IBufferHub { public: Return allocateBuffer(const HardwareBufferDescription& description, diff --git a/services/bufferhub/include/bufferhub/BufferNode.h b/services/bufferhub/include/bufferhub/BufferNode.h index ffeacaccc3..c490e7c709 100644 --- a/services/bufferhub/include/bufferhub/BufferNode.h +++ b/services/bufferhub/include/bufferhub/BufferNode.h @@ -2,6 +2,7 @@ #define ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_BUFFER_NODE_H_ #include +#include #include namespace android { @@ -14,13 +15,16 @@ class BufferNode { public: // Allocates a new BufferNode. BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, - uint64_t usage, size_t user_metadata_size); + uint64_t usage, size_t user_metadata_size, + uint32_t id = UniqueIdGenerator::kInvalidId); ~BufferNode(); // Returns whether the object holds a valid metadata. bool IsValid() const { return metadata_.IsValid(); } + uint32_t id() const { return mId; } + size_t user_metadata_size() const { return metadata_.user_metadata_size(); } // Accessors of the buffer description and handle @@ -59,6 +63,11 @@ private: // Metadata in shared memory. BufferHubMetadata metadata_; + // A system-unique id generated by bufferhub from 1 to std::numeric_limits::max(). + // BufferNodes not created by bufferhub will have id = 0, meaning "not specified". + // TODO(b/118891412): remove default id = 0 and update comments after pdx is no longer in use + const uint32_t mId = 0; + // The following variables are atomic variables in metadata_ that are visible // to Bn object and Bp objects. Please find more info in // BufferHubDefs::MetadataHeader. diff --git a/services/bufferhub/include/bufferhub/UniqueIdGenerator.h b/services/bufferhub/include/bufferhub/UniqueIdGenerator.h new file mode 100644 index 0000000000..d2e702f7e0 --- /dev/null +++ b/services/bufferhub/include/bufferhub/UniqueIdGenerator.h @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_ID_GENERATOR_H +#define ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_ID_GENERATOR_H + +#include +#include + +#include + +namespace android { +namespace frameworks { +namespace bufferhub { +namespace V1_0 { +namespace implementation { + +// A thread-safe incremental uint32_t id generator. +class UniqueIdGenerator { +public: + // 0 is considered invalid + static constexpr uint32_t kInvalidId = 0UL; + + // Gets next available id. If next id is greater than std::numeric_limits::max() (2 ^ + // 32 - 1), it will try to get an id start from 1 again. + uint32_t getId(); + + // Free a specific id. Return true on freed, false on not found. + bool freeId(uint32_t id); + +private: + std::mutex mIdsInUseMutex; + // Start from kInvalidID to avoid generating it. + uint32_t mLastId = kInvalidId; + std::set mIdsInUse GUARDED_BY(mIdsInUseMutex); +}; + +} // namespace implementation +} // namespace V1_0 +} // namespace bufferhub +} // namespace frameworks +} // namespace android + +#endif // ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_ID_GENERATOR_H diff --git a/services/bufferhub/tests/Android.bp b/services/bufferhub/tests/Android.bp index cef31f6ba9..8a30ef5820 100644 --- a/services/bufferhub/tests/Android.bp +++ b/services/bufferhub/tests/Android.bp @@ -21,4 +21,20 @@ cc_test { ], // TODO(b/117568153): Temporarily opt out using libcrt. no_libcrt: true, +} + +cc_test { + name: "UniqueIdGenerator_test", + srcs: ["UniqueIdGenerator_test.cpp"], + cflags: [ + "-DLOG_TAG=\"UniqueIdGenerator_test\"", + "-DTRACE=0", + "-DATRACE_TAG=ATRACE_TAG_GRAPHICS", + ], + shared_libs: [ + "libbufferhubservice", + ], + static_libs: [ + "libgmock", + ], } \ No newline at end of file diff --git a/services/bufferhub/tests/UniqueIdGenerator_test.cpp b/services/bufferhub/tests/UniqueIdGenerator_test.cpp new file mode 100644 index 0000000000..c4d83e0c1f --- /dev/null +++ b/services/bufferhub/tests/UniqueIdGenerator_test.cpp @@ -0,0 +1,45 @@ +#include +#include + +namespace android { +namespace frameworks { +namespace bufferhub { +namespace V1_0 { +namespace implementation { + +namespace { + +class UniqueIdGeneratorTest : public testing::Test { +protected: + UniqueIdGenerator mIdGenerator; +}; + +TEST_F(UniqueIdGeneratorTest, TestGenerateAndFreeID) { + uint32_t id = mIdGenerator.getId(); + EXPECT_NE(id, UniqueIdGenerator::kInvalidId); + + EXPECT_TRUE(mIdGenerator.freeId(id)); + EXPECT_FALSE(mIdGenerator.freeId(id)); +} + +TEST_F(UniqueIdGeneratorTest, TestGenerateUniqueIncrementalID) { + // 10 IDs should not overflow the UniqueIdGenerator to cause a roll back to start, so the + // resulting IDs should still keep incresing. + const size_t kTestSize = 10U; + uint32_t ids[kTestSize]; + for (int i = 0; i < kTestSize; ++i) { + ids[i] = mIdGenerator.getId(); + EXPECT_NE(ids[i], UniqueIdGenerator::kInvalidId); + if (i >= 1) { + EXPECT_GT(ids[i], ids[i - 1]); + } + } +} + +} // namespace + +} // namespace implementation +} // namespace V1_0 +} // namespace bufferhub +} // namespace frameworks +} // namespace android \ No newline at end of file -- cgit v1.2.3-59-g8ed1b From 1c16df541e8b1a67a042fbb20b6e2c76446e9748 Mon Sep 17 00:00:00 2001 From: Fan Xu Date: Mon, 19 Nov 2018 16:27:27 -0800 Subject: Fix freeId() failed in ~BufferNode And rename UniqueIdGenerator to BufferHubIdGenerator. Created a static getter and Use scoped static initialization for UniqueIdGenerator. Test: run BufferHubBuffer_test, then "adb logcat | grep bufferhub -i" to verify that there are no error messages. Fix: 118844348 Change-Id: Ia75ae4dac13cc709f39161c803401bcbb90c70e2 --- services/bufferhub/Android.bp | 2 +- services/bufferhub/BufferHubIdGenerator.cpp | 61 +++++++++++++++++++++ services/bufferhub/BufferHubService.cpp | 3 +- services/bufferhub/BufferNode.cpp | 4 +- services/bufferhub/UniqueIdGenerator.cpp | 55 ------------------- .../include/bufferhub/BufferHubIdGenerator.h | 63 ++++++++++++++++++++++ .../bufferhub/include/bufferhub/BufferHubService.h | 4 +- services/bufferhub/include/bufferhub/BufferNode.h | 4 +- .../include/bufferhub/UniqueIdGenerator.h | 57 -------------------- services/bufferhub/tests/Android.bp | 6 +-- .../bufferhub/tests/BufferHubIdGenerator_test.cpp | 45 ++++++++++++++++ .../bufferhub/tests/UniqueIdGenerator_test.cpp | 45 ---------------- 12 files changed, 180 insertions(+), 169 deletions(-) create mode 100644 services/bufferhub/BufferHubIdGenerator.cpp delete mode 100644 services/bufferhub/UniqueIdGenerator.cpp create mode 100644 services/bufferhub/include/bufferhub/BufferHubIdGenerator.h delete mode 100644 services/bufferhub/include/bufferhub/UniqueIdGenerator.h create mode 100644 services/bufferhub/tests/BufferHubIdGenerator_test.cpp delete mode 100644 services/bufferhub/tests/UniqueIdGenerator_test.cpp (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/services/bufferhub/Android.bp b/services/bufferhub/Android.bp index b747dbca4b..f9aaa2050f 100644 --- a/services/bufferhub/Android.bp +++ b/services/bufferhub/Android.bp @@ -24,9 +24,9 @@ cc_library_shared { ], srcs: [ "BufferClient.cpp", + "BufferHubIdGenerator.cpp", "BufferHubService.cpp", "BufferNode.cpp", - "UniqueIdGenerator.cpp", ], header_libs: [ "libbufferhub_headers", diff --git a/services/bufferhub/BufferHubIdGenerator.cpp b/services/bufferhub/BufferHubIdGenerator.cpp new file mode 100644 index 0000000000..6444a033e1 --- /dev/null +++ b/services/bufferhub/BufferHubIdGenerator.cpp @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +namespace android { +namespace frameworks { +namespace bufferhub { +namespace V1_0 { +namespace implementation { + +constexpr uint32_t BufferHubIdGenerator::kInvalidId; + +BufferHubIdGenerator& BufferHubIdGenerator::getInstance() { + static BufferHubIdGenerator generator; + + return generator; +} + +uint32_t BufferHubIdGenerator::getId() { + std::lock_guard lock(mIdsInUseMutex); + + do { + if (++mLastId >= std::numeric_limits::max()) { + mLastId = kInvalidId + 1; + } + } while (mIdsInUse.find(mLastId) != mIdsInUse.end()); + + mIdsInUse.insert(mLastId); + return mLastId; +} + +bool BufferHubIdGenerator::freeId(uint32_t id) { + std::lock_guard lock(mIdsInUseMutex); + auto iter = mIdsInUse.find(id); + if (iter != mIdsInUse.end()) { + mIdsInUse.erase(iter); + return true; + } + + return false; +} + +} // namespace implementation +} // namespace V1_0 +} // namespace bufferhub +} // namespace frameworks +} // namespace android diff --git a/services/bufferhub/BufferHubService.cpp b/services/bufferhub/BufferHubService.cpp index 6f97f0d8c0..6eaf0abe8e 100644 --- a/services/bufferhub/BufferHubService.cpp +++ b/services/bufferhub/BufferHubService.cpp @@ -35,7 +35,8 @@ Return BufferHubService::allocateBuffer(const HardwareBufferDescription& d std::shared_ptr node = std::make_shared(desc.width, desc.height, desc.layers, desc.format, - desc.usage, userMetadataSize, nodeIdGenerator.getId()); + desc.usage, userMetadataSize, + BufferHubIdGenerator::getInstance().getId()); if (node == nullptr || !node->IsValid()) { ALOGE("%s: creating BufferNode failed.", __FUNCTION__); _hidl_cb(/*bufferClient=*/nullptr, /*status=*/BufferHubStatus::ALLOCATION_FAILED); diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index 715d0a1f67..ec84849e22 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -66,8 +66,8 @@ BufferNode::~BufferNode() { } // Free the id, if valid - if (id() != UniqueIdGenerator::kInvalidId) { - if (nodeIdGenerator.freeId(id())) { + if (id() != BufferHubIdGenerator::kInvalidId) { + if (BufferHubIdGenerator::getInstance().freeId(id())) { ALOGI("%s: id #%u is freed.", __FUNCTION__, id()); } else { ALOGE("%s: Cannot free nonexistent id #%u", __FUNCTION__, id()); diff --git a/services/bufferhub/UniqueIdGenerator.cpp b/services/bufferhub/UniqueIdGenerator.cpp deleted file mode 100644 index 362a026f6c..0000000000 --- a/services/bufferhub/UniqueIdGenerator.cpp +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright (C) 2018 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include - -namespace android { -namespace frameworks { -namespace bufferhub { -namespace V1_0 { -namespace implementation { - -constexpr uint32_t UniqueIdGenerator::kInvalidId; - -uint32_t UniqueIdGenerator::getId() { - std::lock_guard lock(mIdsInUseMutex); - - do { - if (++mLastId >= std::numeric_limits::max()) { - mLastId = kInvalidId + 1; - } - } while (mIdsInUse.find(mLastId) != mIdsInUse.end()); - - mIdsInUse.insert(mLastId); - return mLastId; -} - -bool UniqueIdGenerator::freeId(uint32_t id) { - std::lock_guard lock(mIdsInUseMutex); - auto iter = mIdsInUse.find(id); - if (iter != mIdsInUse.end()) { - mIdsInUse.erase(iter); - return true; - } - - return false; -} - -} // namespace implementation -} // namespace V1_0 -} // namespace bufferhub -} // namespace frameworks -} // namespace android diff --git a/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h b/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h new file mode 100644 index 0000000000..c5b2cdef33 --- /dev/null +++ b/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_ID_GENERATOR_H +#define ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_ID_GENERATOR_H + +#include +#include + +#include + +namespace android { +namespace frameworks { +namespace bufferhub { +namespace V1_0 { +namespace implementation { + +// A thread-safe incremental uint32_t id generator. +class BufferHubIdGenerator { +public: + // 0 is considered invalid + static constexpr uint32_t kInvalidId = 0UL; + + // Get the singleton instance of this class + static BufferHubIdGenerator& getInstance(); + + // Gets next available id. If next id is greater than std::numeric_limits::max() (2 ^ + // 32 - 1), it will try to get an id start from 1 again. + uint32_t getId(); + + // Free a specific id. Return true on freed, false on not found. + bool freeId(uint32_t id); + +private: + BufferHubIdGenerator() = default; + ~BufferHubIdGenerator() = default; + + std::mutex mIdsInUseMutex; + // Start from kInvalidID to avoid generating it. + uint32_t mLastId = kInvalidId; + std::set mIdsInUse GUARDED_BY(mIdsInUseMutex); +}; + +} // namespace implementation +} // namespace V1_0 +} // namespace bufferhub +} // namespace frameworks +} // namespace android + +#endif // ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_ID_GENERATOR_H diff --git a/services/bufferhub/include/bufferhub/BufferHubService.h b/services/bufferhub/include/bufferhub/BufferHubService.h index 6535659ae7..63997aeff9 100644 --- a/services/bufferhub/include/bufferhub/BufferHubService.h +++ b/services/bufferhub/include/bufferhub/BufferHubService.h @@ -22,7 +22,7 @@ #include #include -#include +#include #include namespace android { @@ -35,8 +35,6 @@ using hardware::hidl_handle; using hardware::Return; using hardware::graphics::common::V1_2::HardwareBufferDescription; -static UniqueIdGenerator nodeIdGenerator; - class BufferHubService : public IBufferHub { public: Return allocateBuffer(const HardwareBufferDescription& description, diff --git a/services/bufferhub/include/bufferhub/BufferNode.h b/services/bufferhub/include/bufferhub/BufferNode.h index c490e7c709..94ef505d41 100644 --- a/services/bufferhub/include/bufferhub/BufferNode.h +++ b/services/bufferhub/include/bufferhub/BufferNode.h @@ -2,7 +2,7 @@ #define ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_BUFFER_NODE_H_ #include -#include +#include #include namespace android { @@ -16,7 +16,7 @@ public: // Allocates a new BufferNode. BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, uint64_t usage, size_t user_metadata_size, - uint32_t id = UniqueIdGenerator::kInvalidId); + uint32_t id = BufferHubIdGenerator::kInvalidId); ~BufferNode(); diff --git a/services/bufferhub/include/bufferhub/UniqueIdGenerator.h b/services/bufferhub/include/bufferhub/UniqueIdGenerator.h deleted file mode 100644 index d2e702f7e0..0000000000 --- a/services/bufferhub/include/bufferhub/UniqueIdGenerator.h +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright (C) 2018 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_ID_GENERATOR_H -#define ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_ID_GENERATOR_H - -#include -#include - -#include - -namespace android { -namespace frameworks { -namespace bufferhub { -namespace V1_0 { -namespace implementation { - -// A thread-safe incremental uint32_t id generator. -class UniqueIdGenerator { -public: - // 0 is considered invalid - static constexpr uint32_t kInvalidId = 0UL; - - // Gets next available id. If next id is greater than std::numeric_limits::max() (2 ^ - // 32 - 1), it will try to get an id start from 1 again. - uint32_t getId(); - - // Free a specific id. Return true on freed, false on not found. - bool freeId(uint32_t id); - -private: - std::mutex mIdsInUseMutex; - // Start from kInvalidID to avoid generating it. - uint32_t mLastId = kInvalidId; - std::set mIdsInUse GUARDED_BY(mIdsInUseMutex); -}; - -} // namespace implementation -} // namespace V1_0 -} // namespace bufferhub -} // namespace frameworks -} // namespace android - -#endif // ANDROID_FRAMEWORKS_BUFFERHUB_V1_0_ID_GENERATOR_H diff --git a/services/bufferhub/tests/Android.bp b/services/bufferhub/tests/Android.bp index 8a30ef5820..39678865cb 100644 --- a/services/bufferhub/tests/Android.bp +++ b/services/bufferhub/tests/Android.bp @@ -24,10 +24,10 @@ cc_test { } cc_test { - name: "UniqueIdGenerator_test", - srcs: ["UniqueIdGenerator_test.cpp"], + name: "BufferHubIdGenerator_test", + srcs: ["BufferHubIdGenerator_test.cpp"], cflags: [ - "-DLOG_TAG=\"UniqueIdGenerator_test\"", + "-DLOG_TAG=\"BufferHubIdGenerator_test\"", "-DTRACE=0", "-DATRACE_TAG=ATRACE_TAG_GRAPHICS", ], diff --git a/services/bufferhub/tests/BufferHubIdGenerator_test.cpp b/services/bufferhub/tests/BufferHubIdGenerator_test.cpp new file mode 100644 index 0000000000..4eddfe0e9b --- /dev/null +++ b/services/bufferhub/tests/BufferHubIdGenerator_test.cpp @@ -0,0 +1,45 @@ +#include +#include + +namespace android { +namespace frameworks { +namespace bufferhub { +namespace V1_0 { +namespace implementation { + +namespace { + +class BufferHubIdGeneratorTest : public testing::Test { +protected: + BufferHubIdGenerator* mIdGenerator = &BufferHubIdGenerator::getInstance(); +}; + +TEST_F(BufferHubIdGeneratorTest, TestGenerateAndFreeID) { + uint32_t id = mIdGenerator->getId(); + EXPECT_NE(id, BufferHubIdGenerator::kInvalidId); + + EXPECT_TRUE(mIdGenerator->freeId(id)); + EXPECT_FALSE(mIdGenerator->freeId(id)); +} + +TEST_F(BufferHubIdGeneratorTest, TestGenerateUniqueIncrementalID) { + // 10 IDs should not overflow the UniqueIdGenerator to cause a roll back to start, so the + // resulting IDs should still keep incresing. + const size_t kTestSize = 10U; + uint32_t ids[kTestSize]; + for (int i = 0; i < kTestSize; ++i) { + ids[i] = mIdGenerator->getId(); + EXPECT_NE(ids[i], BufferHubIdGenerator::kInvalidId); + if (i >= 1) { + EXPECT_GT(ids[i], ids[i - 1]); + } + } +} + +} // namespace + +} // namespace implementation +} // namespace V1_0 +} // namespace bufferhub +} // namespace frameworks +} // namespace android \ No newline at end of file diff --git a/services/bufferhub/tests/UniqueIdGenerator_test.cpp b/services/bufferhub/tests/UniqueIdGenerator_test.cpp deleted file mode 100644 index c4d83e0c1f..0000000000 --- a/services/bufferhub/tests/UniqueIdGenerator_test.cpp +++ /dev/null @@ -1,45 +0,0 @@ -#include -#include - -namespace android { -namespace frameworks { -namespace bufferhub { -namespace V1_0 { -namespace implementation { - -namespace { - -class UniqueIdGeneratorTest : public testing::Test { -protected: - UniqueIdGenerator mIdGenerator; -}; - -TEST_F(UniqueIdGeneratorTest, TestGenerateAndFreeID) { - uint32_t id = mIdGenerator.getId(); - EXPECT_NE(id, UniqueIdGenerator::kInvalidId); - - EXPECT_TRUE(mIdGenerator.freeId(id)); - EXPECT_FALSE(mIdGenerator.freeId(id)); -} - -TEST_F(UniqueIdGeneratorTest, TestGenerateUniqueIncrementalID) { - // 10 IDs should not overflow the UniqueIdGenerator to cause a roll back to start, so the - // resulting IDs should still keep incresing. - const size_t kTestSize = 10U; - uint32_t ids[kTestSize]; - for (int i = 0; i < kTestSize; ++i) { - ids[i] = mIdGenerator.getId(); - EXPECT_NE(ids[i], UniqueIdGenerator::kInvalidId); - if (i >= 1) { - EXPECT_GT(ids[i], ids[i - 1]); - } - } -} - -} // namespace - -} // namespace implementation -} // namespace V1_0 -} // namespace bufferhub -} // namespace frameworks -} // namespace android \ No newline at end of file -- cgit v1.2.3-59-g8ed1b From d34a80a63b47dd6c046afb2be47fa55e6ca7a284 Mon Sep 17 00:00:00 2001 From: Fan Xu Date: Tue, 4 Dec 2018 11:32:39 -0800 Subject: Move BufferHubDefs namespace to ui/BufferHubDefs.h And move libbufferhubservice off libbufferhub_headers and libpdx_headers dependency. Test: build passed, including all the tests under libui and libbufferhubservice Bug: 118893702 Change-Id: I8163a7524c6078f9fbb264cf5253d8cbe05dd1c4 --- libs/ui/include/ui/BufferHubDefs.h | 98 ++++++++++++++++++++++ libs/ui/tests/Android.bp | 6 +- libs/ui/tests/BufferHubMetadata_test.cpp | 4 +- .../include/private/dvr/buffer_hub_defs.h | 88 ++++--------------- services/bufferhub/Android.bp | 4 - services/bufferhub/BufferNode.cpp | 10 +-- services/bufferhub/tests/Android.bp | 4 - services/bufferhub/tests/BufferNode_test.cpp | 5 +- 8 files changed, 124 insertions(+), 95 deletions(-) (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/libs/ui/include/ui/BufferHubDefs.h b/libs/ui/include/ui/BufferHubDefs.h index a1948256f5..ef6668bd76 100644 --- a/libs/ui/include/ui/BufferHubDefs.h +++ b/libs/ui/include/ui/BufferHubDefs.h @@ -29,6 +29,104 @@ namespace android { namespace BufferHubDefs { +// Single buffer clients (up to 32) ownership signal. +// 64-bit atomic unsigned int. +// Each client takes 2 bits. The first bit locates in the first 32 bits of +// buffer_state; the second bit locates in the last 32 bits of buffer_state. +// Client states: +// Gained state 11. Exclusive write state. +// Posted state 10. +// Acquired state 01. Shared read state. +// Released state 00. +// +// MSB LSB +// | | +// v v +// [C31|...|C1|C0|C31| ... |C1|C0] + +// Maximum number of clients a buffer can have. +static constexpr int kMaxNumberOfClients = 32; + +// Definition of bit masks. +// MSB LSB +// | kHighBitsMask | kLowbitsMask | +// v v v +// [b63| ... |b32|b31| ... |b0] + +// The location of lower 32 bits in the 64-bit buffer state. +static constexpr uint64_t kLowbitsMask = (1ULL << kMaxNumberOfClients) - 1ULL; + +// The location of higher 32 bits in the 64-bit buffer state. +static constexpr uint64_t kHighBitsMask = ~kLowbitsMask; + +// The client bit mask of the first client. +static constexpr uint64_t kFirstClientBitMask = (1ULL << kMaxNumberOfClients) + 1ULL; + +// Returns true if any of the client is in gained state. +static inline bool AnyClientGained(uint64_t state) { + uint64_t high_bits = state >> kMaxNumberOfClients; + uint64_t low_bits = state & kLowbitsMask; + return high_bits == low_bits && low_bits != 0ULL; +} + +// Returns true if the input client is in gained state. +static inline bool IsClientGained(uint64_t state, uint64_t client_bit_mask) { + return state == client_bit_mask; +} + +// Returns true if any of the client is in posted state. +static inline bool AnyClientPosted(uint64_t state) { + uint64_t high_bits = state >> kMaxNumberOfClients; + uint64_t low_bits = state & kLowbitsMask; + uint64_t posted_or_acquired = high_bits ^ low_bits; + return posted_or_acquired & high_bits; +} + +// Returns true if the input client is in posted state. +static inline bool IsClientPosted(uint64_t state, uint64_t client_bit_mask) { + uint64_t client_bits = state & client_bit_mask; + if (client_bits == 0ULL) return false; + uint64_t low_bits = client_bits & kLowbitsMask; + return low_bits == 0ULL; +} + +// Return true if any of the client is in acquired state. +static inline bool AnyClientAcquired(uint64_t state) { + uint64_t high_bits = state >> kMaxNumberOfClients; + uint64_t low_bits = state & kLowbitsMask; + uint64_t posted_or_acquired = high_bits ^ low_bits; + return posted_or_acquired & low_bits; +} + +// Return true if the input client is in acquired state. +static inline bool IsClientAcquired(uint64_t state, uint64_t client_bit_mask) { + uint64_t client_bits = state & client_bit_mask; + if (client_bits == 0ULL) return false; + uint64_t high_bits = client_bits & kHighBitsMask; + return high_bits == 0ULL; +} + +// Returns true if all clients are in released state. +static inline bool IsBufferReleased(uint64_t state) { + return state == 0ULL; +} + +// Returns true if the input client is in released state. +static inline bool IsClientReleased(uint64_t state, uint64_t client_bit_mask) { + return (state & client_bit_mask) == 0ULL; +} + +// Returns the next available buffer client's client_state_masks. +// @params union_bits. Union of all existing clients' client_state_masks. +static inline uint64_t FindNextAvailableClientStateMask(uint64_t union_bits) { + uint64_t low_union = union_bits & kLowbitsMask; + if (low_union == kLowbitsMask) return 0ULL; + uint64_t incremented = low_union + 1ULL; + uint64_t difference = incremented ^ low_union; + uint64_t new_low_bit = (difference + 1ULL) >> 1; + return new_low_bit + (new_low_bit << kMaxNumberOfClients); +} + struct __attribute__((aligned(8))) MetadataHeader { // Internal data format, which can be updated as long as the size, padding and field alignment // of the struct is consistent within the same ABI. As this part is subject for future updates, diff --git a/libs/ui/tests/Android.bp b/libs/ui/tests/Android.bp index ca73be79d1..c0f4c8916c 100644 --- a/libs/ui/tests/Android.bp +++ b/libs/ui/tests/Android.bp @@ -73,11 +73,7 @@ cc_test { cc_test { name: "BufferHubMetadata_test", - header_libs: [ - "libbufferhub_headers", - "libdvr_headers", - "libpdx_headers", - ], + header_libs: ["libdvr_headers"], shared_libs: [ "libbase", "libui", diff --git a/libs/ui/tests/BufferHubMetadata_test.cpp b/libs/ui/tests/BufferHubMetadata_test.cpp index 2265336c30..11f8e57adc 100644 --- a/libs/ui/tests/BufferHubMetadata_test.cpp +++ b/libs/ui/tests/BufferHubMetadata_test.cpp @@ -15,11 +15,9 @@ */ #include -#include #include -// TODO(b/118893702): move this function to ui/BufferHubDefs.h after ag/5483995 is landed -using android::dvr::BufferHubDefs::IsBufferReleased; +using android::BufferHubDefs::IsBufferReleased; namespace android { namespace dvr { diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h index 62ef475f16..2de36f26a6 100644 --- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h +++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h @@ -19,104 +19,50 @@ static constexpr uint32_t kMetadataFormat = HAL_PIXEL_FORMAT_BLOB; static constexpr uint32_t kMetadataUsage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN; -// Single buffer clients (up to 32) ownership signal. -// 64-bit atomic unsigned int. -// Each client takes 2 bits. The first bit locates in the first 32 bits of -// buffer_state; the second bit locates in the last 32 bits of buffer_state. -// Client states: -// Gained state 11. Exclusive write state. -// Posted state 10. -// Acquired state 01. Shared read state. -// Released state 00. -// -// MSB LSB -// | | -// v v -// [C31|...|C1|C0|C31| ... |C1|C0] - -// Maximum number of clients a buffer can have. -static constexpr int kMaxNumberOfClients = 32; - -// Definition of bit masks. -// MSB LSB -// | kHighBitsMask | kLowbitsMask | -// v v v -// [b63| ... |b32|b31| ... |b0] - -// The location of lower 32 bits in the 64-bit buffer state. -static constexpr uint64_t kLowbitsMask = (1ULL << kMaxNumberOfClients) - 1ULL; - -// The location of higher 32 bits in the 64-bit buffer state. -static constexpr uint64_t kHighBitsMask = ~kLowbitsMask; - -// The client bit mask of the first client. +// See more details in libs/ui/include/ui/BufferHubDefs.h +static constexpr int kMaxNumberOfClients = + android::BufferHubDefs::kMaxNumberOfClients; +static constexpr uint64_t kLowbitsMask = android::BufferHubDefs::kLowbitsMask; +static constexpr uint64_t kHighBitsMask = android::BufferHubDefs::kHighBitsMask; static constexpr uint64_t kFirstClientBitMask = - (1ULL << kMaxNumberOfClients) + 1ULL; + android::BufferHubDefs::kFirstClientBitMask; -// Returns true if any of the client is in gained state. static inline bool AnyClientGained(uint64_t state) { - uint64_t high_bits = state >> kMaxNumberOfClients; - uint64_t low_bits = state & kLowbitsMask; - return high_bits == low_bits && low_bits != 0ULL; + return android::BufferHubDefs::AnyClientGained(state); } -// Returns true if the input client is in gained state. static inline bool IsClientGained(uint64_t state, uint64_t client_bit_mask) { - return state == client_bit_mask; + return android::BufferHubDefs::IsClientGained(state, client_bit_mask); } -// Returns true if any of the client is in posted state. static inline bool AnyClientPosted(uint64_t state) { - uint64_t high_bits = state >> kMaxNumberOfClients; - uint64_t low_bits = state & kLowbitsMask; - uint64_t posted_or_acquired = high_bits ^ low_bits; - return posted_or_acquired & high_bits; + return android::BufferHubDefs::AnyClientPosted(state); } -// Returns true if the input client is in posted state. static inline bool IsClientPosted(uint64_t state, uint64_t client_bit_mask) { - uint64_t client_bits = state & client_bit_mask; - if (client_bits == 0ULL) - return false; - uint64_t low_bits = client_bits & kLowbitsMask; - return low_bits == 0ULL; + return android::BufferHubDefs::IsClientPosted(state, client_bit_mask); } -// Return true if any of the client is in acquired state. static inline bool AnyClientAcquired(uint64_t state) { - uint64_t high_bits = state >> kMaxNumberOfClients; - uint64_t low_bits = state & kLowbitsMask; - uint64_t posted_or_acquired = high_bits ^ low_bits; - return posted_or_acquired & low_bits; + return android::BufferHubDefs::AnyClientAcquired(state); } -// Return true if the input client is in acquired state. static inline bool IsClientAcquired(uint64_t state, uint64_t client_bit_mask) { - uint64_t client_bits = state & client_bit_mask; - if (client_bits == 0ULL) - return false; - uint64_t high_bits = client_bits & kHighBitsMask; - return high_bits == 0ULL; + return android::BufferHubDefs::IsClientAcquired(state, client_bit_mask); } -// Returns true if all clients are in released state. -static inline bool IsBufferReleased(uint64_t state) { return state == 0ULL; } +static inline bool IsBufferReleased(uint64_t state) { + return android::BufferHubDefs::IsBufferReleased(state); +} -// Returns true if the input client is in released state. static inline bool IsClientReleased(uint64_t state, uint64_t client_bit_mask) { - return (state & client_bit_mask) == 0ULL; + return android::BufferHubDefs::IsClientReleased(state, client_bit_mask); } // Returns the next available buffer client's client_state_masks. // @params union_bits. Union of all existing clients' client_state_masks. static inline uint64_t FindNextAvailableClientStateMask(uint64_t union_bits) { - uint64_t low_union = union_bits & kLowbitsMask; - if (low_union == kLowbitsMask) - return 0ULL; - uint64_t incremented = low_union + 1ULL; - uint64_t difference = incremented ^ low_union; - uint64_t new_low_bit = (difference + 1ULL) >> 1; - return new_low_bit + (new_low_bit << kMaxNumberOfClients); + return android::BufferHubDefs::FindNextAvailableClientStateMask(union_bits); } using MetadataHeader = android::BufferHubDefs::MetadataHeader; diff --git a/services/bufferhub/Android.bp b/services/bufferhub/Android.bp index f9aaa2050f..72d210cbb0 100644 --- a/services/bufferhub/Android.bp +++ b/services/bufferhub/Android.bp @@ -29,10 +29,8 @@ cc_library_shared { "BufferNode.cpp", ], header_libs: [ - "libbufferhub_headers", "libdvr_headers", "libnativewindow_headers", - "libpdx_headers", ], shared_libs: [ "android.frameworks.bufferhub@1.0", @@ -56,10 +54,8 @@ cc_binary { "main_bufferhub.cpp" ], header_libs: [ - "libbufferhub_headers", "libdvr_headers", "libnativewindow_headers", - "libpdx_headers", ], shared_libs: [ "android.frameworks.bufferhub@1.0", diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index ec84849e22..4bad829210 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -2,7 +2,6 @@ #include #include -#include #include namespace android { @@ -14,7 +13,7 @@ namespace implementation { void BufferNode::InitializeMetadata() { // Using placement new here to reuse shared memory instead of new allocation // Initialize the atomic variables to zero. - dvr::BufferHubDefs::MetadataHeader* metadata_header = metadata_.metadata_header(); + BufferHubDefs::MetadataHeader* metadata_header = metadata_.metadata_header(); buffer_state_ = new (&metadata_header->buffer_state) std::atomic(0); fence_state_ = new (&metadata_header->fence_state) std::atomic(0); active_clients_bit_mask_ = @@ -84,10 +83,11 @@ uint64_t BufferNode::AddNewActiveClientsBitToMask() { uint64_t client_state_mask = 0ULL; uint64_t updated_active_clients_bit_mask = 0ULL; do { - client_state_mask = dvr::BufferHubDefs::FindNextAvailableClientStateMask( - current_active_clients_bit_mask); + client_state_mask = + BufferHubDefs::FindNextAvailableClientStateMask(current_active_clients_bit_mask); if (client_state_mask == 0ULL) { - ALOGE("%s: reached the maximum number of channels per buffer node: 32.", __FUNCTION__); + ALOGE("%s: reached the maximum number of channels per buffer node: %d.", __FUNCTION__, + BufferHubDefs::kMaxNumberOfClients); errno = E2BIG; return 0ULL; } diff --git a/services/bufferhub/tests/Android.bp b/services/bufferhub/tests/Android.bp index 39678865cb..bf65469784 100644 --- a/services/bufferhub/tests/Android.bp +++ b/services/bufferhub/tests/Android.bp @@ -7,10 +7,8 @@ cc_test { "-DATRACE_TAG=ATRACE_TAG_GRAPHICS", ], header_libs: [ - "libbufferhub_headers", "libdvr_headers", "libnativewindow_headers", - "libpdx_headers", ], shared_libs: [ "libbufferhubservice", @@ -19,8 +17,6 @@ cc_test { static_libs: [ "libgmock", ], - // TODO(b/117568153): Temporarily opt out using libcrt. - no_libcrt: true, } cc_test { diff --git a/services/bufferhub/tests/BufferNode_test.cpp b/services/bufferhub/tests/BufferNode_test.cpp index 3358c87eb8..8555eb7800 100644 --- a/services/bufferhub/tests/BufferNode_test.cpp +++ b/services/bufferhub/tests/BufferNode_test.cpp @@ -3,7 +3,7 @@ #include #include #include -#include +#include #include namespace android { @@ -22,7 +22,6 @@ const uint32_t kLayerCount = 1; const uint32_t kFormat = 1; const uint64_t kUsage = 0; const size_t kUserMetadataSize = 0; -const size_t kMaxClientsCount = dvr::BufferHubDefs::kMaxNumberOfClients; class BufferNodeTest : public ::testing::Test { protected: @@ -73,7 +72,7 @@ TEST_F(BufferNodeTest, TestAddNewActiveClientsBitToMask_32NewClients) { uint64_t current_mask = 0ULL; uint64_t expected_mask = 0ULL; - for (int i = 0; i < kMaxClientsCount; ++i) { + for (int i = 0; i < BufferHubDefs::kMaxNumberOfClients; ++i) { new_client_state_mask = buffer_node->AddNewActiveClientsBitToMask(); EXPECT_NE(new_client_state_mask, 0); EXPECT_FALSE(new_client_state_mask & current_mask); -- cgit v1.2.3-59-g8ed1b From a99f9114d15add6bf1d67264e2a60a73e13009fe Mon Sep 17 00:00:00 2001 From: Tianyu Jiang Date: Thu, 13 Dec 2018 18:23:07 -0800 Subject: Change atomics in ashmem from uint64_t to uint32_t Fix: 117849512 Test: Blueline: atest AHardwareBufferTest BufferHub_test buffer_hub_queue_producer-test libgui_test libsensor_test vrflinger_test buffer_hub-test buffer_hub_queue-test dvr_buffer_queue-test dvr_api-test dvr_display-test Test: in libui_test InputSurfacesTest are segfault on top of master already. Test: Vega: AHardwareBufferTest BufferHubBuffer_test BufferHubMetadata_test buffer_hub_queue_producer-test buffer_hub-test dvr_buffer_queue-test buffer_hub_queue-test dvr_api-test libdvrtracking-test Change-Id: I55f91c21f7ac07615b5451b5413521d7938cf591 --- libs/gui/BufferHubProducer.cpp | 2 +- libs/ui/BufferHubBuffer.cpp | 30 +++--- libs/ui/include/ui/BufferHubBuffer.h | 12 +-- libs/ui/include/ui/BufferHubDefs.h | 101 +++++++++++---------- libs/ui/tests/BufferHubBuffer_test.cpp | 12 +-- libs/vr/libbufferhub/buffer_hub-test.cpp | 22 ++--- libs/vr/libbufferhub/buffer_hub_base.cpp | 8 +- libs/vr/libbufferhub/consumer_buffer.cpp | 22 ++--- .../include/private/dvr/buffer_hub_base.h | 16 ++-- .../include/private/dvr/buffer_hub_defs.h | 30 +++--- .../include/private/dvr/bufferhub_rpc.h | 6 +- libs/vr/libbufferhub/producer_buffer.cpp | 36 ++++---- libs/vr/libdvr/include/dvr/dvr_api.h | 4 +- libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp | 2 +- services/bufferhub/BufferNode.cpp | 22 ++--- .../include/bufferhub/BufferHubIdGenerator.h | 2 +- services/bufferhub/include/bufferhub/BufferNode.h | 14 +-- services/bufferhub/tests/BufferNode_test.cpp | 16 ++-- services/vr/bufferhubd/buffer_channel.cpp | 4 +- services/vr/bufferhubd/consumer_channel.cpp | 2 +- .../include/private/dvr/buffer_channel.h | 4 +- .../include/private/dvr/consumer_channel.h | 6 +- .../include/private/dvr/producer_channel.h | 20 ++-- services/vr/bufferhubd/producer_channel.cpp | 82 ++++++++--------- 24 files changed, 239 insertions(+), 236 deletions(-) (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/libs/gui/BufferHubProducer.cpp b/libs/gui/BufferHubProducer.cpp index ed773e003f..16952a6625 100644 --- a/libs/gui/BufferHubProducer.cpp +++ b/libs/gui/BufferHubProducer.cpp @@ -520,7 +520,7 @@ status_t BufferHubProducer::cancelBuffer(int slot, const sp& fence) { } auto buffer_producer = buffers_[slot].mBufferProducer; - queue_->Enqueue(buffer_producer, size_t(slot), 0ULL); + queue_->Enqueue(buffer_producer, size_t(slot), 0U); buffers_[slot].mBufferState.cancel(); buffers_[slot].mFence = fence; ALOGV("cancelBuffer: slot %d", slot); diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index 1464e48367..5bc113f4b2 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -191,22 +191,22 @@ int BufferHubBuffer::ImportGraphicBuffer() { mClientStateMask = bufferTraits.client_state_mask(); // TODO(b/112012161) Set up shared fences. - ALOGD("BufferHubBuffer::ImportGraphicBuffer: id=%d, buffer_state=%" PRIx64 ".", id(), + ALOGD("BufferHubBuffer::ImportGraphicBuffer: id=%d, buffer_state=%" PRIx32 ".", id(), buffer_state_->load(std::memory_order_acquire)); return 0; } int BufferHubBuffer::Gain() { - uint64_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (IsClientGained(current_buffer_state, mClientStateMask)) { - ALOGV("%s: Buffer is already gained by this client %" PRIx64 ".", __FUNCTION__, + ALOGV("%s: Buffer is already gained by this client %" PRIx32 ".", __FUNCTION__, mClientStateMask); return 0; } do { if (AnyClientGained(current_buffer_state & (~mClientStateMask)) || AnyClientAcquired(current_buffer_state)) { - ALOGE("%s: Buffer is in use, id=%d mClientStateMask=%" PRIx64 " state=%" PRIx64 ".", + ALOGE("%s: Buffer is in use, id=%d mClientStateMask=%" PRIx32 " state=%" PRIx32 ".", __FUNCTION__, mId, mClientStateMask, current_buffer_state); return -EBUSY; } @@ -220,13 +220,13 @@ int BufferHubBuffer::Gain() { } int BufferHubBuffer::Post() { - uint64_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - uint64_t current_active_clients_bit_mask = 0ULL; - uint64_t updated_buffer_state = 0ULL; + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); + uint32_t current_active_clients_bit_mask = 0U; + uint32_t updated_buffer_state = 0U; do { if (!IsClientGained(current_buffer_state, mClientStateMask)) { ALOGE("%s: Cannot post a buffer that is not gained by this client. buffer_id=%d " - "mClientStateMask=%" PRIx64 " state=%" PRIx64 ".", + "mClientStateMask=%" PRIx32 " state=%" PRIx32 ".", __FUNCTION__, mId, mClientStateMask, current_buffer_state); return -EBUSY; } @@ -242,17 +242,17 @@ int BufferHubBuffer::Post() { } int BufferHubBuffer::Acquire() { - uint64_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (IsClientAcquired(current_buffer_state, mClientStateMask)) { - ALOGV("%s: Buffer is already acquired by this client %" PRIx64 ".", __FUNCTION__, + ALOGV("%s: Buffer is already acquired by this client %" PRIx32 ".", __FUNCTION__, mClientStateMask); return 0; } - uint64_t updated_buffer_state = 0ULL; + uint32_t updated_buffer_state = 0U; do { if (!IsClientPosted(current_buffer_state, mClientStateMask)) { ALOGE("%s: Cannot acquire a buffer that is not in posted state. buffer_id=%d " - "mClientStateMask=%" PRIx64 " state=%" PRIx64 ".", + "mClientStateMask=%" PRIx32 " state=%" PRIx32 ".", __FUNCTION__, mId, mClientStateMask, current_buffer_state); return -EBUSY; } @@ -266,13 +266,13 @@ int BufferHubBuffer::Acquire() { } int BufferHubBuffer::Release() { - uint64_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (IsClientReleased(current_buffer_state, mClientStateMask)) { - ALOGV("%s: Buffer is already released by this client %" PRIx64 ".", __FUNCTION__, + ALOGV("%s: Buffer is already released by this client %" PRIx32 ".", __FUNCTION__, mClientStateMask); return 0; } - uint64_t updated_buffer_state = 0ULL; + uint32_t updated_buffer_state = 0U; do { updated_buffer_state = current_buffer_state & (~mClientStateMask); } while (!buffer_state_->compare_exchange_weak(current_buffer_state, updated_buffer_state, diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h index 11e9b5c88d..90dd391d8d 100644 --- a/libs/ui/include/ui/BufferHubBuffer.h +++ b/libs/ui/include/ui/BufferHubBuffer.h @@ -86,13 +86,13 @@ public: const native_handle_t* DuplicateHandle() { return mBufferHandle.DuplicateHandle(); } // Returns the current value of MetadataHeader::buffer_state. - uint64_t buffer_state() { + uint32_t buffer_state() { return mMetadata.metadata_header()->buffer_state.load(std::memory_order_acquire); } // A state mask which is unique to a buffer hub client among all its siblings sharing the same // concrete graphic buffer. - uint64_t client_state_mask() const { return mClientStateMask; } + uint32_t client_state_mask() const { return mClientStateMask; } size_t user_metadata_size() const { return mMetadata.user_metadata_size(); } @@ -154,7 +154,7 @@ private: // Client state mask of this BufferHubBuffer object. It is unique amoung all // clients/users of the buffer. - uint64_t mClientStateMask = 0; + uint32_t mClientStateMask = 0U; // Stores ground truth of the buffer. AHardwareBuffer_Desc mBufferDesc; @@ -166,9 +166,9 @@ private: // bufferhubd daemon and all buffer clients. BufferHubMetadata mMetadata; // Shortcuts to the atomics inside the header of mMetadata. - std::atomic* buffer_state_{nullptr}; - std::atomic* fence_state_{nullptr}; - std::atomic* active_clients_bit_mask_{nullptr}; + std::atomic* buffer_state_ = nullptr; + std::atomic* fence_state_ = nullptr; + std::atomic* active_clients_bit_mask_ = nullptr; // PDX backend. BufferHubClient mClient; diff --git a/libs/ui/include/ui/BufferHubDefs.h b/libs/ui/include/ui/BufferHubDefs.h index ef6668bd76..d259fefb8f 100644 --- a/libs/ui/include/ui/BufferHubDefs.h +++ b/libs/ui/include/ui/BufferHubDefs.h @@ -29,10 +29,10 @@ namespace android { namespace BufferHubDefs { -// Single buffer clients (up to 32) ownership signal. -// 64-bit atomic unsigned int. -// Each client takes 2 bits. The first bit locates in the first 32 bits of -// buffer_state; the second bit locates in the last 32 bits of buffer_state. +// Single buffer clients (up to 16) ownership signal. +// 32-bit atomic unsigned int. +// Each client takes 2 bits. The first bit locates in the first 16 bits of +// buffer_state; the second bit locates in the last 16 bits of buffer_state. // Client states: // Gained state 11. Exclusive write state. // Posted state 10. @@ -42,88 +42,88 @@ namespace BufferHubDefs { // MSB LSB // | | // v v -// [C31|...|C1|C0|C31| ... |C1|C0] +// [C15|...|C1|C0|C15| ... |C1|C0] // Maximum number of clients a buffer can have. -static constexpr int kMaxNumberOfClients = 32; +static constexpr int kMaxNumberOfClients = 16; // Definition of bit masks. // MSB LSB // | kHighBitsMask | kLowbitsMask | // v v v -// [b63| ... |b32|b31| ... |b0] +// [b31| ... |b16|b15| ... |b0] -// The location of lower 32 bits in the 64-bit buffer state. -static constexpr uint64_t kLowbitsMask = (1ULL << kMaxNumberOfClients) - 1ULL; +// The location of lower 16 bits in the 32-bit buffer state. +static constexpr uint32_t kLowbitsMask = (1U << kMaxNumberOfClients) - 1U; -// The location of higher 32 bits in the 64-bit buffer state. -static constexpr uint64_t kHighBitsMask = ~kLowbitsMask; +// The location of higher 16 bits in the 32-bit buffer state. +static constexpr uint32_t kHighBitsMask = ~kLowbitsMask; // The client bit mask of the first client. -static constexpr uint64_t kFirstClientBitMask = (1ULL << kMaxNumberOfClients) + 1ULL; +static constexpr uint32_t kFirstClientBitMask = (1U << kMaxNumberOfClients) + 1U; // Returns true if any of the client is in gained state. -static inline bool AnyClientGained(uint64_t state) { - uint64_t high_bits = state >> kMaxNumberOfClients; - uint64_t low_bits = state & kLowbitsMask; - return high_bits == low_bits && low_bits != 0ULL; +static inline bool AnyClientGained(uint32_t state) { + uint32_t high_bits = state >> kMaxNumberOfClients; + uint32_t low_bits = state & kLowbitsMask; + return high_bits == low_bits && low_bits != 0U; } // Returns true if the input client is in gained state. -static inline bool IsClientGained(uint64_t state, uint64_t client_bit_mask) { +static inline bool IsClientGained(uint32_t state, uint32_t client_bit_mask) { return state == client_bit_mask; } // Returns true if any of the client is in posted state. -static inline bool AnyClientPosted(uint64_t state) { - uint64_t high_bits = state >> kMaxNumberOfClients; - uint64_t low_bits = state & kLowbitsMask; - uint64_t posted_or_acquired = high_bits ^ low_bits; +static inline bool AnyClientPosted(uint32_t state) { + uint32_t high_bits = state >> kMaxNumberOfClients; + uint32_t low_bits = state & kLowbitsMask; + uint32_t posted_or_acquired = high_bits ^ low_bits; return posted_or_acquired & high_bits; } // Returns true if the input client is in posted state. -static inline bool IsClientPosted(uint64_t state, uint64_t client_bit_mask) { - uint64_t client_bits = state & client_bit_mask; - if (client_bits == 0ULL) return false; - uint64_t low_bits = client_bits & kLowbitsMask; - return low_bits == 0ULL; +static inline bool IsClientPosted(uint32_t state, uint32_t client_bit_mask) { + uint32_t client_bits = state & client_bit_mask; + if (client_bits == 0U) return false; + uint32_t low_bits = client_bits & kLowbitsMask; + return low_bits == 0U; } // Return true if any of the client is in acquired state. -static inline bool AnyClientAcquired(uint64_t state) { - uint64_t high_bits = state >> kMaxNumberOfClients; - uint64_t low_bits = state & kLowbitsMask; - uint64_t posted_or_acquired = high_bits ^ low_bits; +static inline bool AnyClientAcquired(uint32_t state) { + uint32_t high_bits = state >> kMaxNumberOfClients; + uint32_t low_bits = state & kLowbitsMask; + uint32_t posted_or_acquired = high_bits ^ low_bits; return posted_or_acquired & low_bits; } // Return true if the input client is in acquired state. -static inline bool IsClientAcquired(uint64_t state, uint64_t client_bit_mask) { - uint64_t client_bits = state & client_bit_mask; - if (client_bits == 0ULL) return false; - uint64_t high_bits = client_bits & kHighBitsMask; - return high_bits == 0ULL; +static inline bool IsClientAcquired(uint32_t state, uint32_t client_bit_mask) { + uint32_t client_bits = state & client_bit_mask; + if (client_bits == 0U) return false; + uint32_t high_bits = client_bits & kHighBitsMask; + return high_bits == 0U; } // Returns true if all clients are in released state. -static inline bool IsBufferReleased(uint64_t state) { - return state == 0ULL; +static inline bool IsBufferReleased(uint32_t state) { + return state == 0U; } // Returns true if the input client is in released state. -static inline bool IsClientReleased(uint64_t state, uint64_t client_bit_mask) { - return (state & client_bit_mask) == 0ULL; +static inline bool IsClientReleased(uint32_t state, uint32_t client_bit_mask) { + return (state & client_bit_mask) == 0U; } // Returns the next available buffer client's client_state_masks. // @params union_bits. Union of all existing clients' client_state_masks. -static inline uint64_t FindNextAvailableClientStateMask(uint64_t union_bits) { - uint64_t low_union = union_bits & kLowbitsMask; - if (low_union == kLowbitsMask) return 0ULL; - uint64_t incremented = low_union + 1ULL; - uint64_t difference = incremented ^ low_union; - uint64_t new_low_bit = (difference + 1ULL) >> 1; +static inline uint32_t FindNextAvailableClientStateMask(uint32_t union_bits) { + uint32_t low_union = union_bits & kLowbitsMask; + if (low_union == kLowbitsMask) return 0U; + uint32_t incremented = low_union + 1U; + uint32_t difference = incremented ^ low_union; + uint32_t new_low_bit = (difference + 1U) >> 1; return new_low_bit + (new_low_bit << kMaxNumberOfClients); } @@ -135,15 +135,18 @@ struct __attribute__((aligned(8))) MetadataHeader { // Every client takes up one bit from the higher 32 bits and one bit from the lower 32 bits in // buffer_state. - std::atomic buffer_state; + std::atomic buffer_state; // Every client takes up one bit in fence_state. Only the lower 32 bits are valid. The upper 32 // bits are there for easier manipulation, but the value should be ignored. - std::atomic fence_state; + std::atomic fence_state; // Every client takes up one bit from the higher 32 bits and one bit from the lower 32 bits in // active_clients_bit_mask. - std::atomic active_clients_bit_mask; + std::atomic active_clients_bit_mask; + + // Explicit padding 4 bytes. + uint32_t padding; // The index of the buffer queue where the buffer belongs to. uint64_t queue_index; @@ -152,7 +155,7 @@ struct __attribute__((aligned(8))) MetadataHeader { DvrNativeBufferMetadata metadata; }; -static_assert(sizeof(MetadataHeader) == 136, "Unexpected MetadataHeader size"); +static_assert(sizeof(MetadataHeader) == 128, "Unexpected MetadataHeader size"); static constexpr size_t kMetadataHeaderSize = sizeof(MetadataHeader); } // namespace BufferHubDefs diff --git a/libs/ui/tests/BufferHubBuffer_test.cpp b/libs/ui/tests/BufferHubBuffer_test.cpp index f616785387..a894f201c5 100644 --- a/libs/ui/tests/BufferHubBuffer_test.cpp +++ b/libs/ui/tests/BufferHubBuffer_test.cpp @@ -67,9 +67,9 @@ protected: } std::unique_ptr b1; - uint64_t b1ClientMask = 0ULL; + uint64_t b1ClientMask = 0U; std::unique_ptr b2; - uint64_t b2ClientMask = 0ULL; + uint64_t b2ClientMask = 0U; private: // Creates b1 and b2 as the clients of the same buffer for testing. @@ -79,13 +79,13 @@ private: void BufferHubBufferStateTransitionTest::CreateTwoClientsOfABuffer() { b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); b1ClientMask = b1->client_state_mask(); - ASSERT_NE(b1ClientMask, 0ULL); + ASSERT_NE(b1ClientMask, 0U); auto statusOrHandle = b1->Duplicate(); ASSERT_TRUE(statusOrHandle); LocalChannelHandle h2 = statusOrHandle.take(); b2 = BufferHubBuffer::Import(std::move(h2)); b2ClientMask = b2->client_state_mask(); - ASSERT_NE(b2ClientMask, 0ULL); + ASSERT_NE(b2ClientMask, 0U); ASSERT_NE(b2ClientMask, b1ClientMask); } @@ -126,7 +126,7 @@ TEST_F(BufferHubBufferTest, DuplicateBufferHubBuffer) { kUserMetadataSize); int id1 = b1->id(); uint64_t bufferStateMask1 = b1->client_state_mask(); - EXPECT_NE(bufferStateMask1, 0ULL); + EXPECT_NE(bufferStateMask1, 0U); EXPECT_TRUE(b1->IsValid()); EXPECT_EQ(b1->user_metadata_size(), kUserMetadataSize); @@ -149,7 +149,7 @@ TEST_F(BufferHubBufferTest, DuplicateBufferHubBuffer) { int id2 = b2->id(); uint64_t bufferStateMask2 = b2->client_state_mask(); - EXPECT_NE(bufferStateMask2, 0ULL); + EXPECT_NE(bufferStateMask2, 0U); // These two buffer instances are based on the same physical buffer under the // hood, so they should share the same id. diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp index c3fa77b958..6cb6541c52 100644 --- a/libs/vr/libbufferhub/buffer_hub-test.cpp +++ b/libs/vr/libbufferhub/buffer_hub-test.cpp @@ -175,7 +175,7 @@ TEST_F(LibBufferHubTest, TestStateMask) { ASSERT_TRUE(p.get() != nullptr); // It's ok to create up to kMaxConsumerCount consumer buffers. - uint64_t client_state_masks = p->client_state_mask(); + uint32_t client_state_masks = p->client_state_mask(); std::array, kMaxConsumerCount> cs; for (size_t i = 0; i < kMaxConsumerCount; i++) { cs[i] = ConsumerBuffer::Import(p->CreateConsumer()); @@ -184,7 +184,7 @@ TEST_F(LibBufferHubTest, TestStateMask) { EXPECT_EQ(client_state_masks & cs[i]->client_state_mask(), 0U); client_state_masks |= cs[i]->client_state_mask(); } - EXPECT_EQ(client_state_masks, ~0ULL); + EXPECT_EQ(client_state_masks, ~0U); // The 64th creation will fail with out-of-memory error. auto state = p->CreateConsumer(); @@ -373,7 +373,7 @@ TEST_F(LibBufferHubTest, TestMaxConsumers) { std::unique_ptr p = ProducerBuffer::Create( kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t)); ASSERT_TRUE(p.get() != nullptr); - uint64_t producer_state_mask = p->client_state_mask(); + uint32_t producer_state_mask = p->client_state_mask(); std::array, kMaxConsumerCount> cs; for (size_t i = 0; i < kMaxConsumerCount; ++i) { @@ -719,7 +719,7 @@ TEST_F(LibBufferHubTest, TestOrphanedAcquire) { std::unique_ptr c1 = ConsumerBuffer::Import(p->CreateConsumer()); ASSERT_TRUE(c1.get() != nullptr); - const uint64_t client_state_mask1 = c1->client_state_mask(); + const uint32_t client_state_mask1 = c1->client_state_mask(); EXPECT_EQ(0, p->GainAsync()); DvrNativeBufferMetadata meta; @@ -739,7 +739,7 @@ TEST_F(LibBufferHubTest, TestOrphanedAcquire) { std::unique_ptr c2 = ConsumerBuffer::Import(p->CreateConsumer()); ASSERT_TRUE(c2.get() != nullptr); - const uint64_t client_state_mask2 = c2->client_state_mask(); + const uint32_t client_state_mask2 = c2->client_state_mask(); EXPECT_NE(client_state_mask1, client_state_mask2); EXPECT_EQ(0, RETRY_EINTR(c2->Poll(kPollTimeoutMs))); EXPECT_EQ(-EBUSY, c2->AcquireAsync(&meta, &fence)); @@ -755,7 +755,7 @@ TEST_F(LibBufferHubTest, TestAcquireLastPosted) { std::unique_ptr c1 = ConsumerBuffer::Import(p->CreateConsumer()); ASSERT_TRUE(c1.get() != nullptr); - const uint64_t client_state_mask1 = c1->client_state_mask(); + const uint32_t client_state_mask1 = c1->client_state_mask(); EXPECT_EQ(0, p->GainAsync()); DvrNativeBufferMetadata meta; @@ -767,7 +767,7 @@ TEST_F(LibBufferHubTest, TestAcquireLastPosted) { std::unique_ptr c2 = ConsumerBuffer::Import(p->CreateConsumer()); ASSERT_TRUE(c2.get() != nullptr); - const uint64_t client_state_mask2 = c2->client_state_mask(); + const uint32_t client_state_mask2 = c2->client_state_mask(); EXPECT_NE(client_state_mask1, client_state_mask2); EXPECT_LT(0, RETRY_EINTR(c2->Poll(kPollTimeoutMs))); LocalHandle invalid_fence; @@ -781,7 +781,7 @@ TEST_F(LibBufferHubTest, TestAcquireLastPosted) { std::unique_ptr c3 = ConsumerBuffer::Import(p->CreateConsumer()); ASSERT_TRUE(c3.get() != nullptr); - const uint64_t client_state_mask3 = c3->client_state_mask(); + const uint32_t client_state_mask3 = c3->client_state_mask(); EXPECT_NE(client_state_mask1, client_state_mask3); EXPECT_NE(client_state_mask2, client_state_mask3); EXPECT_LT(0, RETRY_EINTR(c3->Poll(kPollTimeoutMs))); @@ -802,7 +802,7 @@ TEST_F(LibBufferHubTest, TestAcquireLastPosted) { std::unique_ptr c4 = ConsumerBuffer::Import(p->CreateConsumer()); ASSERT_TRUE(c4.get() != nullptr); - const uint64_t client_state_mask4 = c4->client_state_mask(); + const uint32_t client_state_mask4 = c4->client_state_mask(); EXPECT_NE(client_state_mask3, client_state_mask4); EXPECT_GE(0, RETRY_EINTR(c3->Poll(kPollTimeoutMs))); EXPECT_EQ(-EBUSY, c3->AcquireAsync(&meta, &invalid_fence)); @@ -952,7 +952,7 @@ TEST_F(LibBufferHubTest, TestDuplicateBufferHubBuffer) { int b1_id = b1->id(); EXPECT_TRUE(b1->IsValid()); EXPECT_EQ(b1->user_metadata_size(), kUserMetadataSize); - EXPECT_NE(b1->client_state_mask(), 0ULL); + EXPECT_NE(b1->client_state_mask(), 0U); auto status_or_handle = b1->Duplicate(); EXPECT_TRUE(status_or_handle); @@ -970,7 +970,7 @@ TEST_F(LibBufferHubTest, TestDuplicateBufferHubBuffer) { ASSERT_TRUE(b2 != nullptr); EXPECT_TRUE(b2->IsValid()); EXPECT_EQ(b2->user_metadata_size(), kUserMetadataSize); - EXPECT_NE(b2->client_state_mask(), 0ULL); + EXPECT_NE(b2->client_state_mask(), 0U); int b2_id = b2->id(); diff --git a/libs/vr/libbufferhub/buffer_hub_base.cpp b/libs/vr/libbufferhub/buffer_hub_base.cpp index 2dc427aec1..8497f3edc1 100644 --- a/libs/vr/libbufferhub/buffer_hub_base.cpp +++ b/libs/vr/libbufferhub/buffer_hub_base.cpp @@ -124,16 +124,16 @@ int BufferHubBase::ImportBuffer() { // memory region will be preserved. buffer_state_ = &metadata_header_->buffer_state; ALOGD_IF(TRACE, - "BufferHubBase::ImportBuffer: id=%d, buffer_state=%" PRIx64 ".", + "BufferHubBase::ImportBuffer: id=%d, buffer_state=%" PRIx32 ".", id(), buffer_state_->load(std::memory_order_acquire)); fence_state_ = &metadata_header_->fence_state; ALOGD_IF(TRACE, - "BufferHubBase::ImportBuffer: id=%d, fence_state=%" PRIx64 ".", id(), + "BufferHubBase::ImportBuffer: id=%d, fence_state=%" PRIx32 ".", id(), fence_state_->load(std::memory_order_acquire)); active_clients_bit_mask_ = &metadata_header_->active_clients_bit_mask; ALOGD_IF( TRACE, - "BufferHubBase::ImportBuffer: id=%d, active_clients_bit_mask=%" PRIx64 + "BufferHubBase::ImportBuffer: id=%d, active_clients_bit_mask=%" PRIx32 ".", id(), active_clients_bit_mask_->load(std::memory_order_acquire)); @@ -171,7 +171,7 @@ int BufferHubBase::UpdateSharedFence(const LocalHandle& new_fence, // If ready fence is valid, we put that into the epoll set. epoll_event event; event.events = EPOLLIN; - event.data.u64 = client_state_mask(); + event.data.u32 = client_state_mask(); pending_fence_fd_ = new_fence.Duplicate(); if (epoll_ctl(shared_fence.Get(), EPOLL_CTL_ADD, pending_fence_fd_.Get(), &event) < 0) { diff --git a/libs/vr/libbufferhub/consumer_buffer.cpp b/libs/vr/libbufferhub/consumer_buffer.cpp index 62fb5fd769..b6ca64eef2 100644 --- a/libs/vr/libbufferhub/consumer_buffer.cpp +++ b/libs/vr/libbufferhub/consumer_buffer.cpp @@ -36,33 +36,33 @@ int ConsumerBuffer::LocalAcquire(DvrNativeBufferMetadata* out_meta, return -EINVAL; // The buffer can be acquired iff the buffer state for this client is posted. - uint64_t current_buffer_state = + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (!BufferHubDefs::IsClientPosted(current_buffer_state, client_state_mask())) { ALOGE( "%s: Failed to acquire the buffer. The buffer is not posted, id=%d " - "state=%" PRIx64 " client_state_mask=%" PRIx64 ".", + "state=%" PRIx32 " client_state_mask=%" PRIx32 ".", __FUNCTION__, id(), current_buffer_state, client_state_mask()); return -EBUSY; } // Change the buffer state for this consumer from posted to acquired. - uint64_t updated_buffer_state = current_buffer_state ^ client_state_mask(); + uint32_t updated_buffer_state = current_buffer_state ^ client_state_mask(); while (!buffer_state_->compare_exchange_weak( current_buffer_state, updated_buffer_state, std::memory_order_acq_rel, std::memory_order_acquire)) { ALOGD( "%s Failed to acquire the buffer. Current buffer state was changed to " - "%" PRIx64 + "%" PRIx32 " when trying to acquire the buffer and modify the buffer state to " - "%" PRIx64 ". About to try again if the buffer is still posted.", + "%" PRIx32 ". About to try again if the buffer is still posted.", __FUNCTION__, current_buffer_state, updated_buffer_state); if (!BufferHubDefs::IsClientPosted(current_buffer_state, client_state_mask())) { ALOGE( "%s: Failed to acquire the buffer. The buffer is no longer posted, " - "id=%d state=%" PRIx64 " client_state_mask=%" PRIx64 ".", + "id=%d state=%" PRIx32 " client_state_mask=%" PRIx32 ".", __FUNCTION__, id(), current_buffer_state, client_state_mask()); return -EBUSY; } @@ -81,7 +81,7 @@ int ConsumerBuffer::LocalAcquire(DvrNativeBufferMetadata* out_meta, out_meta->user_metadata_ptr = 0; } - uint64_t fence_state = fence_state_->load(std::memory_order_acquire); + uint32_t fence_state = fence_state_->load(std::memory_order_acquire); // If there is an acquire fence from producer, we need to return it. // The producer state bit mask is kFirstClientBitMask for now. if (fence_state & BufferHubDefs::kFirstClientBitMask) { @@ -142,21 +142,21 @@ int ConsumerBuffer::LocalRelease(const DvrNativeBufferMetadata* meta, // Set the buffer state of this client to released if it is not already in // released state. - uint64_t current_buffer_state = + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (BufferHubDefs::IsClientReleased(current_buffer_state, client_state_mask())) { return 0; } - uint64_t updated_buffer_state = current_buffer_state & (~client_state_mask()); + uint32_t updated_buffer_state = current_buffer_state & (~client_state_mask()); while (!buffer_state_->compare_exchange_weak( current_buffer_state, updated_buffer_state, std::memory_order_acq_rel, std::memory_order_acquire)) { ALOGD( "%s: Failed to release the buffer. Current buffer state was changed to " - "%" PRIx64 + "%" PRIx32 " when trying to release the buffer and modify the buffer state to " - "%" PRIx64 ". About to try again.", + "%" PRIx32 ". About to try again.", __FUNCTION__, current_buffer_state, updated_buffer_state); // The failure of compare_exchange_weak updates current_buffer_state. updated_buffer_state = current_buffer_state & (~client_state_mask()); diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h index 09feb73f81..440a59dc71 100644 --- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h +++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h @@ -90,13 +90,13 @@ class BufferHubBase : public pdx::Client { int cid() const { return cid_; } // Returns the buffer buffer state. - uint64_t buffer_state() { + uint32_t buffer_state() { return buffer_state_->load(std::memory_order_acquire); }; // A state mask which is unique to a buffer hub client among all its siblings // sharing the same concrete graphic buffer. - uint64_t client_state_mask() const { return client_state_mask_; } + uint32_t client_state_mask() const { return client_state_mask_; } // The following methods return settings of the first buffer. Currently, // it is only possible to create multi-buffer BufferHubBases with the same @@ -132,11 +132,11 @@ class BufferHubBase : public pdx::Client { // IonBuffer that is shared between bufferhubd, producer, and consumers. size_t metadata_buf_size_{0}; size_t user_metadata_size_{0}; - BufferHubDefs::MetadataHeader* metadata_header_{nullptr}; - void* user_metadata_ptr_{nullptr}; - std::atomic* buffer_state_{nullptr}; - std::atomic* fence_state_{nullptr}; - std::atomic* active_clients_bit_mask_{nullptr}; + BufferHubDefs::MetadataHeader* metadata_header_ = nullptr; + void* user_metadata_ptr_ = nullptr; + std::atomic* buffer_state_ = nullptr; + std::atomic* fence_state_ = nullptr; + std::atomic* active_clients_bit_mask_ = nullptr; LocalHandle shared_acquire_fence_; LocalHandle shared_release_fence_; @@ -159,7 +159,7 @@ class BufferHubBase : public pdx::Client { // Client bit mask which indicates the locations of this client object in the // buffer_state_. - uint64_t client_state_mask_{0ULL}; + uint32_t client_state_mask_{0U}; IonBuffer buffer_; IonBuffer metadata_buffer_; }; diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h index 2de36f26a6..f2c40fe3d3 100644 --- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h +++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h @@ -22,46 +22,46 @@ static constexpr uint32_t kMetadataUsage = // See more details in libs/ui/include/ui/BufferHubDefs.h static constexpr int kMaxNumberOfClients = android::BufferHubDefs::kMaxNumberOfClients; -static constexpr uint64_t kLowbitsMask = android::BufferHubDefs::kLowbitsMask; -static constexpr uint64_t kHighBitsMask = android::BufferHubDefs::kHighBitsMask; -static constexpr uint64_t kFirstClientBitMask = +static constexpr uint32_t kLowbitsMask = android::BufferHubDefs::kLowbitsMask; +static constexpr uint32_t kHighBitsMask = android::BufferHubDefs::kHighBitsMask; +static constexpr uint32_t kFirstClientBitMask = android::BufferHubDefs::kFirstClientBitMask; -static inline bool AnyClientGained(uint64_t state) { +static inline bool AnyClientGained(uint32_t state) { return android::BufferHubDefs::AnyClientGained(state); } -static inline bool IsClientGained(uint64_t state, uint64_t client_bit_mask) { +static inline bool IsClientGained(uint32_t state, uint32_t client_bit_mask) { return android::BufferHubDefs::IsClientGained(state, client_bit_mask); } -static inline bool AnyClientPosted(uint64_t state) { +static inline bool AnyClientPosted(uint32_t state) { return android::BufferHubDefs::AnyClientPosted(state); } -static inline bool IsClientPosted(uint64_t state, uint64_t client_bit_mask) { +static inline bool IsClientPosted(uint32_t state, uint32_t client_bit_mask) { return android::BufferHubDefs::IsClientPosted(state, client_bit_mask); } -static inline bool AnyClientAcquired(uint64_t state) { +static inline bool AnyClientAcquired(uint32_t state) { return android::BufferHubDefs::AnyClientAcquired(state); } -static inline bool IsClientAcquired(uint64_t state, uint64_t client_bit_mask) { +static inline bool IsClientAcquired(uint32_t state, uint32_t client_bit_mask) { return android::BufferHubDefs::IsClientAcquired(state, client_bit_mask); } -static inline bool IsBufferReleased(uint64_t state) { +static inline bool IsBufferReleased(uint32_t state) { return android::BufferHubDefs::IsBufferReleased(state); } -static inline bool IsClientReleased(uint64_t state, uint64_t client_bit_mask) { +static inline bool IsClientReleased(uint32_t state, uint32_t client_bit_mask) { return android::BufferHubDefs::IsClientReleased(state, client_bit_mask); } // Returns the next available buffer client's client_state_masks. // @params union_bits. Union of all existing clients' client_state_masks. -static inline uint64_t FindNextAvailableClientStateMask(uint64_t union_bits) { +static inline uint32_t FindNextAvailableClientStateMask(uint32_t union_bits) { return android::BufferHubDefs::FindNextAvailableClientStateMask(union_bits); } @@ -77,7 +77,7 @@ class BufferTraits { BufferTraits() = default; BufferTraits(const native_handle_t* buffer_handle, const FileHandleType& metadata_handle, int id, - uint64_t client_state_mask, uint64_t metadata_size, + uint32_t client_state_mask, uint64_t metadata_size, uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, uint64_t usage, uint32_t stride, const FileHandleType& acquire_fence_fd, @@ -107,7 +107,7 @@ class BufferTraits { // same buffer channel has uniqued state bit among its siblings. For a // producer buffer the bit must be kFirstClientBitMask; for a consumer the bit // must be one of the kConsumerStateMask. - uint64_t client_state_mask() const { return client_state_mask_; } + uint32_t client_state_mask() const { return client_state_mask_; } uint64_t metadata_size() const { return metadata_size_; } uint32_t width() { return width_; } @@ -131,7 +131,7 @@ class BufferTraits { private: // BufferHub specific traits. int id_ = -1; - uint64_t client_state_mask_; + uint32_t client_state_mask_; uint64_t metadata_size_; // Traits for a GraphicBuffer. diff --git a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h index 5ff4e00a01..f1cd0b4adc 100644 --- a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h +++ b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h @@ -99,7 +99,7 @@ class BufferDescription { public: BufferDescription() = default; BufferDescription(const IonBuffer& buffer, const IonBuffer& metadata, int id, - int buffer_cid, uint64_t client_state_mask, + int buffer_cid, uint32_t client_state_mask, const FileHandleType& acquire_fence_fd, const FileHandleType& release_fence_fd) : id_(id), @@ -123,7 +123,7 @@ class BufferDescription { // State mask of the buffer client. Each BufferHub client backed by the // same buffer channel has uniqued state bit among its siblings. - uint64_t client_state_mask() const { return client_state_mask_; } + uint32_t client_state_mask() const { return client_state_mask_; } FileHandleType take_acquire_fence() { return std::move(acquire_fence_fd_); } FileHandleType take_release_fence() { return std::move(release_fence_fd_); } @@ -133,7 +133,7 @@ class BufferDescription { private: int id_{-1}; int buffer_cid_{-1}; - uint64_t client_state_mask_{0}; + uint32_t client_state_mask_{0U}; // Two IonBuffers: one for the graphic buffer and one for metadata. NativeBufferHandle buffer_; NativeBufferHandle metadata_; diff --git a/libs/vr/libbufferhub/producer_buffer.cpp b/libs/vr/libbufferhub/producer_buffer.cpp index cd92b625d5..5274bf25d5 100644 --- a/libs/vr/libbufferhub/producer_buffer.cpp +++ b/libs/vr/libbufferhub/producer_buffer.cpp @@ -80,20 +80,20 @@ int ProducerBuffer::LocalPost(const DvrNativeBufferMetadata* meta, return error; // The buffer can be posted iff the buffer state for this client is gained. - uint64_t current_buffer_state = + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (!BufferHubDefs::IsClientGained(current_buffer_state, client_state_mask())) { - ALOGE("%s: not gained, id=%d state=%" PRIx64 ".", __FUNCTION__, id(), + ALOGE("%s: not gained, id=%d state=%" PRIx32 ".", __FUNCTION__, id(), current_buffer_state); return -EBUSY; } // Set the producer client buffer state to released, other clients' buffer // state to posted. - uint64_t current_active_clients_bit_mask = + uint32_t current_active_clients_bit_mask = active_clients_bit_mask_->load(std::memory_order_acquire); - uint64_t updated_buffer_state = current_active_clients_bit_mask & + uint32_t updated_buffer_state = current_active_clients_bit_mask & (~client_state_mask()) & BufferHubDefs::kHighBitsMask; while (!buffer_state_->compare_exchange_weak( @@ -101,16 +101,16 @@ int ProducerBuffer::LocalPost(const DvrNativeBufferMetadata* meta, std::memory_order_acquire)) { ALOGD( "%s: Failed to post the buffer. Current buffer state was changed to " - "%" PRIx64 + "%" PRIx32 " when trying to post the buffer and modify the buffer state to " - "%" PRIx64 + "%" PRIx32 ". About to try again if the buffer is still gained by this client.", __FUNCTION__, current_buffer_state, updated_buffer_state); if (!BufferHubDefs::IsClientGained(current_buffer_state, client_state_mask())) { ALOGE( "%s: Failed to post the buffer. The buffer is no longer gained, " - "id=%d state=%" PRIx64 ".", + "id=%d state=%" PRIx32 ".", __FUNCTION__, id(), current_buffer_state); return -EBUSY; } @@ -164,9 +164,9 @@ int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta, if (!out_meta) return -EINVAL; - uint64_t current_buffer_state = + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - ALOGD_IF(TRACE, "%s: buffer=%d, state=%" PRIx64 ".", __FUNCTION__, id(), + ALOGD_IF(TRACE, "%s: buffer=%d, state=%" PRIx32 ".", __FUNCTION__, id(), current_buffer_state); if (BufferHubDefs::IsClientGained(current_buffer_state, @@ -178,20 +178,20 @@ int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta, BufferHubDefs::AnyClientGained(current_buffer_state) || (BufferHubDefs::AnyClientPosted(current_buffer_state) && !gain_posted_buffer)) { - ALOGE("%s: not released id=%d state=%" PRIx64 ".", __FUNCTION__, id(), + ALOGE("%s: not released id=%d state=%" PRIx32 ".", __FUNCTION__, id(), current_buffer_state); return -EBUSY; } // Change the buffer state to gained state. - uint64_t updated_buffer_state = client_state_mask(); + uint32_t updated_buffer_state = client_state_mask(); while (!buffer_state_->compare_exchange_weak( current_buffer_state, updated_buffer_state, std::memory_order_acq_rel, std::memory_order_acquire)) { ALOGD( "%s: Failed to gain the buffer. Current buffer state was changed to " - "%" PRIx64 + "%" PRIx32 " when trying to gain the buffer and modify the buffer state to " - "%" PRIx64 + "%" PRIx32 ". About to try again if the buffer is still not read by other " "clients.", __FUNCTION__, current_buffer_state, updated_buffer_state); @@ -202,7 +202,7 @@ int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta, !gain_posted_buffer)) { ALOGE( "%s: Failed to gain the buffer. The buffer is no longer released. " - "id=%d state=%" PRIx64 ".", + "id=%d state=%" PRIx32 ".", __FUNCTION__, id(), current_buffer_state); return -EBUSY; } @@ -221,8 +221,8 @@ int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta, out_meta->user_metadata_ptr = 0; } - uint64_t current_fence_state = fence_state_->load(std::memory_order_acquire); - uint64_t current_active_clients_bit_mask = + uint32_t current_fence_state = fence_state_->load(std::memory_order_acquire); + uint32_t current_active_clients_bit_mask = active_clients_bit_mask_->load(std::memory_order_acquire); // If there are release fence(s) from consumer(s), we need to return it to the // consumer(s). @@ -289,11 +289,11 @@ Status ProducerBuffer::Detach() { // TODO(b/112338294) Keep here for reference. Remove it after new logic is // written. - /* uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire); + /* uint32_t buffer_state = buffer_state_->load(std::memory_order_acquire); if (!BufferHubDefs::IsClientGained( buffer_state, BufferHubDefs::kFirstClientStateMask)) { // Can only detach a ProducerBuffer when it's in gained state. - ALOGW("ProducerBuffer::Detach: The buffer (id=%d, state=0x%" PRIx64 + ALOGW("ProducerBuffer::Detach: The buffer (id=%d, state=0x%" PRIx32 ") is not in gained state.", id(), buffer_state); return {}; diff --git a/libs/vr/libdvr/include/dvr/dvr_api.h b/libs/vr/libdvr/include/dvr/dvr_api.h index a204f62fff..e383bb2cb3 100644 --- a/libs/vr/libdvr/include/dvr/dvr_api.h +++ b/libs/vr/libdvr/include/dvr/dvr_api.h @@ -466,11 +466,11 @@ struct ALIGNED_DVR_STRUCT(8) DvrNativeBufferMetadata { // Only applicable for metadata retrieved from GainAsync. This indicates which // consumer has pending fence that producer should epoll on. - uint64_t release_fence_mask; + uint32_t release_fence_mask; // Reserved bytes for so that the struct is forward compatible and padding to // 104 bytes so the size is a multiple of 8. - int32_t reserved[8]; + int32_t reserved[9]; }; #ifdef __cplusplus diff --git a/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp index 2d5f0043e3..df060973ec 100644 --- a/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp +++ b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp @@ -62,7 +62,7 @@ class DvrBufferQueueTest : public DvrApiTest { buffer_removed_count_); } - DvrWriteBufferQueue* write_queue_{nullptr}; + DvrWriteBufferQueue* write_queue_ = nullptr; int buffer_available_count_{0}; int buffer_removed_count_{0}; }; diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index 4bad829210..cc87e15917 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -14,10 +14,10 @@ void BufferNode::InitializeMetadata() { // Using placement new here to reuse shared memory instead of new allocation // Initialize the atomic variables to zero. BufferHubDefs::MetadataHeader* metadata_header = metadata_.metadata_header(); - buffer_state_ = new (&metadata_header->buffer_state) std::atomic(0); - fence_state_ = new (&metadata_header->fence_state) std::atomic(0); + buffer_state_ = new (&metadata_header->buffer_state) std::atomic(0); + fence_state_ = new (&metadata_header->fence_state) std::atomic(0); active_clients_bit_mask_ = - new (&metadata_header->active_clients_bit_mask) std::atomic(0); + new (&metadata_header->active_clients_bit_mask) std::atomic(0); } // Allocates a new BufferNode. @@ -74,22 +74,22 @@ BufferNode::~BufferNode() { } } -uint64_t BufferNode::GetActiveClientsBitMask() const { +uint32_t BufferNode::GetActiveClientsBitMask() const { return active_clients_bit_mask_->load(std::memory_order_acquire); } -uint64_t BufferNode::AddNewActiveClientsBitToMask() { - uint64_t current_active_clients_bit_mask = GetActiveClientsBitMask(); - uint64_t client_state_mask = 0ULL; - uint64_t updated_active_clients_bit_mask = 0ULL; +uint32_t BufferNode::AddNewActiveClientsBitToMask() { + uint32_t current_active_clients_bit_mask = GetActiveClientsBitMask(); + uint32_t client_state_mask = 0U; + uint32_t updated_active_clients_bit_mask = 0U; do { client_state_mask = BufferHubDefs::FindNextAvailableClientStateMask(current_active_clients_bit_mask); - if (client_state_mask == 0ULL) { + if (client_state_mask == 0U) { ALOGE("%s: reached the maximum number of channels per buffer node: %d.", __FUNCTION__, BufferHubDefs::kMaxNumberOfClients); errno = E2BIG; - return 0ULL; + return 0U; } updated_active_clients_bit_mask = current_active_clients_bit_mask | client_state_mask; } while (!(active_clients_bit_mask_->compare_exchange_weak(current_active_clients_bit_mask, @@ -99,7 +99,7 @@ uint64_t BufferNode::AddNewActiveClientsBitToMask() { return client_state_mask; } -void BufferNode::RemoveClientsBitFromMask(const uint64_t& value) { +void BufferNode::RemoveClientsBitFromMask(const uint32_t& value) { active_clients_bit_mask_->fetch_and(~value); } diff --git a/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h b/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h index c5b2cdef33..b51fcda3cd 100644 --- a/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h +++ b/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h @@ -32,7 +32,7 @@ namespace implementation { class BufferHubIdGenerator { public: // 0 is considered invalid - static constexpr uint32_t kInvalidId = 0UL; + static constexpr uint32_t kInvalidId = 0U; // Get the singleton instance of this class static BufferHubIdGenerator& getInstance(); diff --git a/services/bufferhub/include/bufferhub/BufferNode.h b/services/bufferhub/include/bufferhub/BufferNode.h index 02bb5af0e0..cf56c33ec0 100644 --- a/services/bufferhub/include/bufferhub/BufferNode.h +++ b/services/bufferhub/include/bufferhub/BufferNode.h @@ -38,19 +38,19 @@ public: // Gets the current value of active_clients_bit_mask in metadata_ with // std::memory_order_acquire, so that all previous releases of // active_clients_bit_mask from all threads will be returned here. - uint64_t GetActiveClientsBitMask() const; + uint32_t GetActiveClientsBitMask() const; // Find and add a new client_state_mask to active_clients_bit_mask in // metadata_. // Return the new client_state_mask that is added to active_clients_bit_mask. - // Return 0ULL if there are already 32 bp clients of the buffer. - uint64_t AddNewActiveClientsBitToMask(); + // Return 0U if there are already 16 clients of the buffer. + uint32_t AddNewActiveClientsBitToMask(); // Removes the value from active_clients_bit_mask in metadata_ with // std::memory_order_release, so that the change will be visible to any // acquire of active_clients_bit_mask_ in any threads after the succeed of // this operation. - void RemoveClientsBitFromMask(const uint64_t& value); + void RemoveClientsBitFromMask(const uint32_t& value); private: // Helper method for constructors to initialize atomic metadata header @@ -75,14 +75,14 @@ private: // buffer_state_ tracks the state of the buffer. Buffer can be in one of these // four states: gained, posted, acquired, released. - std::atomic* buffer_state_ = nullptr; + std::atomic* buffer_state_ = nullptr; // TODO(b/112012161): add comments to fence_state_. - std::atomic* fence_state_ = nullptr; + std::atomic* fence_state_ = nullptr; // active_clients_bit_mask_ tracks all the bp clients of the buffer. It is the // union of all client_state_mask of all bp clients. - std::atomic* active_clients_bit_mask_ = nullptr; + std::atomic* active_clients_bit_mask_ = nullptr; }; } // namespace implementation diff --git a/services/bufferhub/tests/BufferNode_test.cpp b/services/bufferhub/tests/BufferNode_test.cpp index 8555eb7800..dbf10e8cc5 100644 --- a/services/bufferhub/tests/BufferNode_test.cpp +++ b/services/bufferhub/tests/BufferNode_test.cpp @@ -56,21 +56,21 @@ TEST_F(BufferNodeTest, TestCreateBufferNode) { } TEST_F(BufferNodeTest, TestAddNewActiveClientsBitToMask_twoNewClients) { - uint64_t new_client_state_mask_1 = buffer_node->AddNewActiveClientsBitToMask(); + uint32_t new_client_state_mask_1 = buffer_node->AddNewActiveClientsBitToMask(); EXPECT_EQ(buffer_node->GetActiveClientsBitMask(), new_client_state_mask_1); // Request and add a new client_state_mask again. // Active clients bit mask should be the union of the two new // client_state_masks. - uint64_t new_client_state_mask_2 = buffer_node->AddNewActiveClientsBitToMask(); + uint32_t new_client_state_mask_2 = buffer_node->AddNewActiveClientsBitToMask(); EXPECT_EQ(buffer_node->GetActiveClientsBitMask(), new_client_state_mask_1 | new_client_state_mask_2); } TEST_F(BufferNodeTest, TestAddNewActiveClientsBitToMask_32NewClients) { - uint64_t new_client_state_mask = 0ULL; - uint64_t current_mask = 0ULL; - uint64_t expected_mask = 0ULL; + uint32_t new_client_state_mask = 0U; + uint32_t current_mask = 0U; + uint32_t expected_mask = 0U; for (int i = 0; i < BufferHubDefs::kMaxNumberOfClients; ++i) { new_client_state_mask = buffer_node->AddNewActiveClientsBitToMask(); @@ -83,14 +83,14 @@ TEST_F(BufferNodeTest, TestAddNewActiveClientsBitToMask_32NewClients) { // Method should fail upon requesting for more than maximum allowable clients. new_client_state_mask = buffer_node->AddNewActiveClientsBitToMask(); - EXPECT_EQ(new_client_state_mask, 0ULL); + EXPECT_EQ(new_client_state_mask, 0U); EXPECT_EQ(errno, E2BIG); } TEST_F(BufferNodeTest, TestRemoveActiveClientsBitFromMask) { buffer_node->AddNewActiveClientsBitToMask(); - uint64_t current_mask = buffer_node->GetActiveClientsBitMask(); - uint64_t new_client_state_mask = buffer_node->AddNewActiveClientsBitToMask(); + uint32_t current_mask = buffer_node->GetActiveClientsBitMask(); + uint32_t new_client_state_mask = buffer_node->AddNewActiveClientsBitToMask(); EXPECT_NE(buffer_node->GetActiveClientsBitMask(), current_mask); buffer_node->RemoveClientsBitFromMask(new_client_state_mask); diff --git a/services/vr/bufferhubd/buffer_channel.cpp b/services/vr/bufferhubd/buffer_channel.cpp index cf072b667d..695396caf2 100644 --- a/services/vr/bufferhubd/buffer_channel.cpp +++ b/services/vr/bufferhubd/buffer_channel.cpp @@ -32,7 +32,7 @@ BufferChannel::BufferChannel(BufferHubService* service, int buffer_id, : BufferHubChannel(service, buffer_id, channel_id, kDetachedBufferType), buffer_node_(buffer_node) { client_state_mask_ = buffer_node_->AddNewActiveClientsBitToMask(); - if (client_state_mask_ == 0ULL) { + if (client_state_mask_ == 0U) { ALOGE("BufferChannel::BufferChannel: %s", strerror(errno)); buffer_node_ = nullptr; } @@ -41,7 +41,7 @@ BufferChannel::BufferChannel(BufferHubService* service, int buffer_id, BufferChannel::~BufferChannel() { ALOGD_IF(TRACE, "BufferChannel::~BufferChannel: channel_id=%d buffer_id=%d.", channel_id(), buffer_id()); - if (client_state_mask_ != 0ULL) { + if (client_state_mask_ != 0U) { buffer_node_->RemoveClientsBitFromMask(client_state_mask_); } Hangup(); diff --git a/services/vr/bufferhubd/consumer_channel.cpp b/services/vr/bufferhubd/consumer_channel.cpp index 158ef9ceaa..c7695bc51f 100644 --- a/services/vr/bufferhubd/consumer_channel.cpp +++ b/services/vr/bufferhubd/consumer_channel.cpp @@ -17,7 +17,7 @@ namespace android { namespace dvr { ConsumerChannel::ConsumerChannel(BufferHubService* service, int buffer_id, - int channel_id, uint64_t client_state_mask, + int channel_id, uint32_t client_state_mask, const std::shared_ptr producer) : BufferHubChannel(service, buffer_id, channel_id, kConsumerType), client_state_mask_(client_state_mask), diff --git a/services/vr/bufferhubd/include/private/dvr/buffer_channel.h b/services/vr/bufferhubd/include/private/dvr/buffer_channel.h index 744c095338..9888db6642 100644 --- a/services/vr/bufferhubd/include/private/dvr/buffer_channel.h +++ b/services/vr/bufferhubd/include/private/dvr/buffer_channel.h @@ -51,8 +51,8 @@ class BufferChannel : public BufferHubChannel { // The concrete implementation of the Buffer object. std::shared_ptr buffer_node_ = nullptr; - // The state bit of this buffer. Must be one the lower 63 bits. - uint64_t client_state_mask_ = 0ULL; + // The state bit of this buffer. + uint32_t client_state_mask_ = 0U; }; } // namespace dvr diff --git a/services/vr/bufferhubd/include/private/dvr/consumer_channel.h b/services/vr/bufferhubd/include/private/dvr/consumer_channel.h index 5fb4ec1725..5ee551f115 100644 --- a/services/vr/bufferhubd/include/private/dvr/consumer_channel.h +++ b/services/vr/bufferhubd/include/private/dvr/consumer_channel.h @@ -16,14 +16,14 @@ class ConsumerChannel : public BufferHubChannel { using Message = pdx::Message; ConsumerChannel(BufferHubService* service, int buffer_id, int channel_id, - uint64_t client_state_mask, + uint32_t client_state_mask, const std::shared_ptr producer); ~ConsumerChannel() override; bool HandleMessage(Message& message) override; void HandleImpulse(Message& message) override; - uint64_t client_state_mask() const { return client_state_mask_; } + uint32_t client_state_mask() const { return client_state_mask_; } BufferInfo GetBufferInfo() const override; void OnProducerGained(); @@ -39,7 +39,7 @@ class ConsumerChannel : public BufferHubChannel { pdx::Status OnConsumerRelease(Message& message, LocalFence release_fence); - uint64_t client_state_mask_{0}; + uint32_t client_state_mask_{0U}; bool acquired_{false}; bool released_{true}; std::weak_ptr producer_; diff --git a/services/vr/bufferhubd/include/private/dvr/producer_channel.h b/services/vr/bufferhubd/include/private/dvr/producer_channel.h index 4734439196..96ef1a20a0 100644 --- a/services/vr/bufferhubd/include/private/dvr/producer_channel.h +++ b/services/vr/bufferhubd/include/private/dvr/producer_channel.h @@ -42,7 +42,7 @@ class ProducerChannel : public BufferHubChannel { ~ProducerChannel() override; - uint64_t buffer_state() const { + uint32_t buffer_state() const { return buffer_state_->load(std::memory_order_acquire); } @@ -51,18 +51,18 @@ class ProducerChannel : public BufferHubChannel { BufferInfo GetBufferInfo() const override; - BufferDescription GetBuffer(uint64_t client_state_mask); + BufferDescription GetBuffer(uint32_t client_state_mask); pdx::Status CreateConsumer(Message& message, - uint64_t consumer_state_mask); - pdx::Status CreateConsumerStateMask(); + uint32_t consumer_state_mask); + pdx::Status CreateConsumerStateMask(); pdx::Status OnNewConsumer(Message& message); pdx::Status OnConsumerAcquire(Message& message); pdx::Status OnConsumerRelease(Message& message, LocalFence release_fence); - void OnConsumerOrphaned(const uint64_t& consumer_state_mask); + void OnConsumerOrphaned(const uint32_t& consumer_state_mask); void AddConsumer(ConsumerChannel* channel); void RemoveConsumer(ConsumerChannel* channel); @@ -79,13 +79,13 @@ class ProducerChannel : public BufferHubChannel { // IonBuffer that is shared between bufferhubd, producer, and consumers. IonBuffer metadata_buffer_; BufferHubDefs::MetadataHeader* metadata_header_ = nullptr; - std::atomic* buffer_state_ = nullptr; - std::atomic* fence_state_ = nullptr; - std::atomic* active_clients_bit_mask_ = nullptr; + std::atomic* buffer_state_ = nullptr; + std::atomic* fence_state_ = nullptr; + std::atomic* active_clients_bit_mask_ = nullptr; // All orphaned consumer bits. Valid bits are the lower 63 bits, while the // highest bit is reserved for the producer and should not be set. - uint64_t orphaned_consumer_bit_mask_{0ULL}; + uint32_t orphaned_consumer_bit_mask_{0U}; LocalFence post_fence_; LocalFence returned_fence_; @@ -110,7 +110,7 @@ class ProducerChannel : public BufferHubChannel { // Remove consumer from atomics in shared memory based on consumer_state_mask. // This function is used for clean up for failures in CreateConsumer method. - void RemoveConsumerClientMask(uint64_t consumer_state_mask); + void RemoveConsumerClientMask(uint32_t consumer_state_mask); ProducerChannel(const ProducerChannel&) = delete; void operator=(const ProducerChannel&) = delete; diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp index b541fb3963..1682bfeb33 100644 --- a/services/vr/bufferhubd/producer_channel.cpp +++ b/services/vr/bufferhubd/producer_channel.cpp @@ -93,10 +93,10 @@ int ProducerChannel::InitializeBuffer() { // Using placement new here to reuse shared memory instead of new allocation // and also initialize the value to zero. buffer_state_ = - new (&metadata_header_->buffer_state) std::atomic(0); - fence_state_ = new (&metadata_header_->fence_state) std::atomic(0); + new (&metadata_header_->buffer_state) std::atomic(0); + fence_state_ = new (&metadata_header_->fence_state) std::atomic(0); active_clients_bit_mask_ = - new (&metadata_header_->active_clients_bit_mask) std::atomic(0); + new (&metadata_header_->active_clients_bit_mask) std::atomic(0); // Producer channel is never created after consumer channel, and one buffer // only have one fixed producer for now. Thus, it is correct to assume @@ -119,7 +119,7 @@ int ProducerChannel::InitializeBuffer() { epoll_event event; event.events = 0; - event.data.u64 = 0ULL; + event.data.u32 = 0U; if (epoll_ctl(release_fence_fd_.Get(), EPOLL_CTL_ADD, dummy_fence_fd_.Get(), &event) < 0) { ALOGE( @@ -164,7 +164,7 @@ Status> ProducerChannel::Create( ProducerChannel::~ProducerChannel() { ALOGD_IF(TRACE, "ProducerChannel::~ProducerChannel: channel_id=%d buffer_id=%d " - "state=%" PRIx64 ".", + "state=%" PRIx32 ".", channel_id(), buffer_id(), buffer_state_->load(std::memory_order_acquire)); for (auto consumer : consumer_channels_) { @@ -175,7 +175,7 @@ ProducerChannel::~ProducerChannel() { BufferHubChannel::BufferInfo ProducerChannel::GetBufferInfo() const { // Derive the mask of signaled buffers in this producer / consumer set. - uint64_t signaled_mask = signaled() ? BufferHubDefs::kFirstClientBitMask : 0; + uint32_t signaled_mask = signaled() ? BufferHubDefs::kFirstClientBitMask : 0; for (const ConsumerChannel* consumer : consumer_channels_) { signaled_mask |= consumer->signaled() ? consumer->client_state_mask() : 0; } @@ -228,7 +228,7 @@ bool ProducerChannel::HandleMessage(Message& message) { } BufferDescription ProducerChannel::GetBuffer( - uint64_t client_state_mask) { + uint32_t client_state_mask) { return {buffer_, metadata_buffer_, buffer_id(), @@ -241,27 +241,27 @@ BufferDescription ProducerChannel::GetBuffer( Status> ProducerChannel::OnGetBuffer( Message& /*message*/) { ATRACE_NAME("ProducerChannel::OnGetBuffer"); - ALOGD_IF(TRACE, "ProducerChannel::OnGetBuffer: buffer=%d, state=%" PRIx64 ".", + ALOGD_IF(TRACE, "ProducerChannel::OnGetBuffer: buffer=%d, state=%" PRIx32 ".", buffer_id(), buffer_state_->load(std::memory_order_acquire)); return {GetBuffer(BufferHubDefs::kFirstClientBitMask)}; } -Status ProducerChannel::CreateConsumerStateMask() { +Status ProducerChannel::CreateConsumerStateMask() { // Try find the next consumer state bit which has not been claimed by any // consumer yet. // memory_order_acquire is chosen here because all writes in other threads // that release active_clients_bit_mask_ need to be visible here. - uint64_t current_active_clients_bit_mask = + uint32_t current_active_clients_bit_mask = active_clients_bit_mask_->load(std::memory_order_acquire); - uint64_t consumer_state_mask = + uint32_t consumer_state_mask = BufferHubDefs::FindNextAvailableClientStateMask( current_active_clients_bit_mask | orphaned_consumer_bit_mask_); - if (consumer_state_mask == 0ULL) { + if (consumer_state_mask == 0U) { ALOGE("%s: reached the maximum mumber of consumers per producer: 63.", __FUNCTION__); return ErrorStatus(E2BIG); } - uint64_t updated_active_clients_bit_mask = + uint32_t updated_active_clients_bit_mask = current_active_clients_bit_mask | consumer_state_mask; // Set the updated value only if the current value stays the same as what was // read before. If the comparison succeeds, update the value without @@ -274,15 +274,15 @@ Status ProducerChannel::CreateConsumerStateMask() { while (!active_clients_bit_mask_->compare_exchange_weak( current_active_clients_bit_mask, updated_active_clients_bit_mask, std::memory_order_acq_rel, std::memory_order_acquire)) { - ALOGE("%s: Current active clients bit mask is changed to %" PRIx64 - ", which was expected to be %" PRIx64 + ALOGE("%s: Current active clients bit mask is changed to %" PRIx32 + ", which was expected to be %" PRIx32 ". Trying to generate a new client state mask to resolve race " "condition.", __FUNCTION__, updated_active_clients_bit_mask, current_active_clients_bit_mask); consumer_state_mask = BufferHubDefs::FindNextAvailableClientStateMask( current_active_clients_bit_mask | orphaned_consumer_bit_mask_); - if (consumer_state_mask == 0ULL) { + if (consumer_state_mask == 0U) { ALOGE("%s: reached the maximum mumber of consumers per producer: %d.", __FUNCTION__, (BufferHubDefs::kMaxNumberOfClients - 1)); return ErrorStatus(E2BIG); @@ -294,7 +294,7 @@ Status ProducerChannel::CreateConsumerStateMask() { return {consumer_state_mask}; } -void ProducerChannel::RemoveConsumerClientMask(uint64_t consumer_state_mask) { +void ProducerChannel::RemoveConsumerClientMask(uint32_t consumer_state_mask) { // Clear up the buffer state and fence state in case there is already // something there due to possible race condition between producer post and // consumer failed to create channel. @@ -308,7 +308,7 @@ void ProducerChannel::RemoveConsumerClientMask(uint64_t consumer_state_mask) { } Status ProducerChannel::CreateConsumer( - Message& message, uint64_t consumer_state_mask) { + Message& message, uint32_t consumer_state_mask) { ATRACE_NAME(__FUNCTION__); ALOGD_IF(TRACE, "%s: buffer_id=%d", __FUNCTION__, buffer_id()); @@ -332,7 +332,7 @@ Status ProducerChannel::CreateConsumer( return ErrorStatus(ENOMEM); } - uint64_t current_buffer_state = + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (BufferHubDefs::IsBufferReleased(current_buffer_state) || BufferHubDefs::AnyClientGained(current_buffer_state)) { @@ -343,7 +343,7 @@ Status ProducerChannel::CreateConsumer( bool update_buffer_state = true; if (!BufferHubDefs::IsClientPosted(current_buffer_state, consumer_state_mask)) { - uint64_t updated_buffer_state = + uint32_t updated_buffer_state = current_buffer_state ^ (consumer_state_mask & BufferHubDefs::kHighBitsMask); while (!buffer_state_->compare_exchange_weak( @@ -351,15 +351,15 @@ Status ProducerChannel::CreateConsumer( std::memory_order_acquire)) { ALOGI( "%s: Failed to post to the new consumer. " - "Current buffer state was changed to %" PRIx64 + "Current buffer state was changed to %" PRIx32 " when trying to acquire the buffer and modify the buffer state to " - "%" PRIx64 + "%" PRIx32 ". About to try again if the buffer is still not gained nor fully " "released.", __FUNCTION__, current_buffer_state, updated_buffer_state); if (BufferHubDefs::IsBufferReleased(current_buffer_state) || BufferHubDefs::AnyClientGained(current_buffer_state)) { - ALOGI("%s: buffer is gained or fully released, state=%" PRIx64 ".", + ALOGI("%s: buffer is gained or fully released, state=%" PRIx32 ".", __FUNCTION__, current_buffer_state); update_buffer_state = false; break; @@ -393,7 +393,7 @@ Status ProducerChannel::OnProducerPost(Message&, epoll_event event; event.events = 0; - event.data.u64 = 0ULL; + event.data.u32 = 0U; int ret = epoll_ctl(release_fence_fd_.Get(), EPOLL_CTL_MOD, dummy_fence_fd_.Get(), &event); ALOGE_IF(ret < 0, @@ -401,7 +401,7 @@ Status ProducerChannel::OnProducerPost(Message&, "release fence to include the dummy fence: %s", strerror(errno)); - eventfd_t dummy_fence_count = 0ULL; + eventfd_t dummy_fence_count = 0U; if (eventfd_read(dummy_fence_fd_.Get(), &dummy_fence_count) < 0) { const int error = errno; if (error != EAGAIN) { @@ -451,13 +451,13 @@ Status ProducerChannel::OnProducerGain(Message& /*message*/) { ALOGD_IF(TRACE, "ProducerChannel::OnProducerDetach: buffer_id=%d", buffer_id()); - uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire); + uint32_t buffer_state = buffer_state_->load(std::memory_order_acquire); if (!BufferHubDefs::IsClientGained( buffer_state, BufferHubDefs::kFirstClientStateMask)) { // Can only detach a BufferProducer when it's in gained state. ALOGW( "ProducerChannel::OnProducerDetach: The buffer (id=%d, state=%" - PRIx64 + PRIx32 ") is not in gained state.", buffer_id(), buffer_state); return {}; @@ -534,7 +534,7 @@ Status ProducerChannel::OnConsumerRelease(Message&, } } - uint64_t current_buffer_state = + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (BufferHubDefs::IsBufferReleased(current_buffer_state & ~orphaned_consumer_bit_mask_)) { @@ -542,7 +542,7 @@ Status ProducerChannel::OnConsumerRelease(Message&, if (orphaned_consumer_bit_mask_) { ALOGW( "%s: orphaned buffer detected during the this acquire/release cycle: " - "id=%d orphaned=0x%" PRIx64 " queue_index=%" PRIx64 ".", + "id=%d orphaned=0x%" PRIx32 " queue_index=%" PRIx64 ".", __FUNCTION__, buffer_id(), orphaned_consumer_bit_mask_, metadata_header_->queue_index); orphaned_consumer_bit_mask_ = 0; @@ -552,16 +552,16 @@ Status ProducerChannel::OnConsumerRelease(Message&, return {}; } -void ProducerChannel::OnConsumerOrphaned(const uint64_t& consumer_state_mask) { +void ProducerChannel::OnConsumerOrphaned(const uint32_t& consumer_state_mask) { // Remember the ignored consumer so that newly added consumer won't be // taking the same state mask as this orphaned consumer. ALOGE_IF(orphaned_consumer_bit_mask_ & consumer_state_mask, - "%s: Consumer (consumer_state_mask=%" PRIx64 + "%s: Consumer (consumer_state_mask=%" PRIx32 ") is already orphaned.", __FUNCTION__, consumer_state_mask); orphaned_consumer_bit_mask_ |= consumer_state_mask; - uint64_t current_buffer_state = + uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (BufferHubDefs::IsBufferReleased(current_buffer_state & ~orphaned_consumer_bit_mask_)) { @@ -577,8 +577,8 @@ void ProducerChannel::OnConsumerOrphaned(const uint64_t& consumer_state_mask) { ALOGW( "%s: detected new orphaned consumer buffer_id=%d " - "consumer_state_mask=%" PRIx64 " queue_index=%" PRIx64 - " buffer_state=%" PRIx64 " fence_state=%" PRIx64 ".", + "consumer_state_mask=%" PRIx32 " queue_index=%" PRIx64 + " buffer_state=%" PRIx32 " fence_state=%" PRIx32 ".", __FUNCTION__, buffer_id(), consumer_state_mask, metadata_header_->queue_index, buffer_state_->load(std::memory_order_acquire), @@ -594,18 +594,18 @@ void ProducerChannel::RemoveConsumer(ConsumerChannel* channel) { std::find(consumer_channels_.begin(), consumer_channels_.end(), channel)); // Restore the consumer state bit and make it visible in other threads that // acquire the active_clients_bit_mask_. - uint64_t consumer_state_mask = channel->client_state_mask(); - uint64_t current_active_clients_bit_mask = + uint32_t consumer_state_mask = channel->client_state_mask(); + uint32_t current_active_clients_bit_mask = active_clients_bit_mask_->load(std::memory_order_acquire); - uint64_t updated_active_clients_bit_mask = + uint32_t updated_active_clients_bit_mask = current_active_clients_bit_mask & (~consumer_state_mask); while (!active_clients_bit_mask_->compare_exchange_weak( current_active_clients_bit_mask, updated_active_clients_bit_mask, std::memory_order_acq_rel, std::memory_order_acquire)) { ALOGI( "%s: Failed to remove consumer state mask. Current active clients bit " - "mask is changed to %" PRIu64 - " when trying to acquire and modify it to %" PRIu64 + "mask is changed to %" PRIx32 + " when trying to acquire and modify it to %" PRIx32 ". About to try again.", __FUNCTION__, current_active_clients_bit_mask, updated_active_clients_bit_mask); @@ -613,7 +613,7 @@ void ProducerChannel::RemoveConsumer(ConsumerChannel* channel) { current_active_clients_bit_mask & (~consumer_state_mask); } - const uint64_t current_buffer_state = + const uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); if (BufferHubDefs::IsClientPosted(current_buffer_state, consumer_state_mask) || @@ -634,7 +634,7 @@ void ProducerChannel::RemoveConsumer(ConsumerChannel* channel) { if (fence_state_->load(std::memory_order_acquire) & consumer_state_mask) { epoll_event event; event.events = EPOLLIN; - event.data.u64 = consumer_state_mask; + event.data.u32 = consumer_state_mask; if (epoll_ctl(release_fence_fd_.Get(), EPOLL_CTL_MOD, dummy_fence_fd_.Get(), &event) < 0) { ALOGE( -- cgit v1.2.3-59-g8ed1b From 2ceb320ea35a14ef8f6df460314f85a40a7581de Mon Sep 17 00:00:00 2001 From: Tianyu Jiang Date: Mon, 17 Dec 2018 12:58:34 -0800 Subject: Check atomics in shared memory are lock free when they are created in bufferhub server side in BufferNode, and client side in BufferHubBuffer. Fix: 117849512 Test: BufferHub_test BufferHubServer_test Change-Id: Ifc5b681a6a86fa02cb598b33bf68dfefc07a76f9 --- libs/ui/BufferHubBuffer.cpp | 7 +++++++ services/bufferhub/BufferNode.cpp | 8 ++++++++ 2 files changed, 15 insertions(+) (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index 226b6ee9ad..0582e1afe7 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -169,6 +169,13 @@ int BufferHubBuffer::ImportGraphicBuffer() { buffer_state_ = &metadata_header->buffer_state; fence_state_ = &metadata_header->fence_state; active_clients_bit_mask_ = &metadata_header->active_clients_bit_mask; + // The C++ standard recommends (but does not require) that lock-free atomic operations are + // also address-free, that is, suitable for communication between processes using shared + // memory. + LOG_ALWAYS_FATAL_IF(!std::atomic_is_lock_free(buffer_state_) || + !std::atomic_is_lock_free(fence_state_) || + !std::atomic_is_lock_free(active_clients_bit_mask_), + "Atomic variables in ashmen are not lock free."); // Import the buffer: We only need to hold on the native_handle_t here so that // GraphicBuffer instance can be created in future. diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index cc87e15917..da19a6fb1d 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -2,6 +2,7 @@ #include #include +#include #include namespace android { @@ -18,6 +19,13 @@ void BufferNode::InitializeMetadata() { fence_state_ = new (&metadata_header->fence_state) std::atomic(0); active_clients_bit_mask_ = new (&metadata_header->active_clients_bit_mask) std::atomic(0); + // The C++ standard recommends (but does not require) that lock-free atomic operations are + // also address-free, that is, suitable for communication between processes using shared + // memory. + LOG_ALWAYS_FATAL_IF(!std::atomic_is_lock_free(buffer_state_) || + !std::atomic_is_lock_free(fence_state_) || + !std::atomic_is_lock_free(active_clients_bit_mask_), + "Atomic variables in ashmen are not lock free."); } // Allocates a new BufferNode. -- cgit v1.2.3-59-g8ed1b From 6b4b1e5b1f7ad32dcadc49a2455370003b20897a Mon Sep 17 00:00:00 2001 From: Fan Xu Date: Tue, 18 Dec 2018 14:03:44 -0800 Subject: Change bufferhub's bufferId to int After discussion we decided that our new system should still use int as type of buffer id. Now the Id generator will generate int ids >= 0 instead of uint32_t ids > 0. Remove redundant log in destructor of BufferNode, now only log when freeId failed. Update BufferHubIdGenerator_test.cpp to fit in current API. Add code to cleanup generated ids in TestGenerateUniqueIncrementalID. Test: BufferHubServer_test Change-Id: I2018bfd009a3c311a99b9114762d0f4fbf1e3fe2 Fix: 118844348 --- services/bufferhub/BufferHubIdGenerator.cpp | 16 +++++++--------- services/bufferhub/BufferHubService.cpp | 4 ++-- services/bufferhub/BufferNode.cpp | 10 +++------- .../include/bufferhub/BufferHubIdGenerator.h | 22 ++++++++++------------ .../bufferhub/include/bufferhub/BufferHubService.h | 4 ++-- services/bufferhub/include/bufferhub/BufferNode.h | 13 ++++++------- .../bufferhub/tests/BufferHubIdGenerator_test.cpp | 19 +++++++++++-------- 7 files changed, 41 insertions(+), 47 deletions(-) (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/services/bufferhub/BufferHubIdGenerator.cpp b/services/bufferhub/BufferHubIdGenerator.cpp index 6444a033e1..2c12f0e26d 100644 --- a/services/bufferhub/BufferHubIdGenerator.cpp +++ b/services/bufferhub/BufferHubIdGenerator.cpp @@ -15,6 +15,7 @@ */ #include +#include namespace android { namespace frameworks { @@ -22,20 +23,18 @@ namespace bufferhub { namespace V1_0 { namespace implementation { -constexpr uint32_t BufferHubIdGenerator::kInvalidId; - BufferHubIdGenerator& BufferHubIdGenerator::getInstance() { static BufferHubIdGenerator generator; return generator; } -uint32_t BufferHubIdGenerator::getId() { +int BufferHubIdGenerator::getId() { std::lock_guard lock(mIdsInUseMutex); do { - if (++mLastId >= std::numeric_limits::max()) { - mLastId = kInvalidId + 1; + if (++mLastId >= std::numeric_limits::max()) { + mLastId = 0; } } while (mIdsInUse.find(mLastId) != mIdsInUse.end()); @@ -43,15 +42,14 @@ uint32_t BufferHubIdGenerator::getId() { return mLastId; } -bool BufferHubIdGenerator::freeId(uint32_t id) { +void BufferHubIdGenerator::freeId(int id) { std::lock_guard lock(mIdsInUseMutex); auto iter = mIdsInUse.find(id); if (iter != mIdsInUse.end()) { mIdsInUse.erase(iter); - return true; + } else { + ALOGW("%s: Cannot free nonexistent id #%d", __FUNCTION__, id); } - - return false; } } // namespace implementation diff --git a/services/bufferhub/BufferHubService.cpp b/services/bufferhub/BufferHubService.cpp index 6389521e6b..ad49cd62ed 100644 --- a/services/bufferhub/BufferHubService.cpp +++ b/services/bufferhub/BufferHubService.cpp @@ -343,15 +343,15 @@ void BufferHubService::onClientClosed(const BufferClient* client) { // Implementation of this function should be consistent with the definition of bufferInfo handle in // ui/BufferHubDefs.h. -hidl_handle BufferHubService::buildBufferInfo(uint32_t bufferId, uint32_t clientBitMask, +hidl_handle BufferHubService::buildBufferInfo(int bufferId, uint32_t clientBitMask, uint32_t userMetadataSize, const int metadataFd) { native_handle_t* infoHandle = native_handle_create(BufferHubDefs::kBufferInfoNumFds, BufferHubDefs::kBufferInfoNumInts); infoHandle->data[0] = dup(metadataFd); + infoHandle->data[1] = bufferId; // Use memcpy to convert to int without missing digit. // TOOD(b/121345852): use bit_cast to unpack bufferInfo when C++20 becomes available. - memcpy(&infoHandle->data[1], &bufferId, sizeof(bufferId)); memcpy(&infoHandle->data[2], &clientBitMask, sizeof(clientBitMask)); memcpy(&infoHandle->data[3], &userMetadataSize, sizeof(userMetadataSize)); diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index da19a6fb1d..51063903b5 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -30,7 +30,7 @@ void BufferNode::InitializeMetadata() { // Allocates a new BufferNode. BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, - uint64_t usage, size_t user_metadata_size, uint32_t id) + uint64_t usage, size_t user_metadata_size, int id) : mId(id) { uint32_t out_stride = 0; // graphicBufferId is not used in GraphicBufferAllocator::allocate @@ -73,12 +73,8 @@ BufferNode::~BufferNode() { } // Free the id, if valid - if (id() != BufferHubIdGenerator::kInvalidId) { - if (BufferHubIdGenerator::getInstance().freeId(id())) { - ALOGI("%s: id #%u is freed.", __FUNCTION__, id()); - } else { - ALOGE("%s: Cannot free nonexistent id #%u", __FUNCTION__, id()); - } + if (mId >= 0) { + BufferHubIdGenerator::getInstance().freeId(mId); } } diff --git a/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h b/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h index b51fcda3cd..ef7c077feb 100644 --- a/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h +++ b/services/bufferhub/include/bufferhub/BufferHubIdGenerator.h @@ -28,30 +28,28 @@ namespace bufferhub { namespace V1_0 { namespace implementation { -// A thread-safe incremental uint32_t id generator. +// A thread-safe, non-negative, incremental, int id generator. class BufferHubIdGenerator { public: - // 0 is considered invalid - static constexpr uint32_t kInvalidId = 0U; - // Get the singleton instance of this class static BufferHubIdGenerator& getInstance(); - // Gets next available id. If next id is greater than std::numeric_limits::max() (2 ^ - // 32 - 1), it will try to get an id start from 1 again. - uint32_t getId(); + // Gets next available id. If next id is greater than std::numeric_limits::max(), it + // will try to get an id start from 0 again. + int getId(); - // Free a specific id. Return true on freed, false on not found. - bool freeId(uint32_t id); + // Free a specific id. + void freeId(int id); private: BufferHubIdGenerator() = default; ~BufferHubIdGenerator() = default; + // Start from -1 so all valid ids will be >= 0 + int mLastId = -1; + std::mutex mIdsInUseMutex; - // Start from kInvalidID to avoid generating it. - uint32_t mLastId = kInvalidId; - std::set mIdsInUse GUARDED_BY(mIdsInUseMutex); + std::set mIdsInUse GUARDED_BY(mIdsInUseMutex); }; } // namespace implementation diff --git a/services/bufferhub/include/bufferhub/BufferHubService.h b/services/bufferhub/include/bufferhub/BufferHubService.h index 04ba5b76f9..23e664e036 100644 --- a/services/bufferhub/include/bufferhub/BufferHubService.h +++ b/services/bufferhub/include/bufferhub/BufferHubService.h @@ -58,8 +58,8 @@ public: private: // Helper function to build BufferTraits.bufferInfo handle - hidl_handle buildBufferInfo(uint32_t bufferId, uint32_t clientBitMask, - uint32_t userMetadataSize, const int metadataFd); + hidl_handle buildBufferInfo(int bufferId, uint32_t clientBitMask, uint32_t userMetadataSize, + const int metadataFd); // Helper function to remove all the token belongs to a specific client. void removeTokenByClient(const BufferClient* client); diff --git a/services/bufferhub/include/bufferhub/BufferNode.h b/services/bufferhub/include/bufferhub/BufferNode.h index cf56c33ec0..b7a195b0a1 100644 --- a/services/bufferhub/include/bufferhub/BufferNode.h +++ b/services/bufferhub/include/bufferhub/BufferNode.h @@ -16,15 +16,14 @@ class BufferNode { public: // Allocates a new BufferNode. BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, - uint64_t usage, size_t user_metadata_size, - uint32_t id = BufferHubIdGenerator::kInvalidId); + uint64_t usage, size_t user_metadata_size, int id = -1); ~BufferNode(); // Returns whether the object holds a valid metadata. bool IsValid() const { return metadata_.IsValid(); } - uint32_t id() const { return mId; } + int id() const { return mId; } size_t user_metadata_size() const { return metadata_.user_metadata_size(); } @@ -64,10 +63,10 @@ private: // Metadata in shared memory. BufferHubMetadata metadata_; - // A system-unique id generated by bufferhub from 1 to std::numeric_limits::max(). - // BufferNodes not created by bufferhub will have id = 0, meaning "not specified". - // TODO(b/118891412): remove default id = 0 and update comments after pdx is no longer in use - const uint32_t mId = 0; + // A system-unique id generated by bufferhub from 0 to std::numeric_limits::max(). + // BufferNodes not created by bufferhub will have id < 0, meaning "not specified". + // TODO(b/118891412): remove default id = -1 and update comments after pdx is no longer in use + const int mId = -1; // The following variables are atomic variables in metadata_ that are visible // to Bn object and Bp objects. Please find more info in diff --git a/services/bufferhub/tests/BufferHubIdGenerator_test.cpp b/services/bufferhub/tests/BufferHubIdGenerator_test.cpp index fe010137fd..fb6de0d18e 100644 --- a/services/bufferhub/tests/BufferHubIdGenerator_test.cpp +++ b/services/bufferhub/tests/BufferHubIdGenerator_test.cpp @@ -15,25 +15,28 @@ protected: }; TEST_F(BufferHubIdGeneratorTest, TestGenerateAndFreeID) { - uint32_t id = mIdGenerator->getId(); - EXPECT_NE(id, BufferHubIdGenerator::kInvalidId); + int id = mIdGenerator->getId(); + EXPECT_GE(id, 0); - EXPECT_TRUE(mIdGenerator->freeId(id)); - EXPECT_FALSE(mIdGenerator->freeId(id)); + mIdGenerator->freeId(id); } TEST_F(BufferHubIdGeneratorTest, TestGenerateUniqueIncrementalID) { // 10 IDs should not overflow the UniqueIdGenerator to cause a roll back to start, so the // resulting IDs should still keep incresing. - const size_t kTestSize = 10U; - uint32_t ids[kTestSize]; - for (size_t i = 0UL; i < kTestSize; ++i) { + const int kTestSize = 10; + int ids[kTestSize]; + for (int i = 0; i < kTestSize; ++i) { ids[i] = mIdGenerator->getId(); - EXPECT_NE(ids[i], BufferHubIdGenerator::kInvalidId); + EXPECT_GE(ids[i], 0); if (i >= 1) { EXPECT_GT(ids[i], ids[i - 1]); } } + + for (int i = 0; i < kTestSize; ++i) { + mIdGenerator->freeId(ids[i]); + } } } // namespace -- cgit v1.2.3-59-g8ed1b From 1543d888f5e8ccc01930369a26c371649bc18068 Mon Sep 17 00:00:00 2001 From: Tianyu Jiang Date: Mon, 4 Feb 2019 10:53:53 -0800 Subject: Fix non camelCase member variables Android Framework C++ Code Style Guidelines says that member variables should be camelCase: http://go/droidcppstyle. Test: m, mma in frameworks/native Bug: 68273829 Change-Id: I859b916244f1e2ca2ba35c0a6e816b6250cd71ce --- libs/ui/BufferHubBuffer.cpp | 80 +++++++++++------------ libs/ui/include/ui/BufferHubBuffer.h | 8 +-- libs/ui/tests/BufferHubBuffer_test.cpp | 14 ++-- services/bufferhub/BufferNode.cpp | 59 ++++++++--------- services/bufferhub/include/bufferhub/BufferNode.h | 46 ++++++------- 5 files changed, 103 insertions(+), 104 deletions(-) (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index 4b20772b75..c3144b924d 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -210,15 +210,15 @@ int BufferHubBuffer::initWithBufferTraits(const BufferTraits& bufferTraits) { // Populate shortcuts to the atomics in metadata. auto metadata_header = mMetadata.metadata_header(); - buffer_state_ = &metadata_header->buffer_state; - fence_state_ = &metadata_header->fence_state; - active_clients_bit_mask_ = &metadata_header->active_clients_bit_mask; + mBufferState = &metadata_header->buffer_state; + mFenceState = &metadata_header->fence_state; + mActiveClientsBitMask = &metadata_header->active_clients_bit_mask; // The C++ standard recommends (but does not require) that lock-free atomic operations are // also address-free, that is, suitable for communication between processes using shared // memory. - LOG_ALWAYS_FATAL_IF(!std::atomic_is_lock_free(buffer_state_) || - !std::atomic_is_lock_free(fence_state_) || - !std::atomic_is_lock_free(active_clients_bit_mask_), + LOG_ALWAYS_FATAL_IF(!std::atomic_is_lock_free(mBufferState) || + !std::atomic_is_lock_free(mFenceState) || + !std::atomic_is_lock_free(mActiveClientsBitMask), "Atomic variables in ashmen are not lock free."); // Import the buffer: We only need to hold on the native_handle_t here so that @@ -231,96 +231,96 @@ int BufferHubBuffer::initWithBufferTraits(const BufferTraits& bufferTraits) { // TODO(b/112012161) Set up shared fences. ALOGD("%s: id=%d, buffer_state=%" PRIx32 ".", __FUNCTION__, mId, - buffer_state_->load(std::memory_order_acquire)); + mBufferState->load(std::memory_order_acquire)); return 0; } int BufferHubBuffer::Gain() { - uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - if (IsClientGained(current_buffer_state, mClientStateMask)) { + uint32_t currentBufferState = mBufferState->load(std::memory_order_acquire); + if (IsClientGained(currentBufferState, mClientStateMask)) { ALOGV("%s: Buffer is already gained by this client %" PRIx32 ".", __FUNCTION__, mClientStateMask); return 0; } do { - if (AnyClientGained(current_buffer_state & (~mClientStateMask)) || - AnyClientAcquired(current_buffer_state)) { + if (AnyClientGained(currentBufferState & (~mClientStateMask)) || + AnyClientAcquired(currentBufferState)) { ALOGE("%s: Buffer is in use, id=%d mClientStateMask=%" PRIx32 " state=%" PRIx32 ".", - __FUNCTION__, mId, mClientStateMask, current_buffer_state); + __FUNCTION__, mId, mClientStateMask, currentBufferState); return -EBUSY; } // Change the buffer state to gained state, whose value happens to be the same as // mClientStateMask. - } while (!buffer_state_->compare_exchange_weak(current_buffer_state, mClientStateMask, - std::memory_order_acq_rel, - std::memory_order_acquire)); + } while (!mBufferState->compare_exchange_weak(currentBufferState, mClientStateMask, + std::memory_order_acq_rel, + std::memory_order_acquire)); // TODO(b/119837586): Update fence state and return GPU fence. return 0; } int BufferHubBuffer::Post() { - uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - uint32_t updated_buffer_state = (~mClientStateMask) & BufferHubDefs::kHighBitsMask; + uint32_t currentBufferState = mBufferState->load(std::memory_order_acquire); + uint32_t updatedBufferState = (~mClientStateMask) & BufferHubDefs::kHighBitsMask; do { - if (!IsClientGained(current_buffer_state, mClientStateMask)) { + if (!IsClientGained(currentBufferState, mClientStateMask)) { ALOGE("%s: Cannot post a buffer that is not gained by this client. buffer_id=%d " "mClientStateMask=%" PRIx32 " state=%" PRIx32 ".", - __FUNCTION__, mId, mClientStateMask, current_buffer_state); + __FUNCTION__, mId, mClientStateMask, currentBufferState); return -EBUSY; } // Set the producer client buffer state to released, other clients' buffer state to posted. // Post to all existing and non-existing clients. - } while (!buffer_state_->compare_exchange_weak(current_buffer_state, updated_buffer_state, - std::memory_order_acq_rel, - std::memory_order_acquire)); + } while (!mBufferState->compare_exchange_weak(currentBufferState, updatedBufferState, + std::memory_order_acq_rel, + std::memory_order_acquire)); // TODO(b/119837586): Update fence state and return GPU fence if needed. return 0; } int BufferHubBuffer::Acquire() { - uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - if (IsClientAcquired(current_buffer_state, mClientStateMask)) { + uint32_t currentBufferState = mBufferState->load(std::memory_order_acquire); + if (IsClientAcquired(currentBufferState, mClientStateMask)) { ALOGV("%s: Buffer is already acquired by this client %" PRIx32 ".", __FUNCTION__, mClientStateMask); return 0; } - uint32_t updated_buffer_state = 0U; + uint32_t updatedBufferState = 0U; do { - if (!IsClientPosted(current_buffer_state, mClientStateMask)) { + if (!IsClientPosted(currentBufferState, mClientStateMask)) { ALOGE("%s: Cannot acquire a buffer that is not in posted state. buffer_id=%d " "mClientStateMask=%" PRIx32 " state=%" PRIx32 ".", - __FUNCTION__, mId, mClientStateMask, current_buffer_state); + __FUNCTION__, mId, mClientStateMask, currentBufferState); return -EBUSY; } // Change the buffer state for this consumer from posted to acquired. - updated_buffer_state = current_buffer_state ^ mClientStateMask; - } while (!buffer_state_->compare_exchange_weak(current_buffer_state, updated_buffer_state, - std::memory_order_acq_rel, - std::memory_order_acquire)); + updatedBufferState = currentBufferState ^ mClientStateMask; + } while (!mBufferState->compare_exchange_weak(currentBufferState, updatedBufferState, + std::memory_order_acq_rel, + std::memory_order_acquire)); // TODO(b/119837586): Update fence state and return GPU fence. return 0; } int BufferHubBuffer::Release() { - uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - if (IsClientReleased(current_buffer_state, mClientStateMask)) { + uint32_t currentBufferState = mBufferState->load(std::memory_order_acquire); + if (IsClientReleased(currentBufferState, mClientStateMask)) { ALOGV("%s: Buffer is already released by this client %" PRIx32 ".", __FUNCTION__, mClientStateMask); return 0; } - uint32_t updated_buffer_state = 0U; + uint32_t updatedBufferState = 0U; do { - updated_buffer_state = current_buffer_state & (~mClientStateMask); - } while (!buffer_state_->compare_exchange_weak(current_buffer_state, updated_buffer_state, - std::memory_order_acq_rel, - std::memory_order_acquire)); + updatedBufferState = currentBufferState & (~mClientStateMask); + } while (!mBufferState->compare_exchange_weak(currentBufferState, updatedBufferState, + std::memory_order_acq_rel, + std::memory_order_acquire)); // TODO(b/119837586): Update fence state and return GPU fence if needed. return 0; } bool BufferHubBuffer::IsReleased() const { - return (buffer_state_->load(std::memory_order_acquire) & - active_clients_bit_mask_->load(std::memory_order_acquire)) == 0; + return (mBufferState->load(std::memory_order_acquire) & + mActiveClientsBitMask->load(std::memory_order_acquire)) == 0; } bool BufferHubBuffer::IsValid() const { diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h index 0b6d75a9cb..06955e4161 100644 --- a/libs/ui/include/ui/BufferHubBuffer.h +++ b/libs/ui/include/ui/BufferHubBuffer.h @@ -59,7 +59,7 @@ public: const BufferHubEventFd& eventFd() const { return mEventFd; } // Returns the current value of MetadataHeader::buffer_state. - uint32_t buffer_state() const { return buffer_state_->load(std::memory_order_acquire); } + uint32_t buffer_state() const { return mBufferState->load(std::memory_order_acquire); } // A state mask which is unique to a buffer hub client among all its siblings sharing the same // concrete graphic buffer. @@ -134,9 +134,9 @@ private: // bufferhubd daemon and all buffer clients. BufferHubMetadata mMetadata; // Shortcuts to the atomics inside the header of mMetadata. - std::atomic* buffer_state_ = nullptr; - std::atomic* fence_state_ = nullptr; - std::atomic* active_clients_bit_mask_ = nullptr; + std::atomic* mBufferState = nullptr; + std::atomic* mFenceState = nullptr; + std::atomic* mActiveClientsBitMask = nullptr; // HwBinder backend sp mBufferClient; diff --git a/libs/ui/tests/BufferHubBuffer_test.cpp b/libs/ui/tests/BufferHubBuffer_test.cpp index 3bcd9353fc..4ad2c4c2ce 100644 --- a/libs/ui/tests/BufferHubBuffer_test.cpp +++ b/libs/ui/tests/BufferHubBuffer_test.cpp @@ -239,8 +239,8 @@ TEST_F(BufferHubBufferStateTransitionTest, GainBuffer_fromReleasedState) { TEST_F(BufferHubBufferStateTransitionTest, GainBuffer_fromGainedState) { ASSERT_EQ(b1->Gain(), 0); - auto current_buffer_state = b1->buffer_state(); - ASSERT_TRUE(IsClientGained(current_buffer_state, b1ClientMask)); + auto currentBufferState = b1->buffer_state(); + ASSERT_TRUE(IsClientGained(currentBufferState, b1ClientMask)); // Gaining from gained state by the same client should not return error. EXPECT_EQ(b1->Gain(), 0); @@ -291,9 +291,9 @@ TEST_F(BufferHubBufferStateTransitionTest, PostBuffer_fromSelfInGainedState) { ASSERT_TRUE(IsClientGained(b1->buffer_state(), b1ClientMask)); EXPECT_EQ(b1->Post(), 0); - auto current_buffer_state = b1->buffer_state(); - EXPECT_TRUE(IsClientReleased(current_buffer_state, b1ClientMask)); - EXPECT_TRUE(IsClientPosted(current_buffer_state, b2ClientMask)); + auto currentBufferState = b1->buffer_state(); + EXPECT_TRUE(IsClientReleased(currentBufferState, b1ClientMask)); + EXPECT_TRUE(IsClientPosted(currentBufferState, b2ClientMask)); } TEST_F(BufferHubBufferStateTransitionTest, PostBuffer_fromPostedState) { @@ -348,8 +348,8 @@ TEST_F(BufferHubBufferStateTransitionTest, AcquireBuffer_fromSelfInAcquiredState ASSERT_EQ(b1->Gain(), 0); ASSERT_EQ(b1->Post(), 0); ASSERT_EQ(b2->Acquire(), 0); - auto current_buffer_state = b1->buffer_state(); - ASSERT_TRUE(IsClientAcquired(current_buffer_state, b2ClientMask)); + auto currentBufferState = b1->buffer_state(); + ASSERT_TRUE(IsClientAcquired(currentBufferState, b2ClientMask)); // Acquiring from acquired state by the same client should not error out. EXPECT_EQ(b2->Acquire(), 0); diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index 51063903b5..dfa2de0173 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -14,17 +14,16 @@ namespace implementation { void BufferNode::InitializeMetadata() { // Using placement new here to reuse shared memory instead of new allocation // Initialize the atomic variables to zero. - BufferHubDefs::MetadataHeader* metadata_header = metadata_.metadata_header(); - buffer_state_ = new (&metadata_header->buffer_state) std::atomic(0); - fence_state_ = new (&metadata_header->fence_state) std::atomic(0); - active_clients_bit_mask_ = - new (&metadata_header->active_clients_bit_mask) std::atomic(0); + BufferHubDefs::MetadataHeader* metadataHeader = mMetadata.metadata_header(); + mBufferState = new (&metadataHeader->buffer_state) std::atomic(0); + mFenceState = new (&metadataHeader->fence_state) std::atomic(0); + mActiveClientsBitMask = new (&metadataHeader->active_clients_bit_mask) std::atomic(0); // The C++ standard recommends (but does not require) that lock-free atomic operations are // also address-free, that is, suitable for communication between processes using shared // memory. - LOG_ALWAYS_FATAL_IF(!std::atomic_is_lock_free(buffer_state_) || - !std::atomic_is_lock_free(fence_state_) || - !std::atomic_is_lock_free(active_clients_bit_mask_), + LOG_ALWAYS_FATAL_IF(!std::atomic_is_lock_free(mBufferState) || + !std::atomic_is_lock_free(mFenceState) || + !std::atomic_is_lock_free(mActiveClientsBitMask), "Atomic variables in ashmen are not lock free."); } @@ -38,25 +37,25 @@ BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, ui // hardcoded service name "bufferhub". int ret = GraphicBufferAllocator::get().allocate(width, height, format, layer_count, usage, const_cast( - &buffer_handle_), + &mBufferHandle), &out_stride, /*graphicBufferId=*/0, /*requestor=*/"bufferhub"); - if (ret != OK || buffer_handle_ == nullptr) { + if (ret != OK || mBufferHandle == nullptr) { ALOGE("%s: Failed to allocate buffer: %s", __FUNCTION__, strerror(-ret)); return; } - buffer_desc_.width = width; - buffer_desc_.height = height; - buffer_desc_.layers = layer_count; - buffer_desc_.format = format; - buffer_desc_.usage = usage; - buffer_desc_.stride = out_stride; + mBufferDesc.width = width; + mBufferDesc.height = height; + mBufferDesc.layers = layer_count; + mBufferDesc.format = format; + mBufferDesc.usage = usage; + mBufferDesc.stride = out_stride; - metadata_ = BufferHubMetadata::Create(user_metadata_size); - if (!metadata_.IsValid()) { + mMetadata = BufferHubMetadata::Create(user_metadata_size); + if (!mMetadata.IsValid()) { ALOGE("%s: Failed to allocate metadata.", __FUNCTION__); return; } @@ -65,8 +64,8 @@ BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, ui BufferNode::~BufferNode() { // Free the handle - if (buffer_handle_ != nullptr) { - status_t ret = GraphicBufferAllocator::get().free(buffer_handle_); + if (mBufferHandle != nullptr) { + status_t ret = GraphicBufferAllocator::get().free(mBufferHandle); if (ret != OK) { ALOGE("%s: Failed to free handle; Got error: %d", __FUNCTION__, ret); } @@ -79,32 +78,32 @@ BufferNode::~BufferNode() { } uint32_t BufferNode::GetActiveClientsBitMask() const { - return active_clients_bit_mask_->load(std::memory_order_acquire); + return mActiveClientsBitMask->load(std::memory_order_acquire); } uint32_t BufferNode::AddNewActiveClientsBitToMask() { - uint32_t current_active_clients_bit_mask = GetActiveClientsBitMask(); + uint32_t currentActiveClientsBitMask = GetActiveClientsBitMask(); uint32_t client_state_mask = 0U; - uint32_t updated_active_clients_bit_mask = 0U; + uint32_t updatedActiveClientsBitMask = 0U; do { client_state_mask = - BufferHubDefs::FindNextAvailableClientStateMask(current_active_clients_bit_mask); + BufferHubDefs::FindNextAvailableClientStateMask(currentActiveClientsBitMask); if (client_state_mask == 0U) { ALOGE("%s: reached the maximum number of channels per buffer node: %d.", __FUNCTION__, BufferHubDefs::kMaxNumberOfClients); errno = E2BIG; return 0U; } - updated_active_clients_bit_mask = current_active_clients_bit_mask | client_state_mask; - } while (!(active_clients_bit_mask_->compare_exchange_weak(current_active_clients_bit_mask, - updated_active_clients_bit_mask, - std::memory_order_acq_rel, - std::memory_order_acquire))); + updatedActiveClientsBitMask = currentActiveClientsBitMask | client_state_mask; + } while (!(mActiveClientsBitMask->compare_exchange_weak(currentActiveClientsBitMask, + updatedActiveClientsBitMask, + std::memory_order_acq_rel, + std::memory_order_acquire))); return client_state_mask; } void BufferNode::RemoveClientsBitFromMask(const uint32_t& value) { - active_clients_bit_mask_->fetch_and(~value); + mActiveClientsBitMask->fetch_and(~value); } } // namespace implementation diff --git a/services/bufferhub/include/bufferhub/BufferNode.h b/services/bufferhub/include/bufferhub/BufferNode.h index 4729e1cc73..112a21c9af 100644 --- a/services/bufferhub/include/bufferhub/BufferNode.h +++ b/services/bufferhub/include/bufferhub/BufferNode.h @@ -22,36 +22,36 @@ public: ~BufferNode(); // Returns whether the object holds a valid metadata. - bool IsValid() const { return metadata_.IsValid(); } + bool IsValid() const { return mMetadata.IsValid(); } int id() const { return mId; } - size_t user_metadata_size() const { return metadata_.user_metadata_size(); } + size_t user_metadata_size() const { return mMetadata.user_metadata_size(); } // Accessors of the buffer description and handle - const native_handle_t* buffer_handle() const { return buffer_handle_; } - const AHardwareBuffer_Desc& buffer_desc() const { return buffer_desc_; } + const native_handle_t* buffer_handle() const { return mBufferHandle; } + const AHardwareBuffer_Desc& buffer_desc() const { return mBufferDesc; } // Accessor of event fd. const BufferHubEventFd& eventFd() const { return mEventFd; } - // Accessors of metadata. - const BufferHubMetadata& metadata() const { return metadata_; } + // Accessors of mMetadata. + const BufferHubMetadata& metadata() const { return mMetadata; } - // Gets the current value of active_clients_bit_mask in metadata_ with + // Gets the current value of mActiveClientsBitMask in mMetadata with // std::memory_order_acquire, so that all previous releases of - // active_clients_bit_mask from all threads will be returned here. + // mActiveClientsBitMask from all threads will be returned here. uint32_t GetActiveClientsBitMask() const; - // Find and add a new client_state_mask to active_clients_bit_mask in - // metadata_. - // Return the new client_state_mask that is added to active_clients_bit_mask. + // Find and add a new client state mask to mActiveClientsBitMask in + // mMetadata. + // Return the new client state mask that is added to mActiveClientsBitMask. // Return 0U if there are already 16 clients of the buffer. uint32_t AddNewActiveClientsBitToMask(); - // Removes the value from active_clients_bit_mask in metadata_ with + // Removes the value from active_clients_bit_mask in mMetadata with // std::memory_order_release, so that the change will be visible to any - // acquire of active_clients_bit_mask_ in any threads after the succeed of + // acquire of mActiveClientsBitMask in any threads after the succeed of // this operation. void RemoveClientsBitFromMask(const uint32_t& value); @@ -61,34 +61,34 @@ private: void InitializeMetadata(); // Gralloc buffer handles. - native_handle_t* buffer_handle_; - AHardwareBuffer_Desc buffer_desc_; + native_handle_t* mBufferHandle; + AHardwareBuffer_Desc mBufferDesc; // Eventfd used for signalling buffer events among the clients of the buffer. BufferHubEventFd mEventFd; // Metadata in shared memory. - BufferHubMetadata metadata_; + BufferHubMetadata mMetadata; // A system-unique id generated by bufferhub from 0 to std::numeric_limits::max(). // BufferNodes not created by bufferhub will have id < 0, meaning "not specified". // TODO(b/118891412): remove default id = -1 and update comments after pdx is no longer in use const int mId = -1; - // The following variables are atomic variables in metadata_ that are visible + // The following variables are atomic variables in mMetadata that are visible // to Bn object and Bp objects. Please find more info in // BufferHubDefs::MetadataHeader. - // buffer_state_ tracks the state of the buffer. Buffer can be in one of these + // mBufferState tracks the state of the buffer. Buffer can be in one of these // four states: gained, posted, acquired, released. - std::atomic* buffer_state_ = nullptr; + std::atomic* mBufferState = nullptr; - // TODO(b/112012161): add comments to fence_state_. - std::atomic* fence_state_ = nullptr; + // TODO(b/112012161): add comments to mFenceState. + std::atomic* mFenceState = nullptr; - // active_clients_bit_mask_ tracks all the bp clients of the buffer. It is the + // mActiveClientsBitMask tracks all the bp clients of the buffer. It is the // union of all client_state_mask of all bp clients. - std::atomic* active_clients_bit_mask_ = nullptr; + std::atomic* mActiveClientsBitMask = nullptr; }; } // namespace implementation -- cgit v1.2.3-59-g8ed1b From 727ede4a0fd4fbc6621833b21579da255eb1fb68 Mon Sep 17 00:00:00 2001 From: Tianyu Jiang Date: Fri, 1 Feb 2019 11:44:51 -0800 Subject: Fix non camelCase function names Android Framework C++ Code Style Guidelines says that function names should be camelCase: http://go/droidcppstyle Test: m, mma in frameworks/native Bug: 68273829 Change-Id: I2f661c06b31b2e72cd0eee3d91b95531b60ec939 --- libs/ui/BufferHubBuffer.cpp | 62 ++--- libs/ui/BufferHubMetadata.cpp | 8 +- libs/ui/GraphicBuffer.cpp | 2 +- libs/ui/include/ui/BufferHubBuffer.h | 28 +-- libs/ui/include/ui/BufferHubDefs.h | 16 +- libs/ui/include/ui/BufferHubMetadata.h | 14 +- libs/ui/tests/BufferHubBuffer_test.cpp | 272 ++++++++++----------- libs/ui/tests/BufferHubMetadata_test.cpp | 101 ++++---- libs/ui/tests/GraphicBuffer_test.cpp | 8 +- libs/vr/libbufferhub/buffer_hub-test.cpp | 46 ++-- libs/vr/libbufferhub/consumer_buffer.cpp | 6 +- .../include/private/dvr/buffer_hub_defs.h | 32 +-- libs/vr/libbufferhub/producer_buffer.cpp | 20 +- .../libbufferhubqueue/buffer_hub_queue_client.cpp | 4 +- services/bufferhub/BufferHubService.cpp | 22 +- services/bufferhub/BufferNode.cpp | 20 +- services/bufferhub/include/bufferhub/BufferNode.h | 16 +- services/bufferhub/tests/BufferNode_test.cpp | 47 ++-- services/vr/bufferhubd/producer_channel.cpp | 22 +- services/vr/bufferhubd/producer_queue_channel.cpp | 2 +- 20 files changed, 373 insertions(+), 375 deletions(-) (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index c3144b924d..7693fcbfe0 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -24,12 +24,12 @@ #include using ::android::base::unique_fd; -using ::android::BufferHubDefs::AnyClientAcquired; -using ::android::BufferHubDefs::AnyClientGained; -using ::android::BufferHubDefs::IsClientAcquired; -using ::android::BufferHubDefs::IsClientGained; -using ::android::BufferHubDefs::IsClientPosted; -using ::android::BufferHubDefs::IsClientReleased; +using ::android::BufferHubDefs::isAnyClientAcquired; +using ::android::BufferHubDefs::isAnyClientGained; +using ::android::BufferHubDefs::isClientAcquired; +using ::android::BufferHubDefs::isClientGained; +using ::android::BufferHubDefs::isClientPosted; +using ::android::BufferHubDefs::isClientReleased; using ::android::frameworks::bufferhub::V1_0::BufferHubStatus; using ::android::frameworks::bufferhub::V1_0::BufferTraits; using ::android::frameworks::bufferhub::V1_0::IBufferClient; @@ -39,22 +39,22 @@ using ::android::hardware::graphics::common::V1_2::HardwareBufferDescription; namespace android { -std::unique_ptr BufferHubBuffer::Create(uint32_t width, uint32_t height, +std::unique_ptr BufferHubBuffer::create(uint32_t width, uint32_t height, uint32_t layerCount, uint32_t format, uint64_t usage, size_t userMetadataSize) { auto buffer = std::unique_ptr( new BufferHubBuffer(width, height, layerCount, format, usage, userMetadataSize)); - return buffer->IsValid() ? std::move(buffer) : nullptr; + return buffer->isValid() ? std::move(buffer) : nullptr; } -std::unique_ptr BufferHubBuffer::Import(const native_handle_t* token) { +std::unique_ptr BufferHubBuffer::import(const native_handle_t* token) { if (token == nullptr) { ALOGE("%s: token cannot be nullptr!", __FUNCTION__); return nullptr; } auto buffer = std::unique_ptr(new BufferHubBuffer(token)); - return buffer->IsValid() ? std::move(buffer) : nullptr; + return buffer->isValid() ? std::move(buffer) : nullptr; } BufferHubBuffer::BufferHubBuffer(uint32_t width, uint32_t height, uint32_t layerCount, @@ -169,8 +169,8 @@ int BufferHubBuffer::initWithBufferTraits(const BufferTraits& bufferTraits) { // Import fds. Dup fds because hidl_handle owns the fds. unique_fd ashmemFd(fcntl(bufferTraits.bufferInfo->data[0], F_DUPFD_CLOEXEC, 0)); - mMetadata = BufferHubMetadata::Import(std::move(ashmemFd)); - if (!mMetadata.IsValid()) { + mMetadata = BufferHubMetadata::import(std::move(ashmemFd)); + if (!mMetadata.isValid()) { ALOGE("%s: Received an invalid metadata.", __FUNCTION__); return -EINVAL; } @@ -196,20 +196,20 @@ int BufferHubBuffer::initWithBufferTraits(const BufferTraits& bufferTraits) { uint32_t userMetadataSize; memcpy(&userMetadataSize, &bufferTraits.bufferInfo->data[4], sizeof(userMetadataSize)); - if (mMetadata.user_metadata_size() != userMetadataSize) { + if (mMetadata.userMetadataSize() != userMetadataSize) { ALOGE("%s: user metadata size not match: expected %u, actual %zu.", __FUNCTION__, - userMetadataSize, mMetadata.user_metadata_size()); + userMetadataSize, mMetadata.userMetadataSize()); return -EINVAL; } - size_t metadataSize = static_cast(mMetadata.metadata_size()); + size_t metadataSize = static_cast(mMetadata.metadataSize()); if (metadataSize < BufferHubDefs::kMetadataHeaderSize) { ALOGE("%s: metadata too small: %zu", __FUNCTION__, metadataSize); return -EINVAL; } // Populate shortcuts to the atomics in metadata. - auto metadata_header = mMetadata.metadata_header(); + auto metadata_header = mMetadata.metadataHeader(); mBufferState = &metadata_header->buffer_state; mFenceState = &metadata_header->fence_state; mActiveClientsBitMask = &metadata_header->active_clients_bit_mask; @@ -235,16 +235,16 @@ int BufferHubBuffer::initWithBufferTraits(const BufferTraits& bufferTraits) { return 0; } -int BufferHubBuffer::Gain() { +int BufferHubBuffer::gain() { uint32_t currentBufferState = mBufferState->load(std::memory_order_acquire); - if (IsClientGained(currentBufferState, mClientStateMask)) { + if (isClientGained(currentBufferState, mClientStateMask)) { ALOGV("%s: Buffer is already gained by this client %" PRIx32 ".", __FUNCTION__, mClientStateMask); return 0; } do { - if (AnyClientGained(currentBufferState & (~mClientStateMask)) || - AnyClientAcquired(currentBufferState)) { + if (isAnyClientGained(currentBufferState & (~mClientStateMask)) || + isAnyClientAcquired(currentBufferState)) { ALOGE("%s: Buffer is in use, id=%d mClientStateMask=%" PRIx32 " state=%" PRIx32 ".", __FUNCTION__, mId, mClientStateMask, currentBufferState); return -EBUSY; @@ -258,11 +258,11 @@ int BufferHubBuffer::Gain() { return 0; } -int BufferHubBuffer::Post() { +int BufferHubBuffer::post() { uint32_t currentBufferState = mBufferState->load(std::memory_order_acquire); uint32_t updatedBufferState = (~mClientStateMask) & BufferHubDefs::kHighBitsMask; do { - if (!IsClientGained(currentBufferState, mClientStateMask)) { + if (!isClientGained(currentBufferState, mClientStateMask)) { ALOGE("%s: Cannot post a buffer that is not gained by this client. buffer_id=%d " "mClientStateMask=%" PRIx32 " state=%" PRIx32 ".", __FUNCTION__, mId, mClientStateMask, currentBufferState); @@ -277,16 +277,16 @@ int BufferHubBuffer::Post() { return 0; } -int BufferHubBuffer::Acquire() { +int BufferHubBuffer::acquire() { uint32_t currentBufferState = mBufferState->load(std::memory_order_acquire); - if (IsClientAcquired(currentBufferState, mClientStateMask)) { + if (isClientAcquired(currentBufferState, mClientStateMask)) { ALOGV("%s: Buffer is already acquired by this client %" PRIx32 ".", __FUNCTION__, mClientStateMask); return 0; } uint32_t updatedBufferState = 0U; do { - if (!IsClientPosted(currentBufferState, mClientStateMask)) { + if (!isClientPosted(currentBufferState, mClientStateMask)) { ALOGE("%s: Cannot acquire a buffer that is not in posted state. buffer_id=%d " "mClientStateMask=%" PRIx32 " state=%" PRIx32 ".", __FUNCTION__, mId, mClientStateMask, currentBufferState); @@ -301,9 +301,9 @@ int BufferHubBuffer::Acquire() { return 0; } -int BufferHubBuffer::Release() { +int BufferHubBuffer::release() { uint32_t currentBufferState = mBufferState->load(std::memory_order_acquire); - if (IsClientReleased(currentBufferState, mClientStateMask)) { + if (isClientReleased(currentBufferState, mClientStateMask)) { ALOGV("%s: Buffer is already released by this client %" PRIx32 ".", __FUNCTION__, mClientStateMask); return 0; @@ -318,17 +318,17 @@ int BufferHubBuffer::Release() { return 0; } -bool BufferHubBuffer::IsReleased() const { +bool BufferHubBuffer::isReleased() const { return (mBufferState->load(std::memory_order_acquire) & mActiveClientsBitMask->load(std::memory_order_acquire)) == 0; } -bool BufferHubBuffer::IsValid() const { +bool BufferHubBuffer::isValid() const { return mBufferHandle.getNativeHandle() != nullptr && mId >= 0 && mClientStateMask != 0U && - mEventFd.get() >= 0 && mMetadata.IsValid() && mBufferClient != nullptr; + mEventFd.get() >= 0 && mMetadata.isValid() && mBufferClient != nullptr; } -native_handle_t* BufferHubBuffer::Duplicate() { +native_handle_t* BufferHubBuffer::duplicate() { if (mBufferClient == nullptr) { ALOGE("%s: missing BufferClient!", __FUNCTION__); return nullptr; diff --git a/libs/ui/BufferHubMetadata.cpp b/libs/ui/BufferHubMetadata.cpp index 816707db9d..05bc7ddfbe 100644 --- a/libs/ui/BufferHubMetadata.cpp +++ b/libs/ui/BufferHubMetadata.cpp @@ -34,7 +34,7 @@ using BufferHubDefs::kMetadataHeaderSize; using BufferHubDefs::MetadataHeader; /* static */ -BufferHubMetadata BufferHubMetadata::Create(size_t userMetadataSize) { +BufferHubMetadata BufferHubMetadata::create(size_t userMetadataSize) { // The size the of metadata buffer is used as the "width" parameter during allocation. Thus it // cannot overflow uint32_t. if (userMetadataSize >= (std::numeric_limits::max() - kMetadataHeaderSize)) { @@ -59,11 +59,11 @@ BufferHubMetadata BufferHubMetadata::Create(size_t userMetadataSize) { return {}; } - return BufferHubMetadata::Import(std::move(ashmemFd)); + return BufferHubMetadata::import(std::move(ashmemFd)); } /* static */ -BufferHubMetadata BufferHubMetadata::Import(unique_fd ashmemFd) { +BufferHubMetadata BufferHubMetadata::import(unique_fd ashmemFd) { if (!ashmem_valid(ashmemFd.get())) { ALOGE("BufferHubMetadata::Import: invalid ashmem fd."); return {}; @@ -94,7 +94,7 @@ BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd BufferHubMetadata::~BufferHubMetadata() { if (mMetadataHeader != nullptr) { - int ret = munmap(mMetadataHeader, metadata_size()); + int ret = munmap(mMetadataHeader, metadataSize()); ALOGE_IF(ret != 0, "BufferHubMetadata::~BufferHubMetadata: failed to unmap ashmem, error=%d.", errno); mMetadataHeader = nullptr; diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp index 41ae2531cd..f487dfa81b 100644 --- a/libs/ui/GraphicBuffer.cpp +++ b/libs/ui/GraphicBuffer.cpp @@ -100,7 +100,7 @@ GraphicBuffer::GraphicBuffer(std::unique_ptr buffer) : GraphicB return; } - mInitCheck = initWithHandle(buffer->DuplicateHandle(), /*method=*/TAKE_UNREGISTERED_HANDLE, + mInitCheck = initWithHandle(buffer->duplicateHandle(), /*method=*/TAKE_UNREGISTERED_HANDLE, buffer->desc().width, buffer->desc().height, static_cast(buffer->desc().format), buffer->desc().layers, buffer->desc().usage, buffer->desc().stride); diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h index 06955e4161..eac8c8437b 100644 --- a/libs/ui/include/ui/BufferHubBuffer.h +++ b/libs/ui/include/ui/BufferHubBuffer.h @@ -29,14 +29,14 @@ namespace android { class BufferHubBuffer { public: // Allocates a standalone BufferHubBuffer. - static std::unique_ptr Create(uint32_t width, uint32_t height, + static std::unique_ptr create(uint32_t width, uint32_t height, uint32_t layerCount, uint32_t format, uint64_t usage, size_t userMetadataSize); // Imports the given token to a BufferHubBuffer. Not taking ownership of the token. Caller // should close and destroy the token after calling this function regardless of output. // TODO(b/122543147): use a movable wrapper for token - static std::unique_ptr Import(const native_handle_t* token); + static std::unique_ptr import(const native_handle_t* token); BufferHubBuffer(const BufferHubBuffer&) = delete; void operator=(const BufferHubBuffer&) = delete; @@ -52,51 +52,51 @@ public: // Duplicate the underlying Gralloc buffer handle. Caller is responsible to free the handle // after use. - native_handle_t* DuplicateHandle() { + native_handle_t* duplicateHandle() { return native_handle_clone(mBufferHandle.getNativeHandle()); } const BufferHubEventFd& eventFd() const { return mEventFd; } // Returns the current value of MetadataHeader::buffer_state. - uint32_t buffer_state() const { return mBufferState->load(std::memory_order_acquire); } + uint32_t bufferState() const { return mBufferState->load(std::memory_order_acquire); } // A state mask which is unique to a buffer hub client among all its siblings sharing the same // concrete graphic buffer. - uint32_t client_state_mask() const { return mClientStateMask; } + uint32_t clientStateMask() const { return mClientStateMask; } - size_t user_metadata_size() const { return mMetadata.user_metadata_size(); } + size_t userMetadataSize() const { return mMetadata.userMetadataSize(); } // Returns true if the BufferClient is still alive. - bool IsConnected() const { return mBufferClient->ping().isOk(); } + bool isConnected() const { return mBufferClient->ping().isOk(); } // Returns true if the buffer is valid: non-null buffer handle, valid id, valid client bit mask, // valid metadata and valid buffer client - bool IsValid() const; + bool isValid() const; // Gains the buffer for exclusive write permission. Read permission is implied once a buffer is // gained. // The buffer can be gained as long as there is no other client in acquired or gained state. - int Gain(); + int gain(); // Posts the gained buffer for other buffer clients to use the buffer. // The buffer can be posted iff the buffer state for this client is gained. // After posting the buffer, this client is put to released state and does not have access to // the buffer for this cycle of the usage of the buffer. - int Post(); + int post(); // Acquires the buffer for shared read permission. // The buffer can be acquired iff the buffer state for this client is posted. - int Acquire(); + int acquire(); // Releases the buffer. // The buffer can be released from any buffer state. // After releasing the buffer, this client no longer have any permissions to the buffer for the // current cycle of the usage of the buffer. - int Release(); + int release(); // Returns whether the buffer is released by all active clients or not. - bool IsReleased() const; + bool isReleased() const; // Creates a token that stands for this BufferHubBuffer client and could be used for Import to // create another BufferHubBuffer. The new BufferHubBuffer will share the same underlying @@ -104,7 +104,7 @@ public: // should free it after use. // Returns a valid token on success, nullptr on failure. // TODO(b/122543147): use a movable wrapper for token - native_handle_t* Duplicate(); + native_handle_t* duplicate(); private: BufferHubBuffer(uint32_t width, uint32_t height, uint32_t layerCount, uint32_t format, diff --git a/libs/ui/include/ui/BufferHubDefs.h b/libs/ui/include/ui/BufferHubDefs.h index ff970cbd64..722a060597 100644 --- a/libs/ui/include/ui/BufferHubDefs.h +++ b/libs/ui/include/ui/BufferHubDefs.h @@ -63,19 +63,19 @@ static constexpr uint32_t kHighBitsMask = ~kLowbitsMask; static constexpr uint32_t kFirstClientBitMask = (1U << kMaxNumberOfClients) + 1U; // Returns true if any of the client is in gained state. -static inline bool AnyClientGained(uint32_t state) { +static inline bool isAnyClientGained(uint32_t state) { uint32_t high_bits = state >> kMaxNumberOfClients; uint32_t low_bits = state & kLowbitsMask; return high_bits == low_bits && low_bits != 0U; } // Returns true if the input client is in gained state. -static inline bool IsClientGained(uint32_t state, uint32_t client_bit_mask) { +static inline bool isClientGained(uint32_t state, uint32_t client_bit_mask) { return state == client_bit_mask; } // Returns true if any of the client is in posted state. -static inline bool AnyClientPosted(uint32_t state) { +static inline bool isAnyClientPosted(uint32_t state) { uint32_t high_bits = state >> kMaxNumberOfClients; uint32_t low_bits = state & kLowbitsMask; uint32_t posted_or_acquired = high_bits ^ low_bits; @@ -83,7 +83,7 @@ static inline bool AnyClientPosted(uint32_t state) { } // Returns true if the input client is in posted state. -static inline bool IsClientPosted(uint32_t state, uint32_t client_bit_mask) { +static inline bool isClientPosted(uint32_t state, uint32_t client_bit_mask) { uint32_t client_bits = state & client_bit_mask; if (client_bits == 0U) return false; uint32_t low_bits = client_bits & kLowbitsMask; @@ -91,7 +91,7 @@ static inline bool IsClientPosted(uint32_t state, uint32_t client_bit_mask) { } // Return true if any of the client is in acquired state. -static inline bool AnyClientAcquired(uint32_t state) { +static inline bool isAnyClientAcquired(uint32_t state) { uint32_t high_bits = state >> kMaxNumberOfClients; uint32_t low_bits = state & kLowbitsMask; uint32_t posted_or_acquired = high_bits ^ low_bits; @@ -99,7 +99,7 @@ static inline bool AnyClientAcquired(uint32_t state) { } // Return true if the input client is in acquired state. -static inline bool IsClientAcquired(uint32_t state, uint32_t client_bit_mask) { +static inline bool isClientAcquired(uint32_t state, uint32_t client_bit_mask) { uint32_t client_bits = state & client_bit_mask; if (client_bits == 0U) return false; uint32_t high_bits = client_bits & kHighBitsMask; @@ -107,13 +107,13 @@ static inline bool IsClientAcquired(uint32_t state, uint32_t client_bit_mask) { } // Returns true if the input client is in released state. -static inline bool IsClientReleased(uint32_t state, uint32_t client_bit_mask) { +static inline bool isClientReleased(uint32_t state, uint32_t client_bit_mask) { return (state & client_bit_mask) == 0U; } // Returns the next available buffer client's client_state_masks. // @params union_bits. Union of all existing clients' client_state_masks. -static inline uint32_t FindNextAvailableClientStateMask(uint32_t union_bits) { +static inline uint32_t findNextAvailableClientStateMask(uint32_t union_bits) { uint32_t low_union = union_bits & kLowbitsMask; if (low_union == kLowbitsMask) return 0U; uint32_t incremented = low_union + 1U; diff --git a/libs/ui/include/ui/BufferHubMetadata.h b/libs/ui/include/ui/BufferHubMetadata.h index 212189497a..3482507399 100644 --- a/libs/ui/include/ui/BufferHubMetadata.h +++ b/libs/ui/include/ui/BufferHubMetadata.h @@ -33,12 +33,12 @@ public: // @param userMetadataSize Size in bytes of the user defined metadata. The entire metadata // shared memory region to be allocated is the size of canonical // BufferHubDefs::MetadataHeader plus userMetadataSize. - static BufferHubMetadata Create(size_t userMetadataSize); + static BufferHubMetadata create(size_t userMetadataSize); // Imports an existing BufferHubMetadata from an ashmem FD. // // @param ashmemFd Ashmem file descriptor representing an ashmem region. - static BufferHubMetadata Import(unique_fd ashmemFd); + static BufferHubMetadata import(unique_fd ashmemFd); BufferHubMetadata() = default; @@ -63,13 +63,13 @@ public: // Returns true if the metadata is valid, i.e. the metadata has a valid ashmem fd and the ashmem // has been mapped into virtual address space. - bool IsValid() const { return mAshmemFd.get() != -1 && mMetadataHeader != nullptr; } + bool isValid() const { return mAshmemFd.get() != -1 && mMetadataHeader != nullptr; } - size_t user_metadata_size() const { return mUserMetadataSize; } - size_t metadata_size() const { return mUserMetadataSize + BufferHubDefs::kMetadataHeaderSize; } + size_t userMetadataSize() const { return mUserMetadataSize; } + size_t metadataSize() const { return mUserMetadataSize + BufferHubDefs::kMetadataHeaderSize; } - const unique_fd& ashmem_fd() const { return mAshmemFd; } - BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; } + const unique_fd& ashmemFd() const { return mAshmemFd; } + BufferHubDefs::MetadataHeader* metadataHeader() { return mMetadataHeader; } private: BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd, diff --git a/libs/ui/tests/BufferHubBuffer_test.cpp b/libs/ui/tests/BufferHubBuffer_test.cpp index 4ad2c4c2ce..3087a90787 100644 --- a/libs/ui/tests/BufferHubBuffer_test.cpp +++ b/libs/ui/tests/BufferHubBuffer_test.cpp @@ -32,13 +32,13 @@ namespace android { namespace { -using ::android::BufferHubDefs::AnyClientAcquired; -using ::android::BufferHubDefs::AnyClientGained; -using ::android::BufferHubDefs::AnyClientPosted; -using ::android::BufferHubDefs::IsClientAcquired; -using ::android::BufferHubDefs::IsClientGained; -using ::android::BufferHubDefs::IsClientPosted; -using ::android::BufferHubDefs::IsClientReleased; +using ::android::BufferHubDefs::isAnyClientAcquired; +using ::android::BufferHubDefs::isAnyClientGained; +using ::android::BufferHubDefs::isAnyClientPosted; +using ::android::BufferHubDefs::isClientAcquired; +using ::android::BufferHubDefs::isClientGained; +using ::android::BufferHubDefs::isClientPosted; +using ::android::BufferHubDefs::isClientReleased; using ::android::BufferHubDefs::kMetadataHeaderSize; using ::testing::IsNull; using ::testing::NotNull; @@ -81,88 +81,88 @@ private: }; void BufferHubBufferStateTransitionTest::CreateTwoClientsOfABuffer() { - b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); + b1 = BufferHubBuffer::create(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); ASSERT_THAT(b1, NotNull()); - b1ClientMask = b1->client_state_mask(); + b1ClientMask = b1->clientStateMask(); ASSERT_NE(b1ClientMask, 0U); - native_handle_t* token = b1->Duplicate(); + native_handle_t* token = b1->duplicate(); ASSERT_THAT(token, NotNull()); // TODO(b/122543147): use a movalbe wrapper for token - b2 = BufferHubBuffer::Import(token); + b2 = BufferHubBuffer::import(token); native_handle_close(token); native_handle_delete(token); ASSERT_THAT(b2, NotNull()); - b2ClientMask = b2->client_state_mask(); + b2ClientMask = b2->clientStateMask(); ASSERT_NE(b2ClientMask, 0U); ASSERT_NE(b2ClientMask, b1ClientMask); } TEST_F(BufferHubBufferTest, CreateBufferFails) { // Buffer Creation will fail: BLOB format requires height to be 1. - auto b1 = BufferHubBuffer::Create(kWidth, /*height=*/2, kLayerCount, + auto b1 = BufferHubBuffer::create(kWidth, /*height=*/2, kLayerCount, /*format=*/HAL_PIXEL_FORMAT_BLOB, kUsage, kUserMetadataSize); EXPECT_THAT(b1, IsNull()); // Buffer Creation will fail: user metadata size too large. - auto b2 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, + auto b2 = BufferHubBuffer::create(kWidth, kHeight, kLayerCount, kFormat, kUsage, /*userMetadataSize=*/std::numeric_limits::max()); EXPECT_THAT(b2, IsNull()); // Buffer Creation will fail: user metadata size too large. const size_t userMetadataSize = std::numeric_limits::max() - kMetadataHeaderSize; - auto b3 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, + auto b3 = BufferHubBuffer::create(kWidth, kHeight, kLayerCount, kFormat, kUsage, userMetadataSize); EXPECT_THAT(b3, IsNull()); } TEST_F(BufferHubBufferTest, CreateBuffer) { - auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, + auto b1 = BufferHubBuffer::create(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); ASSERT_THAT(b1, NotNull()); - EXPECT_TRUE(b1->IsConnected()); - EXPECT_TRUE(b1->IsValid()); + EXPECT_TRUE(b1->isConnected()); + EXPECT_TRUE(b1->isValid()); EXPECT_TRUE(cmpAHardwareBufferDesc(b1->desc(), kDesc)); - EXPECT_EQ(b1->user_metadata_size(), kUserMetadataSize); + EXPECT_EQ(b1->userMetadataSize(), kUserMetadataSize); } TEST_F(BufferHubBufferTest, DuplicateAndImportBuffer) { - auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, + auto b1 = BufferHubBuffer::create(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); ASSERT_THAT(b1, NotNull()); - EXPECT_TRUE(b1->IsValid()); + EXPECT_TRUE(b1->isValid()); - native_handle_t* token = b1->Duplicate(); + native_handle_t* token = b1->duplicate(); EXPECT_TRUE(token); // The detached buffer should still be valid. - EXPECT_TRUE(b1->IsConnected()); - EXPECT_TRUE(b1->IsValid()); + EXPECT_TRUE(b1->isConnected()); + EXPECT_TRUE(b1->isValid()); - std::unique_ptr b2 = BufferHubBuffer::Import(token); + std::unique_ptr b2 = BufferHubBuffer::import(token); native_handle_close(token); native_handle_delete(token); ASSERT_THAT(b2, NotNull()); - EXPECT_TRUE(b2->IsValid()); + EXPECT_TRUE(b2->isValid()); EXPECT_TRUE(cmpAHardwareBufferDesc(b1->desc(), b2->desc())); - EXPECT_EQ(b1->user_metadata_size(), b2->user_metadata_size()); + EXPECT_EQ(b1->userMetadataSize(), b2->userMetadataSize()); // These two buffer instances are based on the same physical buffer under the // hood, so they should share the same id. EXPECT_EQ(b1->id(), b2->id()); - // We use client_state_mask() to tell those two instances apart. - EXPECT_NE(b1->client_state_mask(), b2->client_state_mask()); + // We use clientStateMask() to tell those two instances apart. + EXPECT_NE(b1->clientStateMask(), b2->clientStateMask()); // Both buffer instances should be in released state currently. - EXPECT_TRUE(b1->IsReleased()); - EXPECT_TRUE(b2->IsReleased()); + EXPECT_TRUE(b1->isReleased()); + EXPECT_TRUE(b2->isReleased()); // The event fd should behave like duped event fds. const BufferHubEventFd& eventFd1 = b1->eventFd(); @@ -192,19 +192,19 @@ TEST_F(BufferHubBufferTest, DuplicateAndImportBuffer) { } TEST_F(BufferHubBufferTest, ImportFreedBuffer) { - auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, + auto b1 = BufferHubBuffer::create(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); ASSERT_THAT(b1, NotNull()); - EXPECT_TRUE(b1->IsValid()); + EXPECT_TRUE(b1->isValid()); - native_handle_t* token = b1->Duplicate(); + native_handle_t* token = b1->duplicate(); EXPECT_TRUE(token); // Explicitly destroy b1. Backend buffer should be freed and token becomes invalid b1.reset(); // TODO(b/122543147): use a movalbe wrapper for token - std::unique_ptr b2 = BufferHubBuffer::Import(token); + std::unique_ptr b2 = BufferHubBuffer::import(token); native_handle_close(token); native_handle_delete(token); @@ -214,7 +214,7 @@ TEST_F(BufferHubBufferTest, ImportFreedBuffer) { // nullptr must not crash the service TEST_F(BufferHubBufferTest, ImportNullToken) { - auto b1 = BufferHubBuffer::Import(nullptr); + auto b1 = BufferHubBuffer::import(nullptr); EXPECT_THAT(b1, IsNull()); } @@ -222,185 +222,185 @@ TEST_F(BufferHubBufferTest, ImportInvalidToken) { native_handle_t* token = native_handle_create(/*numFds=*/0, /*numInts=*/1); token->data[0] = 0; - auto b1 = BufferHubBuffer::Import(token); + auto b1 = BufferHubBuffer::import(token); native_handle_delete(token); EXPECT_THAT(b1, IsNull()); } TEST_F(BufferHubBufferStateTransitionTest, GainBuffer_fromReleasedState) { - ASSERT_TRUE(b1->IsReleased()); + ASSERT_TRUE(b1->isReleased()); // Successful gaining the buffer should change the buffer state bit of b1 to // gained state, other client state bits to released state. - EXPECT_EQ(b1->Gain(), 0); - EXPECT_TRUE(IsClientGained(b1->buffer_state(), b1ClientMask)); + EXPECT_EQ(b1->gain(), 0); + EXPECT_TRUE(isClientGained(b1->bufferState(), b1ClientMask)); } TEST_F(BufferHubBufferStateTransitionTest, GainBuffer_fromGainedState) { - ASSERT_EQ(b1->Gain(), 0); - auto currentBufferState = b1->buffer_state(); - ASSERT_TRUE(IsClientGained(currentBufferState, b1ClientMask)); + ASSERT_EQ(b1->gain(), 0); + auto currentBufferState = b1->bufferState(); + ASSERT_TRUE(isClientGained(currentBufferState, b1ClientMask)); // Gaining from gained state by the same client should not return error. - EXPECT_EQ(b1->Gain(), 0); + EXPECT_EQ(b1->gain(), 0); // Gaining from gained state by another client should return error. - EXPECT_EQ(b2->Gain(), -EBUSY); + EXPECT_EQ(b2->gain(), -EBUSY); } TEST_F(BufferHubBufferStateTransitionTest, GainBuffer_fromAcquiredState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_EQ(b2->Acquire(), 0); - ASSERT_TRUE(AnyClientAcquired(b1->buffer_state())); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_EQ(b2->acquire(), 0); + ASSERT_TRUE(isAnyClientAcquired(b1->bufferState())); // Gaining from acquired state should fail. - EXPECT_EQ(b1->Gain(), -EBUSY); - EXPECT_EQ(b2->Gain(), -EBUSY); + EXPECT_EQ(b1->gain(), -EBUSY); + EXPECT_EQ(b2->gain(), -EBUSY); } TEST_F(BufferHubBufferStateTransitionTest, GainBuffer_fromOtherClientInPostedState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_TRUE(AnyClientPosted(b1->buffer_state())); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_TRUE(isAnyClientPosted(b1->bufferState())); // Gaining a buffer who has other posted client should succeed. - EXPECT_EQ(b1->Gain(), 0); + EXPECT_EQ(b1->gain(), 0); } TEST_F(BufferHubBufferStateTransitionTest, GainBuffer_fromSelfInPostedState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_TRUE(AnyClientPosted(b1->buffer_state())); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_TRUE(isAnyClientPosted(b1->bufferState())); // A posted client should be able to gain the buffer when there is no other clients in // acquired state. - EXPECT_EQ(b2->Gain(), 0); + EXPECT_EQ(b2->gain(), 0); } TEST_F(BufferHubBufferStateTransitionTest, PostBuffer_fromOtherInGainedState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_TRUE(IsClientGained(b1->buffer_state(), b1ClientMask)); + ASSERT_EQ(b1->gain(), 0); + ASSERT_TRUE(isClientGained(b1->bufferState(), b1ClientMask)); - EXPECT_EQ(b2->Post(), -EBUSY); + EXPECT_EQ(b2->post(), -EBUSY); } TEST_F(BufferHubBufferStateTransitionTest, PostBuffer_fromSelfInGainedState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_TRUE(IsClientGained(b1->buffer_state(), b1ClientMask)); + ASSERT_EQ(b1->gain(), 0); + ASSERT_TRUE(isClientGained(b1->bufferState(), b1ClientMask)); - EXPECT_EQ(b1->Post(), 0); - auto currentBufferState = b1->buffer_state(); - EXPECT_TRUE(IsClientReleased(currentBufferState, b1ClientMask)); - EXPECT_TRUE(IsClientPosted(currentBufferState, b2ClientMask)); + EXPECT_EQ(b1->post(), 0); + auto currentBufferState = b1->bufferState(); + EXPECT_TRUE(isClientReleased(currentBufferState, b1ClientMask)); + EXPECT_TRUE(isClientPosted(currentBufferState, b2ClientMask)); } TEST_F(BufferHubBufferStateTransitionTest, PostBuffer_fromPostedState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_TRUE(AnyClientPosted(b1->buffer_state())); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_TRUE(isAnyClientPosted(b1->bufferState())); // Post from posted state should fail. - EXPECT_EQ(b1->Post(), -EBUSY); - EXPECT_EQ(b2->Post(), -EBUSY); + EXPECT_EQ(b1->post(), -EBUSY); + EXPECT_EQ(b2->post(), -EBUSY); } TEST_F(BufferHubBufferStateTransitionTest, PostBuffer_fromAcquiredState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_EQ(b2->Acquire(), 0); - ASSERT_TRUE(AnyClientAcquired(b1->buffer_state())); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_EQ(b2->acquire(), 0); + ASSERT_TRUE(isAnyClientAcquired(b1->bufferState())); // Posting from acquired state should fail. - EXPECT_EQ(b1->Post(), -EBUSY); - EXPECT_EQ(b2->Post(), -EBUSY); + EXPECT_EQ(b1->post(), -EBUSY); + EXPECT_EQ(b2->post(), -EBUSY); } TEST_F(BufferHubBufferStateTransitionTest, PostBuffer_fromReleasedState) { - ASSERT_TRUE(b1->IsReleased()); + ASSERT_TRUE(b1->isReleased()); // Posting from released state should fail. - EXPECT_EQ(b1->Post(), -EBUSY); - EXPECT_EQ(b2->Post(), -EBUSY); + EXPECT_EQ(b1->post(), -EBUSY); + EXPECT_EQ(b2->post(), -EBUSY); } TEST_F(BufferHubBufferStateTransitionTest, AcquireBuffer_fromSelfInPostedState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_TRUE(IsClientPosted(b1->buffer_state(), b2ClientMask)); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_TRUE(isClientPosted(b1->bufferState(), b2ClientMask)); // Acquire from posted state should pass. - EXPECT_EQ(b2->Acquire(), 0); + EXPECT_EQ(b2->acquire(), 0); } TEST_F(BufferHubBufferStateTransitionTest, AcquireBuffer_fromOtherInPostedState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_TRUE(IsClientPosted(b1->buffer_state(), b2ClientMask)); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_TRUE(isClientPosted(b1->bufferState(), b2ClientMask)); // Acquire from released state should fail, although there are other clients // in posted state. - EXPECT_EQ(b1->Acquire(), -EBUSY); + EXPECT_EQ(b1->acquire(), -EBUSY); } TEST_F(BufferHubBufferStateTransitionTest, AcquireBuffer_fromSelfInAcquiredState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_EQ(b2->Acquire(), 0); - auto currentBufferState = b1->buffer_state(); - ASSERT_TRUE(IsClientAcquired(currentBufferState, b2ClientMask)); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_EQ(b2->acquire(), 0); + auto currentBufferState = b1->bufferState(); + ASSERT_TRUE(isClientAcquired(currentBufferState, b2ClientMask)); // Acquiring from acquired state by the same client should not error out. - EXPECT_EQ(b2->Acquire(), 0); + EXPECT_EQ(b2->acquire(), 0); } TEST_F(BufferHubBufferStateTransitionTest, AcquireBuffer_fromReleasedState) { - ASSERT_TRUE(b1->IsReleased()); + ASSERT_TRUE(b1->isReleased()); // Acquiring form released state should fail. - EXPECT_EQ(b1->Acquire(), -EBUSY); - EXPECT_EQ(b2->Acquire(), -EBUSY); + EXPECT_EQ(b1->acquire(), -EBUSY); + EXPECT_EQ(b2->acquire(), -EBUSY); } TEST_F(BufferHubBufferStateTransitionTest, AcquireBuffer_fromGainedState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_TRUE(AnyClientGained(b1->buffer_state())); + ASSERT_EQ(b1->gain(), 0); + ASSERT_TRUE(isAnyClientGained(b1->bufferState())); // Acquiring from gained state should fail. - EXPECT_EQ(b1->Acquire(), -EBUSY); - EXPECT_EQ(b2->Acquire(), -EBUSY); + EXPECT_EQ(b1->acquire(), -EBUSY); + EXPECT_EQ(b2->acquire(), -EBUSY); } TEST_F(BufferHubBufferStateTransitionTest, ReleaseBuffer_fromSelfInReleasedState) { - ASSERT_TRUE(b1->IsReleased()); + ASSERT_TRUE(b1->isReleased()); - EXPECT_EQ(b1->Release(), 0); + EXPECT_EQ(b1->release(), 0); } TEST_F(BufferHubBufferStateTransitionTest, ReleaseBuffer_fromSelfInGainedState) { - ASSERT_TRUE(b1->IsReleased()); - ASSERT_EQ(b1->Gain(), 0); - ASSERT_TRUE(AnyClientGained(b1->buffer_state())); + ASSERT_TRUE(b1->isReleased()); + ASSERT_EQ(b1->gain(), 0); + ASSERT_TRUE(isAnyClientGained(b1->bufferState())); - EXPECT_EQ(b1->Release(), 0); + EXPECT_EQ(b1->release(), 0); } TEST_F(BufferHubBufferStateTransitionTest, ReleaseBuffer_fromSelfInPostedState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_TRUE(AnyClientPosted(b1->buffer_state())); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_TRUE(isAnyClientPosted(b1->bufferState())); - EXPECT_EQ(b2->Release(), 0); + EXPECT_EQ(b2->release(), 0); } TEST_F(BufferHubBufferStateTransitionTest, ReleaseBuffer_fromSelfInAcquiredState) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_EQ(b2->Acquire(), 0); - ASSERT_TRUE(AnyClientAcquired(b1->buffer_state())); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_EQ(b2->acquire(), 0); + ASSERT_TRUE(isAnyClientAcquired(b1->bufferState())); - EXPECT_EQ(b2->Release(), 0); + EXPECT_EQ(b2->release(), 0); } TEST_F(BufferHubBufferStateTransitionTest, BasicUsage) { @@ -408,60 +408,60 @@ TEST_F(BufferHubBufferStateTransitionTest, BasicUsage) { // Test if this set of basic operation succeed: // Producer post three times to the consumer, and released by consumer. for (int i = 0; i < 3; ++i) { - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); - ASSERT_EQ(b2->Acquire(), 0); - ASSERT_EQ(b2->Release(), 0); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); + ASSERT_EQ(b2->acquire(), 0); + ASSERT_EQ(b2->release(), 0); } } TEST_F(BufferHubBufferTest, createNewConsumerAfterGain) { // Create a poducer buffer and gain. std::unique_ptr b1 = - BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, + BufferHubBuffer::create(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); ASSERT_THAT(b1, NotNull()); - ASSERT_EQ(b1->Gain(), 0); + ASSERT_EQ(b1->gain(), 0); // Create a consumer of the buffer and test if the consumer can acquire the // buffer if producer posts. // TODO(b/122543147): use a movalbe wrapper for token - native_handle_t* token = b1->Duplicate(); + native_handle_t* token = b1->duplicate(); ASSERT_TRUE(token); - std::unique_ptr b2 = BufferHubBuffer::Import(token); + std::unique_ptr b2 = BufferHubBuffer::import(token); native_handle_close(token); native_handle_delete(token); ASSERT_THAT(b2, NotNull()); - ASSERT_NE(b1->client_state_mask(), b2->client_state_mask()); + ASSERT_NE(b1->clientStateMask(), b2->clientStateMask()); - ASSERT_EQ(b1->Post(), 0); - EXPECT_EQ(b2->Acquire(), 0); + ASSERT_EQ(b1->post(), 0); + EXPECT_EQ(b2->acquire(), 0); } TEST_F(BufferHubBufferTest, createNewConsumerAfterPost) { // Create a poducer buffer and post. std::unique_ptr b1 = - BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, + BufferHubBuffer::create(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); - ASSERT_EQ(b1->Gain(), 0); - ASSERT_EQ(b1->Post(), 0); + ASSERT_EQ(b1->gain(), 0); + ASSERT_EQ(b1->post(), 0); // Create a consumer of the buffer and test if the consumer can acquire the // buffer if producer posts. // TODO(b/122543147): use a movalbe wrapper for token - native_handle_t* token = b1->Duplicate(); + native_handle_t* token = b1->duplicate(); ASSERT_TRUE(token); - std::unique_ptr b2 = BufferHubBuffer::Import(token); + std::unique_ptr b2 = BufferHubBuffer::import(token); native_handle_close(token); native_handle_delete(token); ASSERT_THAT(b2, NotNull()); - ASSERT_NE(b1->client_state_mask(), b2->client_state_mask()); + ASSERT_NE(b1->clientStateMask(), b2->clientStateMask()); - EXPECT_EQ(b2->Acquire(), 0); + EXPECT_EQ(b2->acquire(), 0); } } // namespace diff --git a/libs/ui/tests/BufferHubMetadata_test.cpp b/libs/ui/tests/BufferHubMetadata_test.cpp index b7f0b4b140..f02c4fc178 100644 --- a/libs/ui/tests/BufferHubMetadata_test.cpp +++ b/libs/ui/tests/BufferHubMetadata_test.cpp @@ -25,74 +25,73 @@ constexpr size_t kEmptyUserMetadataSize = 0; class BufferHubMetadataTest : public ::testing::Test {}; TEST_F(BufferHubMetadataTest, Create_UserMetdataSizeTooBig) { - BufferHubMetadata m1 = - BufferHubMetadata::Create(std::numeric_limits::max()); - EXPECT_FALSE(m1.IsValid()); + BufferHubMetadata m1 = BufferHubMetadata::create(std::numeric_limits::max()); + EXPECT_FALSE(m1.isValid()); } TEST_F(BufferHubMetadataTest, Create_Success) { - BufferHubMetadata m1 = BufferHubMetadata::Create(kEmptyUserMetadataSize); - EXPECT_TRUE(m1.IsValid()); - EXPECT_NE(m1.metadata_header(), nullptr); + BufferHubMetadata m1 = BufferHubMetadata::create(kEmptyUserMetadataSize); + EXPECT_TRUE(m1.isValid()); + EXPECT_NE(m1.metadataHeader(), nullptr); } TEST_F(BufferHubMetadataTest, Import_Success) { - BufferHubMetadata m1 = BufferHubMetadata::Create(kEmptyUserMetadataSize); - EXPECT_TRUE(m1.IsValid()); - EXPECT_NE(m1.metadata_header(), nullptr); - - unique_fd h2 = unique_fd(dup(m1.ashmem_fd().get())); - EXPECT_NE(h2.get(), -1); - - BufferHubMetadata m2 = BufferHubMetadata::Import(std::move(h2)); - EXPECT_EQ(h2.get(), -1); - EXPECT_TRUE(m1.IsValid()); - BufferHubDefs::MetadataHeader* mh1 = m1.metadata_header(); - EXPECT_NE(mh1, nullptr); - - // Check if the newly allocated buffer is initialized in released state (i.e. - // state equals to 0U). - EXPECT_TRUE(mh1->buffer_state.load() == 0U); - - EXPECT_TRUE(m2.IsValid()); - BufferHubDefs::MetadataHeader* mh2 = m2.metadata_header(); - EXPECT_NE(mh2, nullptr); - - // Check if the newly allocated buffer is initialized in released state (i.e. - // state equals to 0U). - EXPECT_TRUE(mh2->buffer_state.load() == 0U); + BufferHubMetadata m1 = BufferHubMetadata::create(kEmptyUserMetadataSize); + EXPECT_TRUE(m1.isValid()); + EXPECT_NE(m1.metadataHeader(), nullptr); + + unique_fd h2 = unique_fd(dup(m1.ashmemFd().get())); + EXPECT_NE(h2.get(), -1); + + BufferHubMetadata m2 = BufferHubMetadata::import(std::move(h2)); + EXPECT_EQ(h2.get(), -1); + EXPECT_TRUE(m1.isValid()); + BufferHubDefs::MetadataHeader* mh1 = m1.metadataHeader(); + EXPECT_NE(mh1, nullptr); + + // Check if the newly allocated buffer is initialized in released state (i.e. + // state equals to 0U). + EXPECT_TRUE(mh1->buffer_state.load() == 0U); + + EXPECT_TRUE(m2.isValid()); + BufferHubDefs::MetadataHeader* mh2 = m2.metadataHeader(); + EXPECT_NE(mh2, nullptr); + + // Check if the newly allocated buffer is initialized in released state (i.e. + // state equals to 0U). + EXPECT_TRUE(mh2->buffer_state.load() == 0U); } TEST_F(BufferHubMetadataTest, MoveMetadataInvalidatesOldOne) { - BufferHubMetadata m1 = BufferHubMetadata::Create(sizeof(int)); - EXPECT_TRUE(m1.IsValid()); - EXPECT_NE(m1.metadata_header(), nullptr); - EXPECT_NE(m1.ashmem_fd().get(), -1); - EXPECT_EQ(m1.user_metadata_size(), sizeof(int)); + BufferHubMetadata m1 = BufferHubMetadata::create(sizeof(int)); + EXPECT_TRUE(m1.isValid()); + EXPECT_NE(m1.metadataHeader(), nullptr); + EXPECT_NE(m1.ashmemFd().get(), -1); + EXPECT_EQ(m1.userMetadataSize(), sizeof(int)); - BufferHubMetadata m2 = std::move(m1); + BufferHubMetadata m2 = std::move(m1); - // After the move, the metadata header (a raw pointer) should be reset in the older buffer. - EXPECT_EQ(m1.metadata_header(), nullptr); - EXPECT_NE(m2.metadata_header(), nullptr); + // After the move, the metadata header (a raw pointer) should be reset in the older buffer. + EXPECT_EQ(m1.metadataHeader(), nullptr); + EXPECT_NE(m2.metadataHeader(), nullptr); - EXPECT_EQ(m1.ashmem_fd().get(), -1); - EXPECT_NE(m2.ashmem_fd().get(), -1); + EXPECT_EQ(m1.ashmemFd().get(), -1); + EXPECT_NE(m2.ashmemFd().get(), -1); - EXPECT_EQ(m1.user_metadata_size(), 0U); - EXPECT_EQ(m2.user_metadata_size(), sizeof(int)); + EXPECT_EQ(m1.userMetadataSize(), 0U); + EXPECT_EQ(m2.userMetadataSize(), sizeof(int)); - BufferHubMetadata m3{std::move(m2)}; + BufferHubMetadata m3{std::move(m2)}; - // After the move, the metadata header (a raw pointer) should be reset in the older buffer. - EXPECT_EQ(m2.metadata_header(), nullptr); - EXPECT_NE(m3.metadata_header(), nullptr); + // After the move, the metadata header (a raw pointer) should be reset in the older buffer. + EXPECT_EQ(m2.metadataHeader(), nullptr); + EXPECT_NE(m3.metadataHeader(), nullptr); - EXPECT_EQ(m2.ashmem_fd().get(), -1); - EXPECT_NE(m3.ashmem_fd().get(), -1); + EXPECT_EQ(m2.ashmemFd().get(), -1); + EXPECT_NE(m3.ashmemFd().get(), -1); - EXPECT_EQ(m2.user_metadata_size(), 0U); - EXPECT_EQ(m3.user_metadata_size(), sizeof(int)); + EXPECT_EQ(m2.userMetadataSize(), 0U); + EXPECT_EQ(m3.userMetadataSize(), sizeof(int)); } } // namespace dvr diff --git a/libs/ui/tests/GraphicBuffer_test.cpp b/libs/ui/tests/GraphicBuffer_test.cpp index 5b46454c60..c767ce02fc 100644 --- a/libs/ui/tests/GraphicBuffer_test.cpp +++ b/libs/ui/tests/GraphicBuffer_test.cpp @@ -37,10 +37,10 @@ class GraphicBufferTest : public testing::Test {}; TEST_F(GraphicBufferTest, CreateFromBufferHubBuffer) { std::unique_ptr b1 = - BufferHubBuffer::Create(kTestWidth, kTestHeight, kTestLayerCount, kTestFormat, + BufferHubBuffer::create(kTestWidth, kTestHeight, kTestLayerCount, kTestFormat, kTestUsage, /*userMetadataSize=*/0); ASSERT_NE(b1, nullptr); - EXPECT_TRUE(b1->IsValid()); + EXPECT_TRUE(b1->isValid()); sp gb(new GraphicBuffer(std::move(b1))); EXPECT_TRUE(gb->isBufferHubBuffer()); @@ -61,10 +61,10 @@ TEST_F(GraphicBufferTest, InvalidBufferIdForNoneBufferHubBuffer) { TEST_F(GraphicBufferTest, BufferIdMatchesBufferHubBufferId) { std::unique_ptr b1 = - BufferHubBuffer::Create(kTestWidth, kTestHeight, kTestLayerCount, kTestFormat, + BufferHubBuffer::create(kTestWidth, kTestHeight, kTestLayerCount, kTestFormat, kTestUsage, /*userMetadataSize=*/0); EXPECT_NE(b1, nullptr); - EXPECT_TRUE(b1->IsValid()); + EXPECT_TRUE(b1->isValid()); int b1_id = b1->id(); EXPECT_GE(b1_id, 0); diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp index 527a27d5dd..27ab0242ad 100644 --- a/libs/vr/libbufferhub/buffer_hub-test.cpp +++ b/libs/vr/libbufferhub/buffer_hub-test.cpp @@ -20,12 +20,12 @@ namespace { return result; \ })() -using android::BufferHubDefs::AnyClientAcquired; -using android::BufferHubDefs::AnyClientGained; -using android::BufferHubDefs::AnyClientPosted; -using android::BufferHubDefs::IsClientAcquired; -using android::BufferHubDefs::IsClientPosted; -using android::BufferHubDefs::IsClientReleased; +using android::BufferHubDefs::isAnyClientAcquired; +using android::BufferHubDefs::isAnyClientGained; +using android::BufferHubDefs::isAnyClientPosted; +using android::BufferHubDefs::isClientAcquired; +using android::BufferHubDefs::isClientPosted; +using android::BufferHubDefs::isClientReleased; using android::BufferHubDefs::kFirstClientBitMask; using android::dvr::ConsumerBuffer; using android::dvr::ProducerBuffer; @@ -268,7 +268,7 @@ TEST_F(LibBufferHubTest, TestAsyncStateTransitions) { // Post in gained state should succeed. EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence)); EXPECT_EQ(p->buffer_state(), c->buffer_state()); - EXPECT_TRUE(AnyClientPosted(p->buffer_state())); + EXPECT_TRUE(isAnyClientPosted(p->buffer_state())); // Post and gain in posted state should fail. EXPECT_EQ(-EBUSY, p->PostAsync(&metadata, invalid_fence)); @@ -280,7 +280,7 @@ TEST_F(LibBufferHubTest, TestAsyncStateTransitions) { EXPECT_EQ(0, c->AcquireAsync(&metadata, &invalid_fence)); EXPECT_FALSE(invalid_fence.IsValid()); EXPECT_EQ(p->buffer_state(), c->buffer_state()); - EXPECT_TRUE(AnyClientAcquired(p->buffer_state())); + EXPECT_TRUE(isAnyClientAcquired(p->buffer_state())); // Acquire, post, and gain in acquired state should fail. EXPECT_EQ(-EBUSY, c->AcquireAsync(&metadata, &invalid_fence)); @@ -304,7 +304,7 @@ TEST_F(LibBufferHubTest, TestAsyncStateTransitions) { EXPECT_EQ(0, p->GainAsync(&metadata, &invalid_fence)); EXPECT_FALSE(invalid_fence.IsValid()); EXPECT_EQ(p->buffer_state(), c->buffer_state()); - EXPECT_TRUE(AnyClientGained(p->buffer_state())); + EXPECT_TRUE(isAnyClientGained(p->buffer_state())); // Acquire and gain in gained state should fail. EXPECT_EQ(-EBUSY, c->AcquireAsync(&metadata, &invalid_fence)); @@ -329,7 +329,7 @@ TEST_F(LibBufferHubTest, TestGainPostedBuffer) { ASSERT_TRUE(c.get() != nullptr); ASSERT_EQ(0, p->GainAsync()); ASSERT_EQ(0, p->Post(LocalHandle())); - ASSERT_TRUE(AnyClientPosted(p->buffer_state())); + ASSERT_TRUE(isAnyClientPosted(p->buffer_state())); // Gain in posted state should only succeed with gain_posted_buffer = true. LocalHandle invalid_fence; @@ -346,7 +346,7 @@ TEST_F(LibBufferHubTest, TestGainPostedBufferAsync) { ASSERT_TRUE(c.get() != nullptr); ASSERT_EQ(0, p->GainAsync()); ASSERT_EQ(0, p->Post(LocalHandle())); - ASSERT_TRUE(AnyClientPosted(p->buffer_state())); + ASSERT_TRUE(isAnyClientPosted(p->buffer_state())); // GainAsync in posted state should only succeed with gain_posted_buffer // equals true. @@ -364,9 +364,9 @@ TEST_F(LibBufferHubTest, TestGainPostedBuffer_noConsumer) { ASSERT_EQ(0, p->Post(LocalHandle())); // Producer state bit is in released state after post, other clients shall be // in posted state although there is no consumer of this buffer yet. - ASSERT_TRUE(IsClientReleased(p->buffer_state(), p->client_state_mask())); + ASSERT_TRUE(isClientReleased(p->buffer_state(), p->client_state_mask())); ASSERT_TRUE(p->is_released()); - ASSERT_TRUE(AnyClientPosted(p->buffer_state())); + ASSERT_TRUE(isAnyClientPosted(p->buffer_state())); // Gain in released state should succeed. LocalHandle invalid_fence; @@ -393,14 +393,14 @@ TEST_F(LibBufferHubTest, TestMaxConsumers) { // Post the producer should trigger all consumers to be available. EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence)); - EXPECT_TRUE(IsClientReleased(p->buffer_state(), p->client_state_mask())); + EXPECT_TRUE(isClientReleased(p->buffer_state(), p->client_state_mask())); for (size_t i = 0; i < kMaxConsumerCount; ++i) { EXPECT_TRUE( - IsClientPosted(cs[i]->buffer_state(), cs[i]->client_state_mask())); + isClientPosted(cs[i]->buffer_state(), cs[i]->client_state_mask())); EXPECT_LT(0, RETRY_EINTR(PollBufferEvent(cs[i]))); EXPECT_EQ(0, cs[i]->AcquireAsync(&metadata, &invalid_fence)); EXPECT_TRUE( - IsClientAcquired(p->buffer_state(), cs[i]->client_state_mask())); + isClientAcquired(p->buffer_state(), cs[i]->client_state_mask())); } // All consumers have to release before the buffer is considered to be @@ -424,22 +424,22 @@ TEST_F(LibBufferHubTest, TestCreateConsumerWhenBufferGained) { kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t)); ASSERT_TRUE(p.get() != nullptr); EXPECT_EQ(0, p->GainAsync()); - EXPECT_TRUE(AnyClientGained(p->buffer_state())); + EXPECT_TRUE(isAnyClientGained(p->buffer_state())); std::unique_ptr c = ConsumerBuffer::Import(p->CreateConsumer()); ASSERT_TRUE(c.get() != nullptr); - EXPECT_TRUE(AnyClientGained(c->buffer_state())); + EXPECT_TRUE(isAnyClientGained(c->buffer_state())); DvrNativeBufferMetadata metadata; LocalHandle invalid_fence; // Post the gained buffer should signal already created consumer. EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence)); - EXPECT_TRUE(AnyClientPosted(p->buffer_state())); + EXPECT_TRUE(isAnyClientPosted(p->buffer_state())); EXPECT_LT(0, RETRY_EINTR(PollBufferEvent(c))); EXPECT_EQ(0, c->AcquireAsync(&metadata, &invalid_fence)); - EXPECT_TRUE(AnyClientAcquired(c->buffer_state())); + EXPECT_TRUE(isAnyClientAcquired(c->buffer_state())); } TEST_F(LibBufferHubTest, TestCreateTheFirstConsumerAfterPostingBuffer) { @@ -447,7 +447,7 @@ TEST_F(LibBufferHubTest, TestCreateTheFirstConsumerAfterPostingBuffer) { kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t)); ASSERT_TRUE(p.get() != nullptr); EXPECT_EQ(0, p->GainAsync()); - EXPECT_TRUE(AnyClientGained(p->buffer_state())); + EXPECT_TRUE(isAnyClientGained(p->buffer_state())); DvrNativeBufferMetadata metadata; LocalHandle invalid_fence; @@ -462,7 +462,7 @@ TEST_F(LibBufferHubTest, TestCreateTheFirstConsumerAfterPostingBuffer) { std::unique_ptr c = ConsumerBuffer::Import(p->CreateConsumer()); ASSERT_TRUE(c.get() != nullptr); - EXPECT_TRUE(IsClientPosted(c->buffer_state(), c->client_state_mask())); + EXPECT_TRUE(isClientPosted(c->buffer_state(), c->client_state_mask())); EXPECT_EQ(0, c->AcquireAsync(&metadata, &invalid_fence)); } @@ -500,7 +500,7 @@ TEST_F(LibBufferHubTest, TestCreateConsumerWhenBufferReleased) { EXPECT_TRUE(p->is_released()); EXPECT_EQ(0, p->GainAsync(&metadata, &invalid_fence)); - EXPECT_TRUE(AnyClientGained(p->buffer_state())); + EXPECT_TRUE(isAnyClientGained(p->buffer_state())); } TEST_F(LibBufferHubTest, TestWithCustomMetadata) { diff --git a/libs/vr/libbufferhub/consumer_buffer.cpp b/libs/vr/libbufferhub/consumer_buffer.cpp index b6ca64eef2..115e8666e5 100644 --- a/libs/vr/libbufferhub/consumer_buffer.cpp +++ b/libs/vr/libbufferhub/consumer_buffer.cpp @@ -38,7 +38,7 @@ int ConsumerBuffer::LocalAcquire(DvrNativeBufferMetadata* out_meta, // The buffer can be acquired iff the buffer state for this client is posted. uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - if (!BufferHubDefs::IsClientPosted(current_buffer_state, + if (!BufferHubDefs::isClientPosted(current_buffer_state, client_state_mask())) { ALOGE( "%s: Failed to acquire the buffer. The buffer is not posted, id=%d " @@ -58,7 +58,7 @@ int ConsumerBuffer::LocalAcquire(DvrNativeBufferMetadata* out_meta, " when trying to acquire the buffer and modify the buffer state to " "%" PRIx32 ". About to try again if the buffer is still posted.", __FUNCTION__, current_buffer_state, updated_buffer_state); - if (!BufferHubDefs::IsClientPosted(current_buffer_state, + if (!BufferHubDefs::isClientPosted(current_buffer_state, client_state_mask())) { ALOGE( "%s: Failed to acquire the buffer. The buffer is no longer posted, " @@ -144,7 +144,7 @@ int ConsumerBuffer::LocalRelease(const DvrNativeBufferMetadata* meta, // released state. uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - if (BufferHubDefs::IsClientReleased(current_buffer_state, + if (BufferHubDefs::isClientReleased(current_buffer_state, client_state_mask())) { return 0; } diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h index bab7367caa..e610e18849 100644 --- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h +++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h @@ -27,38 +27,38 @@ static constexpr uint32_t kHighBitsMask = android::BufferHubDefs::kHighBitsMask; static constexpr uint32_t kFirstClientBitMask = android::BufferHubDefs::kFirstClientBitMask; -static inline bool AnyClientGained(uint32_t state) { - return android::BufferHubDefs::AnyClientGained(state); +static inline bool isAnyClientGained(uint32_t state) { + return android::BufferHubDefs::isAnyClientGained(state); } -static inline bool IsClientGained(uint32_t state, uint32_t client_bit_mask) { - return android::BufferHubDefs::IsClientGained(state, client_bit_mask); +static inline bool isClientGained(uint32_t state, uint32_t client_bit_mask) { + return android::BufferHubDefs::isClientGained(state, client_bit_mask); } -static inline bool AnyClientPosted(uint32_t state) { - return android::BufferHubDefs::AnyClientPosted(state); +static inline bool isAnyClientPosted(uint32_t state) { + return android::BufferHubDefs::isAnyClientPosted(state); } -static inline bool IsClientPosted(uint32_t state, uint32_t client_bit_mask) { - return android::BufferHubDefs::IsClientPosted(state, client_bit_mask); +static inline bool isClientPosted(uint32_t state, uint32_t client_bit_mask) { + return android::BufferHubDefs::isClientPosted(state, client_bit_mask); } -static inline bool AnyClientAcquired(uint32_t state) { - return android::BufferHubDefs::AnyClientAcquired(state); +static inline bool isAnyClientAcquired(uint32_t state) { + return android::BufferHubDefs::isAnyClientAcquired(state); } -static inline bool IsClientAcquired(uint32_t state, uint32_t client_bit_mask) { - return android::BufferHubDefs::IsClientAcquired(state, client_bit_mask); +static inline bool isClientAcquired(uint32_t state, uint32_t client_bit_mask) { + return android::BufferHubDefs::isClientAcquired(state, client_bit_mask); } -static inline bool IsClientReleased(uint32_t state, uint32_t client_bit_mask) { - return android::BufferHubDefs::IsClientReleased(state, client_bit_mask); +static inline bool isClientReleased(uint32_t state, uint32_t client_bit_mask) { + return android::BufferHubDefs::isClientReleased(state, client_bit_mask); } // Returns the next available buffer client's client_state_masks. // @params union_bits. Union of all existing clients' client_state_masks. -static inline uint32_t FindNextAvailableClientStateMask(uint32_t union_bits) { - return android::BufferHubDefs::FindNextAvailableClientStateMask(union_bits); +static inline uint32_t findNextAvailableClientStateMask(uint32_t union_bits) { + return android::BufferHubDefs::findNextAvailableClientStateMask(union_bits); } using MetadataHeader = android::BufferHubDefs::MetadataHeader; diff --git a/libs/vr/libbufferhub/producer_buffer.cpp b/libs/vr/libbufferhub/producer_buffer.cpp index edfdddf363..3d88ba5dbe 100644 --- a/libs/vr/libbufferhub/producer_buffer.cpp +++ b/libs/vr/libbufferhub/producer_buffer.cpp @@ -82,7 +82,7 @@ int ProducerBuffer::LocalPost(const DvrNativeBufferMetadata* meta, // The buffer can be posted iff the buffer state for this client is gained. uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - if (!BufferHubDefs::IsClientGained(current_buffer_state, + if (!BufferHubDefs::isClientGained(current_buffer_state, client_state_mask())) { ALOGE("%s: not gained, id=%d state=%" PRIx32 ".", __FUNCTION__, id(), current_buffer_state); @@ -103,7 +103,7 @@ int ProducerBuffer::LocalPost(const DvrNativeBufferMetadata* meta, "%" PRIx32 ". About to try again if the buffer is still gained by this client.", __FUNCTION__, current_buffer_state, updated_buffer_state); - if (!BufferHubDefs::IsClientGained(current_buffer_state, + if (!BufferHubDefs::isClientGained(current_buffer_state, client_state_mask())) { ALOGE( "%s: Failed to post the buffer. The buffer is no longer gained, " @@ -166,14 +166,14 @@ int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta, ALOGD_IF(TRACE, "%s: buffer=%d, state=%" PRIx32 ".", __FUNCTION__, id(), current_buffer_state); - if (BufferHubDefs::IsClientGained(current_buffer_state, + if (BufferHubDefs::isClientGained(current_buffer_state, client_state_mask())) { ALOGV("%s: already gained id=%d.", __FUNCTION__, id()); return 0; } - if (BufferHubDefs::AnyClientAcquired(current_buffer_state) || - BufferHubDefs::AnyClientGained(current_buffer_state) || - (BufferHubDefs::AnyClientPosted( + if (BufferHubDefs::isAnyClientAcquired(current_buffer_state) || + BufferHubDefs::isAnyClientGained(current_buffer_state) || + (BufferHubDefs::isAnyClientPosted( current_buffer_state & active_clients_bit_mask_->load(std::memory_order_acquire)) && !gain_posted_buffer)) { @@ -195,9 +195,9 @@ int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta, "clients.", __FUNCTION__, current_buffer_state, updated_buffer_state); - if (BufferHubDefs::AnyClientAcquired(current_buffer_state) || - BufferHubDefs::AnyClientGained(current_buffer_state) || - (BufferHubDefs::AnyClientPosted( + if (BufferHubDefs::isAnyClientAcquired(current_buffer_state) || + BufferHubDefs::isAnyClientGained(current_buffer_state) || + (BufferHubDefs::isAnyClientPosted( current_buffer_state & active_clients_bit_mask_->load(std::memory_order_acquire)) && !gain_posted_buffer)) { @@ -291,7 +291,7 @@ Status ProducerBuffer::Detach() { // TODO(b/112338294) Keep here for reference. Remove it after new logic is // written. /* uint32_t buffer_state = buffer_state_->load(std::memory_order_acquire); - if (!BufferHubDefs::IsClientGained( + if (!BufferHubDefs::isClientGained( buffer_state, BufferHubDefs::kFirstClientStateMask)) { // Can only detach a ProducerBuffer when it's in gained state. ALOGW("ProducerBuffer::Detach: The buffer (id=%d, state=0x%" PRIx32 diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp index d7833f382b..2d3fa4aec0 100644 --- a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp +++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp @@ -532,7 +532,7 @@ Status ProducerQueue::AddBuffer( Status ProducerQueue::InsertBuffer( const std::shared_ptr& buffer) { if (buffer == nullptr || - !BufferHubDefs::IsClientGained(buffer->buffer_state(), + !BufferHubDefs::isClientGained(buffer->buffer_state(), buffer->client_state_mask())) { ALOGE( "ProducerQueue::InsertBuffer: Can only insert a buffer when it's in " @@ -638,7 +638,7 @@ Status> ProducerQueue::DequeueUnacquiredBuffer( static_cast(*slot)); return ErrorStatus(EIO); } - if (!BufferHubDefs::AnyClientAcquired(buffer->buffer_state())) { + if (!BufferHubDefs::isAnyClientAcquired(buffer->buffer_state())) { *slot = *iter; unavailable_buffers_slot_.erase(iter); unavailable_buffers_slot_.push_back(*slot); diff --git a/services/bufferhub/BufferHubService.cpp b/services/bufferhub/BufferHubService.cpp index 6b802fbf83..90ac1c2535 100644 --- a/services/bufferhub/BufferHubService.cpp +++ b/services/bufferhub/BufferHubService.cpp @@ -53,7 +53,7 @@ Return BufferHubService::allocateBuffer(const HardwareBufferDescription& d std::make_shared(desc.width, desc.height, desc.layers, desc.format, desc.usage, userMetadataSize, BufferHubIdGenerator::getInstance().getId()); - if (node == nullptr || !node->IsValid()) { + if (node == nullptr || !node->isValid()) { ALOGE("%s: creating BufferNode failed.", __FUNCTION__); _hidl_cb(/*status=*/BufferHubStatus::ALLOCATION_FAILED, /*bufferClient=*/nullptr, /*bufferTraits=*/{}); @@ -70,11 +70,11 @@ Return BufferHubService::allocateBuffer(const HardwareBufferDescription& d NATIVE_HANDLE_DECLARE_STORAGE(bufferInfoStorage, BufferHubDefs::kBufferInfoNumFds, BufferHubDefs::kBufferInfoNumInts); hidl_handle bufferInfo = - buildBufferInfo(bufferInfoStorage, node->id(), node->AddNewActiveClientsBitToMask(), - node->user_metadata_size(), node->metadata().ashmem_fd(), + buildBufferInfo(bufferInfoStorage, node->id(), node->addNewActiveClientsBitToMask(), + node->userMetadataSize(), node->metadata().ashmemFd(), node->eventFd().get()); BufferTraits bufferTraits = {/*bufferDesc=*/description, - /*bufferHandle=*/hidl_handle(node->buffer_handle()), + /*bufferHandle=*/hidl_handle(node->bufferHandle()), /*bufferInfo=*/std::move(bufferInfo)}; _hidl_cb(/*status=*/BufferHubStatus::NO_ERROR, /*bufferClient=*/client, @@ -141,7 +141,7 @@ Return BufferHubService::importBuffer(const hidl_handle& tokenHandle, } sp client = new BufferClient(*originClient); - uint32_t clientStateMask = client->getBufferNode()->AddNewActiveClientsBitToMask(); + uint32_t clientStateMask = client->getBufferNode()->addNewActiveClientsBitToMask(); if (clientStateMask == 0U) { // Reach max client count ALOGE("%s: import failed, BufferNode#%u reached maximum clients.", __FUNCTION__, @@ -157,17 +157,17 @@ Return BufferHubService::importBuffer(const hidl_handle& tokenHandle, std::shared_ptr node = client->getBufferNode(); HardwareBufferDescription bufferDesc; - memcpy(&bufferDesc, &node->buffer_desc(), sizeof(HardwareBufferDescription)); + memcpy(&bufferDesc, &node->bufferDesc(), sizeof(HardwareBufferDescription)); // Allocate memory for bufferInfo of type hidl_handle on the stack. See // http://aosp/286282 for the usage of NATIVE_HANDLE_DECLARE_STORAGE. NATIVE_HANDLE_DECLARE_STORAGE(bufferInfoStorage, BufferHubDefs::kBufferInfoNumFds, BufferHubDefs::kBufferInfoNumInts); hidl_handle bufferInfo = buildBufferInfo(bufferInfoStorage, node->id(), clientStateMask, - node->user_metadata_size(), - node->metadata().ashmem_fd(), node->eventFd().get()); + node->userMetadataSize(), node->metadata().ashmemFd(), + node->eventFd().get()); BufferTraits bufferTraits = {/*bufferDesc=*/bufferDesc, - /*bufferHandle=*/hidl_handle(node->buffer_handle()), + /*bufferHandle=*/hidl_handle(node->bufferHandle()), /*bufferInfo=*/std::move(bufferInfo)}; _hidl_cb(/*status=*/BufferHubStatus::NO_ERROR, /*bufferClient=*/client, @@ -231,10 +231,10 @@ Return BufferHubService::debug(const hidl_handle& fd, const hidl_vec node = std::move(iter->second.first); const uint32_t clientCount = iter->second.second; - AHardwareBuffer_Desc desc = node->buffer_desc(); + AHardwareBuffer_Desc desc = node->bufferDesc(); MetadataHeader* metadataHeader = - const_cast(&node->metadata())->metadata_header(); + const_cast(&node->metadata())->metadataHeader(); const uint32_t state = metadataHeader->buffer_state.load(std::memory_order_acquire); const uint64_t index = metadataHeader->queue_index; diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index dfa2de0173..4f877b264a 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -11,10 +11,10 @@ namespace bufferhub { namespace V1_0 { namespace implementation { -void BufferNode::InitializeMetadata() { +void BufferNode::initializeMetadata() { // Using placement new here to reuse shared memory instead of new allocation // Initialize the atomic variables to zero. - BufferHubDefs::MetadataHeader* metadataHeader = mMetadata.metadata_header(); + BufferHubDefs::MetadataHeader* metadataHeader = mMetadata.metadataHeader(); mBufferState = new (&metadataHeader->buffer_state) std::atomic(0); mFenceState = new (&metadataHeader->fence_state) std::atomic(0); mActiveClientsBitMask = new (&metadataHeader->active_clients_bit_mask) std::atomic(0); @@ -54,12 +54,12 @@ BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, ui mBufferDesc.usage = usage; mBufferDesc.stride = out_stride; - mMetadata = BufferHubMetadata::Create(user_metadata_size); - if (!mMetadata.IsValid()) { + mMetadata = BufferHubMetadata::create(user_metadata_size); + if (!mMetadata.isValid()) { ALOGE("%s: Failed to allocate metadata.", __FUNCTION__); return; } - InitializeMetadata(); + initializeMetadata(); } BufferNode::~BufferNode() { @@ -77,17 +77,17 @@ BufferNode::~BufferNode() { } } -uint32_t BufferNode::GetActiveClientsBitMask() const { +uint32_t BufferNode::getActiveClientsBitMask() const { return mActiveClientsBitMask->load(std::memory_order_acquire); } -uint32_t BufferNode::AddNewActiveClientsBitToMask() { - uint32_t currentActiveClientsBitMask = GetActiveClientsBitMask(); +uint32_t BufferNode::addNewActiveClientsBitToMask() { + uint32_t currentActiveClientsBitMask = getActiveClientsBitMask(); uint32_t client_state_mask = 0U; uint32_t updatedActiveClientsBitMask = 0U; do { client_state_mask = - BufferHubDefs::FindNextAvailableClientStateMask(currentActiveClientsBitMask); + BufferHubDefs::findNextAvailableClientStateMask(currentActiveClientsBitMask); if (client_state_mask == 0U) { ALOGE("%s: reached the maximum number of channels per buffer node: %d.", __FUNCTION__, BufferHubDefs::kMaxNumberOfClients); @@ -102,7 +102,7 @@ uint32_t BufferNode::AddNewActiveClientsBitToMask() { return client_state_mask; } -void BufferNode::RemoveClientsBitFromMask(const uint32_t& value) { +void BufferNode::removeClientsBitFromMask(const uint32_t& value) { mActiveClientsBitMask->fetch_and(~value); } diff --git a/services/bufferhub/include/bufferhub/BufferNode.h b/services/bufferhub/include/bufferhub/BufferNode.h index 112a21c9af..04970fd14d 100644 --- a/services/bufferhub/include/bufferhub/BufferNode.h +++ b/services/bufferhub/include/bufferhub/BufferNode.h @@ -22,15 +22,15 @@ public: ~BufferNode(); // Returns whether the object holds a valid metadata. - bool IsValid() const { return mMetadata.IsValid(); } + bool isValid() const { return mMetadata.isValid(); } int id() const { return mId; } - size_t user_metadata_size() const { return mMetadata.user_metadata_size(); } + size_t userMetadataSize() const { return mMetadata.userMetadataSize(); } // Accessors of the buffer description and handle - const native_handle_t* buffer_handle() const { return mBufferHandle; } - const AHardwareBuffer_Desc& buffer_desc() const { return mBufferDesc; } + const native_handle_t* bufferHandle() const { return mBufferHandle; } + const AHardwareBuffer_Desc& bufferDesc() const { return mBufferDesc; } // Accessor of event fd. const BufferHubEventFd& eventFd() const { return mEventFd; } @@ -41,24 +41,24 @@ public: // Gets the current value of mActiveClientsBitMask in mMetadata with // std::memory_order_acquire, so that all previous releases of // mActiveClientsBitMask from all threads will be returned here. - uint32_t GetActiveClientsBitMask() const; + uint32_t getActiveClientsBitMask() const; // Find and add a new client state mask to mActiveClientsBitMask in // mMetadata. // Return the new client state mask that is added to mActiveClientsBitMask. // Return 0U if there are already 16 clients of the buffer. - uint32_t AddNewActiveClientsBitToMask(); + uint32_t addNewActiveClientsBitToMask(); // Removes the value from active_clients_bit_mask in mMetadata with // std::memory_order_release, so that the change will be visible to any // acquire of mActiveClientsBitMask in any threads after the succeed of // this operation. - void RemoveClientsBitFromMask(const uint32_t& value); + void removeClientsBitFromMask(const uint32_t& value); private: // Helper method for constructors to initialize atomic metadata header // variables in shared memory. - void InitializeMetadata(); + void initializeMetadata(); // Gralloc buffer handles. native_handle_t* mBufferHandle; diff --git a/services/bufferhub/tests/BufferNode_test.cpp b/services/bufferhub/tests/BufferNode_test.cpp index ccb1197498..b9f1c81c63 100644 --- a/services/bufferhub/tests/BufferNode_test.cpp +++ b/services/bufferhub/tests/BufferNode_test.cpp @@ -28,7 +28,7 @@ protected: void SetUp() override { buffer_node = new BufferNode(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); - ASSERT_TRUE(buffer_node->IsValid()); + ASSERT_TRUE(buffer_node->isValid()); } void TearDown() override { @@ -41,65 +41,64 @@ protected: }; TEST_F(BufferNodeTest, TestCreateBufferNode) { - EXPECT_EQ(buffer_node->user_metadata_size(), kUserMetadataSize); + EXPECT_EQ(buffer_node->userMetadataSize(), kUserMetadataSize); // Test the handle just allocated is good (i.e. able to be imported) GraphicBufferMapper& mapper = GraphicBufferMapper::get(); const native_handle_t* outHandle; status_t ret = - mapper.importBuffer(buffer_node->buffer_handle(), buffer_node->buffer_desc().width, - buffer_node->buffer_desc().height, - buffer_node->buffer_desc().layers, - buffer_node->buffer_desc().format, buffer_node->buffer_desc().usage, - buffer_node->buffer_desc().stride, &outHandle); + mapper.importBuffer(buffer_node->bufferHandle(), buffer_node->bufferDesc().width, + buffer_node->bufferDesc().height, buffer_node->bufferDesc().layers, + buffer_node->bufferDesc().format, buffer_node->bufferDesc().usage, + buffer_node->bufferDesc().stride, &outHandle); EXPECT_EQ(ret, OK); EXPECT_THAT(outHandle, NotNull()); } -TEST_F(BufferNodeTest, TestAddNewActiveClientsBitToMask_twoNewClients) { - uint32_t new_client_state_mask_1 = buffer_node->AddNewActiveClientsBitToMask(); - EXPECT_EQ(buffer_node->GetActiveClientsBitMask(), new_client_state_mask_1); +TEST_F(BufferNodeTest, TestaddNewActiveClientsBitToMask_twoNewClients) { + uint32_t new_client_state_mask_1 = buffer_node->addNewActiveClientsBitToMask(); + EXPECT_EQ(buffer_node->getActiveClientsBitMask(), new_client_state_mask_1); // Request and add a new client_state_mask again. // Active clients bit mask should be the union of the two new // client_state_masks. - uint32_t new_client_state_mask_2 = buffer_node->AddNewActiveClientsBitToMask(); - EXPECT_EQ(buffer_node->GetActiveClientsBitMask(), + uint32_t new_client_state_mask_2 = buffer_node->addNewActiveClientsBitToMask(); + EXPECT_EQ(buffer_node->getActiveClientsBitMask(), new_client_state_mask_1 | new_client_state_mask_2); } -TEST_F(BufferNodeTest, TestAddNewActiveClientsBitToMask_32NewClients) { +TEST_F(BufferNodeTest, TestaddNewActiveClientsBitToMask_32NewClients) { uint32_t new_client_state_mask = 0U; uint32_t current_mask = 0U; uint32_t expected_mask = 0U; for (int i = 0; i < BufferHubDefs::kMaxNumberOfClients; ++i) { - new_client_state_mask = buffer_node->AddNewActiveClientsBitToMask(); + new_client_state_mask = buffer_node->addNewActiveClientsBitToMask(); EXPECT_NE(new_client_state_mask, 0U); EXPECT_FALSE(new_client_state_mask & current_mask); expected_mask = current_mask | new_client_state_mask; - current_mask = buffer_node->GetActiveClientsBitMask(); + current_mask = buffer_node->getActiveClientsBitMask(); EXPECT_EQ(current_mask, expected_mask); } // Method should fail upon requesting for more than maximum allowable clients. - new_client_state_mask = buffer_node->AddNewActiveClientsBitToMask(); + new_client_state_mask = buffer_node->addNewActiveClientsBitToMask(); EXPECT_EQ(new_client_state_mask, 0U); EXPECT_EQ(errno, E2BIG); } TEST_F(BufferNodeTest, TestRemoveActiveClientsBitFromMask) { - buffer_node->AddNewActiveClientsBitToMask(); - uint32_t current_mask = buffer_node->GetActiveClientsBitMask(); - uint32_t new_client_state_mask = buffer_node->AddNewActiveClientsBitToMask(); - EXPECT_NE(buffer_node->GetActiveClientsBitMask(), current_mask); + buffer_node->addNewActiveClientsBitToMask(); + uint32_t current_mask = buffer_node->getActiveClientsBitMask(); + uint32_t new_client_state_mask = buffer_node->addNewActiveClientsBitToMask(); + EXPECT_NE(buffer_node->getActiveClientsBitMask(), current_mask); - buffer_node->RemoveClientsBitFromMask(new_client_state_mask); - EXPECT_EQ(buffer_node->GetActiveClientsBitMask(), current_mask); + buffer_node->removeClientsBitFromMask(new_client_state_mask); + EXPECT_EQ(buffer_node->getActiveClientsBitMask(), current_mask); // Remove the test_mask again to the active client bit mask should not modify // the value of active clients bit mask. - buffer_node->RemoveClientsBitFromMask(new_client_state_mask); - EXPECT_EQ(buffer_node->GetActiveClientsBitMask(), current_mask); + buffer_node->removeClientsBitFromMask(new_client_state_mask); + EXPECT_EQ(buffer_node->getActiveClientsBitMask(), current_mask); } } // namespace diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp index 164d9e6b19..f3e54a0ed2 100644 --- a/services/vr/bufferhubd/producer_channel.cpp +++ b/services/vr/bufferhubd/producer_channel.cpp @@ -253,7 +253,7 @@ Status ProducerChannel::CreateConsumerStateMask() { uint32_t current_active_clients_bit_mask = active_clients_bit_mask_->load(std::memory_order_acquire); uint32_t consumer_state_mask = - BufferHubDefs::FindNextAvailableClientStateMask( + BufferHubDefs::findNextAvailableClientStateMask( current_active_clients_bit_mask | orphaned_consumer_bit_mask_); if (consumer_state_mask == 0U) { ALOGE("%s: reached the maximum mumber of consumers per producer: 63.", @@ -279,7 +279,7 @@ Status ProducerChannel::CreateConsumerStateMask() { "condition.", __FUNCTION__, updated_active_clients_bit_mask, current_active_clients_bit_mask); - consumer_state_mask = BufferHubDefs::FindNextAvailableClientStateMask( + consumer_state_mask = BufferHubDefs::findNextAvailableClientStateMask( current_active_clients_bit_mask | orphaned_consumer_bit_mask_); if (consumer_state_mask == 0U) { ALOGE("%s: reached the maximum mumber of consumers per producer: %d.", @@ -337,13 +337,13 @@ Status ProducerChannel::CreateConsumer( // consumer to a buffer that is available to producer (a.k.a a fully-released // buffer) or a gained buffer. if (current_buffer_state == 0U || - BufferHubDefs::AnyClientGained(current_buffer_state)) { + BufferHubDefs::isAnyClientGained(current_buffer_state)) { return {status.take()}; } // Signal the new consumer when adding it to a posted producer. bool update_buffer_state = true; - if (!BufferHubDefs::IsClientPosted(current_buffer_state, + if (!BufferHubDefs::isClientPosted(current_buffer_state, consumer_state_mask)) { uint32_t updated_buffer_state = current_buffer_state ^ @@ -360,7 +360,7 @@ Status ProducerChannel::CreateConsumer( "released.", __FUNCTION__, current_buffer_state, updated_buffer_state); if (current_buffer_state == 0U || - BufferHubDefs::AnyClientGained(current_buffer_state)) { + BufferHubDefs::isAnyClientGained(current_buffer_state)) { ALOGI("%s: buffer is gained or fully released, state=%" PRIx32 ".", __FUNCTION__, current_buffer_state); update_buffer_state = false; @@ -371,7 +371,7 @@ Status ProducerChannel::CreateConsumer( (consumer_state_mask & BufferHubDefs::kHighBitsMask); } } - if (update_buffer_state || BufferHubDefs::IsClientPosted( + if (update_buffer_state || BufferHubDefs::isClientPosted( buffer_state_->load(std::memory_order_acquire), consumer_state_mask)) { consumer->OnProducerPosted(); @@ -457,7 +457,7 @@ Status ProducerChannel::OnProducerGain(Message& /*message*/) { buffer_id()); uint32_t buffer_state = buffer_state_->load(std::memory_order_acquire); - if (!BufferHubDefs::IsClientGained( + if (!BufferHubDefs::isClientGained( buffer_state, BufferHubDefs::kFirstClientStateMask)) { // Can only detach a ProducerBuffer when it's in gained state. ALOGW( @@ -616,9 +616,9 @@ void ProducerChannel::RemoveConsumer(ConsumerChannel* channel) { const uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire); - if (BufferHubDefs::IsClientPosted(current_buffer_state, + if (BufferHubDefs::isClientPosted(current_buffer_state, consumer_state_mask) || - BufferHubDefs::IsClientAcquired(current_buffer_state, + BufferHubDefs::isClientAcquired(current_buffer_state, consumer_state_mask)) { // The consumer client is being destoryed without releasing. This could // happen in corner cases when the consumer crashes. Here we mark it @@ -627,9 +627,9 @@ void ProducerChannel::RemoveConsumer(ConsumerChannel* channel) { return; } - if (BufferHubDefs::IsClientReleased(current_buffer_state, + if (BufferHubDefs::isClientReleased(current_buffer_state, consumer_state_mask) || - BufferHubDefs::AnyClientGained(current_buffer_state)) { + BufferHubDefs::isAnyClientGained(current_buffer_state)) { // The consumer is being close while it is suppose to signal a release // fence. Signal the dummy fence here. if (fence_state_->load(std::memory_order_acquire) & consumer_state_mask) { diff --git a/services/vr/bufferhubd/producer_queue_channel.cpp b/services/vr/bufferhubd/producer_queue_channel.cpp index 6b33f5094c..004dc7cb4f 100644 --- a/services/vr/bufferhubd/producer_queue_channel.cpp +++ b/services/vr/bufferhubd/producer_queue_channel.cpp @@ -323,7 +323,7 @@ Status ProducerQueueChannel::OnProducerQueueInsertBuffer( // memory to indicate which client is the last producer of the buffer. // Currently, the first client is the only producer to the buffer. // Thus, it checks whether the first client gains the buffer below. - if (!BufferHubDefs::IsClientGained(buffer_state, + if (!BufferHubDefs::isClientGained(buffer_state, BufferHubDefs::kFirstClientBitMask)) { // Rejects the request if the requested buffer is not in Gained state. ALOGE( -- cgit v1.2.3-59-g8ed1b From c9d8d589b838234f372e1c83578f0a6247f5cced Mon Sep 17 00:00:00 2001 From: Tianyu Jiang Date: Sun, 10 Feb 2019 22:05:34 -0800 Subject: Fix style violation according to go/droidcppstyle Local variable should be lowerCamelCase. This change leave the buffer_state fence_state, active_clients_bit_mask and queue_index in MetadataHeader unchanged because there are too many instances of them that need to be changed in both new bufferhub system and the old pdx-backed bufferhubd system. I will do a separate change for that. Test: m, mma Bug: 68273829 Change-Id: Ia91cf7debe01a402f61171f069988e0ac13cec6a --- libs/ui/BufferHubBuffer.cpp | 8 +-- services/bufferhub/BufferNode.cpp | 26 ++++----- .../bufferhub/include/bufferhub/BufferClient.h | 2 +- services/bufferhub/include/bufferhub/BufferNode.h | 4 +- services/bufferhub/tests/BufferNode_test.cpp | 67 +++++++++++----------- 5 files changed, 53 insertions(+), 54 deletions(-) (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index 7693fcbfe0..cce35fdf29 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -209,10 +209,10 @@ int BufferHubBuffer::initWithBufferTraits(const BufferTraits& bufferTraits) { } // Populate shortcuts to the atomics in metadata. - auto metadata_header = mMetadata.metadataHeader(); - mBufferState = &metadata_header->buffer_state; - mFenceState = &metadata_header->fence_state; - mActiveClientsBitMask = &metadata_header->active_clients_bit_mask; + auto metadataHeader = mMetadata.metadataHeader(); + mBufferState = &metadataHeader->buffer_state; + mFenceState = &metadataHeader->fence_state; + mActiveClientsBitMask = &metadataHeader->active_clients_bit_mask; // The C++ standard recommends (but does not require) that lock-free atomic operations are // also address-free, that is, suitable for communication between processes using shared // memory. diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index 4f877b264a..1efb27ee0a 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -28,17 +28,17 @@ void BufferNode::initializeMetadata() { } // Allocates a new BufferNode. -BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, - uint64_t usage, size_t user_metadata_size, int id) +BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layerCount, uint32_t format, + uint64_t usage, size_t userMetadataSize, int id) : mId(id) { - uint32_t out_stride = 0; + uint32_t outStride = 0; // graphicBufferId is not used in GraphicBufferAllocator::allocate // TODO(b/112338294) After move to the service folder, stop using the // hardcoded service name "bufferhub". - int ret = GraphicBufferAllocator::get().allocate(width, height, format, layer_count, usage, + int ret = GraphicBufferAllocator::get().allocate(width, height, format, layerCount, usage, const_cast( &mBufferHandle), - &out_stride, + &outStride, /*graphicBufferId=*/0, /*requestor=*/"bufferhub"); @@ -49,12 +49,12 @@ BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, ui mBufferDesc.width = width; mBufferDesc.height = height; - mBufferDesc.layers = layer_count; + mBufferDesc.layers = layerCount; mBufferDesc.format = format; mBufferDesc.usage = usage; - mBufferDesc.stride = out_stride; + mBufferDesc.stride = outStride; - mMetadata = BufferHubMetadata::create(user_metadata_size); + mMetadata = BufferHubMetadata::create(userMetadataSize); if (!mMetadata.isValid()) { ALOGE("%s: Failed to allocate metadata.", __FUNCTION__); return; @@ -83,23 +83,23 @@ uint32_t BufferNode::getActiveClientsBitMask() const { uint32_t BufferNode::addNewActiveClientsBitToMask() { uint32_t currentActiveClientsBitMask = getActiveClientsBitMask(); - uint32_t client_state_mask = 0U; + uint32_t clientStateMask = 0U; uint32_t updatedActiveClientsBitMask = 0U; do { - client_state_mask = + clientStateMask = BufferHubDefs::findNextAvailableClientStateMask(currentActiveClientsBitMask); - if (client_state_mask == 0U) { + if (clientStateMask == 0U) { ALOGE("%s: reached the maximum number of channels per buffer node: %d.", __FUNCTION__, BufferHubDefs::kMaxNumberOfClients); errno = E2BIG; return 0U; } - updatedActiveClientsBitMask = currentActiveClientsBitMask | client_state_mask; + updatedActiveClientsBitMask = currentActiveClientsBitMask | clientStateMask; } while (!(mActiveClientsBitMask->compare_exchange_weak(currentActiveClientsBitMask, updatedActiveClientsBitMask, std::memory_order_acq_rel, std::memory_order_acquire))); - return client_state_mask; + return clientStateMask; } void BufferNode::removeClientsBitFromMask(const uint32_t& value) { diff --git a/services/bufferhub/include/bufferhub/BufferClient.h b/services/bufferhub/include/bufferhub/BufferClient.h index 66ed4bd13f..644b40377d 100644 --- a/services/bufferhub/include/bufferhub/BufferClient.h +++ b/services/bufferhub/include/bufferhub/BufferClient.h @@ -72,4 +72,4 @@ private: } // namespace frameworks } // namespace android -#endif \ No newline at end of file +#endif diff --git a/services/bufferhub/include/bufferhub/BufferNode.h b/services/bufferhub/include/bufferhub/BufferNode.h index 04970fd14d..62a8d63b38 100644 --- a/services/bufferhub/include/bufferhub/BufferNode.h +++ b/services/bufferhub/include/bufferhub/BufferNode.h @@ -16,8 +16,8 @@ namespace implementation { class BufferNode { public: // Allocates a new BufferNode. - BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, - uint64_t usage, size_t user_metadata_size, int id = -1); + BufferNode(uint32_t width, uint32_t height, uint32_t layerCount, uint32_t format, + uint64_t usage, size_t userMetadataSize, int id = -1); ~BufferNode(); diff --git a/services/bufferhub/tests/BufferNode_test.cpp b/services/bufferhub/tests/BufferNode_test.cpp index b9f1c81c63..2dfd4fc52e 100644 --- a/services/bufferhub/tests/BufferNode_test.cpp +++ b/services/bufferhub/tests/BufferNode_test.cpp @@ -26,79 +26,78 @@ const size_t kUserMetadataSize = 0; class BufferNodeTest : public ::testing::Test { protected: void SetUp() override { - buffer_node = + mBufferNode = new BufferNode(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize); - ASSERT_TRUE(buffer_node->isValid()); + ASSERT_TRUE(mBufferNode->isValid()); } void TearDown() override { - if (buffer_node != nullptr) { - delete buffer_node; + if (mBufferNode != nullptr) { + delete mBufferNode; } } - BufferNode* buffer_node = nullptr; + BufferNode* mBufferNode = nullptr; }; TEST_F(BufferNodeTest, TestCreateBufferNode) { - EXPECT_EQ(buffer_node->userMetadataSize(), kUserMetadataSize); + EXPECT_EQ(mBufferNode->userMetadataSize(), kUserMetadataSize); // Test the handle just allocated is good (i.e. able to be imported) GraphicBufferMapper& mapper = GraphicBufferMapper::get(); const native_handle_t* outHandle; status_t ret = - mapper.importBuffer(buffer_node->bufferHandle(), buffer_node->bufferDesc().width, - buffer_node->bufferDesc().height, buffer_node->bufferDesc().layers, - buffer_node->bufferDesc().format, buffer_node->bufferDesc().usage, - buffer_node->bufferDesc().stride, &outHandle); + mapper.importBuffer(mBufferNode->bufferHandle(), mBufferNode->bufferDesc().width, + mBufferNode->bufferDesc().height, mBufferNode->bufferDesc().layers, + mBufferNode->bufferDesc().format, mBufferNode->bufferDesc().usage, + mBufferNode->bufferDesc().stride, &outHandle); EXPECT_EQ(ret, OK); EXPECT_THAT(outHandle, NotNull()); } TEST_F(BufferNodeTest, TestaddNewActiveClientsBitToMask_twoNewClients) { - uint32_t new_client_state_mask_1 = buffer_node->addNewActiveClientsBitToMask(); - EXPECT_EQ(buffer_node->getActiveClientsBitMask(), new_client_state_mask_1); + uint32_t newClientStateMask1 = mBufferNode->addNewActiveClientsBitToMask(); + EXPECT_EQ(mBufferNode->getActiveClientsBitMask(), newClientStateMask1); // Request and add a new client_state_mask again. // Active clients bit mask should be the union of the two new // client_state_masks. - uint32_t new_client_state_mask_2 = buffer_node->addNewActiveClientsBitToMask(); - EXPECT_EQ(buffer_node->getActiveClientsBitMask(), - new_client_state_mask_1 | new_client_state_mask_2); + uint32_t newClientStateMask2 = mBufferNode->addNewActiveClientsBitToMask(); + EXPECT_EQ(mBufferNode->getActiveClientsBitMask(), newClientStateMask1 | newClientStateMask2); } TEST_F(BufferNodeTest, TestaddNewActiveClientsBitToMask_32NewClients) { - uint32_t new_client_state_mask = 0U; - uint32_t current_mask = 0U; - uint32_t expected_mask = 0U; + uint32_t newClientStateMask = 0U; + uint32_t currentMask = 0U; + uint32_t expectedMask = 0U; for (int i = 0; i < BufferHubDefs::kMaxNumberOfClients; ++i) { - new_client_state_mask = buffer_node->addNewActiveClientsBitToMask(); - EXPECT_NE(new_client_state_mask, 0U); - EXPECT_FALSE(new_client_state_mask & current_mask); - expected_mask = current_mask | new_client_state_mask; - current_mask = buffer_node->getActiveClientsBitMask(); - EXPECT_EQ(current_mask, expected_mask); + newClientStateMask = mBufferNode->addNewActiveClientsBitToMask(); + EXPECT_NE(newClientStateMask, 0U); + EXPECT_FALSE(newClientStateMask & currentMask); + expectedMask = currentMask | newClientStateMask; + currentMask = mBufferNode->getActiveClientsBitMask(); + EXPECT_EQ(currentMask, expectedMask); } // Method should fail upon requesting for more than maximum allowable clients. - new_client_state_mask = buffer_node->addNewActiveClientsBitToMask(); - EXPECT_EQ(new_client_state_mask, 0U); + newClientStateMask = mBufferNode->addNewActiveClientsBitToMask(); + EXPECT_EQ(newClientStateMask, 0U); EXPECT_EQ(errno, E2BIG); } TEST_F(BufferNodeTest, TestRemoveActiveClientsBitFromMask) { - buffer_node->addNewActiveClientsBitToMask(); - uint32_t current_mask = buffer_node->getActiveClientsBitMask(); - uint32_t new_client_state_mask = buffer_node->addNewActiveClientsBitToMask(); - EXPECT_NE(buffer_node->getActiveClientsBitMask(), current_mask); + mBufferNode->addNewActiveClientsBitToMask(); + uint32_t currentMask = mBufferNode->getActiveClientsBitMask(); + uint32_t newClientStateMask = mBufferNode->addNewActiveClientsBitToMask(); + EXPECT_NE(mBufferNode->getActiveClientsBitMask(), currentMask); - buffer_node->removeClientsBitFromMask(new_client_state_mask); - EXPECT_EQ(buffer_node->getActiveClientsBitMask(), current_mask); + mBufferNode->removeClientsBitFromMask(newClientStateMask); + EXPECT_EQ(mBufferNode->getActiveClientsBitMask(), currentMask); // Remove the test_mask again to the active client bit mask should not modify // the value of active clients bit mask. - buffer_node->removeClientsBitFromMask(new_client_state_mask); - EXPECT_EQ(buffer_node->getActiveClientsBitMask(), current_mask); + mBufferNode->removeClientsBitFromMask(newClientStateMask); + EXPECT_EQ(mBufferNode->getActiveClientsBitMask(), currentMask); } } // namespace -- cgit v1.2.3-59-g8ed1b From f377a76a47cdcfd15437c94d547a30a9da82fb7c Mon Sep 17 00:00:00 2001 From: Tianyu Jiang Date: Wed, 13 Feb 2019 13:46:42 -0800 Subject: Fix code style violation in the variables in BufferHubDefs::MetadataHeader Violation: variables should be lowerCaseCamel in the following directories: frameworks/native/libs/ui and frameworks/native/services/bufferhub Test: m, mma Bug: 68273829 Change-Id: I7dc56ec17089456f98b018032f04f779b12632b2 --- libs/ui/BufferHubBuffer.cpp | 25 ++++---- libs/ui/include/ui/BufferHubBuffer.h | 2 +- libs/ui/include/ui/BufferHubDefs.h | 66 +++++++++++----------- libs/ui/tests/BufferHubMetadata_test.cpp | 4 +- libs/vr/libbufferhub/buffer_hub_base.cpp | 6 +- .../include/private/dvr/buffer_hub_base.h | 4 +- services/bufferhub/BufferHubService.cpp | 4 +- services/bufferhub/BufferNode.cpp | 6 +- services/vr/bufferhubd/producer_channel.cpp | 13 ++--- 9 files changed, 64 insertions(+), 66 deletions(-) (limited to 'services/bufferhub/BufferNode.cpp') diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index 604665bc91..da91a979fe 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -78,15 +78,14 @@ BufferHubBuffer::BufferHubBuffer(uint32_t width, uint32_t height, uint32_t layer BufferHubStatus ret; sp client; BufferTraits bufferTraits; - IBufferHub::allocateBuffer_cb alloc_cb = [&](const auto& status, const auto& outClient, - const auto& outTraits) { + IBufferHub::allocateBuffer_cb allocCb = [&](const auto& status, const auto& outClient, + const auto& outTraits) { ret = status; client = std::move(outClient); bufferTraits = std::move(outTraits); }; - if (!bufferhub->allocateBuffer(desc, static_cast(userMetadataSize), alloc_cb) - .isOk()) { + if (!bufferhub->allocateBuffer(desc, static_cast(userMetadataSize), allocCb).isOk()) { ALOGE("%s: allocateBuffer transaction failed!", __FUNCTION__); return; } else if (ret != BufferHubStatus::NO_ERROR) { @@ -115,8 +114,8 @@ BufferHubBuffer::BufferHubBuffer(const sp& token) { BufferHubStatus ret; sp client; BufferTraits bufferTraits; - IBufferHub::importBuffer_cb import_cb = [&](const auto& status, const auto& outClient, - const auto& outTraits) { + IBufferHub::importBuffer_cb importCb = [&](const auto& status, const auto& outClient, + const auto& outTraits) { ret = status; client = std::move(outClient); bufferTraits = std::move(outTraits); @@ -124,7 +123,7 @@ BufferHubBuffer::BufferHubBuffer(const sp& token) { // hidl_handle(native_handle_t*) simply creates a raw pointer reference withouth ownership // transfer. - if (!bufferhub->importBuffer(hidl_handle(token.get()->handle()), import_cb).isOk()) { + if (!bufferhub->importBuffer(hidl_handle(token.get()->handle()), importCb).isOk()) { ALOGE("%s: importBuffer transaction failed!", __FUNCTION__); return; } else if (ret != BufferHubStatus::NO_ERROR) { @@ -210,9 +209,9 @@ int BufferHubBuffer::initWithBufferTraits(const BufferTraits& bufferTraits) { // Populate shortcuts to the atomics in metadata. auto metadataHeader = mMetadata.metadataHeader(); - mBufferState = &metadataHeader->buffer_state; - mFenceState = &metadataHeader->fence_state; - mActiveClientsBitMask = &metadataHeader->active_clients_bit_mask; + mBufferState = &metadataHeader->bufferState; + mFenceState = &metadataHeader->fenceState; + mActiveClientsBitMask = &metadataHeader->activeClientsBitMask; // The C++ standard recommends (but does not require) that lock-free atomic operations are // also address-free, that is, suitable for communication between processes using shared // memory. @@ -230,7 +229,7 @@ int BufferHubBuffer::initWithBufferTraits(const BufferTraits& bufferTraits) { mClientStateMask = clientBitMask; // TODO(b/112012161) Set up shared fences. - ALOGD("%s: id=%d, buffer_state=%" PRIx32 ".", __FUNCTION__, mId, + ALOGD("%s: id=%d, mBufferState=%" PRIx32 ".", __FUNCTION__, mId, mBufferState->load(std::memory_order_acquire)); return 0; } @@ -336,12 +335,12 @@ sp BufferHubBuffer::duplicate() { hidl_handle token; BufferHubStatus ret; - IBufferClient::duplicate_cb dup_cb = [&](const auto& outToken, const auto& status) { + IBufferClient::duplicate_cb dupCb = [&](const auto& outToken, const auto& status) { token = std::move(outToken); ret = status; }; - if (!mBufferClient->duplicate(dup_cb).isOk()) { + if (!mBufferClient->duplicate(dupCb).isOk()) { ALOGE("%s: duplicate transaction failed!", __FUNCTION__); return nullptr; } else if (ret != BufferHubStatus::NO_ERROR) { diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h index 5c0903251a..5ba189c2c3 100644 --- a/libs/ui/include/ui/BufferHubBuffer.h +++ b/libs/ui/include/ui/BufferHubBuffer.h @@ -57,7 +57,7 @@ public: const BufferHubEventFd& eventFd() const { return mEventFd; } - // Returns the current value of MetadataHeader::buffer_state. + // Returns the current value of MetadataHeader::bufferState. uint32_t bufferState() const { return mBufferState->load(std::memory_order_acquire); } // A state mask which is unique to a buffer hub client among all its siblings sharing the same diff --git a/libs/ui/include/ui/BufferHubDefs.h b/libs/ui/include/ui/BufferHubDefs.h index 722a060597..10f274f569 100644 --- a/libs/ui/include/ui/BufferHubDefs.h +++ b/libs/ui/include/ui/BufferHubDefs.h @@ -32,7 +32,7 @@ namespace BufferHubDefs { // Single buffer clients (up to 16) ownership signal. // 32-bit atomic unsigned int. // Each client takes 2 bits. The first bit locates in the first 16 bits of -// buffer_state; the second bit locates in the last 16 bits of buffer_state. +// bufferState; the second bit locates in the last 16 bits of bufferState. // Client states: // Gained state 11. Exclusive write state. // Posted state 10. @@ -64,9 +64,9 @@ static constexpr uint32_t kFirstClientBitMask = (1U << kMaxNumberOfClients) + 1U // Returns true if any of the client is in gained state. static inline bool isAnyClientGained(uint32_t state) { - uint32_t high_bits = state >> kMaxNumberOfClients; - uint32_t low_bits = state & kLowbitsMask; - return high_bits == low_bits && low_bits != 0U; + uint32_t highBits = state >> kMaxNumberOfClients; + uint32_t lowBits = state & kLowbitsMask; + return highBits == lowBits && lowBits != 0U; } // Returns true if the input client is in gained state. @@ -76,34 +76,34 @@ static inline bool isClientGained(uint32_t state, uint32_t client_bit_mask) { // Returns true if any of the client is in posted state. static inline bool isAnyClientPosted(uint32_t state) { - uint32_t high_bits = state >> kMaxNumberOfClients; - uint32_t low_bits = state & kLowbitsMask; - uint32_t posted_or_acquired = high_bits ^ low_bits; - return posted_or_acquired & high_bits; + uint32_t highBits = state >> kMaxNumberOfClients; + uint32_t lowBits = state & kLowbitsMask; + uint32_t postedOrAcquired = highBits ^ lowBits; + return postedOrAcquired & highBits; } // Returns true if the input client is in posted state. static inline bool isClientPosted(uint32_t state, uint32_t client_bit_mask) { - uint32_t client_bits = state & client_bit_mask; - if (client_bits == 0U) return false; - uint32_t low_bits = client_bits & kLowbitsMask; - return low_bits == 0U; + uint32_t clientBits = state & client_bit_mask; + if (clientBits == 0U) return false; + uint32_t lowBits = clientBits & kLowbitsMask; + return lowBits == 0U; } // Return true if any of the client is in acquired state. static inline bool isAnyClientAcquired(uint32_t state) { - uint32_t high_bits = state >> kMaxNumberOfClients; - uint32_t low_bits = state & kLowbitsMask; - uint32_t posted_or_acquired = high_bits ^ low_bits; - return posted_or_acquired & low_bits; + uint32_t highBits = state >> kMaxNumberOfClients; + uint32_t lowBits = state & kLowbitsMask; + uint32_t postedOrAcquired = highBits ^ lowBits; + return postedOrAcquired & lowBits; } // Return true if the input client is in acquired state. static inline bool isClientAcquired(uint32_t state, uint32_t client_bit_mask) { - uint32_t client_bits = state & client_bit_mask; - if (client_bits == 0U) return false; - uint32_t high_bits = client_bits & kHighBitsMask; - return high_bits == 0U; + uint32_t clientBits = state & client_bit_mask; + if (clientBits == 0U) return false; + uint32_t highBits = clientBits & kHighBitsMask; + return highBits == 0U; } // Returns true if the input client is in released state. @@ -114,12 +114,12 @@ static inline bool isClientReleased(uint32_t state, uint32_t client_bit_mask) { // Returns the next available buffer client's client_state_masks. // @params union_bits. Union of all existing clients' client_state_masks. static inline uint32_t findNextAvailableClientStateMask(uint32_t union_bits) { - uint32_t low_union = union_bits & kLowbitsMask; - if (low_union == kLowbitsMask) return 0U; - uint32_t incremented = low_union + 1U; - uint32_t difference = incremented ^ low_union; - uint32_t new_low_bit = (difference + 1U) >> 1; - return new_low_bit + (new_low_bit << kMaxNumberOfClients); + uint32_t lowUnion = union_bits & kLowbitsMask; + if (lowUnion == kLowbitsMask) return 0U; + uint32_t incremented = lowUnion + 1U; + uint32_t difference = incremented ^ lowUnion; + uint32_t newLowBit = (difference + 1U) >> 1; + return newLowBit + (newLowBit << kMaxNumberOfClients); } struct __attribute__((aligned(8))) MetadataHeader { @@ -129,22 +129,22 @@ struct __attribute__((aligned(8))) MetadataHeader { // platform (include Apps and vendor HAL). // Every client takes up one bit from the higher 32 bits and one bit from the lower 32 bits in - // buffer_state. - std::atomic buffer_state; + // bufferState. + std::atomic bufferState; - // Every client takes up one bit in fence_state. Only the lower 32 bits are valid. The upper 32 + // Every client takes up one bit in fenceState. Only the lower 32 bits are valid. The upper 32 // bits are there for easier manipulation, but the value should be ignored. - std::atomic fence_state; + std::atomic fenceState; // Every client takes up one bit from the higher 32 bits and one bit from the lower 32 bits in - // active_clients_bit_mask. - std::atomic active_clients_bit_mask; + // activeClientsBitMask. + std::atomic activeClientsBitMask; // Explicit padding 4 bytes. uint32_t padding; // The index of the buffer queue where the buffer belongs to. - uint64_t queue_index; + uint64_t queueIndex; // Public data format, which should be updated with caution. See more details in dvr_api.h DvrNativeBufferMetadata metadata; diff --git a/libs/ui/tests/BufferHubMetadata_test.cpp b/libs/ui/tests/BufferHubMetadata_test.cpp index f02c4fc178..eb978cabc6 100644 --- a/libs/ui/tests/BufferHubMetadata_test.cpp +++ b/libs/ui/tests/BufferHubMetadata_test.cpp @@ -51,7 +51,7 @@ TEST_F(BufferHubMetadataTest, Import_Success) { // Check if the newly allocated buffer is initialized in released state (i.e. // state equals to 0U). - EXPECT_TRUE(mh1->buffer_state.load() == 0U); + EXPECT_TRUE(mh1->bufferState.load() == 0U); EXPECT_TRUE(m2.isValid()); BufferHubDefs::MetadataHeader* mh2 = m2.metadataHeader(); @@ -59,7 +59,7 @@ TEST_F(BufferHubMetadataTest, Import_Success) { // Check if the newly allocated buffer is initialized in released state (i.e. // state equals to 0U). - EXPECT_TRUE(mh2->buffer_state.load() == 0U); + EXPECT_TRUE(mh2->bufferState.load() == 0U); } TEST_F(BufferHubMetadataTest, MoveMetadataInvalidatesOldOne) { diff --git a/libs/vr/libbufferhub/buffer_hub_base.cpp b/libs/vr/libbufferhub/buffer_hub_base.cpp index b28d10147b..17930b4405 100644 --- a/libs/vr/libbufferhub/buffer_hub_base.cpp +++ b/libs/vr/libbufferhub/buffer_hub_base.cpp @@ -122,15 +122,15 @@ int BufferHubBase::ImportBuffer() { // are mapped from shared memory as an atomic object. The std::atomic's // constructor will not be called so that the original value stored in the // memory region will be preserved. - buffer_state_ = &metadata_header_->buffer_state; + buffer_state_ = &metadata_header_->bufferState; ALOGD_IF(TRACE, "BufferHubBase::ImportBuffer: id=%d, buffer_state=%" PRIx32 ".", id(), buffer_state_->load(std::memory_order_acquire)); - fence_state_ = &metadata_header_->fence_state; + fence_state_ = &metadata_header_->fenceState; ALOGD_IF(TRACE, "BufferHubBase::ImportBuffer: id=%d, fence_state=%" PRIx32 ".", id(), fence_state_->load(std::memory_order_acquire)); - active_clients_bit_mask_ = &metadata_header_->active_clients_bit_mask; + active_clients_bit_mask_ = &metadata_header_->activeClientsBitMask; ALOGD_IF( TRACE, "BufferHubBase::ImportBuffer: id=%d, active_clients_bit_mask=%" PRIx32 diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h index fa39d081e1..8a490d9983 100644 --- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h +++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h @@ -97,8 +97,8 @@ class BufferHubBase : public pdx::Client { uint32_t usage() const { return buffer_.usage(); } uint32_t layer_count() const { return buffer_.layer_count(); } - uint64_t GetQueueIndex() const { return metadata_header_->queue_index; } - void SetQueueIndex(uint64_t index) { metadata_header_->queue_index = index; } + uint64_t GetQueueIndex() const { return metadata_header_->queueIndex; } + void SetQueueIndex(uint64_t index) { metadata_header_->queueIndex = index; } protected: explicit BufferHubBase(LocalChannelHandle channel); diff --git a/services/bufferhub/BufferHubService.cpp b/services/bufferhub/BufferHubService.cpp index ade08e7d17..7a3472fa7e 100644 --- a/services/bufferhub/BufferHubService.cpp +++ b/services/bufferhub/BufferHubService.cpp @@ -241,8 +241,8 @@ Return BufferHubService::debug(const hidl_handle& fd, const hidl_vec(&node->metadata())->metadataHeader(); - const uint32_t state = metadataHeader->buffer_state.load(std::memory_order_acquire); - const uint64_t index = metadataHeader->queue_index; + const uint32_t state = metadataHeader->bufferState.load(std::memory_order_acquire); + const uint64_t index = metadataHeader->queueIndex; stream << std::right; stream << std::setw(6) << /*Id=*/node->id(); diff --git a/services/bufferhub/BufferNode.cpp b/services/bufferhub/BufferNode.cpp index 1efb27ee0a..04ca6493f7 100644 --- a/services/bufferhub/BufferNode.cpp +++ b/services/bufferhub/BufferNode.cpp @@ -15,9 +15,9 @@ void BufferNode::initializeMetadata() { // Using placement new here to reuse shared memory instead of new allocation // Initialize the atomic variables to zero. BufferHubDefs::MetadataHeader* metadataHeader = mMetadata.metadataHeader(); - mBufferState = new (&metadataHeader->buffer_state) std::atomic(0); - mFenceState = new (&metadataHeader->fence_state) std::atomic(0); - mActiveClientsBitMask = new (&metadataHeader->active_clients_bit_mask) std::atomic(0); + mBufferState = new (&metadataHeader->bufferState) std::atomic(0); + mFenceState = new (&metadataHeader->fenceState) std::atomic(0); + mActiveClientsBitMask = new (&metadataHeader->activeClientsBitMask) std::atomic(0); // The C++ standard recommends (but does not require) that lock-free atomic operations are // also address-free, that is, suitable for communication between processes using shared // memory. diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp index f3e54a0ed2..b49d89470f 100644 --- a/services/vr/bufferhubd/producer_channel.cpp +++ b/services/vr/bufferhubd/producer_channel.cpp @@ -91,11 +91,10 @@ int ProducerChannel::InitializeBuffer() { // Using placement new here to reuse shared memory instead of new allocation // and also initialize the value to zero. - buffer_state_ = - new (&metadata_header_->buffer_state) std::atomic(0); - fence_state_ = new (&metadata_header_->fence_state) std::atomic(0); + buffer_state_ = new (&metadata_header_->bufferState) std::atomic(0); + fence_state_ = new (&metadata_header_->fenceState) std::atomic(0); active_clients_bit_mask_ = - new (&metadata_header_->active_clients_bit_mask) std::atomic(0); + new (&metadata_header_->activeClientsBitMask) std::atomic(0); // Producer channel is never created after consumer channel, and one buffer // only have one fixed producer for now. Thus, it is correct to assume @@ -183,7 +182,7 @@ BufferHubChannel::BufferInfo ProducerChannel::GetBufferInfo() const { buffer_.height(), buffer_.layer_count(), buffer_.format(), buffer_.usage(), buffer_state_->load(std::memory_order_acquire), - signaled_mask, metadata_header_->queue_index); + signaled_mask, metadata_header_->queueIndex); } void ProducerChannel::HandleImpulse(Message& message) { @@ -547,7 +546,7 @@ Status ProducerChannel::OnConsumerRelease(Message&, "%s: orphaned buffer detected during the this acquire/release cycle: " "id=%d orphaned=0x%" PRIx32 " queue_index=%" PRId64 ".", __FUNCTION__, buffer_id(), orphaned_consumer_bit_mask_, - metadata_header_->queue_index); + metadata_header_->queueIndex); orphaned_consumer_bit_mask_ = 0; } } @@ -581,7 +580,7 @@ void ProducerChannel::OnConsumerOrphaned(const uint32_t& consumer_state_mask) { "consumer_state_mask=%" PRIx32 " queue_index=%" PRId64 " buffer_state=%" PRIx32 " fence_state=%" PRIx32 ".", __FUNCTION__, buffer_id(), consumer_state_mask, - metadata_header_->queue_index, + metadata_header_->queueIndex, buffer_state_->load(std::memory_order_acquire), fence_state_->load(std::memory_order_acquire)); } -- cgit v1.2.3-59-g8ed1b