From 8f51ec64984c1f03cdb6edef95b1e4098939da45 Mon Sep 17 00:00:00 2001 From: Jiwen 'Steve' Cai Date: Tue, 7 Aug 2018 21:50:51 -0700 Subject: Move detached buffer to libui Move it into libui early so that new modifications towards it can be coded in the libui style This CL only moves the file and updates Android.bp with proper dependencies. Will re-format the coding style in a follow up CL. The reason behind this is to keep this CL small enough so that the "git mv" operation will be considered as an renaming rather than a complete rewrite. Note that DetachedBuffer is not exposed to VNDK, so that we won't need to worry about ABI compatibility. Also, it temporarily introduces some clang warning exceptions, we should be able to remove them very soon once pdx to binder refactor is done for detached buffer. Bug: 112010261 Test: atest BufferHubMetadata_test Change-Id: I63659b9a9b7cb56f30fc2ae8cc5b87977d79b59c --- libs/ui/BufferHubMetadata.cpp | 111 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 libs/ui/BufferHubMetadata.cpp (limited to 'libs/ui/BufferHubMetadata.cpp') diff --git a/libs/ui/BufferHubMetadata.cpp b/libs/ui/BufferHubMetadata.cpp new file mode 100644 index 0000000000..0e5fce0c46 --- /dev/null +++ b/libs/ui/BufferHubMetadata.cpp @@ -0,0 +1,111 @@ +/* + * 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 +#include + +#include +#include +#include + +namespace android { +namespace dvr { + +namespace { + +static const int kAshmemProt = PROT_READ | PROT_WRITE; + +} // namespace + +using BufferHubDefs::kMetadataHeaderSize; +using BufferHubDefs::MetadataHeader; + +/* static */ +BufferHubMetadata BufferHubMetadata::Create(size_t user_metadata_size) { + // The size the of metadata buffer is used as the "width" parameter during + // allocation. Thus it cannot overflow uint32_t. + if (user_metadata_size >= + (std::numeric_limits::max() - kMetadataHeaderSize)) { + ALOGE("BufferHubMetadata::Create: metadata size too big: %zu.", + user_metadata_size); + return {}; + } + + const size_t metadata_size = user_metadata_size + kMetadataHeaderSize; + int fd = ashmem_create_region(/*name=*/"BufferHubMetadata", metadata_size); + if (fd < 0) { + ALOGE("BufferHubMetadata::Create: failed to create ashmem region."); + return {}; + } + + // Hand over the ownership of the fd to a pdx::LocalHandle immediately after + // the successful return of ashmem_create_region. The ashmem_handle is going + // to own the fd and to prevent fd leaks during error handling. + pdx::LocalHandle ashmem_handle{fd}; + + if (ashmem_set_prot_region(ashmem_handle.Get(), kAshmemProt) != 0) { + ALOGE("BufferHubMetadata::Create: failed to set protect region."); + return {}; + } + + return BufferHubMetadata::Import(std::move(ashmem_handle)); +} + +/* static */ +BufferHubMetadata BufferHubMetadata::Import(pdx::LocalHandle ashmem_handle) { + if (!ashmem_valid(ashmem_handle.Get())) { + ALOGE("BufferHubMetadata::Import: invalid ashmem fd."); + return {}; + } + + size_t metadata_size = static_cast(ashmem_get_size_region(ashmem_handle.Get())); + size_t user_metadata_size = metadata_size - kMetadataHeaderSize; + + // Note that here the buffer state is 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 can be preserved. + auto metadata_header = static_cast( + mmap(nullptr, metadata_size, kAshmemProt, MAP_SHARED, ashmem_handle.Get(), + /*offset=*/0)); + if (metadata_header == nullptr) { + ALOGE("BufferHubMetadata::Import: failed to map region."); + return {}; + } + + return BufferHubMetadata(user_metadata_size, std::move(ashmem_handle), + metadata_header); +} + +BufferHubMetadata::BufferHubMetadata(size_t user_metadata_size, + pdx::LocalHandle ashmem_handle, + MetadataHeader* metadata_header) + : user_metadata_size_(user_metadata_size), + ashmem_handle_(std::move(ashmem_handle)), + metadata_header_(metadata_header) {} + +BufferHubMetadata::~BufferHubMetadata() { + if (metadata_header_ != nullptr) { + int ret = munmap(metadata_header_, metadata_size()); + ALOGE_IF(ret != 0, + "BufferHubMetadata::~BufferHubMetadata: failed to unmap ashmem, " + "error=%d.", + errno); + metadata_header_ = nullptr; + } +} + +} // namespace dvr +} // namespace android -- cgit v1.2.3-59-g8ed1b From ff675b716822808a93e1f445754cddc4458f1249 Mon Sep 17 00:00:00 2001 From: Jiwen 'Steve' Cai Date: Tue, 9 Oct 2018 18:08:29 -0700 Subject: Format BufferHub{Buffer,Metadata}.{h,cpp} to match libui style 1/ Run clang-format 2/ Rename variables to use camel case 3/ 100 column per line 4/ Rename DetachedBuffer to BufferHubBuffer Bug: 68273829 Bug: 112010261 Bug: 112940221 Test: BufferHubMetadata_test, buffer_hub-test Change-Id: Iaf0dbbe797daf8b61864dc2e3ea1693703ef9d7f --- libs/gui/BufferHubProducer.cpp | 6 +- libs/ui/BufferHubBuffer.cpp | 252 +++++++++++++++---------------- libs/ui/BufferHubMetadata.cpp | 128 ++++++++-------- libs/ui/include/ui/BufferHubBuffer.h | 204 +++++++++++++------------ libs/ui/include/ui/BufferHubMetadata.h | 114 +++++++------- libs/vr/libbufferhub/buffer_hub-test.cpp | 44 +++--- 6 files changed, 372 insertions(+), 376 deletions(-) (limited to 'libs/ui/BufferHubMetadata.cpp') diff --git a/libs/gui/BufferHubProducer.cpp b/libs/gui/BufferHubProducer.cpp index 0623a8ee11..3dac037880 100644 --- a/libs/gui/BufferHubProducer.cpp +++ b/libs/gui/BufferHubProducer.cpp @@ -400,14 +400,14 @@ status_t BufferHubProducer::attachBuffer(int* out_slot, const sp& ALOGE("attachBuffer: DetachedBufferHandle cannot be NULL."); return BAD_VALUE; } - auto detached_buffer = DetachedBuffer::Import(std::move(detached_handle->handle())); + auto detached_buffer = BufferHubBuffer::Import(std::move(detached_handle->handle())); if (detached_buffer == nullptr) { - ALOGE("attachBuffer: DetachedBuffer cannot be NULL."); + ALOGE("attachBuffer: BufferHubBuffer cannot be NULL."); return BAD_VALUE; } auto status_or_handle = detached_buffer->Promote(); if (!status_or_handle.ok()) { - ALOGE("attachBuffer: Failed to promote a DetachedBuffer into a BufferProducer, error=%d.", + ALOGE("attachBuffer: Failed to promote a BufferHubBuffer into a BufferProducer, error=%d.", status_or_handle.error()); return BAD_VALUE; } diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index 9293711d0e..606386c740 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -45,6 +45,8 @@ using android::dvr::BufferHubMetadata; using android::dvr::BufferTraits; using android::dvr::DetachedBufferRPC; using android::dvr::NativeHandleWrapper; + +// TODO(b/112338294): Remove PDX dependencies from libui. using android::pdx::LocalChannelHandle; using android::pdx::LocalHandle; using android::pdx::Status; @@ -59,13 +61,12 @@ namespace { // to use Binder. static constexpr char kBufferHubClientPath[] = "system/buffer_hub/client"; -} // namespace +} // namespace -BufferHubClient::BufferHubClient() - : Client(ClientChannelFactory::Create(kBufferHubClientPath)) {} +BufferHubClient::BufferHubClient() : Client(ClientChannelFactory::Create(kBufferHubClientPath)) {} -BufferHubClient::BufferHubClient(LocalChannelHandle channel_handle) - : Client(ClientChannel::Create(std::move(channel_handle))) {} +BufferHubClient::BufferHubClient(LocalChannelHandle mChannelHandle) + : Client(ClientChannel::Create(std::move(mChannelHandle))) {} BufferHubClient::~BufferHubClient() {} @@ -74,142 +75,137 @@ bool BufferHubClient::IsValid() const { } LocalChannelHandle BufferHubClient::TakeChannelHandle() { - if (IsConnected()) { - return std::move(GetChannelHandle()); - } else { - return {}; - } + if (IsConnected()) { + return std::move(GetChannelHandle()); + } else { + return {}; + } } -DetachedBuffer::DetachedBuffer(uint32_t width, uint32_t height, - uint32_t layer_count, uint32_t format, - uint64_t usage, size_t user_metadata_size) { - ATRACE_NAME("DetachedBuffer::DetachedBuffer"); - ALOGD("DetachedBuffer::DetachedBuffer: width=%u height=%u layer_count=%u, format=%u " - "usage=%" PRIx64 " user_metadata_size=%zu", - width, height, layer_count, format, usage, user_metadata_size); - - auto status = client_.InvokeRemoteMethod( - width, height, layer_count, format, usage, user_metadata_size); - if (!status) { - ALOGE( - "DetachedBuffer::DetachedBuffer: Failed to create detached buffer: %s", - status.GetErrorMessage().c_str()); - client_.Close(-status.error()); - } - - const int ret = ImportGraphicBuffer(); - if (ret < 0) { - ALOGE("DetachedBuffer::DetachedBuffer: Failed to import buffer: %s", - strerror(-ret)); - client_.Close(ret); - } +BufferHubBuffer::BufferHubBuffer(uint32_t width, uint32_t height, uint32_t layerCount, + uint32_t format, uint64_t usage, size_t mUserMetadataSize) { + ATRACE_CALL(); + ALOGD("BufferHubBuffer::BufferHubBuffer: width=%u height=%u layerCount=%u, format=%u " + "usage=%" PRIx64 " mUserMetadataSize=%zu", + width, height, layerCount, format, usage, mUserMetadataSize); + + auto status = + mClient.InvokeRemoteMethod(width, height, layerCount, format, + usage, mUserMetadataSize); + if (!status) { + ALOGE("BufferHubBuffer::BufferHubBuffer: Failed to create detached buffer: %s", + status.GetErrorMessage().c_str()); + mClient.Close(-status.error()); + } + + const int ret = ImportGraphicBuffer(); + if (ret < 0) { + ALOGE("BufferHubBuffer::BufferHubBuffer: Failed to import buffer: %s", strerror(-ret)); + mClient.Close(ret); + } } -DetachedBuffer::DetachedBuffer(LocalChannelHandle channel_handle) - : client_(std::move(channel_handle)) { - const int ret = ImportGraphicBuffer(); - if (ret < 0) { - ALOGE("DetachedBuffer::DetachedBuffer: Failed to import buffer: %s", - strerror(-ret)); - client_.Close(ret); - } +BufferHubBuffer::BufferHubBuffer(LocalChannelHandle mChannelHandle) + : mClient(std::move(mChannelHandle)) { + const int ret = ImportGraphicBuffer(); + if (ret < 0) { + ALOGE("BufferHubBuffer::BufferHubBuffer: Failed to import buffer: %s", strerror(-ret)); + mClient.Close(ret); + } } -int DetachedBuffer::ImportGraphicBuffer() { - ATRACE_NAME("DetachedBuffer::ImportGraphicBuffer"); - - auto status = client_.InvokeRemoteMethod(); - if (!status) { - ALOGE("DetachedBuffer::DetachedBuffer: Failed to import GraphicBuffer: %s", - status.GetErrorMessage().c_str()); - return -status.error(); - } - - BufferTraits buffer_traits = status.take(); - if (buffer_traits.id() < 0) { - ALOGE("DetachedBuffer::DetachedBuffer: Received an invalid id!"); - return -EIO; - } - - // Stash the buffer id to replace the value in id_. - const int buffer_id = buffer_traits.id(); - - // Import the metadata. - metadata_ = BufferHubMetadata::Import(buffer_traits.take_metadata_handle()); - - if (!metadata_.IsValid()) { - ALOGE("DetachedBuffer::ImportGraphicBuffer: invalid metadata."); - return -ENOMEM; - } - - if (metadata_.metadata_size() != buffer_traits.metadata_size()) { - ALOGE( - "DetachedBuffer::ImportGraphicBuffer: metadata buffer too small: " - "%zu, expected: %" PRIu64 ".", - metadata_.metadata_size(), buffer_traits.metadata_size()); - return -ENOMEM; - } - - size_t metadata_buf_size = static_cast(buffer_traits.metadata_size()); - if (metadata_buf_size < dvr::BufferHubDefs::kMetadataHeaderSize) { - ALOGE("DetachedBuffer::ImportGraphicBuffer: metadata too small: %zu", - metadata_buf_size); - return -EINVAL; - } - - // Import the buffer: We only need to hold on the native_handle_t here so that - // GraphicBuffer instance can be created in future. - buffer_handle_ = buffer_traits.take_buffer_handle(); - - // If all imports succeed, replace the previous buffer and id. - id_ = buffer_id; - buffer_state_bit_ = buffer_traits.buffer_state_bit(); - - // TODO(b/112012161) Set up shared fences. - ALOGD("DetachedBuffer::ImportGraphicBuffer: id=%d, buffer_state=%" PRIx64 ".", id(), - metadata_.metadata_header()->buffer_state.load(std::memory_order_acquire)); - return 0; +int BufferHubBuffer::ImportGraphicBuffer() { + ATRACE_CALL(); + + auto status = mClient.InvokeRemoteMethod(); + if (!status) { + ALOGE("BufferHubBuffer::BufferHubBuffer: Failed to import GraphicBuffer: %s", + status.GetErrorMessage().c_str()); + return -status.error(); + } + + BufferTraits bufferTraits = status.take(); + if (bufferTraits.id() < 0) { + ALOGE("BufferHubBuffer::BufferHubBuffer: Received an invalid id!"); + return -EIO; + } + + // Stash the buffer id to replace the value in mId. + const int bufferId = bufferTraits.id(); + + // Import the metadata. + mMetadata = BufferHubMetadata::Import(bufferTraits.take_metadata_handle()); + + if (!mMetadata.IsValid()) { + ALOGE("BufferHubBuffer::ImportGraphicBuffer: invalid metadata."); + return -ENOMEM; + } + + if (mMetadata.metadata_size() != bufferTraits.metadata_size()) { + ALOGE("BufferHubBuffer::ImportGraphicBuffer: metadata buffer too small: " + "%zu, expected: %" PRIu64 ".", + mMetadata.metadata_size(), bufferTraits.metadata_size()); + return -ENOMEM; + } + + size_t metadataSize = static_cast(bufferTraits.metadata_size()); + if (metadataSize < dvr::BufferHubDefs::kMetadataHeaderSize) { + ALOGE("BufferHubBuffer::ImportGraphicBuffer: metadata too small: %zu", metadataSize); + return -EINVAL; + } + + // Import the buffer: We only need to hold on the native_handle_t here so that + // GraphicBuffer instance can be created in future. + mBufferHandle = bufferTraits.take_buffer_handle(); + + // If all imports succeed, replace the previous buffer and id. + mId = bufferId; + mBfferStateBit = bufferTraits.buffer_state_bit(); + + // TODO(b/112012161) Set up shared fences. + ALOGD("BufferHubBuffer::ImportGraphicBuffer: id=%d, buffer_state=%" PRIx64 ".", id(), + mMetadata.metadata_header()->buffer_state.load(std::memory_order_acquire)); + return 0; } -int DetachedBuffer::Poll(int timeout_ms) { - ATRACE_NAME("DetachedBuffer::Poll"); - pollfd p = {client_.event_fd(), POLLIN, 0}; - return poll(&p, 1, timeout_ms); +int BufferHubBuffer::Poll(int timeoutMs) { + ATRACE_CALL(); + + pollfd p = {mClient.event_fd(), POLLIN, 0}; + return poll(&p, 1, timeoutMs); } -Status DetachedBuffer::Promote() { - // TODO(b/112338294) remove after migrate producer buffer to binder - ALOGW("DetachedBuffer::Promote: not supported operation during migration"); - return {}; - - ATRACE_NAME("DetachedBuffer::Promote"); - ALOGD("DetachedBuffer::Promote: id=%d.", id_); - - auto status_or_handle = - client_.InvokeRemoteMethod(); - if (status_or_handle.ok()) { - // Invalidate the buffer. - buffer_handle_ = {}; - } else { - ALOGE("DetachedBuffer::Promote: Failed to promote buffer (id=%d): %s.", id_, - status_or_handle.GetErrorMessage().c_str()); - } - return status_or_handle; +Status BufferHubBuffer::Promote() { + ATRACE_CALL(); + + // TODO(b/112338294) remove after migrate producer buffer to binder + ALOGW("BufferHubBuffer::Promote: not supported operation during migration"); + return {}; + + ALOGD("BufferHubBuffer::Promote: id=%d.", mId); + + auto statusOrHandle = mClient.InvokeRemoteMethod(); + if (statusOrHandle.ok()) { + // Invalidate the buffer. + mBufferHandle = {}; + } else { + ALOGE("BufferHubBuffer::Promote: Failed to promote buffer (id=%d): %s.", mId, + statusOrHandle.GetErrorMessage().c_str()); + } + return statusOrHandle; } -Status DetachedBuffer::Duplicate() { - ATRACE_NAME("DetachedBuffer::Duplicate"); - ALOGD("DetachedBuffer::Duplicate: id=%d.", id_); +Status BufferHubBuffer::Duplicate() { + ATRACE_CALL(); + ALOGD("BufferHubBuffer::Duplicate: id=%d.", mId); - auto status_or_handle = - client_.InvokeRemoteMethod(); + auto statusOrHandle = mClient.InvokeRemoteMethod(); - if (!status_or_handle.ok()) { - ALOGE("DetachedBuffer::Duplicate: Failed to duplicate buffer (id=%d): %s.", - id_, status_or_handle.GetErrorMessage().c_str()); - } - return status_or_handle; + if (!statusOrHandle.ok()) { + ALOGE("BufferHubBuffer::Duplicate: Failed to duplicate buffer (id=%d): %s.", mId, + statusOrHandle.GetErrorMessage().c_str()); + } + return statusOrHandle; } -} // namespace android +} // namespace android diff --git a/libs/ui/BufferHubMetadata.cpp b/libs/ui/BufferHubMetadata.cpp index 0e5fce0c46..b0c451061d 100644 --- a/libs/ui/BufferHubMetadata.cpp +++ b/libs/ui/BufferHubMetadata.cpp @@ -28,84 +28,78 @@ namespace { static const int kAshmemProt = PROT_READ | PROT_WRITE; -} // namespace +} // namespace using BufferHubDefs::kMetadataHeaderSize; using BufferHubDefs::MetadataHeader; /* static */ -BufferHubMetadata BufferHubMetadata::Create(size_t user_metadata_size) { - // The size the of metadata buffer is used as the "width" parameter during - // allocation. Thus it cannot overflow uint32_t. - if (user_metadata_size >= - (std::numeric_limits::max() - kMetadataHeaderSize)) { - ALOGE("BufferHubMetadata::Create: metadata size too big: %zu.", - user_metadata_size); - return {}; - } - - const size_t metadata_size = user_metadata_size + kMetadataHeaderSize; - int fd = ashmem_create_region(/*name=*/"BufferHubMetadata", metadata_size); - if (fd < 0) { - ALOGE("BufferHubMetadata::Create: failed to create ashmem region."); - return {}; - } - - // Hand over the ownership of the fd to a pdx::LocalHandle immediately after - // the successful return of ashmem_create_region. The ashmem_handle is going - // to own the fd and to prevent fd leaks during error handling. - pdx::LocalHandle ashmem_handle{fd}; - - if (ashmem_set_prot_region(ashmem_handle.Get(), kAshmemProt) != 0) { - ALOGE("BufferHubMetadata::Create: failed to set protect region."); - return {}; - } - - return BufferHubMetadata::Import(std::move(ashmem_handle)); +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)) { + ALOGE("BufferHubMetadata::Create: metadata size too big: %zu.", userMetadataSize); + return {}; + } + + const size_t metadataSize = userMetadataSize + kMetadataHeaderSize; + int fd = ashmem_create_region(/*name=*/"BufferHubMetadata", metadataSize); + if (fd < 0) { + ALOGE("BufferHubMetadata::Create: failed to create ashmem region."); + return {}; + } + + // Hand over the ownership of the fd to a pdx::LocalHandle immediately after the successful + // return of ashmem_create_region. The ashmemHandle is going to own the fd and to prevent fd + // leaks during error handling. + pdx::LocalHandle ashmemHandle{fd}; + + if (ashmem_set_prot_region(ashmemHandle.Get(), kAshmemProt) != 0) { + ALOGE("BufferHubMetadata::Create: failed to set protect region."); + return {}; + } + + return BufferHubMetadata::Import(std::move(ashmemHandle)); } /* static */ -BufferHubMetadata BufferHubMetadata::Import(pdx::LocalHandle ashmem_handle) { - if (!ashmem_valid(ashmem_handle.Get())) { - ALOGE("BufferHubMetadata::Import: invalid ashmem fd."); - return {}; - } - - size_t metadata_size = static_cast(ashmem_get_size_region(ashmem_handle.Get())); - size_t user_metadata_size = metadata_size - kMetadataHeaderSize; - - // Note that here the buffer state is 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 can be preserved. - auto metadata_header = static_cast( - mmap(nullptr, metadata_size, kAshmemProt, MAP_SHARED, ashmem_handle.Get(), - /*offset=*/0)); - if (metadata_header == nullptr) { - ALOGE("BufferHubMetadata::Import: failed to map region."); - return {}; - } - - return BufferHubMetadata(user_metadata_size, std::move(ashmem_handle), - metadata_header); +BufferHubMetadata BufferHubMetadata::Import(pdx::LocalHandle ashmemHandle) { + if (!ashmem_valid(ashmemHandle.Get())) { + ALOGE("BufferHubMetadata::Import: invalid ashmem fd."); + return {}; + } + + size_t metadataSize = static_cast(ashmem_get_size_region(ashmemHandle.Get())); + size_t userMetadataSize = metadataSize - kMetadataHeaderSize; + + // Note that here the buffer state is 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 can be preserved. + auto metadataHeader = static_cast(mmap(nullptr, metadataSize, kAshmemProt, + MAP_SHARED, ashmemHandle.Get(), + /*offset=*/0)); + if (metadataHeader == nullptr) { + ALOGE("BufferHubMetadata::Import: failed to map region."); + return {}; + } + + return BufferHubMetadata(userMetadataSize, std::move(ashmemHandle), metadataHeader); } -BufferHubMetadata::BufferHubMetadata(size_t user_metadata_size, - pdx::LocalHandle ashmem_handle, - MetadataHeader* metadata_header) - : user_metadata_size_(user_metadata_size), - ashmem_handle_(std::move(ashmem_handle)), - metadata_header_(metadata_header) {} +BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle, + MetadataHeader* metadataHeader) + : mUserMetadataSize(userMetadataSize), + mAshmemHandle(std::move(ashmemHandle)), + mMetadataHeader(metadataHeader) {} BufferHubMetadata::~BufferHubMetadata() { - if (metadata_header_ != nullptr) { - int ret = munmap(metadata_header_, metadata_size()); - ALOGE_IF(ret != 0, - "BufferHubMetadata::~BufferHubMetadata: failed to unmap ashmem, " - "error=%d.", - errno); - metadata_header_ = nullptr; - } + if (mMetadataHeader != nullptr) { + int ret = munmap(mMetadataHeader, metadata_size()); + ALOGE_IF(ret != 0, + "BufferHubMetadata::~BufferHubMetadata: failed to unmap ashmem, error=%d.", errno); + mMetadataHeader = nullptr; + } } -} // namespace dvr -} // namespace android +} // namespace dvr +} // namespace android diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h index 8cf1a6d805..e8ae244549 100644 --- a/libs/ui/include/ui/BufferHubBuffer.h +++ b/libs/ui/include/ui/BufferHubBuffer.h @@ -1,5 +1,21 @@ -#ifndef ANDROID_DVR_DETACHED_BUFFER_H_ -#define ANDROID_DVR_DETACHED_BUFFER_H_ +/* + * 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_BUFFER_HUB_BUFFER_H_ +#define ANDROID_BUFFER_HUB_BUFFER_H_ // We would eliminate the clang warnings introduced by libdpx. // TODO(b/112338294): Remove those once BufferHub moved to use Binder @@ -20,7 +36,6 @@ #include #include #include -#include #pragma clang diagnostic pop #include @@ -28,114 +43,107 @@ namespace android { class BufferHubClient : public pdx::Client { - public: - BufferHubClient(); - virtual ~BufferHubClient(); - explicit BufferHubClient(pdx::LocalChannelHandle channel_handle); - - bool IsValid() const; - pdx::LocalChannelHandle TakeChannelHandle(); - - using pdx::Client::Close; - using pdx::Client::event_fd; - using pdx::Client::GetChannel; - using pdx::Client::InvokeRemoteMethod; +public: + BufferHubClient(); + virtual ~BufferHubClient(); + explicit BufferHubClient(pdx::LocalChannelHandle mChannelHandle); + + bool IsValid() const; + pdx::LocalChannelHandle TakeChannelHandle(); + + using pdx::Client::Close; + using pdx::Client::event_fd; + using pdx::Client::GetChannel; + using pdx::Client::InvokeRemoteMethod; }; -class DetachedBuffer { - public: - // Allocates a standalone DetachedBuffer not associated with any producer - // consumer set. - static std::unique_ptr Create(uint32_t width, uint32_t height, - uint32_t layer_count, - uint32_t format, uint64_t usage, - size_t user_metadata_size) { - return std::unique_ptr(new DetachedBuffer( - width, height, layer_count, format, usage, user_metadata_size)); - } - - // Imports the given channel handle to a DetachedBuffer, taking ownership. - static std::unique_ptr Import( - pdx::LocalChannelHandle channel_handle) { - return std::unique_ptr( - new DetachedBuffer(std::move(channel_handle))); - } - - DetachedBuffer(const DetachedBuffer&) = delete; - void operator=(const DetachedBuffer&) = delete; - - // Gets ID of the buffer client. All DetachedBuffer clients derived from the - // same buffer in bufferhubd share the same buffer id. - int id() const { return id_; } - - const native_handle_t* DuplicateHandle() { - return buffer_handle_.DuplicateHandle(); - } - - // Returns the current value of MetadataHeader::buffer_state. - uint64_t buffer_state() { - return metadata_.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 buffer_state_bit() const { return buffer_state_bit_; } - - size_t user_metadata_size() const { return metadata_.user_metadata_size(); } - - // Returns true if the buffer holds an open PDX channels towards bufferhubd. - bool IsConnected() const { return client_.IsValid(); } - - // Returns true if the buffer holds an valid native buffer handle that's - // availble for the client to read from and/or write into. - bool IsValid() const { return buffer_handle_.IsValid(); } - - // Returns the event mask for all the events that are pending on this buffer - // (see sys/poll.h for all possible bits). - pdx::Status GetEventMask(int events) { - if (auto* channel = client_.GetChannel()) { - return channel->GetEventMask(events); - } else { - return pdx::ErrorStatus(EINVAL); +class BufferHubBuffer { +public: + // Allocates a standalone BufferHubBuffer not associated with any producer consumer set. + static std::unique_ptr Create(uint32_t width, uint32_t height, + uint32_t layerCount, uint32_t format, + uint64_t usage, size_t mUserMetadataSize) { + return std::unique_ptr( + new BufferHubBuffer(width, height, layerCount, format, usage, mUserMetadataSize)); + } + + // Imports the given channel handle to a BufferHubBuffer, taking ownership. + static std::unique_ptr Import(pdx::LocalChannelHandle mChannelHandle) { + return std::unique_ptr(new BufferHubBuffer(std::move(mChannelHandle))); + } + + BufferHubBuffer(const BufferHubBuffer&) = delete; + void operator=(const BufferHubBuffer&) = delete; + + // Gets ID of the buffer client. All BufferHubBuffer clients derived from the same buffer in + // bufferhubd share the same buffer id. + int id() const { return mId; } + + const native_handle_t* DuplicateHandle() { return mBufferHandle.DuplicateHandle(); } + + // Returns the current value of MetadataHeader::buffer_state. + uint64_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 buffer_state_bit() const { return mBfferStateBit; } + + size_t user_metadata_size() const { return mMetadata.user_metadata_size(); } + + // Returns true if the buffer holds an open PDX channels towards bufferhubd. + bool IsConnected() const { return mClient.IsValid(); } + + // Returns true if the buffer holds an valid native buffer handle that's availble for the client + // to read from and/or write into. + bool IsValid() const { return mBufferHandle.IsValid(); } + + // Returns the event mask for all the events that are pending on this buffer (see sys/poll.h for + // all possible bits). + pdx::Status GetEventMask(int events) { + if (auto* channel = mClient.GetChannel()) { + return channel->GetEventMask(events); + } else { + return pdx::ErrorStatus(EINVAL); + } } - } - // Polls the fd for |timeout_ms| milliseconds (-1 for infinity). - int Poll(int timeout_ms); + // Polls the fd for |timeoutMs| milliseconds (-1 for infinity). + int Poll(int timeoutMs); - // Promotes a DetachedBuffer to become a ProducerBuffer. Once promoted the - // DetachedBuffer channel will be closed automatically on successful IPC - // return. Further IPCs towards this channel will return error. - pdx::Status Promote(); + // Promotes a BufferHubBuffer to become a ProducerBuffer. Once promoted the BufferHubBuffer + // channel will be closed automatically on successful IPC return. Further IPCs towards this + // channel will return error. + pdx::Status Promote(); - // Creates a DetachedBuffer client from an existing one. The new client will - // share the same underlying gralloc buffer and ashmem region for metadata. - pdx::Status Duplicate(); + // Creates a BufferHubBuffer client from an existing one. The new client will + // share the same underlying gralloc buffer and ashmem region for metadata. + pdx::Status Duplicate(); - private: - DetachedBuffer(uint32_t width, uint32_t height, uint32_t layer_count, - uint32_t format, uint64_t usage, size_t user_metadata_size); +private: + BufferHubBuffer(uint32_t width, uint32_t height, uint32_t layerCount, uint32_t format, + uint64_t usage, size_t mUserMetadataSize); - DetachedBuffer(pdx::LocalChannelHandle channel_handle); + BufferHubBuffer(pdx::LocalChannelHandle mChannelHandle); - int ImportGraphicBuffer(); + int ImportGraphicBuffer(); - // Global id for the buffer that is consistent across processes. - int id_; - uint64_t buffer_state_bit_; + // Global id for the buffer that is consistent across processes. + int mId; + uint64_t mBfferStateBit; - // Wrapps the gralloc buffer handle of this buffer. - dvr::NativeHandleWrapper buffer_handle_; + // Wrapps the gralloc buffer handle of this buffer. + dvr::NativeHandleWrapper mBufferHandle; - // An ashmem-based metadata object. The same shared memory are mapped to the - // bufferhubd daemon and all buffer clients. - dvr::BufferHubMetadata metadata_; + // An ashmem-based metadata object. The same shared memory are mapped to the + // bufferhubd daemon and all buffer clients. + dvr::BufferHubMetadata mMetadata; - // PDX backend. - BufferHubClient client_; + // PDX backend. + BufferHubClient mClient; }; -} // namespace android +} // namespace android -#endif // ANDROID_DVR_DETACHED_BUFFER_H_ +#endif // ANDROID_BUFFER_HUB_BUFFER_H_ diff --git a/libs/ui/include/ui/BufferHubMetadata.h b/libs/ui/include/ui/BufferHubMetadata.h index 52d156dbd5..46adc6a02e 100644 --- a/libs/ui/include/ui/BufferHubMetadata.h +++ b/libs/ui/include/ui/BufferHubMetadata.h @@ -14,8 +14,8 @@ * limitations under the License. */ -#ifndef ANDROID_DVR_BUFFER_HUB_METADATA_H_ -#define ANDROID_DVR_BUFFER_HUB_METADATA_H_ +#ifndef ANDROID_BUFFER_HUB_METADATA_H_ +#define ANDROID_BUFFER_HUB_METADATA_H_ // We would eliminate the clang warnings introduced by libdpx. // TODO(b/112338294): Remove those once BufferHub moved to use Binder @@ -41,70 +41,66 @@ namespace android { namespace dvr { class BufferHubMetadata { - public: - // Creates a new BufferHubMetadata backed by an ashmem region. - // - // @param user_metadata_size 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 user_metadata_size. - static BufferHubMetadata Create(size_t user_metadata_size); - - // Imports an existing BufferHubMetadata from an ashmem FD. - // - // TODO(b/112338294): Refactor BufferHub to use Binder as its internal IPC - // backend instead of UDS. - // - // @param ashmem_handle Ashmem file handle representing an ashmem region. - static BufferHubMetadata Import(pdx::LocalHandle ashmem_handle); - - BufferHubMetadata() = default; - - BufferHubMetadata(BufferHubMetadata&& other) { *this = std::move(other); } - - ~BufferHubMetadata(); - - BufferHubMetadata& operator=(BufferHubMetadata&& other) { - if (this != &other) { - user_metadata_size_ = other.user_metadata_size_; - other.user_metadata_size_ = 0; - - ashmem_handle_ = std::move(other.ashmem_handle_); - - // The old raw metadata_header_ pointer must be cleared, otherwise the - // destructor will automatically mummap() the shared memory. - metadata_header_ = other.metadata_header_; - other.metadata_header_ = nullptr; +public: + // Creates a new BufferHubMetadata backed by an ashmem region. + // + // @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); + + // Imports an existing BufferHubMetadata from an ashmem FD. + // + // TODO(b/112338294): Refactor BufferHub to use Binder as its internal IPC backend instead of + // UDS. + // + // @param ashmemHandle Ashmem file handle representing an ashmem region. + static BufferHubMetadata Import(pdx::LocalHandle ashmemHandle); + + BufferHubMetadata() = default; + + BufferHubMetadata(BufferHubMetadata&& other) { *this = std::move(other); } + + ~BufferHubMetadata(); + + BufferHubMetadata& operator=(BufferHubMetadata&& other) { + if (this != &other) { + mUserMetadataSize = other.mUserMetadataSize; + other.mUserMetadataSize = 0; + + mAshmemHandle = std::move(other.mAshmemHandle); + + // The old raw mMetadataHeader pointer must be cleared, otherwise the destructor will + // automatically mummap() the shared memory. + mMetadataHeader = other.mMetadataHeader; + other.mMetadataHeader = nullptr; + } + return *this; } - return *this; - } - // 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 ashmem_handle_.IsValid() && metadata_header_ != nullptr; - } + // 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 mAshmemHandle.IsValid() && mMetadataHeader != nullptr; } - size_t user_metadata_size() const { return user_metadata_size_; } - size_t metadata_size() const { - return user_metadata_size_ + BufferHubDefs::kMetadataHeaderSize; - } + size_t user_metadata_size() const { return mUserMetadataSize; } + size_t metadata_size() const { return mUserMetadataSize + BufferHubDefs::kMetadataHeaderSize; } - const pdx::LocalHandle& ashmem_handle() const { return ashmem_handle_; } - BufferHubDefs::MetadataHeader* metadata_header() { return metadata_header_; } + const pdx::LocalHandle& ashmem_handle() const { return mAshmemHandle; } + BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; } - private: - BufferHubMetadata(size_t user_metadata_size, pdx::LocalHandle ashmem_handle, - BufferHubDefs::MetadataHeader* metadata_header); +private: + BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle, + BufferHubDefs::MetadataHeader* metadataHeader); - BufferHubMetadata(const BufferHubMetadata&) = delete; - void operator=(const BufferHubMetadata&) = delete; + BufferHubMetadata(const BufferHubMetadata&) = delete; + void operator=(const BufferHubMetadata&) = delete; - size_t user_metadata_size_ = 0; - pdx::LocalHandle ashmem_handle_; - BufferHubDefs::MetadataHeader* metadata_header_ = nullptr; + size_t mUserMetadataSize = 0; + pdx::LocalHandle mAshmemHandle; + BufferHubDefs::MetadataHeader* mMetadataHeader = nullptr; }; -} // namespace dvr -} // namespace android +} // namespace dvr +} // namespace android -#endif // ANDROID_DVR_BUFFER_HUB_METADATA_H_ +#endif // ANDROID_BUFFER_HUB_METADATA_H_ diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp index 08c6bededc..b477f1a571 100644 --- a/libs/vr/libbufferhub/buffer_hub-test.cpp +++ b/libs/vr/libbufferhub/buffer_hub-test.cpp @@ -19,7 +19,7 @@ return result; \ })() -using android::DetachedBuffer; +using android::BufferHubBuffer; using android::GraphicBuffer; using android::sp; using android::dvr::ConsumerBuffer; @@ -784,8 +784,9 @@ TEST_F(LibBufferHubTest, TestDetachBufferFromProducer) { // is gone. EXPECT_EQ(s3.error(), EPIPE); - // Detached buffer handle can be use to construct a new DetachedBuffer object. - auto d = DetachedBuffer::Import(std::move(handle)); + // Detached buffer handle can be use to construct a new BufferHubBuffer + // object. + auto d = BufferHubBuffer::Import(std::move(handle)); EXPECT_FALSE(handle.valid()); EXPECT_TRUE(d->IsConnected()); EXPECT_TRUE(d->IsValid()); @@ -793,17 +794,17 @@ TEST_F(LibBufferHubTest, TestDetachBufferFromProducer) { EXPECT_EQ(d->id(), p_id); } -TEST_F(LibBufferHubTest, TestCreateDetachedBufferFails) { +TEST_F(LibBufferHubTest, TestCreateBufferHubBufferFails) { // Buffer Creation will fail: BLOB format requires height to be 1. - auto b1 = DetachedBuffer::Create(kWidth, /*height=2*/ 2, kLayerCount, - /*format=*/HAL_PIXEL_FORMAT_BLOB, kUsage, - kUserMetadataSize); + auto b1 = BufferHubBuffer::Create(kWidth, /*height=2*/ 2, kLayerCount, + /*format=*/HAL_PIXEL_FORMAT_BLOB, kUsage, + kUserMetadataSize); EXPECT_FALSE(b1->IsConnected()); EXPECT_FALSE(b1->IsValid()); // Buffer Creation will fail: user metadata size too large. - auto b2 = DetachedBuffer::Create( + auto b2 = BufferHubBuffer::Create( kWidth, kHeight, kLayerCount, kFormat, kUsage, /*user_metadata_size=*/std::numeric_limits::max()); @@ -811,7 +812,7 @@ TEST_F(LibBufferHubTest, TestCreateDetachedBufferFails) { EXPECT_FALSE(b2->IsValid()); // Buffer Creation will fail: user metadata size too large. - auto b3 = DetachedBuffer::Create( + auto b3 = BufferHubBuffer::Create( kWidth, kHeight, kLayerCount, kFormat, kUsage, /*user_metadata_size=*/std::numeric_limits::max() - kMetadataHeaderSize); @@ -820,20 +821,20 @@ TEST_F(LibBufferHubTest, TestCreateDetachedBufferFails) { EXPECT_FALSE(b3->IsValid()); } -TEST_F(LibBufferHubTest, TestCreateDetachedBuffer) { - auto b1 = DetachedBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, - kUsage, kUserMetadataSize); +TEST_F(LibBufferHubTest, TestCreateBufferHubBuffer) { + auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, + kUsage, kUserMetadataSize); EXPECT_TRUE(b1->IsConnected()); EXPECT_TRUE(b1->IsValid()); EXPECT_NE(b1->id(), 0); } -TEST_F(LibBufferHubTest, TestPromoteDetachedBuffer) { +TEST_F(LibBufferHubTest, TestPromoteBufferHubBuffer) { // TODO(b/112338294) rewrite test after migration return; - auto b1 = DetachedBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, - kUsage, kUserMetadataSize); + auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, + kUsage, kUserMetadataSize); int b1_id = b1->id(); EXPECT_TRUE(b1->IsValid()); @@ -879,8 +880,9 @@ TEST_F(LibBufferHubTest, TestDetachThenPromote) { LocalChannelHandle h1 = status_or_handle.take(); EXPECT_TRUE(h1.valid()); - // Detached buffer handle can be use to construct a new DetachedBuffer object. - auto b1 = DetachedBuffer::Import(std::move(h1)); + // Detached buffer handle can be use to construct a new BufferHubBuffer + // object. + auto b1 = BufferHubBuffer::Import(std::move(h1)); EXPECT_FALSE(h1.valid()); EXPECT_TRUE(b1->IsValid()); int b1_id = b1->id(); @@ -907,9 +909,9 @@ TEST_F(LibBufferHubTest, TestDetachThenPromote) { EXPECT_TRUE(IsBufferGained(p2->buffer_state())); } -TEST_F(LibBufferHubTest, TestDuplicateDetachedBuffer) { - auto b1 = DetachedBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, - kUsage, kUserMetadataSize); +TEST_F(LibBufferHubTest, TestDuplicateBufferHubBuffer) { + auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, + kUsage, kUserMetadataSize); int b1_id = b1->id(); EXPECT_TRUE(b1->IsValid()); EXPECT_EQ(b1->user_metadata_size(), kUserMetadataSize); @@ -925,7 +927,7 @@ TEST_F(LibBufferHubTest, TestDuplicateDetachedBuffer) { LocalChannelHandle h2 = status_or_handle.take(); EXPECT_TRUE(h2.valid()); - std::unique_ptr b2 = DetachedBuffer::Import(std::move(h2)); + std::unique_ptr b2 = BufferHubBuffer::Import(std::move(h2)); EXPECT_FALSE(h2.valid()); ASSERT_TRUE(b2 != nullptr); EXPECT_TRUE(b2->IsValid()); -- cgit v1.2.3-59-g8ed1b From 1850da231eafb185add6db88251343a6e6e333db Mon Sep 17 00:00:00 2001 From: Jiwen 'Steve' Cai Date: Mon, 15 Oct 2018 21:29:14 -0700 Subject: BufferHub{Buffer,Metadata}: Some other minor changes 1/ Move BufferHubMetadata out of android::dvr namespace 2/ Fix some misnamed variables Bug: 111976433 Test: BufferHubBuffer_test Change-Id: I06ec1c3cc4831c865b672680b49adb006d6e1835 --- libs/ui/BufferHubBuffer.cpp | 3 +-- libs/ui/BufferHubMetadata.cpp | 6 ++---- libs/ui/include/ui/BufferHubBuffer.h | 8 ++++---- libs/ui/include/ui/BufferHubMetadata.h | 12 ++++++------ 4 files changed, 13 insertions(+), 16 deletions(-) (limited to 'libs/ui/BufferHubMetadata.cpp') diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index 606386c740..2696c6c213 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -41,7 +41,6 @@ #include -using android::dvr::BufferHubMetadata; using android::dvr::BufferTraits; using android::dvr::DetachedBufferRPC; using android::dvr::NativeHandleWrapper; @@ -160,7 +159,7 @@ int BufferHubBuffer::ImportGraphicBuffer() { // If all imports succeed, replace the previous buffer and id. mId = bufferId; - mBfferStateBit = bufferTraits.buffer_state_bit(); + mBufferStateBit = bufferTraits.buffer_state_bit(); // TODO(b/112012161) Set up shared fences. ALOGD("BufferHubBuffer::ImportGraphicBuffer: id=%d, buffer_state=%" PRIx64 ".", id(), diff --git a/libs/ui/BufferHubMetadata.cpp b/libs/ui/BufferHubMetadata.cpp index b0c451061d..1e08ed1388 100644 --- a/libs/ui/BufferHubMetadata.cpp +++ b/libs/ui/BufferHubMetadata.cpp @@ -22,7 +22,6 @@ #include namespace android { -namespace dvr { namespace { @@ -30,8 +29,8 @@ static const int kAshmemProt = PROT_READ | PROT_WRITE; } // namespace -using BufferHubDefs::kMetadataHeaderSize; -using BufferHubDefs::MetadataHeader; +using dvr::BufferHubDefs::kMetadataHeaderSize; +using dvr::BufferHubDefs::MetadataHeader; /* static */ BufferHubMetadata BufferHubMetadata::Create(size_t userMetadataSize) { @@ -101,5 +100,4 @@ BufferHubMetadata::~BufferHubMetadata() { } } -} // namespace dvr } // namespace android diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h index e8ae244549..d33be8cf87 100644 --- a/libs/ui/include/ui/BufferHubBuffer.h +++ b/libs/ui/include/ui/BufferHubBuffer.h @@ -88,7 +88,7 @@ public: // A state mask which is unique to a buffer hub client among all its siblings sharing the same // concrete graphic buffer. - uint64_t buffer_state_bit() const { return mBfferStateBit; } + uint64_t buffer_state_bit() const { return mBufferStateBit; } size_t user_metadata_size() const { return mMetadata.user_metadata_size(); } @@ -130,15 +130,15 @@ private: int ImportGraphicBuffer(); // Global id for the buffer that is consistent across processes. - int mId; - uint64_t mBfferStateBit; + int mId = -1; + uint64_t mBufferStateBit = 0; // Wrapps the gralloc buffer handle of this buffer. dvr::NativeHandleWrapper mBufferHandle; // An ashmem-based metadata object. The same shared memory are mapped to the // bufferhubd daemon and all buffer clients. - dvr::BufferHubMetadata mMetadata; + BufferHubMetadata mMetadata; // PDX backend. BufferHubClient mClient; diff --git a/libs/ui/include/ui/BufferHubMetadata.h b/libs/ui/include/ui/BufferHubMetadata.h index 46adc6a02e..94a9000a32 100644 --- a/libs/ui/include/ui/BufferHubMetadata.h +++ b/libs/ui/include/ui/BufferHubMetadata.h @@ -38,7 +38,6 @@ #pragma clang diagnostic pop namespace android { -namespace dvr { class BufferHubMetadata { public: @@ -83,24 +82,25 @@ public: bool IsValid() const { return mAshmemHandle.IsValid() && mMetadataHeader != nullptr; } size_t user_metadata_size() const { return mUserMetadataSize; } - size_t metadata_size() const { return mUserMetadataSize + BufferHubDefs::kMetadataHeaderSize; } + size_t metadata_size() const { + return mUserMetadataSize + dvr::BufferHubDefs::kMetadataHeaderSize; + } const pdx::LocalHandle& ashmem_handle() const { return mAshmemHandle; } - BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; } + dvr::BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; } private: BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle, - BufferHubDefs::MetadataHeader* metadataHeader); + dvr::BufferHubDefs::MetadataHeader* metadataHeader); BufferHubMetadata(const BufferHubMetadata&) = delete; void operator=(const BufferHubMetadata&) = delete; size_t mUserMetadataSize = 0; pdx::LocalHandle mAshmemHandle; - BufferHubDefs::MetadataHeader* mMetadataHeader = nullptr; + dvr::BufferHubDefs::MetadataHeader* mMetadataHeader = nullptr; }; -} // namespace dvr } // namespace android #endif // ANDROID_BUFFER_HUB_METADATA_H_ -- cgit v1.2.3-59-g8ed1b From 069e8389df7aee7e8812a036bcda43c3fe24cabc Mon Sep 17 00:00:00 2001 From: Fan Xu Date: Fri, 16 Nov 2018 16:28:08 -0800 Subject: Move BufferHubMetadata off pdx::fileHandle Use android::base::unique_fd to replace LocalHandle. For BorrowedHandle, a const reference of the unique_fd is returned. Updated the test cases to fit in current behavior. Test: BufferHubMetadata_test, BufferHubBuffer_test, BufferNode_test, buffer_hub-test (passed) Fix: 118888072 Change-Id: I34de335ed9a10864ac226cd4d7d261ba0078045d --- libs/ui/BufferHubBuffer.cpp | 10 +++++++--- libs/ui/BufferHubMetadata.cpp | 24 ++++++++++++------------ libs/ui/include/ui/BufferHubMetadata.h | 24 +++++++++++++----------- libs/ui/tests/BufferHubMetadata_test.cpp | 22 ++++++++++------------ services/vr/bufferhubd/buffer_channel.cpp | 5 ++++- 5 files changed, 46 insertions(+), 39 deletions(-) (limited to 'libs/ui/BufferHubMetadata.cpp') diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index 8cc1a4e3b4..dd79775d01 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -36,10 +36,12 @@ #include #pragma clang diagnostic pop -#include - #include +#include +#include + +using android::base::unique_fd; using android::dvr::BufferTraits; using android::dvr::DetachedBufferRPC; using android::dvr::NativeHandleWrapper; @@ -132,7 +134,9 @@ int BufferHubBuffer::ImportGraphicBuffer() { const int bufferId = bufferTraits.id(); // Import the metadata. - mMetadata = BufferHubMetadata::Import(bufferTraits.take_metadata_handle()); + LocalHandle metadataHandle = bufferTraits.take_metadata_handle(); + unique_fd metadataFd(metadataHandle.Release()); + mMetadata = BufferHubMetadata::Import(std::move(metadataFd)); if (!mMetadata.IsValid()) { ALOGE("BufferHubBuffer::ImportGraphicBuffer: invalid metadata."); diff --git a/libs/ui/BufferHubMetadata.cpp b/libs/ui/BufferHubMetadata.cpp index 1e08ed1388..18d9a2c963 100644 --- a/libs/ui/BufferHubMetadata.cpp +++ b/libs/ui/BufferHubMetadata.cpp @@ -48,47 +48,47 @@ BufferHubMetadata BufferHubMetadata::Create(size_t userMetadataSize) { return {}; } - // Hand over the ownership of the fd to a pdx::LocalHandle immediately after the successful - // return of ashmem_create_region. The ashmemHandle is going to own the fd and to prevent fd + // Hand over the ownership of the fd to a unique_fd immediately after the successful + // return of ashmem_create_region. The ashmemFd is going to own the fd and to prevent fd // leaks during error handling. - pdx::LocalHandle ashmemHandle{fd}; + unique_fd ashmemFd{fd}; - if (ashmem_set_prot_region(ashmemHandle.Get(), kAshmemProt) != 0) { + if (ashmem_set_prot_region(ashmemFd.get(), kAshmemProt) != 0) { ALOGE("BufferHubMetadata::Create: failed to set protect region."); return {}; } - return BufferHubMetadata::Import(std::move(ashmemHandle)); + return BufferHubMetadata::Import(std::move(ashmemFd)); } /* static */ -BufferHubMetadata BufferHubMetadata::Import(pdx::LocalHandle ashmemHandle) { - if (!ashmem_valid(ashmemHandle.Get())) { +BufferHubMetadata BufferHubMetadata::Import(unique_fd ashmemFd) { + if (!ashmem_valid(ashmemFd.get())) { ALOGE("BufferHubMetadata::Import: invalid ashmem fd."); return {}; } - size_t metadataSize = static_cast(ashmem_get_size_region(ashmemHandle.Get())); + size_t metadataSize = static_cast(ashmem_get_size_region(ashmemFd.get())); size_t userMetadataSize = metadataSize - kMetadataHeaderSize; // Note that here the buffer state is 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 can be preserved. auto metadataHeader = static_cast(mmap(nullptr, metadataSize, kAshmemProt, - MAP_SHARED, ashmemHandle.Get(), + MAP_SHARED, ashmemFd.get(), /*offset=*/0)); if (metadataHeader == nullptr) { ALOGE("BufferHubMetadata::Import: failed to map region."); return {}; } - return BufferHubMetadata(userMetadataSize, std::move(ashmemHandle), metadataHeader); + return BufferHubMetadata(userMetadataSize, std::move(ashmemFd), metadataHeader); } -BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle, +BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd, MetadataHeader* metadataHeader) : mUserMetadataSize(userMetadataSize), - mAshmemHandle(std::move(ashmemHandle)), + mAshmemFd(std::move(ashmemFd)), mMetadataHeader(metadataHeader) {} BufferHubMetadata::~BufferHubMetadata() { diff --git a/libs/ui/include/ui/BufferHubMetadata.h b/libs/ui/include/ui/BufferHubMetadata.h index 94a9000a32..4261971f55 100644 --- a/libs/ui/include/ui/BufferHubMetadata.h +++ b/libs/ui/include/ui/BufferHubMetadata.h @@ -33,12 +33,17 @@ #pragma clang diagnostic ignored "-Wundefined-func-template" #pragma clang diagnostic ignored "-Wunused-template" #pragma clang diagnostic ignored "-Wweak-vtables" -#include #include #pragma clang diagnostic pop +#include + namespace android { +namespace { +using base::unique_fd; +} // namespace + class BufferHubMetadata { public: // Creates a new BufferHubMetadata backed by an ashmem region. @@ -50,11 +55,8 @@ public: // Imports an existing BufferHubMetadata from an ashmem FD. // - // TODO(b/112338294): Refactor BufferHub to use Binder as its internal IPC backend instead of - // UDS. - // - // @param ashmemHandle Ashmem file handle representing an ashmem region. - static BufferHubMetadata Import(pdx::LocalHandle ashmemHandle); + // @param ashmemFd Ashmem file descriptor representing an ashmem region. + static BufferHubMetadata Import(unique_fd ashmemFd); BufferHubMetadata() = default; @@ -67,7 +69,7 @@ public: mUserMetadataSize = other.mUserMetadataSize; other.mUserMetadataSize = 0; - mAshmemHandle = std::move(other.mAshmemHandle); + mAshmemFd = std::move(other.mAshmemFd); // The old raw mMetadataHeader pointer must be cleared, otherwise the destructor will // automatically mummap() the shared memory. @@ -79,25 +81,25 @@ 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 mAshmemHandle.IsValid() && 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 + dvr::BufferHubDefs::kMetadataHeaderSize; } - const pdx::LocalHandle& ashmem_handle() const { return mAshmemHandle; } + const unique_fd& ashmem_fd() const { return mAshmemFd; } dvr::BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; } private: - BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle, + BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd, dvr::BufferHubDefs::MetadataHeader* metadataHeader); BufferHubMetadata(const BufferHubMetadata&) = delete; void operator=(const BufferHubMetadata&) = delete; size_t mUserMetadataSize = 0; - pdx::LocalHandle mAshmemHandle; + unique_fd mAshmemFd; dvr::BufferHubDefs::MetadataHeader* mMetadataHeader = nullptr; }; diff --git a/libs/ui/tests/BufferHubMetadata_test.cpp b/libs/ui/tests/BufferHubMetadata_test.cpp index 4209392afa..70f86b3ef2 100644 --- a/libs/ui/tests/BufferHubMetadata_test.cpp +++ b/libs/ui/tests/BufferHubMetadata_test.cpp @@ -43,11 +43,11 @@ TEST_F(BufferHubMetadataTest, Import_Success) { EXPECT_TRUE(m1.IsValid()); EXPECT_NE(m1.metadata_header(), nullptr); - pdx::LocalHandle h2 = m1.ashmem_handle().Duplicate(); - EXPECT_TRUE(h2.IsValid()); + unique_fd h2 = unique_fd(dup(m1.ashmem_fd().get())); + EXPECT_NE(h2.get(), -1); BufferHubMetadata m2 = BufferHubMetadata::Import(std::move(h2)); - EXPECT_FALSE(h2.IsValid()); + EXPECT_EQ(h2.get(), -1); EXPECT_TRUE(m1.IsValid()); BufferHubDefs::MetadataHeader* mh1 = m1.metadata_header(); EXPECT_NE(mh1, nullptr); @@ -71,31 +71,29 @@ TEST_F(BufferHubMetadataTest, MoveMetadataInvalidatesOldOne) { BufferHubMetadata m1 = BufferHubMetadata::Create(sizeof(int)); EXPECT_TRUE(m1.IsValid()); EXPECT_NE(m1.metadata_header(), nullptr); - EXPECT_TRUE(m1.ashmem_handle().IsValid()); + EXPECT_NE(m1.ashmem_fd().get(), -1); EXPECT_EQ(m1.user_metadata_size(), sizeof(int)); BufferHubMetadata m2 = std::move(m1); - // After the move, the metadata header (a raw pointer) should be reset in the - // older buffer. + // 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); - EXPECT_FALSE(m1.ashmem_handle().IsValid()); - EXPECT_TRUE(m2.ashmem_handle().IsValid()); + EXPECT_EQ(m1.ashmem_fd().get(), -1); + EXPECT_NE(m2.ashmem_fd().get(), -1); EXPECT_EQ(m1.user_metadata_size(), 0U); EXPECT_EQ(m2.user_metadata_size(), sizeof(int)); BufferHubMetadata m3{std::move(m2)}; - // After the move, the metadata header (a raw pointer) should be reset in the - // older buffer. + // 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); - EXPECT_FALSE(m2.ashmem_handle().IsValid()); - EXPECT_TRUE(m3.ashmem_handle().IsValid()); + EXPECT_EQ(m2.ashmem_fd().get(), -1); + EXPECT_NE(m3.ashmem_fd().get(), -1); EXPECT_EQ(m2.user_metadata_size(), 0U); EXPECT_EQ(m3.user_metadata_size(), sizeof(int)); diff --git a/services/vr/bufferhubd/buffer_channel.cpp b/services/vr/bufferhubd/buffer_channel.cpp index 589b31a285..6c64bcd279 100644 --- a/services/vr/bufferhubd/buffer_channel.cpp +++ b/services/vr/bufferhubd/buffer_channel.cpp @@ -83,10 +83,13 @@ Status> BufferChannel::OnImport( ATRACE_NAME("BufferChannel::OnImport"); ALOGD_IF(TRACE, "BufferChannel::OnImport: buffer=%d.", buffer_id()); + BorrowedHandle ashmem_handle = + BorrowedHandle(buffer_node_->metadata().ashmem_fd().get()); + // TODO(b/112057680) Move away from the GraphicBuffer-based IonBuffer. return BufferTraits{ /*buffer_handle=*/buffer_node_->buffer_handle(), - /*metadata_handle=*/buffer_node_->metadata().ashmem_handle().Borrow(), + /*metadata_handle=*/ashmem_handle, /*id=*/buffer_id(), /*client_state_mask=*/client_state_mask_, /*metadata_size=*/buffer_node_->metadata().metadata_size(), -- cgit v1.2.3-59-g8ed1b From cfbe07453ebd7cde74a7ed75568ee2c81d726049 Mon Sep 17 00:00:00 2001 From: Fan Xu Date: Wed, 21 Nov 2018 15:03:32 -0800 Subject: Move MetadataHeader to libui To remove BufferHubMetadata off pdx dependency. Note that the __attribute__(packed) is removed as part of this CL, as it's not really needed and is triggering clang warnings. Test: build passed. Not test needed as no behavior changes. Bug: 118893702 Change-Id: Ifae94a143a2bedef68a653c57f089b95d166e6d7 --- libs/ui/BufferHubBuffer.cpp | 4 +- libs/ui/BufferHubMetadata.cpp | 5 +- libs/ui/include/ui/BufferHubDefs.h | 64 ++++++++++++++++++++++ libs/ui/include/ui/BufferHubMetadata.h | 30 ++-------- libs/ui/tests/Android.bp | 7 ++- libs/ui/tests/BufferHubMetadata_test.cpp | 2 + .../include/private/dvr/buffer_hub_defs.h | 36 ++---------- libs/vr/libdvr/include/dvr/dvr_api.h | 1 + services/bufferhub/BufferClient.cpp | 1 + services/bufferhub/include/bufferhub/BufferNode.h | 1 + services/bufferhub/tests/BufferNode_test.cpp | 4 +- 11 files changed, 91 insertions(+), 64 deletions(-) create mode 100644 libs/ui/include/ui/BufferHubDefs.h (limited to 'libs/ui/BufferHubMetadata.cpp') diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp index 3816c1bc4f..36eaed93ad 100644 --- a/libs/ui/BufferHubBuffer.cpp +++ b/libs/ui/BufferHubBuffer.cpp @@ -40,6 +40,7 @@ #include #include +#include using android::base::unique_fd; using android::dvr::BufferTraits; @@ -69,7 +70,6 @@ using dvr::BufferHubDefs::IsClientGained; using dvr::BufferHubDefs::IsClientPosted; using dvr::BufferHubDefs::IsClientReleased; using dvr::BufferHubDefs::kHighBitsMask; -using dvr::BufferHubDefs::kMetadataHeaderSize; } // namespace @@ -161,7 +161,7 @@ int BufferHubBuffer::ImportGraphicBuffer() { } size_t metadataSize = static_cast(bufferTraits.metadata_size()); - if (metadataSize < kMetadataHeaderSize) { + if (metadataSize < BufferHubDefs::kMetadataHeaderSize) { ALOGE("BufferHubBuffer::ImportGraphicBuffer: metadata too small: %zu", metadataSize); return -EINVAL; } diff --git a/libs/ui/BufferHubMetadata.cpp b/libs/ui/BufferHubMetadata.cpp index 18d9a2c963..816707db9d 100644 --- a/libs/ui/BufferHubMetadata.cpp +++ b/libs/ui/BufferHubMetadata.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -29,8 +30,8 @@ static const int kAshmemProt = PROT_READ | PROT_WRITE; } // namespace -using dvr::BufferHubDefs::kMetadataHeaderSize; -using dvr::BufferHubDefs::MetadataHeader; +using BufferHubDefs::kMetadataHeaderSize; +using BufferHubDefs::MetadataHeader; /* static */ BufferHubMetadata BufferHubMetadata::Create(size_t userMetadataSize) { diff --git a/libs/ui/include/ui/BufferHubDefs.h b/libs/ui/include/ui/BufferHubDefs.h new file mode 100644 index 0000000000..a1948256f5 --- /dev/null +++ b/libs/ui/include/ui/BufferHubDefs.h @@ -0,0 +1,64 @@ +/* + * 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_BUFFER_HUB_DEFS_H_ +#define ANDROID_BUFFER_HUB_DEFS_H_ + +#include + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wpacked" +// TODO(b/118893702): remove dependency once DvrNativeBufferMetadata moved out of libdvr +#include +#pragma clang diagnostic pop + +namespace android { + +namespace BufferHubDefs { + +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, + // it's not stable cross Android version, so don't have it visible from outside of the Android + // 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; + + // 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; + + // 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; + + // The index of the buffer queue where the buffer belongs to. + uint64_t queue_index; + + // Public data format, which should be updated with caution. See more details in dvr_api.h + DvrNativeBufferMetadata metadata; +}; + +static_assert(sizeof(MetadataHeader) == 136, "Unexpected MetadataHeader size"); +static constexpr size_t kMetadataHeaderSize = sizeof(MetadataHeader); + +} // namespace BufferHubDefs + +} // namespace android + +#endif // ANDROID_BUFFER_HUB_DEFS_H_ diff --git a/libs/ui/include/ui/BufferHubMetadata.h b/libs/ui/include/ui/BufferHubMetadata.h index 4261971f55..212189497a 100644 --- a/libs/ui/include/ui/BufferHubMetadata.h +++ b/libs/ui/include/ui/BufferHubMetadata.h @@ -17,26 +17,8 @@ #ifndef ANDROID_BUFFER_HUB_METADATA_H_ #define ANDROID_BUFFER_HUB_METADATA_H_ -// We would eliminate the clang warnings introduced by libdpx. -// TODO(b/112338294): Remove those once BufferHub moved to use Binder -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wconversion" -#pragma clang diagnostic ignored "-Wdouble-promotion" -#pragma clang diagnostic ignored "-Wgnu-case-range" -#pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" -#pragma clang diagnostic ignored "-Winconsistent-missing-destructor-override" -#pragma clang diagnostic ignored "-Wnested-anon-types" -#pragma clang diagnostic ignored "-Wpacked" -#pragma clang diagnostic ignored "-Wshadow" -#pragma clang diagnostic ignored "-Wsign-conversion" -#pragma clang diagnostic ignored "-Wswitch-enum" -#pragma clang diagnostic ignored "-Wundefined-func-template" -#pragma clang diagnostic ignored "-Wunused-template" -#pragma clang diagnostic ignored "-Wweak-vtables" -#include -#pragma clang diagnostic pop - #include +#include namespace android { @@ -84,23 +66,21 @@ public: bool IsValid() const { return mAshmemFd.get() != -1 && mMetadataHeader != nullptr; } size_t user_metadata_size() const { return mUserMetadataSize; } - size_t metadata_size() const { - return mUserMetadataSize + dvr::BufferHubDefs::kMetadataHeaderSize; - } + size_t metadata_size() const { return mUserMetadataSize + BufferHubDefs::kMetadataHeaderSize; } const unique_fd& ashmem_fd() const { return mAshmemFd; } - dvr::BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; } + BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; } private: BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd, - dvr::BufferHubDefs::MetadataHeader* metadataHeader); + BufferHubDefs::MetadataHeader* metadataHeader); BufferHubMetadata(const BufferHubMetadata&) = delete; void operator=(const BufferHubMetadata&) = delete; size_t mUserMetadataSize = 0; unique_fd mAshmemFd; - dvr::BufferHubDefs::MetadataHeader* mMetadataHeader = nullptr; + BufferHubDefs::MetadataHeader* mMetadataHeader = nullptr; }; } // namespace android diff --git a/libs/ui/tests/Android.bp b/libs/ui/tests/Android.bp index 18bbb3e876..ca73be79d1 100644 --- a/libs/ui/tests/Android.bp +++ b/libs/ui/tests/Android.bp @@ -73,10 +73,13 @@ cc_test { cc_test { name: "BufferHubMetadata_test", - header_libs: ["libbufferhub_headers", "libdvr_headers"], + header_libs: [ + "libbufferhub_headers", + "libdvr_headers", + "libpdx_headers", + ], shared_libs: [ "libbase", - "libpdx_default_transport", "libui", "libutils", ], diff --git a/libs/ui/tests/BufferHubMetadata_test.cpp b/libs/ui/tests/BufferHubMetadata_test.cpp index 14422bf987..2265336c30 100644 --- a/libs/ui/tests/BufferHubMetadata_test.cpp +++ b/libs/ui/tests/BufferHubMetadata_test.cpp @@ -15,8 +15,10 @@ */ #include +#include #include +// TODO(b/118893702): move this function to ui/BufferHubDefs.h after ag/5483995 is landed using android::dvr::BufferHubDefs::IsBufferReleased; namespace android { 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 400def7341..62ef475f16 100644 --- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h +++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h @@ -8,8 +8,7 @@ #include #include #include - -#include +#include namespace android { namespace dvr { @@ -120,36 +119,9 @@ static inline uint64_t FindNextAvailableClientStateMask(uint64_t union_bits) { return new_low_bit + (new_low_bit << kMaxNumberOfClients); } -struct __attribute__((packed, 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, it's not stable cross Android version, - // so don't have it visible from outside of the Android 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; - - // 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; - - // 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; - - // The index of the buffer queue where the buffer belongs to. - uint64_t queue_index; - - // Public data format, which should be updated with caution. See more details - // in dvr_api.h - DvrNativeBufferMetadata metadata; -}; - -static_assert(sizeof(MetadataHeader) == 136, "Unexpected MetadataHeader size"); -static constexpr size_t kMetadataHeaderSize = sizeof(MetadataHeader); +using MetadataHeader = android::BufferHubDefs::MetadataHeader; +static constexpr size_t kMetadataHeaderSize = + android::BufferHubDefs::kMetadataHeaderSize; } // namespace BufferHubDefs diff --git a/libs/vr/libdvr/include/dvr/dvr_api.h b/libs/vr/libdvr/include/dvr/dvr_api.h index fef8512266..a204f62fff 100644 --- a/libs/vr/libdvr/include/dvr/dvr_api.h +++ b/libs/vr/libdvr/include/dvr/dvr_api.h @@ -412,6 +412,7 @@ typedef int (*DvrTrackingSensorsStopPtr)(DvrTrackingSensors* sensors); // existing data members. If new fields need to be added, please take extra care // to make sure that new data field is padded properly the size of the struct // stays same. +// TODO(b/118893702): move the definition to libnativewindow or libui struct ALIGNED_DVR_STRUCT(8) DvrNativeBufferMetadata { #ifdef __cplusplus DvrNativeBufferMetadata() diff --git a/services/bufferhub/BufferClient.cpp b/services/bufferhub/BufferClient.cpp index 745951745a..e312011696 100644 --- a/services/bufferhub/BufferClient.cpp +++ b/services/bufferhub/BufferClient.cpp @@ -17,6 +17,7 @@ #include #include #include +#include namespace android { namespace frameworks { diff --git a/services/bufferhub/include/bufferhub/BufferNode.h b/services/bufferhub/include/bufferhub/BufferNode.h index 94ef505d41..02bb5af0e0 100644 --- a/services/bufferhub/include/bufferhub/BufferNode.h +++ b/services/bufferhub/include/bufferhub/BufferNode.h @@ -3,6 +3,7 @@ #include #include +#include #include namespace android { diff --git a/services/bufferhub/tests/BufferNode_test.cpp b/services/bufferhub/tests/BufferNode_test.cpp index df31d78b89..3358c87eb8 100644 --- a/services/bufferhub/tests/BufferNode_test.cpp +++ b/services/bufferhub/tests/BufferNode_test.cpp @@ -1,7 +1,9 @@ -#include #include + +#include #include #include +#include #include namespace android { -- 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 'libs/ui/BufferHubMetadata.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