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/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);