diff options
| -rw-r--r-- | services/bufferhub/Android.bp | 1 | ||||
| -rw-r--r-- | services/bufferhub/BufferHubService.cpp | 2 | ||||
| -rw-r--r-- | services/bufferhub/BufferNode.cpp | 15 | ||||
| -rw-r--r-- | services/bufferhub/UniqueIdGenerator.cpp | 55 | ||||
| -rw-r--r-- | services/bufferhub/include/bufferhub/BufferHubService.h | 3 | ||||
| -rw-r--r-- | services/bufferhub/include/bufferhub/BufferNode.h | 11 | ||||
| -rw-r--r-- | services/bufferhub/include/bufferhub/UniqueIdGenerator.h | 57 | ||||
| -rw-r--r-- | services/bufferhub/tests/Android.bp | 16 | ||||
| -rw-r--r-- | services/bufferhub/tests/UniqueIdGenerator_test.cpp | 45 |
9 files changed, 201 insertions, 4 deletions
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<void> BufferHubService::allocateBuffer(const HardwareBufferDescription& d std::shared_ptr<BufferNode> node = std::make_shared<BufferNode>(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 <errno.h> +#include <bufferhub/BufferHubService.h> #include <bufferhub/BufferNode.h> #include <private/dvr/buffer_hub_defs.h> #include <ui/GraphicBufferAllocator.h> @@ -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 <bufferhub/UniqueIdGenerator.h> + +namespace android { +namespace frameworks { +namespace bufferhub { +namespace V1_0 { +namespace implementation { + +constexpr uint32_t UniqueIdGenerator::kInvalidId; + +uint32_t UniqueIdGenerator::getId() { + std::lock_guard<std::mutex> lock(mIdsInUseMutex); + + do { + if (++mLastId >= std::numeric_limits<uint32_t>::max()) { + mLastId = kInvalidId + 1; + } + } while (mIdsInUse.find(mLastId) != mIdsInUse.end()); + + mIdsInUse.insert(mLastId); + return mLastId; +} + +bool UniqueIdGenerator::freeId(uint32_t id) { + std::lock_guard<std::mutex> 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 <android/frameworks/bufferhub/1.0/IBufferHub.h> #include <bufferhub/BufferClient.h> +#include <bufferhub/UniqueIdGenerator.h> #include <utils/Mutex.h> 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<void> 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 <android/hardware_buffer.h> +#include <bufferhub/UniqueIdGenerator.h> #include <ui/BufferHubMetadata.h> 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<uint32_t>::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 <mutex> +#include <set> + +#include <utils/Mutex.h> + +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<uint32_t>::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<uint32_t> 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 <bufferhub/UniqueIdGenerator.h> +#include <gtest/gtest.h> + +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 |