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