Move detached buffer off IonBuffer
1/ Migration DetachedBuffer's metadata to use ashmem-based
BufferHubMetadata.
2/ Avoid import the actual gralloc buffer into the IonBuffer. Instead,
just store the native_handle_t of the gralloc buffer in DetachedBuffer.
3/ Replace the usage of BufferDescription/NativeBufferHandle with
BufferTraits/NativeHandleWrapper, as they both depend on
IonBuffer. Currently dvr::ProdcuerBuffer and dvr::ConsumerBuffer
are still using BufferDescription/NativeBufferHandle so that we are
reimplementing them for DetachedBuffer to avoid chaning
dvr::ProdcuerBuffer and dvr::ConsumerBuffer at this point.
Bug: 112940221
Bug: 112011098
Bug: 70048475
Test: atest buffer_hub-test
Change-Id: I435180ba80a27b0ff35f0d95fcdbc23412978e22
diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp
index cf4a0e9..a77dbf1 100644
--- a/libs/vr/libbufferhub/buffer_hub-test.cpp
+++ b/libs/vr/libbufferhub/buffer_hub-test.cpp
@@ -920,6 +920,7 @@
kUsage, kUserMetadataSize);
int b1_id = b1->id();
EXPECT_TRUE(b1->IsValid());
+ EXPECT_EQ(b1->user_metadata_size(), kUserMetadataSize);
auto status_or_handle = b1->Duplicate();
EXPECT_TRUE(status_or_handle);
@@ -935,6 +936,9 @@
std::unique_ptr<DetachedBuffer> b2 = DetachedBuffer::Import(std::move(h2));
EXPECT_FALSE(h2.valid());
ASSERT_TRUE(b2 != nullptr);
+ EXPECT_TRUE(b2->IsValid());
+ EXPECT_EQ(b2->user_metadata_size(), kUserMetadataSize);
+
int b2_id = b2->id();
// These two buffer instances are based on the same physical buffer under the
diff --git a/libs/vr/libbufferhub/buffer_hub_metadata.cpp b/libs/vr/libbufferhub/buffer_hub_metadata.cpp
index 8243fc5..788c22c 100644
--- a/libs/vr/libbufferhub/buffer_hub_metadata.cpp
+++ b/libs/vr/libbufferhub/buffer_hub_metadata.cpp
@@ -74,6 +74,9 @@
size_t metadata_size = 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<MetadataHeader*>(
mmap(nullptr, metadata_size, kAshmemProt, MAP_SHARED, ashmem_handle.Get(),
/*offset=*/0));
diff --git a/libs/vr/libbufferhub/detached_buffer.cpp b/libs/vr/libbufferhub/detached_buffer.cpp
index 420ce24..795ad19 100644
--- a/libs/vr/libbufferhub/detached_buffer.cpp
+++ b/libs/vr/libbufferhub/detached_buffer.cpp
@@ -16,8 +16,16 @@
namespace android {
namespace dvr {
+namespace {
+
+// TODO(b/112338294): Remove this string literal after refactoring BufferHub
+// to use Binder.
+static constexpr char kBufferHubClientPath[] = "system/buffer_hub/client";
+
+} // namespace
+
BufferHubClient::BufferHubClient()
- : Client(ClientChannelFactory::Create(BufferHubRPC::kClientPath)) {}
+ : Client(ClientChannelFactory::Create(kBufferHubClientPath)) {}
BufferHubClient::BufferHubClient(LocalChannelHandle channel_handle)
: Client(ClientChannel::Create(std::move(channel_handle))) {}
@@ -80,71 +88,53 @@
return -status.error();
}
- BufferDescription<LocalHandle> buffer_desc = status.take();
- if (buffer_desc.id() < 0) {
+ BufferTraits<LocalHandle> 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_desc.id();
-
- // Import the buffer.
- IonBuffer ion_buffer;
- ALOGD_IF(TRACE, "DetachedBuffer::DetachedBuffer: id=%d.", buffer_id);
-
- if (const int ret = buffer_desc.ImportBuffer(&ion_buffer)) {
- ALOGE("Failed to import GraphicBuffer, error=%d", ret);
- return ret;
- }
+ const int buffer_id = buffer_traits.id();
// Import the metadata.
- IonBuffer metadata_buffer;
- if (const int ret = buffer_desc.ImportMetadata(&metadata_buffer)) {
- ALOGE("Failed to import metadata buffer, error=%d", ret);
- return ret;
+ metadata_ = BufferHubMetadata::Import(buffer_traits.take_metadata_handle());
+
+ if (!metadata_.IsValid()) {
+ ALOGE("DetachedBuffer::ImportGraphicBuffer: invalid metadata.");
+ return -ENOMEM;
}
- size_t metadata_buf_size = metadata_buffer.width();
+
+ 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 = buffer_traits.metadata_size();
if (metadata_buf_size < BufferHubDefs::kMetadataHeaderSize) {
- ALOGE("DetachedBuffer::ImportGraphicBuffer: metadata buffer too small: %zu",
+ 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_ = std::move(ion_buffer);
- metadata_buffer_ = std::move(metadata_buffer);
- user_metadata_size_ = metadata_buf_size - BufferHubDefs::kMetadataHeaderSize;
-
- void* metadata_ptr = nullptr;
- if (const int ret =
- metadata_buffer_.Lock(BufferHubDefs::kMetadataUsage, /*x=*/0,
- /*y=*/0, metadata_buf_size,
- /*height=*/1, &metadata_ptr)) {
- ALOGE("DetachedBuffer::ImportGraphicBuffer: Failed to lock metadata.");
- return ret;
- }
+ buffer_state_bit_ = buffer_traits.buffer_state_bit();
// TODO(b/112012161) Set up shared fences.
-
- // 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.
- metadata_header_ = static_cast<BufferHubDefs::MetadataHeader*>(metadata_ptr);
- if (user_metadata_size_) {
- user_metadata_ptr_ = static_cast<void*>(metadata_header_ + 1);
- } else {
- user_metadata_ptr_ = nullptr;
- }
-
- id_ = buffer_desc.id();
- buffer_state_bit_ = buffer_desc.buffer_state_bit();
-
ALOGD_IF(TRACE,
"DetachedBuffer::ImportGraphicBuffer: id=%d, buffer_state=%" PRIx64
".",
- id(), metadata_header_->buffer_state.load());
+ id(),
+ metadata_.metadata_header()->buffer_state.load(
+ std::memory_order_acquire));
return 0;
}
@@ -166,7 +156,7 @@
client_.InvokeRemoteMethod<DetachedBufferRPC::Promote>();
if (status_or_handle.ok()) {
// Invalidate the buffer.
- buffer_ = {};
+ buffer_handle_ = {};
} else {
ALOGE("DetachedBuffer::Promote: Failed to promote buffer (id=%d): %s.", id_,
status_or_handle.GetErrorMessage().c_str());
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 8b2bf91..5a24253 100644
--- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
+++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
@@ -3,6 +3,11 @@
#include <dvr/dvr_api.h>
#include <hardware/gralloc.h>
+#include <pdx/channel_handle.h>
+#include <pdx/file_handle.h>
+#include <pdx/rpc/remote_method.h>
+#include <pdx/rpc/serializable.h>
+#include <private/dvr/native_handle_wrapper.h>
#include <atomic>
@@ -82,8 +87,137 @@
} // namespace BufferHubDefs
+template <typename FileHandleType>
+class BufferTraits {
+ public:
+ BufferTraits() = default;
+ BufferTraits(const native_handle_t* buffer_handle,
+ const FileHandleType& metadata_handle, int id,
+ uint64_t buffer_state_bit, uint64_t metadata_size,
+ uint32_t width, uint32_t height, uint32_t layer_count,
+ uint32_t format, uint64_t usage, uint32_t stride,
+ const FileHandleType& acquire_fence_fd,
+ const FileHandleType& release_fence_fd)
+ : id_(id),
+ buffer_state_bit_(buffer_state_bit),
+ metadata_size_(metadata_size),
+ width_(width),
+ height_(height),
+ layer_count_(layer_count),
+ format_(format),
+ usage_(usage),
+ stride_(stride),
+ buffer_handle_(buffer_handle),
+ metadata_handle_(metadata_handle.Borrow()),
+ acquire_fence_fd_(acquire_fence_fd.Borrow()),
+ release_fence_fd_(release_fence_fd.Borrow()) {}
+
+ BufferTraits(BufferTraits&& other) = default;
+ BufferTraits& operator=(BufferTraits&& other) = default;
+
+ // ID of the buffer client. All BufferHubBuffer clients derived from the same
+ // buffer in bufferhubd share the same buffer id.
+ int id() const { return id_; }
+
+ // State mask of the buffer client. Each BufferHubBuffer client backed by the
+ // same buffer channel has uniqued state bit among its siblings. For a
+ // producer buffer the bit must be kProducerStateBit; for a consumer the bit
+ // must be one of the kConsumerStateMask.
+ uint64_t buffer_state_bit() const { return buffer_state_bit_; }
+ uint64_t metadata_size() const { return metadata_size_; }
+
+ uint32_t width() { return width_; }
+ uint32_t height() { return height_; }
+ uint32_t layer_count() { return layer_count_; }
+ uint32_t format() { return format_; }
+ uint64_t usage() { return usage_; }
+ uint32_t stride() { return stride_; }
+
+ const NativeHandleWrapper<FileHandleType>& buffer_handle() const {
+ return buffer_handle_;
+ }
+
+ NativeHandleWrapper<FileHandleType> take_buffer_handle() {
+ return std::move(buffer_handle_);
+ }
+ FileHandleType take_metadata_handle() { return std::move(metadata_handle_); }
+ FileHandleType take_acquire_fence() { return std::move(acquire_fence_fd_); }
+ FileHandleType take_release_fence() { return std::move(release_fence_fd_); }
+
+ private:
+ // BufferHub specific traits.
+ int id_ = -1;
+ uint64_t buffer_state_bit_;
+ uint64_t metadata_size_;
+
+ // Traits for a GraphicBuffer.
+ uint32_t width_;
+ uint32_t height_;
+ uint32_t layer_count_;
+ uint32_t format_;
+ uint64_t usage_;
+ uint32_t stride_;
+
+ // Native handle for the graphic buffer.
+ NativeHandleWrapper<FileHandleType> buffer_handle_;
+
+ // File handle of an ashmem that holds buffer metadata.
+ FileHandleType metadata_handle_;
+
+ // Pamameters for shared fences.
+ FileHandleType acquire_fence_fd_;
+ FileHandleType release_fence_fd_;
+
+ PDX_SERIALIZABLE_MEMBERS(BufferTraits<FileHandleType>, id_, buffer_state_bit_,
+ metadata_size_, stride_, width_, height_,
+ layer_count_, format_, usage_, buffer_handle_,
+ metadata_handle_, acquire_fence_fd_,
+ release_fence_fd_);
+
+ BufferTraits(const BufferTraits&) = delete;
+ void operator=(const BufferTraits&) = delete;
+};
+
+struct DetachedBufferRPC {
+ private:
+ enum {
+ kOpDetachedBufferBase = 1000,
+
+ // Allocates a standalone DetachedBuffer not associated with any producer
+ // consumer set.
+ kOpCreate,
+
+ // Imports the given channel handle to a DetachedBuffer, taking ownership.
+ kOpImport,
+
+ // 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.
+ kOpPromote,
+
+ // Creates a DetachedBuffer client from an existing one. The new client will
+ // share the same underlying gralloc buffer and ashmem region for metadata.
+ kOpDuplicate,
+ };
+
+ // Aliases.
+ using LocalChannelHandle = pdx::LocalChannelHandle;
+ using LocalHandle = pdx::LocalHandle;
+ using Void = pdx::rpc::Void;
+
+ public:
+ PDX_REMOTE_METHOD(Create, kOpCreate,
+ void(uint32_t width, uint32_t height, uint32_t layer_count,
+ uint32_t format, uint64_t usage,
+ size_t user_metadata_size));
+ PDX_REMOTE_METHOD(Import, kOpImport, BufferTraits<LocalHandle>(Void));
+ PDX_REMOTE_METHOD(Promote, kOpPromote, LocalChannelHandle(Void));
+ PDX_REMOTE_METHOD(Duplicate, kOpDuplicate, LocalChannelHandle(Void));
+
+ PDX_REMOTE_API(API, Create, Import, Promote, Duplicate);
+};
+
} // namespace dvr
} // namespace android
-
#endif // ANDROID_DVR_BUFFER_HUB_DEFS_H_
diff --git a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
index 0c7fc90..7330aa1 100644
--- a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
+++ b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
@@ -316,8 +316,6 @@
kOpConsumerRelease,
kOpProducerBufferDetach,
kOpConsumerBufferDetach,
- kOpDetachedBufferCreate,
- kOpDetachedBufferPromote,
kOpCreateProducerQueue,
kOpCreateConsumerQueue,
kOpGetQueueInfo,
@@ -326,7 +324,6 @@
kOpProducerQueueRemoveBuffer,
kOpConsumerQueueImportBuffers,
// TODO(b/77153033): Separate all those RPC operations into subclasses.
- kOpDetachedBufferBase = 1000,
};
// Aliases.
@@ -379,27 +376,6 @@
std::vector<std::pair<LocalChannelHandle, size_t>>(Void));
};
-struct DetachedBufferRPC final : public BufferHubRPC {
- private:
- enum {
- kOpCreate = kOpDetachedBufferBase,
- kOpImport,
- kOpPromote,
- kOpDuplicate,
- };
-
- public:
- PDX_REMOTE_METHOD(Create, kOpCreate,
- void(uint32_t width, uint32_t height, uint32_t layer_count,
- uint32_t format, uint64_t usage,
- size_t user_metadata_size));
- PDX_REMOTE_METHOD(Import, kOpImport, BufferDescription<LocalHandle>(Void));
- PDX_REMOTE_METHOD(Promote, kOpPromote, LocalChannelHandle(Void));
- PDX_REMOTE_METHOD(Duplicate, kOpDuplicate, LocalChannelHandle(Void));
-
- PDX_REMOTE_API(API, Create, Import, Promote, Duplicate);
-};
-
} // namespace dvr
} // namespace android
diff --git a/libs/vr/libbufferhub/include/private/dvr/detached_buffer.h b/libs/vr/libbufferhub/include/private/dvr/detached_buffer.h
index d280d94..ff61145 100644
--- a/libs/vr/libbufferhub/include/private/dvr/detached_buffer.h
+++ b/libs/vr/libbufferhub/include/private/dvr/detached_buffer.h
@@ -49,19 +49,28 @@
// 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_header_->buffer_state.load(); }
+ 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 gralloc buffer handle that's
+ // 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_.IsValid(); }
+ 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).
@@ -81,7 +90,8 @@
// return. Further IPCs towards this channel will return error.
pdx::Status<pdx::LocalChannelHandle> Promote();
- // Creates a DetachedBuffer from an existing one.
+ // 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<pdx::LocalChannelHandle> Duplicate();
private:
@@ -96,14 +106,12 @@
int id_;
uint64_t buffer_state_bit_;
- // The concrete Ion buffers.
- IonBuffer buffer_;
- IonBuffer metadata_buffer_;
+ // Wrapps the gralloc buffer handle of this buffer.
+ NativeHandleWrapper<pdx::LocalHandle> buffer_handle_;
- // buffer metadata.
- size_t user_metadata_size_ = 0;
- BufferHubDefs::MetadataHeader* metadata_header_ = nullptr;
- void* user_metadata_ptr_ = nullptr;
+ // An ashmem-based metadata object. The same shared memory are mapped to the
+ // bufferhubd daemon and all buffer clients.
+ BufferHubMetadata metadata_;
// PDX backend.
BufferHubClient client_;
diff --git a/libs/vr/libbufferhub/include/private/dvr/native_handle_wrapper.h b/libs/vr/libbufferhub/include/private/dvr/native_handle_wrapper.h
new file mode 100644
index 0000000..3086982
--- /dev/null
+++ b/libs/vr/libbufferhub/include/private/dvr/native_handle_wrapper.h
@@ -0,0 +1,100 @@
+#ifndef ANDROID_DVR_NATIVE_HANDLE_WRAPPER_H_
+#define ANDROID_DVR_NATIVE_HANDLE_WRAPPER_H_
+
+#include <cutils/native_handle.h>
+
+#include <vector>
+
+namespace android {
+namespace dvr {
+
+// A PDX-friendly wrapper to maintain the life cycle of a native_handle_t
+// object.
+//
+// See https://source.android.com/devices/architecture/hidl/types#handle_t for
+// more information about native_handle_t.
+template <typename FileHandleType>
+class NativeHandleWrapper {
+ public:
+ NativeHandleWrapper() = default;
+ NativeHandleWrapper(NativeHandleWrapper&& other) = default;
+ NativeHandleWrapper& operator=(NativeHandleWrapper&& other) = default;
+
+ // Create a new NativeHandleWrapper by duplicating the handle.
+ explicit NativeHandleWrapper(const native_handle_t* handle) {
+ const int fd_count = handle->numFds;
+ const int int_count = handle->numInts;
+
+ // Populate the fd and int vectors: native_handle->data[] is an array of fds
+ // followed by an array of opaque ints.
+ for (int i = 0; i < fd_count; i++) {
+ fds_.emplace_back(FileHandleType::AsDuplicate(handle->data[i]));
+ }
+ for (int i = 0; i < int_count; i++) {
+ ints_.push_back(handle->data[fd_count + i]);
+ }
+ }
+
+ size_t int_count() const { return ints_.size(); }
+ size_t fd_count() const { return fds_.size(); }
+ bool IsValid() const { return ints_.size() != 0 || fds_.size() != 0; }
+
+ // Duplicate a native handle from the wrapper.
+ const native_handle_t* DuplicateHandle() const {
+ if (!IsValid()) {
+ return nullptr;
+ }
+
+ // numFds + numInts ints.
+ std::vector<FileHandleType> fds;
+ for (const auto& fd : fds_) {
+ if (!fd.IsValid()) {
+ return nullptr;
+ }
+ fds.emplace_back(fd.Duplicate());
+ }
+
+ return FromFdsAndInts(std::move(fds), ints_);
+ }
+
+ // Takes the native handle out of the wrapper.
+ const native_handle_t* TakeHandle() {
+ if (!IsValid()) {
+ return nullptr;
+ }
+
+ return FromFdsAndInts(std::move(fds_), std::move(ints_));
+ }
+
+ private:
+ NativeHandleWrapper(const NativeHandleWrapper&) = delete;
+ void operator=(const NativeHandleWrapper&) = delete;
+
+ static const native_handle_t* FromFdsAndInts(std::vector<FileHandleType> fds,
+ std::vector<int> ints) {
+ native_handle_t* handle = native_handle_create(fds.size(), ints.size());
+ if (!handle) {
+ ALOGE("NativeHandleWrapper::TakeHandle: Failed to create new handle.");
+ return nullptr;
+ }
+
+ // numFds + numInts ints.
+ for (int i = 0; i < handle->numFds; i++) {
+ handle->data[i] = fds[i].Release();
+ }
+ memcpy(&handle->data[handle->numFds], ints.data(),
+ sizeof(int) * handle->numInts);
+
+ return handle;
+ }
+
+ std::vector<int> ints_;
+ std::vector<FileHandleType> fds_;
+
+ PDX_SERIALIZABLE_MEMBERS(NativeHandleWrapper<FileHandleType>, ints_, fds_);
+};
+
+} // namespace dvr
+} // namespace android
+
+#endif // ANDROID_DVR_NATIVE_HANDLE_WRAPPER_H_
diff --git a/services/vr/bufferhubd/Android.bp b/services/vr/bufferhubd/Android.bp
index 04b9511..fc8ad80 100644
--- a/services/vr/bufferhubd/Android.bp
+++ b/services/vr/bufferhubd/Android.bp
@@ -16,6 +16,7 @@
"libbase",
"libbinder",
"libcutils",
+ "libgtest_prod",
"libgui",
"liblog",
"libpdx_default_transport",
diff --git a/services/vr/bufferhubd/buffer_channel.cpp b/services/vr/bufferhubd/buffer_channel.cpp
index 6d22dee..dcc6ea4 100644
--- a/services/vr/bufferhubd/buffer_channel.cpp
+++ b/services/vr/bufferhubd/buffer_channel.cpp
@@ -13,11 +13,10 @@
BufferChannel::BufferChannel(BufferHubService* service, int buffer_id,
int channel_id, IonBuffer buffer,
- IonBuffer metadata_buffer,
size_t user_metadata_size)
: BufferHubChannel(service, buffer_id, channel_id, kDetachedBufferType),
- buffer_node_(std::make_shared<BufferNode>(
- std::move(buffer), std::move(metadata_buffer), user_metadata_size)),
+ buffer_node_(
+ std::make_shared<BufferNode>(std::move(buffer), user_metadata_size)),
buffer_state_bit_(BufferHubDefs::FindFirstClearedBit()) {
buffer_node_->set_buffer_state_bit(buffer_state_bit_);
}
@@ -85,19 +84,26 @@
}
}
-Status<BufferDescription<BorrowedHandle>> BufferChannel::OnImport(
+Status<BufferTraits<BorrowedHandle>> BufferChannel::OnImport(
Message& /*message*/) {
ATRACE_NAME("BufferChannel::OnImport");
- ALOGD_IF(TRACE, "BufferChannel::OnImport: buffer=%d.",
- buffer_id());
+ ALOGD_IF(TRACE, "BufferChannel::OnImport: buffer=%d.", buffer_id());
- return BufferDescription<BorrowedHandle>{buffer_node_->buffer(),
- buffer_node_->metadata_buffer(),
- buffer_id(),
- channel_id(),
- buffer_state_bit_,
- BorrowedHandle{},
- BorrowedHandle{}};
+ // TODO(b/112057680) Move away from the GraphicBuffer-based IonBuffer.
+ return BufferTraits<BorrowedHandle>{
+ /*buffer_handle=*/buffer_node_->buffer().handle(),
+ /*metadata_handle=*/buffer_node_->metadata().ashmem_handle().Borrow(),
+ /*id=*/buffer_id(),
+ /*buffer_state_bit=*/buffer_state_bit_,
+ /*metadata_size=*/buffer_node_->metadata().metadata_size(),
+ /*width=*/buffer_node_->buffer().width(),
+ /*height=*/buffer_node_->buffer().height(),
+ /*layer_count=*/buffer_node_->buffer().layer_count(),
+ /*format=*/buffer_node_->buffer().format(),
+ /*usage=*/buffer_node_->buffer().usage(),
+ /*stride=*/buffer_node_->buffer().stride(),
+ /*acquire_fence_fd=*/BorrowedHandle{},
+ /*released_fence_fd=*/BorrowedHandle{}};
}
Status<RemoteChannelHandle> BufferChannel::OnDuplicate(
@@ -175,7 +181,17 @@
}
IonBuffer buffer = std::move(buffer_node_->buffer());
- IonBuffer metadata_buffer = std::move(buffer_node_->metadata_buffer());
+ IonBuffer metadata_buffer;
+ if (int ret = metadata_buffer.Alloc(buffer_node_->metadata().metadata_size(),
+ /*height=*/1,
+ /*layer_count=*/1,
+ BufferHubDefs::kMetadataFormat,
+ BufferHubDefs::kMetadataUsage)) {
+ ALOGE("BufferChannel::OnPromote: Failed to allocate metadata: %s",
+ strerror(-ret));
+ return ErrorStatus(EINVAL);
+ }
+
size_t user_metadata_size = buffer_node_->user_metadata_size();
std::unique_ptr<ProducerChannel> channel = ProducerChannel::Create(
diff --git a/services/vr/bufferhubd/buffer_node.cpp b/services/vr/bufferhubd/buffer_node.cpp
index 5a04d0c..782b9c2 100644
--- a/services/vr/bufferhubd/buffer_node.cpp
+++ b/services/vr/bufferhubd/buffer_node.cpp
@@ -4,26 +4,15 @@
namespace android {
namespace dvr {
-BufferNode::BufferNode(IonBuffer buffer, IonBuffer metadata_buffer,
- size_t user_metadata_size)
- : buffer_(std::move(buffer)),
- metadata_buffer_(std::move(metadata_buffer)),
- user_metadata_size_(user_metadata_size) {}
+BufferNode::BufferNode(IonBuffer buffer, size_t user_metadata_size)
+ : buffer_(std::move(buffer)) {
+ metadata_ = BufferHubMetadata::Create(user_metadata_size);
+}
// Allocates a new BufferNode.
BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count,
uint32_t format, uint64_t usage,
- size_t user_metadata_size)
- : user_metadata_size_(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<uint32_t>::max() -
- BufferHubDefs::kMetadataHeaderSize)) {
- ALOGE(
- "DetachedBufferChannel::DetachedBufferChannel: metadata size too big.");
- return;
- }
-
+ size_t user_metadata_size) {
if (int ret = buffer_.Alloc(width, height, layer_count, format, usage)) {
ALOGE(
"DetachedBufferChannel::DetachedBufferChannel: Failed to allocate "
@@ -32,20 +21,7 @@
return;
}
- // Buffer metadata has two parts: 1) a fixed sized metadata header; and 2)
- // user requested metadata.
- const size_t size = BufferHubDefs::kMetadataHeaderSize + user_metadata_size_;
- if (int ret = metadata_buffer_.Alloc(size,
- /*height=*/1,
- /*layer_count=*/1,
- BufferHubDefs::kMetadataFormat,
- BufferHubDefs::kMetadataUsage)) {
- ALOGE(
- "DetachedBufferChannel::DetachedBufferChannel: Failed to allocate "
- "metadata: %s",
- strerror(-ret));
- return;
- }
+ metadata_ = BufferHubMetadata::Create(user_metadata_size);
}
} // namespace dvr
diff --git a/services/vr/bufferhubd/include/private/dvr/buffer_channel.h b/services/vr/bufferhubd/include/private/dvr/buffer_channel.h
index bcc93a1..1697251 100644
--- a/services/vr/bufferhubd/include/private/dvr/buffer_channel.h
+++ b/services/vr/bufferhubd/include/private/dvr/buffer_channel.h
@@ -4,6 +4,7 @@
#include <pdx/channel_handle.h>
#include <pdx/file_handle.h>
#include <private/dvr/buffer_hub.h>
+#include <private/dvr/buffer_hub_defs.h>
#include <private/dvr/buffer_node.h>
namespace android {
@@ -34,8 +35,7 @@
private:
// Creates a detached buffer from existing IonBuffers.
BufferChannel(BufferHubService* service, int buffer_id, int channel_id,
- IonBuffer buffer, IonBuffer metadata_buffer,
- size_t user_metadata_size);
+ IonBuffer buffer, size_t user_metadata_size);
// Allocates a new detached buffer.
BufferChannel(BufferHubService* service, int buffer_id, uint32_t width,
@@ -47,7 +47,7 @@
std::shared_ptr<BufferNode> buffer_node,
uint64_t buffer_state_bit);
- pdx::Status<BufferDescription<pdx::BorrowedHandle>> OnImport(
+ pdx::Status<BufferTraits<pdx::BorrowedHandle>> OnImport(
pdx::Message& message);
pdx::Status<pdx::RemoteChannelHandle> OnDuplicate(pdx::Message& message);
pdx::Status<pdx::RemoteChannelHandle> OnPromote(pdx::Message& message);
diff --git a/services/vr/bufferhubd/include/private/dvr/buffer_node.h b/services/vr/bufferhubd/include/private/dvr/buffer_node.h
index 4bcf4e3..ebab97c 100644
--- a/services/vr/bufferhubd/include/private/dvr/buffer_node.h
+++ b/services/vr/bufferhubd/include/private/dvr/buffer_node.h
@@ -1,6 +1,7 @@
#ifndef ANDROID_DVR_BUFFERHUBD_BUFFER_NODE_H_
#define ANDROID_DVR_BUFFERHUBD_BUFFER_NODE_H_
+#include <private/dvr/buffer_hub_metadata.h>
#include <private/dvr/ion_buffer.h>
namespace android {
@@ -10,39 +11,32 @@
public:
// Creates a BufferNode from existing IonBuffers, i.e. creating from an
// existing ProducerChannel.
- BufferNode(IonBuffer buffer, IonBuffer metadata_buffer,
- size_t user_metadata_size);
+ BufferNode(IonBuffer buffer, size_t user_metadata_size);
// Allocates a new BufferNode.
BufferNode(uint32_t width, uint32_t height, uint32_t layer_count,
uint32_t format, uint64_t usage, size_t user_metadata_size);
// Returns whether the object holds a valid graphic buffer.
- bool IsValid() const {
- return buffer_.IsValid() && metadata_buffer_.IsValid();
- }
+ bool IsValid() const { return buffer_.IsValid() && metadata_.IsValid(); }
- size_t user_metadata_size() const { return user_metadata_size_; }
+ size_t user_metadata_size() const { return metadata_.user_metadata_size(); }
uint64_t active_buffer_bit_mask() const { return active_buffer_bit_mask_; }
void set_buffer_state_bit(uint64_t buffer_state_bit) {
active_buffer_bit_mask_ |= buffer_state_bit;
}
- // Used to take out IonBuffers.
+ // Accessor of the IonBuffer.
IonBuffer& buffer() { return buffer_; }
- IonBuffer& metadata_buffer() { return metadata_buffer_; }
-
- // Used to access IonBuffers.
const IonBuffer& buffer() const { return buffer_; }
- const IonBuffer& metadata_buffer() const { return metadata_buffer_; }
+
+ // Accessor of the metadata.
+ const BufferHubMetadata& metadata() const { return metadata_; }
private:
// Gralloc buffer handles.
IonBuffer buffer_;
- IonBuffer metadata_buffer_;
-
- // Size of user requested metadata.
- const size_t user_metadata_size_;
+ BufferHubMetadata metadata_;
// All active buffer bits. Valid bits are the lower 63 bits, while the
// highest bit is reserved for the exclusive writing and should not be set.
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index 0b5257d..e7622ba 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -409,9 +409,9 @@
return ErrorStatus(-ret);
};
- std::unique_ptr<BufferChannel> channel = BufferChannel::Create(
- service(), buffer_id(), channel_id, std::move(buffer_),
- std::move(metadata_buffer_), user_metadata_size_);
+ std::unique_ptr<BufferChannel> channel =
+ BufferChannel::Create(service(), buffer_id(), channel_id,
+ std::move(buffer_), user_metadata_size_);
if (!channel) {
ALOGE("ProducerChannel::OnProducerDetach: Invalid buffer.");
return ErrorStatus(EINVAL);