Refactor BufferHubBuffer to use hwbinder
Entirely move BufferHubBuffer off pdx dependency and using hwbinder
backend and rewrite test cases to fit in current behavior.
Remove duplicated test cases in buffer_hub-test. Commented out
BufferHubBuffer related parts and remove related include and using
statement.
Add hidl interface dependency to libs/gui build file to avoid compile
errors.
Test: BufferHub_test, GraphicBuffer_test, buffer_hub-test
Bug: 116681016
Change-Id: I9d8f734f681e04d3d50b7a02c27b17df8c66cbad
diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index 1184960..c70f188 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -14,150 +14,190 @@
* limitations under the License.
*/
-// 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 <pdx/default_transport/client_channel.h>
-#include <pdx/default_transport/client_channel_factory.h>
-#include <pdx/file_handle.h>
-#include <private/dvr/bufferhub_rpc.h>
-#pragma clang diagnostic pop
-
#include <poll.h>
#include <android-base/unique_fd.h>
+#include <android/frameworks/bufferhub/1.0/IBufferHub.h>
+#include <log/log.h>
#include <ui/BufferHubBuffer.h>
#include <ui/BufferHubDefs.h>
+#include <utils/Trace.h>
-using android::base::unique_fd;
-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;
-using android::pdx::default_transport::ClientChannel;
-using android::pdx::default_transport::ClientChannelFactory;
+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::frameworks::bufferhub::V1_0::BufferHubStatus;
+using ::android::frameworks::bufferhub::V1_0::BufferTraits;
+using ::android::frameworks::bufferhub::V1_0::IBufferClient;
+using ::android::frameworks::bufferhub::V1_0::IBufferHub;
+using ::android::hardware::hidl_handle;
+using ::android::hardware::graphics::common::V1_2::HardwareBufferDescription;
namespace android {
-namespace {
-
-// TODO(b/112338294): Remove this string literal after refactoring BufferHub
-// to use Binder.
-static constexpr char kBufferHubClientPath[] = "system/buffer_hub/client";
-
-using BufferHubDefs::AnyClientAcquired;
-using BufferHubDefs::AnyClientGained;
-using BufferHubDefs::IsClientAcquired;
-using BufferHubDefs::IsClientGained;
-using BufferHubDefs::IsClientPosted;
-using BufferHubDefs::IsClientReleased;
-using BufferHubDefs::kHighBitsMask;
-
-} // namespace
-
-BufferHubClient::BufferHubClient() : Client(ClientChannelFactory::Create(kBufferHubClientPath)) {}
-
-BufferHubClient::BufferHubClient(LocalChannelHandle mChannelHandle)
- : Client(ClientChannel::Create(std::move(mChannelHandle))) {}
-
-BufferHubClient::~BufferHubClient() {}
-
-bool BufferHubClient::IsValid() const {
- return IsConnected() && GetChannelHandle().valid();
+std::unique_ptr<BufferHubBuffer> 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<BufferHubBuffer>(
+ new BufferHubBuffer(width, height, layerCount, format, usage, userMetadataSize));
+ return buffer->IsValid() ? std::move(buffer) : nullptr;
}
-LocalChannelHandle BufferHubClient::TakeChannelHandle() {
- if (IsConnected()) {
- return std::move(GetChannelHandle());
- } else {
- return {};
+std::unique_ptr<BufferHubBuffer> BufferHubBuffer::Import(const native_handle_t* token) {
+ if (token == nullptr) {
+ ALOGE("%s: token cannot be nullptr!", __FUNCTION__);
+ return nullptr;
}
+
+ auto buffer = std::unique_ptr<BufferHubBuffer>(new BufferHubBuffer(token));
+ return buffer->IsValid() ? std::move(buffer) : nullptr;
}
BufferHubBuffer::BufferHubBuffer(uint32_t width, uint32_t height, uint32_t layerCount,
- uint32_t format, uint64_t usage, size_t mUserMetadataSize) {
+ uint32_t format, uint64_t usage, size_t userMetadataSize) {
ATRACE_CALL();
- ALOGD("%s: width=%u height=%u layerCount=%u, format=%u usage=%" PRIx64 " mUserMetadataSize=%zu",
- __FUNCTION__, width, height, layerCount, format, usage, mUserMetadataSize);
+ ALOGD("%s: width=%u height=%u layerCount=%u, format=%u "
+ "usage=%" PRIx64 " mUserMetadataSize=%zu",
+ __FUNCTION__, width, height, layerCount, format, usage, userMetadataSize);
- auto status =
- mClient.InvokeRemoteMethod<DetachedBufferRPC::Create>(width, height, layerCount, format,
- usage, mUserMetadataSize);
- if (!status) {
- ALOGE("%s: Failed to create detached buffer: %s", __FUNCTION__,
- status.GetErrorMessage().c_str());
- mClient.Close(-status.error());
+ sp<IBufferHub> bufferhub = IBufferHub::getService();
+ if (bufferhub.get() == nullptr) {
+ ALOGE("%s: BufferHub service not found!", __FUNCTION__);
+ return;
}
- const int ret = ImportGraphicBuffer();
- if (ret < 0) {
- ALOGE("%s: Failed to import buffer: %s", __FUNCTION__, strerror(-ret));
- mClient.Close(ret);
+ AHardwareBuffer_Desc aDesc = {width, height, layerCount, format,
+ usage, /*stride=*/0UL, /*rfu0=*/0UL, /*rfu1=*/0ULL};
+ HardwareBufferDescription desc;
+ memcpy(&desc, &aDesc, sizeof(HardwareBufferDescription));
+
+ BufferHubStatus ret;
+ sp<IBufferClient> client;
+ BufferTraits bufferTraits;
+ IBufferHub::allocateBuffer_cb alloc_cb = [&](const auto& status, const auto& outClient,
+ const auto& traits) {
+ ret = status;
+ client = std::move(outClient);
+ bufferTraits = std::move(traits);
+ };
+
+ if (!bufferhub->allocateBuffer(desc, static_cast<uint32_t>(userMetadataSize), alloc_cb)
+ .isOk()) {
+ ALOGE("%s: allocateBuffer transaction failed!", __FUNCTION__);
+ return;
+ } else if (ret != BufferHubStatus::NO_ERROR) {
+ ALOGE("%s: allocateBuffer failed with error %u.", __FUNCTION__, ret);
+ return;
+ } else if (client == nullptr) {
+ ALOGE("%s: allocateBuffer got null BufferClient.", __FUNCTION__);
+ return;
+ }
+
+ const int importRet = initWithBufferTraits(bufferTraits);
+ if (importRet < 0) {
+ ALOGE("%s: Failed to import buffer: %s", __FUNCTION__, strerror(-importRet));
+ client->close();
+ }
+ mBufferClient = std::move(client);
+}
+
+BufferHubBuffer::BufferHubBuffer(const native_handle_t* token) {
+ sp<IBufferHub> bufferhub = IBufferHub::getService();
+ if (bufferhub.get() == nullptr) {
+ ALOGE("%s: BufferHub service not found!", __FUNCTION__);
+ return;
+ }
+
+ BufferHubStatus ret;
+ sp<IBufferClient> client;
+ BufferTraits bufferTraits;
+ IBufferHub::importBuffer_cb import_cb = [&](const auto& status, const auto& outClient,
+ const auto& traits) {
+ ret = status;
+ client = std::move(outClient);
+ bufferTraits = std::move(traits);
+ };
+
+ // hidl_handle(native_handle_t*) simply creates a raw pointer reference withouth ownership
+ // transfer.
+ if (!bufferhub->importBuffer(hidl_handle(token), import_cb).isOk()) {
+ ALOGE("%s: importBuffer transaction failed!", __FUNCTION__);
+ return;
+ } else if (ret != BufferHubStatus::NO_ERROR) {
+ ALOGE("%s: importBuffer failed with error %u.", __FUNCTION__, ret);
+ return;
+ } else if (client == nullptr) {
+ ALOGE("%s: importBuffer got null BufferClient.", __FUNCTION__);
+ return;
+ }
+
+ const int importRet = initWithBufferTraits(bufferTraits);
+ if (importRet < 0) {
+ ALOGE("%s: Failed to import buffer: %s", __FUNCTION__, strerror(-importRet));
+ client->close();
+ }
+ mBufferClient = std::move(client);
+}
+
+BufferHubBuffer::~BufferHubBuffer() {
+ // Close buffer client to avoid possible race condition: user could first duplicate and hold
+ // token with the original buffer gone, and then try to import the token. The close function
+ // will explicitly invalidate the token to avoid this.
+ if (mBufferClient != nullptr) {
+ if (!mBufferClient->close().isOk()) {
+ ALOGE("%s: close BufferClient transaction failed!", __FUNCTION__);
+ }
}
}
-BufferHubBuffer::BufferHubBuffer(LocalChannelHandle mChannelHandle)
- : mClient(std::move(mChannelHandle)) {
- const int ret = ImportGraphicBuffer();
- if (ret < 0) {
- ALOGE("%s: Failed to import buffer: %s", __FUNCTION__, strerror(-ret));
- mClient.Close(ret);
- }
-}
-
-int BufferHubBuffer::ImportGraphicBuffer() {
+int BufferHubBuffer::initWithBufferTraits(const BufferTraits& bufferTraits) {
ATRACE_CALL();
- auto status = mClient.InvokeRemoteMethod<DetachedBufferRPC::Import>();
- if (!status) {
- ALOGE("%s: Failed to import GraphicBuffer: %s", __FUNCTION__,
- status.GetErrorMessage().c_str());
- return -status.error();
+ if (bufferTraits.bufferInfo.getNativeHandle() == nullptr) {
+ ALOGE("%s: missing buffer info handle.", __FUNCTION__);
+ return -EINVAL;
}
- BufferTraits<LocalHandle> bufferTraits = status.take();
- if (bufferTraits.id() < 0) {
- ALOGE("%s: Received an invalid id!", __FUNCTION__);
- return -EIO;
+ if (bufferTraits.bufferHandle.getNativeHandle() == nullptr) {
+ ALOGE("%s: missing gralloc handle.", __FUNCTION__);
+ return -EINVAL;
}
- // Stash the buffer id to replace the value in mId.
- const int bufferId = bufferTraits.id();
+ int bufferId = bufferTraits.bufferInfo->data[1];
+ if (bufferId < 0) {
+ ALOGE("%s: Received an invalid (negative) id!", __FUNCTION__);
+ return -EINVAL;
+ }
- // Import the metadata.
- LocalHandle metadataHandle = bufferTraits.take_metadata_handle();
- unique_fd metadataFd(metadataHandle.Release());
- mMetadata = BufferHubMetadata::Import(std::move(metadataFd));
+ uint32_t clientBitMask;
+ memcpy(&clientBitMask, &bufferTraits.bufferInfo->data[2], sizeof(clientBitMask));
+ if (clientBitMask == 0U) {
+ ALOGE("%s: Received a invalid client state mask!", __FUNCTION__);
+ return -EINVAL;
+ }
+
+ // Import the metadata. Dup since hidl_handle owns the fd
+ unique_fd ashmemFd(dup(bufferTraits.bufferInfo->data[0]));
+ mMetadata = BufferHubMetadata::Import(std::move(ashmemFd));
if (!mMetadata.IsValid()) {
ALOGE("%s: invalid metadata.", __FUNCTION__);
- return -ENOMEM;
+ return -EINVAL;
}
- if (mMetadata.metadata_size() != bufferTraits.metadata_size()) {
- ALOGE("%s: metadata buffer too small: %zu, expected: %" PRIu64 ".", __FUNCTION__,
- mMetadata.metadata_size(), bufferTraits.metadata_size());
- return -ENOMEM;
+ uint32_t userMetadataSize;
+ memcpy(&userMetadataSize, &bufferTraits.bufferInfo->data[3], sizeof(userMetadataSize));
+ if (mMetadata.user_metadata_size() != userMetadataSize) {
+ ALOGE("%s: user metadata size not match: expected %u, actual %zu.", __FUNCTION__,
+ userMetadataSize, mMetadata.user_metadata_size());
+ return -EINVAL;
}
- size_t metadataSize = static_cast<size_t>(bufferTraits.metadata_size());
+ size_t metadataSize = static_cast<size_t>(mMetadata.metadata_size());
if (metadataSize < BufferHubDefs::kMetadataHeaderSize) {
ALOGE("%s: metadata too small: %zu", __FUNCTION__, metadataSize);
return -EINVAL;
@@ -178,24 +218,14 @@
// 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();
+ mBufferHandle = std::move(bufferTraits.bufferHandle);
+ memcpy(&mBufferDesc, &bufferTraits.bufferDesc, sizeof(AHardwareBuffer_Desc));
- // Populate buffer desc based on buffer traits.
- mBufferDesc.width = bufferTraits.width();
- mBufferDesc.height = bufferTraits.height();
- mBufferDesc.layers = bufferTraits.layer_count();
- mBufferDesc.format = bufferTraits.format();
- mBufferDesc.usage = bufferTraits.usage();
- mBufferDesc.stride = bufferTraits.stride();
- mBufferDesc.rfu0 = 0U;
- mBufferDesc.rfu1 = 0U;
-
- // If all imports succeed, replace the previous buffer and id.
mId = bufferId;
- mClientStateMask = bufferTraits.client_state_mask();
+ mClientStateMask = clientBitMask;
// TODO(b/112012161) Set up shared fences.
- ALOGD("%s: id=%d, buffer_state=%" PRIx32 ".", __FUNCTION__, id(),
+ ALOGD("%s: id=%d, buffer_state=%" PRIx32 ".", __FUNCTION__, mId,
buffer_state_->load(std::memory_order_acquire));
return 0;
}
@@ -225,7 +255,7 @@
int BufferHubBuffer::Post() {
uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
- uint32_t updated_buffer_state = (~mClientStateMask) & kHighBitsMask;
+ uint32_t updated_buffer_state = (~mClientStateMask) & BufferHubDefs::kHighBitsMask;
do {
if (!IsClientGained(current_buffer_state, mClientStateMask)) {
ALOGE("%s: Cannot post a buffer that is not gained by this client. buffer_id=%d "
@@ -283,24 +313,42 @@
return 0;
}
-int BufferHubBuffer::Poll(int timeoutMs) {
- ATRACE_CALL();
-
- pollfd p = {mClient.event_fd(), POLLIN, 0};
- return poll(&p, 1, timeoutMs);
+bool BufferHubBuffer::IsValid() const {
+ // TODO(b/68770788): check eventFd once implemented
+ return mBufferHandle.getNativeHandle() != nullptr && mId >= 0 && mClientStateMask != 0U &&
+ mMetadata.IsValid() && mBufferClient != nullptr;
}
-Status<LocalChannelHandle> BufferHubBuffer::Duplicate() {
- ATRACE_CALL();
- ALOGD("%s: id=%d.", __FUNCTION__, mId);
+// TODO(b/68770788): implement Poll() once we have event fd
+int BufferHubBuffer::Poll(int /* timeoutMs */) {
+ return -ENOSYS;
+}
- auto statusOrHandle = mClient.InvokeRemoteMethod<DetachedBufferRPC::Duplicate>();
-
- if (!statusOrHandle.ok()) {
- ALOGE("%s: Failed to duplicate buffer (id=%d): %s.", __FUNCTION__, mId,
- statusOrHandle.GetErrorMessage().c_str());
+native_handle_t* BufferHubBuffer::Duplicate() {
+ if (mBufferClient == nullptr) {
+ ALOGE("%s: missing BufferClient!", __FUNCTION__);
+ return nullptr;
}
- return statusOrHandle;
+
+ hidl_handle token;
+ BufferHubStatus ret;
+ IBufferClient::duplicate_cb dup_cb = [&](const auto& outToken, const auto& status) {
+ token = std::move(outToken);
+ ret = status;
+ };
+
+ if (!mBufferClient->duplicate(dup_cb).isOk()) {
+ ALOGE("%s: duplicate transaction failed!", __FUNCTION__);
+ return nullptr;
+ } else if (ret != BufferHubStatus::NO_ERROR) {
+ ALOGE("%s: duplicate failed with error %u.", __FUNCTION__, ret);
+ return nullptr;
+ } else if (token.getNativeHandle() == nullptr) {
+ ALOGE("%s: duplicate got null token.", __FUNCTION__);
+ return nullptr;
+ }
+
+ return native_handle_clone(token.getNativeHandle());
}
} // namespace android