bufferhubd: Implement more DetachedBuffer logic
1/ Separate DetachedBuffer related logic into a dedicated subclass of
BufferHubRPC. This actually is the right thing to do as it utilizes
the PDX's client/service programming pattern better.
2/ Add IsValid() check for the DetachedBufferChannel object.
3/ Add BufferHubClient to handle general PDX operations.
4/ Add DetachedBuffer which composites a BufferHubClient.
5/ Fully functional logic of allocating a DetachedBuffer, converting it
to a BufferHub-backed GraphicBuffer, then converting it back to a
DetachedBuffer.
Bug: 38137191
Bug: 70046255
Bug: 70912269
Test: buffer_hub-test
Change-Id: I81bf9259cbbaeb29a6df2769363b5a03464e7864
diff --git a/libs/vr/libbufferhub/Android.bp b/libs/vr/libbufferhub/Android.bp
index 39814cc..b38ecc7 100644
--- a/libs/vr/libbufferhub/Android.bp
+++ b/libs/vr/libbufferhub/Android.bp
@@ -15,6 +15,7 @@
sourceFiles = [
"buffer_hub_client.cpp",
"buffer_hub_rpc.cpp",
+ "detached_buffer.cpp",
"ion_buffer.cpp",
]
@@ -59,6 +60,11 @@
vndk: {
enabled: true,
},
+ target: {
+ vendor: {
+ exclude_srcs: ["detached_buffer.cpp"],
+ },
+ },
}
cc_test {
diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp
index 660a200..2302828 100644
--- a/libs/vr/libbufferhub/buffer_hub-test.cpp
+++ b/libs/vr/libbufferhub/buffer_hub-test.cpp
@@ -2,8 +2,10 @@
#include <poll.h>
#include <private/dvr/buffer_hub_client.h>
#include <private/dvr/bufferhub_rpc.h>
+#include <private/dvr/detached_buffer.h>
#include <sys/epoll.h>
#include <sys/eventfd.h>
+#include <ui/DetachedBufferHandle.h>
#include <mutex>
#include <thread>
@@ -17,22 +19,28 @@
return result; \
})()
+using android::sp;
+using android::GraphicBuffer;
using android::dvr::BufferConsumer;
using android::dvr::BufferHubDefs::kConsumerStateMask;
+using android::dvr::BufferHubDefs::kMetadataHeaderSize;
using android::dvr::BufferHubDefs::kProducerStateBit;
using android::dvr::BufferHubDefs::IsBufferGained;
using android::dvr::BufferHubDefs::IsBufferPosted;
using android::dvr::BufferHubDefs::IsBufferAcquired;
using android::dvr::BufferHubDefs::IsBufferReleased;
using android::dvr::BufferProducer;
+using android::dvr::DetachedBuffer;
using android::pdx::LocalChannelHandle;
using android::pdx::LocalHandle;
using android::pdx::Status;
const int kWidth = 640;
const int kHeight = 480;
+const int kLayerCount = 1;
const int kFormat = HAL_PIXEL_FORMAT_RGBA_8888;
const int kUsage = 0;
+const size_t kUserMetadataSize = 0;
const uint64_t kContext = 42;
const size_t kMaxConsumerCount = 63;
const int kPollTimeoutMs = 100;
@@ -730,6 +738,7 @@
DvrNativeBufferMetadata metadata;
LocalHandle invalid_fence;
+ int p_id = p->id();
// Detach in posted state should fail.
EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
@@ -753,8 +762,8 @@
s1 = p->Detach();
EXPECT_TRUE(s1);
- LocalChannelHandle detached_buffer = s1.take();
- EXPECT_TRUE(detached_buffer.valid());
+ LocalChannelHandle handle = s1.take();
+ EXPECT_TRUE(handle.valid());
// Both producer and consumer should have hangup.
EXPECT_GT(RETRY_EINTR(p->Poll(kPollTimeoutMs)), 0);
@@ -779,4 +788,80 @@
// ConsumerChannel::HandleMessage as the socket is still open but the producer
// 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));
+ EXPECT_FALSE(handle.valid());
+ EXPECT_TRUE(d->IsValid());
+
+ ASSERT_TRUE(d->buffer() != nullptr);
+ EXPECT_EQ(d->buffer()->initCheck(), 0);
+ EXPECT_EQ(d->id(), p_id);
+}
+
+TEST_F(LibBufferHubTest, TestCreateDetachedBufferFails) {
+ // 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);
+
+ EXPECT_FALSE(b1->IsValid());
+ EXPECT_TRUE(b1->buffer() == nullptr);
+
+ // Buffer Creation will fail: user metadata size too large.
+ auto b2 = DetachedBuffer::Create(
+ kWidth, kHeight, kLayerCount, kFormat, kUsage,
+ /*user_metadata_size=*/std::numeric_limits<size_t>::max());
+
+ EXPECT_FALSE(b2->IsValid());
+ EXPECT_TRUE(b2->buffer() == nullptr);
+
+ // Buffer Creation will fail: user metadata size too large.
+ auto b3 = DetachedBuffer::Create(
+ kWidth, kHeight, kLayerCount, kFormat, kUsage,
+ /*user_metadata_size=*/std::numeric_limits<size_t>::max() -
+ kMetadataHeaderSize);
+
+ EXPECT_FALSE(b3->IsValid());
+ EXPECT_TRUE(b3->buffer() == nullptr);
+}
+
+TEST_F(LibBufferHubTest, TestCreateDetachedBuffer) {
+ auto b1 = DetachedBuffer::Create(kWidth, kHeight, kLayerCount, kFormat,
+ kUsage, kUserMetadataSize);
+ int b1_id = b1->id();
+
+ EXPECT_TRUE(b1->IsValid());
+ ASSERT_TRUE(b1->buffer() != nullptr);
+ EXPECT_NE(b1->id(), 0);
+ EXPECT_EQ(b1->buffer()->initCheck(), 0);
+ EXPECT_FALSE(b1->buffer()->isDetachedBuffer());
+
+ // Takes a standalone GraphicBuffer which still holds on an
+ // PDX::LocalChannelHandle towards BufferHub.
+ sp<GraphicBuffer> g1 = b1->TakeGraphicBuffer();
+ ASSERT_TRUE(g1 != nullptr);
+ EXPECT_TRUE(g1->isDetachedBuffer());
+
+ EXPECT_FALSE(b1->IsValid());
+ EXPECT_TRUE(b1->buffer() == nullptr);
+
+ sp<GraphicBuffer> g2 = b1->TakeGraphicBuffer();
+ ASSERT_TRUE(g2 == nullptr);
+
+ auto h1 = g1->takeDetachedBufferHandle();
+ ASSERT_TRUE(h1 != nullptr);
+ ASSERT_TRUE(h1->isValid());
+ EXPECT_FALSE(g1->isDetachedBuffer());
+
+ auto b2 = DetachedBuffer::Import(std::move(h1->handle()));
+ ASSERT_FALSE(h1->isValid());
+ EXPECT_TRUE(b2->IsValid());
+
+ ASSERT_TRUE(b2->buffer() != nullptr);
+ EXPECT_EQ(b2->buffer()->initCheck(), 0);
+
+ // The newly created DetachedBuffer should share the original buffer_id.
+ EXPECT_EQ(b2->id(), b1_id);
+ EXPECT_FALSE(b2->buffer()->isDetachedBuffer());
}
diff --git a/libs/vr/libbufferhub/buffer_hub_client.cpp b/libs/vr/libbufferhub/buffer_hub_client.cpp
index 13971eb..159f2bd 100644
--- a/libs/vr/libbufferhub/buffer_hub_client.cpp
+++ b/libs/vr/libbufferhub/buffer_hub_client.cpp
@@ -15,10 +15,30 @@
using android::pdx::LocalChannelHandle;
using android::pdx::LocalHandle;
using android::pdx::Status;
+using android::pdx::default_transport::ClientChannel;
+using android::pdx::default_transport::ClientChannelFactory;
namespace android {
namespace dvr {
+BufferHubClient::BufferHubClient()
+ : Client(ClientChannelFactory::Create(BufferHubRPC::kClientPath)) {}
+
+BufferHubClient::BufferHubClient(LocalChannelHandle channel_handle)
+ : Client(ClientChannel::Create(std::move(channel_handle))) {}
+
+bool BufferHubClient::IsValid() const {
+ return IsConnected() && GetChannelHandle().valid();
+}
+
+LocalChannelHandle BufferHubClient::TakeChannelHandle() {
+ if (IsConnected()) {
+ return std::move(GetChannelHandle());
+ } else {
+ return {};
+ }
+}
+
BufferHubBuffer::BufferHubBuffer(LocalChannelHandle channel_handle)
: Client{pdx::default_transport::ClientChannel::Create(
std::move(channel_handle))},
diff --git a/libs/vr/libbufferhub/detached_buffer.cpp b/libs/vr/libbufferhub/detached_buffer.cpp
new file mode 100644
index 0000000..1d59cf3
--- /dev/null
+++ b/libs/vr/libbufferhub/detached_buffer.cpp
@@ -0,0 +1,104 @@
+#include <private/dvr/detached_buffer.h>
+
+#include <pdx/file_handle.h>
+#include <ui/DetachedBufferHandle.h>
+
+using android::pdx::LocalHandle;
+
+namespace android {
+namespace dvr {
+
+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_IF(TRACE,
+ "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<DetachedBufferRPC::Create>(
+ 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);
+ }
+}
+
+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);
+ }
+}
+
+int DetachedBuffer::ImportGraphicBuffer() {
+ ATRACE_NAME("DetachedBuffer::DetachedBuffer");
+
+ auto status = client_.InvokeRemoteMethod<DetachedBufferRPC::Import>();
+ if (!status) {
+ ALOGE("DetachedBuffer::DetachedBuffer: Failed to import GraphicBuffer: %s",
+ status.GetErrorMessage().c_str());
+ return -status.error();
+ }
+
+ BufferDescription<LocalHandle> buffer_desc = status.take();
+ if (buffer_desc.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;
+ }
+
+ // If all imports succeed, replace the previous buffer and id.
+ id_ = buffer_id;
+ buffer_ = std::move(ion_buffer);
+ return 0;
+}
+
+std::unique_ptr<BufferProducer> DetachedBuffer::Promote() {
+ ALOGE("DetachedBuffer::Promote: Not implemented.");
+ return nullptr;
+}
+
+sp<GraphicBuffer> DetachedBuffer::TakeGraphicBuffer() {
+ if (!client_.IsValid() || !buffer_.buffer()) {
+ ALOGE("DetachedBuffer::TakeGraphicBuffer: Invalid buffer.");
+ return nullptr;
+ }
+
+ // Technically this should never happen.
+ LOG_FATAL_IF(
+ buffer_.buffer()->isDetachedBuffer(),
+ "DetachedBuffer::TakeGraphicBuffer: GraphicBuffer is already detached.");
+
+ sp<GraphicBuffer> buffer = std::move(buffer_.buffer());
+ buffer->setDetachedBufferHandle(
+ DetachedBufferHandle::Create(client_.TakeChannelHandle()));
+ return buffer;
+}
+
+} // namespace dvr
+} // namespace android
diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h
index 32448a1..c1cc7f3 100644
--- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h
+++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h
@@ -16,6 +16,21 @@
namespace android {
namespace dvr {
+class BufferHubClient : public pdx::Client {
+ public:
+ using LocalChannelHandle = pdx::LocalChannelHandle;
+
+ BufferHubClient();
+ explicit BufferHubClient(LocalChannelHandle channel_handle);
+
+ bool IsValid() const;
+ LocalChannelHandle TakeChannelHandle();
+
+ using pdx::Client::Close;
+ using pdx::Client::InvokeRemoteMethod;
+ using pdx::Client::IsConnected;
+};
+
class BufferHubBuffer : public pdx::Client {
public:
using LocalHandle = pdx::LocalHandle;
diff --git a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
index fabefd5..f4918c4 100644
--- a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
+++ b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
@@ -375,7 +375,7 @@
kOpConsumerSetIgnore,
kOpProducerBufferDetach,
kOpConsumerBufferDetach,
- kOpCreateDetachedBuffer,
+ kOpDetachedBufferCreate,
kOpDetachedBufferPromote,
kOpCreateProducerQueue,
kOpCreateConsumerQueue,
@@ -383,6 +383,8 @@
kOpProducerQueueAllocateBuffers,
kOpProducerQueueRemoveBuffer,
kOpConsumerQueueImportBuffers,
+ // TODO(b/77153033): Separate all those RPC operations into subclasses.
+ kOpDetachedBufferBase = 1000,
};
// Aliases.
@@ -416,17 +418,6 @@
PDX_REMOTE_METHOD(ConsumerBufferDetach, kOpConsumerBufferDetach,
LocalChannelHandle(Void));
- // Creates a standalone DetachedBuffer not associated with any
- // producer/consumer set.
- PDX_REMOTE_METHOD(CreateDetachedBuffer, kOpCreateDetachedBuffer,
- LocalChannelHandle(Void));
-
- // 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_REMOTE_METHOD(DetachedBufferPromote, kOpDetachedBufferPromote,
- LocalChannelHandle(Void));
-
// Buffer Queue Methods.
PDX_REMOTE_METHOD(CreateProducerQueue, kOpCreateProducerQueue,
QueueInfo(const ProducerQueueConfig& producer_config,
@@ -445,6 +436,25 @@
std::vector<std::pair<LocalChannelHandle, size_t>>(Void));
};
+struct DetachedBufferRPC final : public BufferHubRPC {
+ private:
+ enum {
+ kOpCreate = kOpDetachedBufferBase,
+ kOpImport,
+ kOpPromote,
+ };
+
+ 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_API(API, Create, Promote);
+};
+
} // 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
new file mode 100644
index 0000000..73e895d
--- /dev/null
+++ b/libs/vr/libbufferhub/include/private/dvr/detached_buffer.h
@@ -0,0 +1,65 @@
+#ifndef ANDROID_DVR_DETACHED_BUFFER_H_
+#define ANDROID_DVR_DETACHED_BUFFER_H_
+
+#include <private/dvr/buffer_hub_client.h>
+
+namespace android {
+namespace dvr {
+
+class DetachedBuffer {
+ public:
+ using LocalChannelHandle = pdx::LocalChannelHandle;
+
+ // Allocates a standalone DetachedBuffer not associated with any producer
+ // consumer set.
+ static std::unique_ptr<DetachedBuffer> 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<DetachedBuffer>(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<DetachedBuffer> Import(
+ LocalChannelHandle channel_handle) {
+ return std::unique_ptr<DetachedBuffer>(
+ new DetachedBuffer(std::move(channel_handle)));
+ }
+
+ DetachedBuffer(const DetachedBuffer&) = delete;
+ void operator=(const DetachedBuffer&) = delete;
+
+ const sp<GraphicBuffer>& buffer() const { return buffer_.buffer(); }
+
+ int id() const { return id_; }
+ bool IsValid() const { return client_.IsValid(); }
+
+ // 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.
+ std::unique_ptr<BufferProducer> Promote();
+
+ // Takes the underlying graphic buffer out of this DetachedBuffer. This call
+ // immediately invalidates this DetachedBuffer object and transfers the
+ // underlying pdx::LocalChannelHandle into the GraphicBuffer.
+ sp<GraphicBuffer> TakeGraphicBuffer();
+
+ private:
+ DetachedBuffer(uint32_t width, uint32_t height, uint32_t layer_count,
+ uint32_t format, uint64_t usage, size_t user_metadata_size);
+
+ DetachedBuffer(LocalChannelHandle channel_handle);
+
+ int ImportGraphicBuffer();
+
+ // Global id for the buffer that is consistent across processes.
+ int id_;
+ IonBuffer buffer_;
+ BufferHubClient client_;
+};
+
+} // namespace dvr
+} // namespace android
+
+#endif // ANDROID_DVR_DETACHED_BUFFER_H_
diff --git a/libs/vr/libbufferhub/include/private/dvr/ion_buffer.h b/libs/vr/libbufferhub/include/private/dvr/ion_buffer.h
index 0d337f7..f6bc547 100644
--- a/libs/vr/libbufferhub/include/private/dvr/ion_buffer.h
+++ b/libs/vr/libbufferhub/include/private/dvr/ion_buffer.h
@@ -23,6 +23,9 @@
IonBuffer(IonBuffer&& other);
IonBuffer& operator=(IonBuffer&& other);
+ // Returns check this IonBuffer holds a valid Gralloc buffer.
+ bool IsValid() const { return buffer_ && buffer_->initCheck() == NO_ERROR; }
+
// Frees the underlying native handle and leaves the instance initialized to
// empty.
void FreeHandle();
@@ -66,6 +69,7 @@
struct android_ycbcr* yuv);
int Unlock();
+ sp<GraphicBuffer>& buffer() { return buffer_; }
const sp<GraphicBuffer>& buffer() const { return buffer_; }
buffer_handle_t handle() const {
return buffer_.get() ? buffer_->handle : nullptr;
diff --git a/libs/vr/libpdx/client.cpp b/libs/vr/libpdx/client.cpp
index a01c4d6..3c66a40 100644
--- a/libs/vr/libpdx/client.cpp
+++ b/libs/vr/libpdx/client.cpp
@@ -119,6 +119,10 @@
return channel_->GetChannelHandle();
}
+const LocalChannelHandle& Client::GetChannelHandle() const {
+ return channel_->GetChannelHandle();
+}
+
///////////////////////////// Transaction implementation //////////////////////
Transaction::Transaction(Client& client) : client_{client} {}
diff --git a/libs/vr/libpdx/private/pdx/client.h b/libs/vr/libpdx/private/pdx/client.h
index 656de7e..c35dabd 100644
--- a/libs/vr/libpdx/private/pdx/client.h
+++ b/libs/vr/libpdx/private/pdx/client.h
@@ -49,6 +49,7 @@
// Returns a reference to IPC channel handle.
LocalChannelHandle& GetChannelHandle();
+ const LocalChannelHandle& GetChannelHandle() const;
protected:
friend Transaction;
diff --git a/libs/vr/libpdx/private/pdx/client_channel.h b/libs/vr/libpdx/private/pdx/client_channel.h
index 8f5fdfe..f33a60e 100644
--- a/libs/vr/libpdx/private/pdx/client_channel.h
+++ b/libs/vr/libpdx/private/pdx/client_channel.h
@@ -33,6 +33,7 @@
virtual std::vector<EventSource> GetEventSources() const = 0;
virtual LocalChannelHandle& GetChannelHandle() = 0;
+ virtual const LocalChannelHandle& GetChannelHandle() const = 0;
virtual void* AllocateTransactionState() = 0;
virtual void FreeTransactionState(void* state) = 0;
diff --git a/libs/vr/libpdx/private/pdx/mock_client_channel.h b/libs/vr/libpdx/private/pdx/mock_client_channel.h
index ecc20b3..b1a1299 100644
--- a/libs/vr/libpdx/private/pdx/mock_client_channel.h
+++ b/libs/vr/libpdx/private/pdx/mock_client_channel.h
@@ -14,6 +14,7 @@
MOCK_CONST_METHOD0(GetEventSources, std::vector<EventSource>());
MOCK_METHOD1(GetEventMask, Status<int>(int));
MOCK_METHOD0(GetChannelHandle, LocalChannelHandle&());
+ MOCK_CONST_METHOD0(GetChannelHandle, const LocalChannelHandle&());
MOCK_METHOD0(AllocateTransactionState, void*());
MOCK_METHOD1(FreeTransactionState, void(void* state));
MOCK_METHOD3(SendImpulse,
diff --git a/libs/vr/libpdx_uds/private/uds/client_channel.h b/libs/vr/libpdx_uds/private/uds/client_channel.h
index b5524d8..3561c6f 100644
--- a/libs/vr/libpdx_uds/private/uds/client_channel.h
+++ b/libs/vr/libpdx_uds/private/uds/client_channel.h
@@ -41,6 +41,9 @@
}
LocalChannelHandle& GetChannelHandle() override { return channel_handle_; }
+ const LocalChannelHandle& GetChannelHandle() const override {
+ return channel_handle_;
+ }
void* AllocateTransactionState() override;
void FreeTransactionState(void* state) override;