Implement DetachedBuffer::Promote
1/ This enables a BufferHub-backed DetachedBuffer to be promoted into
the ProducerBuffer.
2/ Add DetachedBuffer::IsConnected in addition to
DetachedBuffer::IsValid.
3/ Cleaned up some using pdx:: namespace statements.
Bug: 69982239
Bug: 69981968
Bug: 70046255
Test: buffer_hub-test
Change-Id: I6ee99507b190d142647455532cdce0c2c780b2b0
diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp
index 2302828..e247398 100644
--- a/libs/vr/libbufferhub/buffer_hub-test.cpp
+++ b/libs/vr/libbufferhub/buffer_hub-test.cpp
@@ -19,18 +19,18 @@
return result; \
})()
-using android::sp;
using android::GraphicBuffer;
+using android::sp;
using android::dvr::BufferConsumer;
+using android::dvr::BufferProducer;
+using android::dvr::DetachedBuffer;
+using android::dvr::BufferHubDefs::IsBufferAcquired;
+using android::dvr::BufferHubDefs::IsBufferGained;
+using android::dvr::BufferHubDefs::IsBufferPosted;
+using android::dvr::BufferHubDefs::IsBufferReleased;
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;
@@ -376,8 +376,8 @@
EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
EXPECT_TRUE(IsBufferPosted(p->buffer_state()));
for (size_t i = 0; i < kMaxConsumerCount; i++) {
- EXPECT_TRUE(IsBufferPosted(cs[i]->buffer_state(),
- cs[i]->buffer_state_bit()));
+ EXPECT_TRUE(
+ IsBufferPosted(cs[i]->buffer_state(), cs[i]->buffer_state_bit()));
EXPECT_LT(0, RETRY_EINTR(cs[i]->Poll(kPollTimeoutMs)));
EXPECT_EQ(0, cs[i]->AcquireAsync(&metadata, &invalid_fence));
EXPECT_TRUE(IsBufferAcquired(p->buffer_state()));
@@ -792,6 +792,7 @@
// 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->IsConnected());
EXPECT_TRUE(d->IsValid());
ASSERT_TRUE(d->buffer() != nullptr);
@@ -801,10 +802,11 @@
TEST_F(LibBufferHubTest, TestCreateDetachedBufferFails) {
// Buffer Creation will fail: BLOB format requires height to be 1.
- auto b1 = DetachedBuffer::Create(kWidth, /*height=2*/2, kLayerCount,
+ auto b1 = DetachedBuffer::Create(kWidth, /*height=2*/ 2, kLayerCount,
/*format=*/HAL_PIXEL_FORMAT_BLOB, kUsage,
kUserMetadataSize);
+ EXPECT_FALSE(b1->IsConnected());
EXPECT_FALSE(b1->IsValid());
EXPECT_TRUE(b1->buffer() == nullptr);
@@ -813,6 +815,7 @@
kWidth, kHeight, kLayerCount, kFormat, kUsage,
/*user_metadata_size=*/std::numeric_limits<size_t>::max());
+ EXPECT_FALSE(b2->IsConnected());
EXPECT_FALSE(b2->IsValid());
EXPECT_TRUE(b2->buffer() == nullptr);
@@ -822,6 +825,7 @@
/*user_metadata_size=*/std::numeric_limits<size_t>::max() -
kMetadataHeaderSize);
+ EXPECT_FALSE(b3->IsConnected());
EXPECT_FALSE(b3->IsValid());
EXPECT_TRUE(b3->buffer() == nullptr);
}
@@ -831,6 +835,7 @@
kUsage, kUserMetadataSize);
int b1_id = b1->id();
+ EXPECT_TRUE(b1->IsConnected());
EXPECT_TRUE(b1->IsValid());
ASSERT_TRUE(b1->buffer() != nullptr);
EXPECT_NE(b1->id(), 0);
@@ -843,6 +848,7 @@
ASSERT_TRUE(g1 != nullptr);
EXPECT_TRUE(g1->isDetachedBuffer());
+ EXPECT_FALSE(b1->IsConnected());
EXPECT_FALSE(b1->IsValid());
EXPECT_TRUE(b1->buffer() == nullptr);
@@ -856,6 +862,7 @@
auto b2 = DetachedBuffer::Import(std::move(h1->handle()));
ASSERT_FALSE(h1->isValid());
+ EXPECT_TRUE(b2->IsConnected());
EXPECT_TRUE(b2->IsValid());
ASSERT_TRUE(b2->buffer() != nullptr);
@@ -865,3 +872,76 @@
EXPECT_EQ(b2->id(), b1_id);
EXPECT_FALSE(b2->buffer()->isDetachedBuffer());
}
+
+TEST_F(LibBufferHubTest, TestPromoteDetachedBuffer) {
+ auto b1 = DetachedBuffer::Create(kWidth, kHeight, kLayerCount, kFormat,
+ kUsage, kUserMetadataSize);
+ int b1_id = b1->id();
+ EXPECT_TRUE(b1->IsValid());
+
+ auto status_or_handle = b1->Promote();
+ EXPECT_TRUE(status_or_handle);
+
+ // The detached buffer should have hangup.
+ EXPECT_GT(RETRY_EINTR(b1->Poll(kPollTimeoutMs)), 0);
+ auto status_or_int = b1->GetEventMask(POLLHUP);
+ EXPECT_TRUE(status_or_int.ok());
+ EXPECT_EQ(status_or_int.get(), POLLHUP);
+
+ // The buffer client is still considered as connected but invalid.
+ EXPECT_TRUE(b1->IsConnected());
+ EXPECT_FALSE(b1->IsValid());
+
+ // Gets the channel handle for the producer.
+ LocalChannelHandle h1 = status_or_handle.take();
+ EXPECT_TRUE(h1.valid());
+
+ std::unique_ptr<BufferProducer> p1 = BufferProducer::Import(std::move(h1));
+ EXPECT_FALSE(h1.valid());
+ ASSERT_TRUE(p1 != nullptr);
+ int p1_id = p1->id();
+
+ // A newly promoted ProducerBuffer should inherit the same buffer id.
+ EXPECT_EQ(b1_id, p1_id);
+ EXPECT_TRUE(IsBufferGained(p1->buffer_state()));
+}
+
+TEST_F(LibBufferHubTest, TestDetachThenPromote) {
+ std::unique_ptr<BufferProducer> p1 = BufferProducer::Create(
+ kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
+ ASSERT_TRUE(p1.get() != nullptr);
+ int p1_id = p1->id();
+
+ // Detached the producer.
+ auto status_or_handle = p1->Detach();
+ EXPECT_TRUE(status_or_handle.ok());
+ 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));
+ EXPECT_FALSE(h1.valid());
+ EXPECT_TRUE(b1->IsValid());
+ int b1_id = b1->id();
+ EXPECT_EQ(b1_id, p1_id);
+
+ // Promote the detached buffer.
+ status_or_handle = b1->Promote();
+ // The buffer client is still considered as connected but invalid.
+ EXPECT_TRUE(b1->IsConnected());
+ EXPECT_FALSE(b1->IsValid());
+ EXPECT_TRUE(status_or_handle.ok());
+
+ // Gets the channel handle for the producer.
+ LocalChannelHandle h2 = status_or_handle.take();
+ EXPECT_TRUE(h2.valid());
+
+ std::unique_ptr<BufferProducer> p2 = BufferProducer::Import(std::move(h2));
+ EXPECT_FALSE(h2.valid());
+ ASSERT_TRUE(p2 != nullptr);
+ int p2_id = p2->id();
+
+ // A newly promoted ProducerBuffer should inherit the same buffer id.
+ EXPECT_EQ(b1_id, p2_id);
+ EXPECT_TRUE(IsBufferGained(p2->buffer_state()));
+}
diff --git a/libs/vr/libbufferhub/detached_buffer.cpp b/libs/vr/libbufferhub/detached_buffer.cpp
index 1d59cf3..6fae16d 100644
--- a/libs/vr/libbufferhub/detached_buffer.cpp
+++ b/libs/vr/libbufferhub/detached_buffer.cpp
@@ -3,7 +3,11 @@
#include <pdx/file_handle.h>
#include <ui/DetachedBufferHandle.h>
+#include <poll.h>
+
+using android::pdx::LocalChannelHandle;
using android::pdx::LocalHandle;
+using android::pdx::Status;
namespace android {
namespace dvr {
@@ -78,9 +82,26 @@
return 0;
}
-std::unique_ptr<BufferProducer> DetachedBuffer::Promote() {
- ALOGE("DetachedBuffer::Promote: Not implemented.");
- return nullptr;
+int DetachedBuffer::Poll(int timeout_ms) {
+ ATRACE_NAME("DetachedBuffer::Poll");
+ pollfd p = {client_.event_fd(), POLLIN, 0};
+ return poll(&p, 1, timeout_ms);
+}
+
+Status<LocalChannelHandle> DetachedBuffer::Promote() {
+ ATRACE_NAME("DetachedBuffer::Promote");
+ ALOGD_IF(TRACE, "DetachedBuffer::Promote: id=%d.", id_);
+
+ auto status_or_handle =
+ client_.InvokeRemoteMethod<DetachedBufferRPC::Promote>();
+ if (status_or_handle.ok()) {
+ // Invalidate the buffer.
+ buffer_ = {};
+ } else {
+ ALOGE("DetachedBuffer::Promote: Failed to promote buffer (id=%d): %s.", id_,
+ status_or_handle.GetErrorMessage().c_str());
+ }
+ return status_or_handle;
}
sp<GraphicBuffer> DetachedBuffer::TakeGraphicBuffer() {
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 c1cc7f3..0ea77c8 100644
--- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h
+++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h
@@ -18,17 +18,17 @@
class BufferHubClient : public pdx::Client {
public:
- using LocalChannelHandle = pdx::LocalChannelHandle;
-
BufferHubClient();
- explicit BufferHubClient(LocalChannelHandle channel_handle);
+ explicit BufferHubClient(pdx::LocalChannelHandle channel_handle);
bool IsValid() const;
- LocalChannelHandle TakeChannelHandle();
+ pdx::LocalChannelHandle TakeChannelHandle();
using pdx::Client::Close;
+ using pdx::Client::GetChannel;
using pdx::Client::InvokeRemoteMethod;
using pdx::Client::IsConnected;
+ using pdx::Client::event_fd;
};
class BufferHubBuffer : public pdx::Client {
diff --git a/libs/vr/libbufferhub/include/private/dvr/detached_buffer.h b/libs/vr/libbufferhub/include/private/dvr/detached_buffer.h
index 73e895d..6d0b502 100644
--- a/libs/vr/libbufferhub/include/private/dvr/detached_buffer.h
+++ b/libs/vr/libbufferhub/include/private/dvr/detached_buffer.h
@@ -8,8 +8,6 @@
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,
@@ -22,7 +20,7 @@
// Imports the given channel handle to a DetachedBuffer, taking ownership.
static std::unique_ptr<DetachedBuffer> Import(
- LocalChannelHandle channel_handle) {
+ pdx::LocalChannelHandle channel_handle) {
return std::unique_ptr<DetachedBuffer>(
new DetachedBuffer(std::move(channel_handle)));
}
@@ -33,12 +31,31 @@
const sp<GraphicBuffer>& buffer() const { return buffer_.buffer(); }
int id() const { return id_; }
- bool IsValid() const { return client_.IsValid(); }
+
+ // 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
+ // availble for the client to read from and/or write into.
+ bool IsValid() const { return buffer_.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<int> GetEventMask(int events) {
+ if (auto* channel = client_.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);
// 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();
+ pdx::Status<pdx::LocalChannelHandle> Promote();
// Takes the underlying graphic buffer out of this DetachedBuffer. This call
// immediately invalidates this DetachedBuffer object and transfers the
@@ -49,7 +66,7 @@
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);
+ DetachedBuffer(pdx::LocalChannelHandle channel_handle);
int ImportGraphicBuffer();
diff --git a/services/vr/bufferhubd/buffer_hub.cpp b/services/vr/bufferhubd/buffer_hub.cpp
index 72bf6f2..e57c8ed 100644
--- a/services/vr/bufferhubd/buffer_hub.cpp
+++ b/services/vr/bufferhubd/buffer_hub.cpp
@@ -266,6 +266,14 @@
SetChannel(channel->channel_id(), nullptr);
return {};
+ case DetachedBufferRPC::Promote::Opcode:
+ // In addition to the message handler in the DetachedBufferChannel's
+ // HandleMessage method, we also need to invalid the channel. Note that
+ // this has to be done after HandleMessage returns to make sure the IPC
+ // request has went back to the client first.
+ SetChannel(channel->channel_id(), nullptr);
+ return {};
+
default:
return DefaultHandleMessage(message);
}
diff --git a/services/vr/bufferhubd/detached_buffer_channel.cpp b/services/vr/bufferhubd/detached_buffer_channel.cpp
index 4f4160a..a5cf68d 100644
--- a/services/vr/bufferhubd/detached_buffer_channel.cpp
+++ b/services/vr/bufferhubd/detached_buffer_channel.cpp
@@ -1,4 +1,5 @@
#include "detached_buffer_channel.h"
+#include "producer_channel.h"
using android::pdx::BorrowedHandle;
using android::pdx::ErrorStatus;
@@ -62,6 +63,14 @@
}
}
+DetachedBufferChannel::~DetachedBufferChannel() {
+ ALOGD_IF(TRACE,
+ "DetachedBufferChannel::~DetachedBufferChannel: channel_id=%d "
+ "buffer_id=%d.",
+ channel_id(), buffer_id());
+ Hangup();
+}
+
BufferHubChannel::BufferInfo DetachedBufferChannel::GetBufferInfo() const {
return BufferInfo(buffer_id(), /*consumer_count=*/0, buffer_.width(),
buffer_.height(), buffer_.layer_count(), buffer_.format(),
@@ -106,13 +115,44 @@
}
Status<RemoteChannelHandle> DetachedBufferChannel::OnPromote(
- Message& /*message*/) {
+ Message& message) {
ATRACE_NAME("DetachedBufferChannel::OnPromote");
ALOGD_IF(TRACE, "DetachedBufferChannel::OnPromote: buffer_id=%d",
buffer_id());
- // TODO(b/69982239): Implement the logic to promote a detached buffer.
- return ErrorStatus(ENOSYS);
+ // Note that the new ProducerChannel will have different channel_id, but
+ // inherits the buffer_id from the DetachedBuffer.
+ int channel_id;
+ auto status = message.PushChannel(0, nullptr, &channel_id);
+ if (!status) {
+ ALOGE(
+ "DetachedBufferChannel::OnPromote: Failed to push ProducerChannel: %s.",
+ status.GetErrorMessage().c_str());
+ return ErrorStatus(ENOMEM);
+ }
+
+ std::unique_ptr<ProducerChannel> channel = ProducerChannel::Create(
+ service(), buffer_id(), channel_id, std::move(buffer_),
+ std::move(metadata_buffer_), user_metadata_size_);
+ if (!channel) {
+ ALOGE(
+ "DetachedBufferChannel::OnPromote: Failed to create ProducerChannel "
+ "from a DetachedBufferChannel, buffer_id=%d.",
+ buffer_id());
+ }
+
+ const auto channel_status =
+ service()->SetChannel(channel_id, std::move(channel));
+ if (!channel_status) {
+ // Technically, this should never fail, as we just pushed the channel. Note
+ // that LOG_FATAL will be stripped out in non-debug build.
+ LOG_FATAL(
+ "DetachedBufferChannel::OnPromote: Failed to set new producer buffer "
+ "channel: %s.",
+ channel_status.GetErrorMessage().c_str());
+ }
+
+ return status;
}
} // namespace dvr
diff --git a/services/vr/bufferhubd/detached_buffer_channel.h b/services/vr/bufferhubd/detached_buffer_channel.h
index 079ba72..8b6dab8 100644
--- a/services/vr/bufferhubd/detached_buffer_channel.h
+++ b/services/vr/bufferhubd/detached_buffer_channel.h
@@ -11,6 +11,8 @@
class DetachedBufferChannel : public BufferHubChannel {
public:
+ ~DetachedBufferChannel() override;
+
template <typename... Args>
static std::unique_ptr<DetachedBufferChannel> Create(Args&&... args) {
auto buffer = std::unique_ptr<DetachedBufferChannel>(
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index a753168..b6977aa 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -35,6 +35,30 @@
} // namespace
+ProducerChannel::ProducerChannel(BufferHubService* service, int buffer_id,
+ int channel_id, IonBuffer buffer,
+ IonBuffer metadata_buffer,
+ size_t user_metadata_size, int* error)
+ : BufferHubChannel(service, buffer_id, channel_id, kProducerType),
+ buffer_(std::move(buffer)),
+ metadata_buffer_(std::move(metadata_buffer)),
+ user_metadata_size_(user_metadata_size),
+ metadata_buf_size_(BufferHubDefs::kMetadataHeaderSize +
+ user_metadata_size) {
+ if (!buffer_.IsValid()) {
+ ALOGE("ProducerChannel::ProducerChannel: Invalid buffer.");
+ *error = -EINVAL;
+ return;
+ }
+ if (!metadata_buffer_.IsValid()) {
+ ALOGE("ProducerChannel::ProducerChannel: Invalid metadata buffer.");
+ *error = -EINVAL;
+ return;
+ }
+
+ *error = InitializeBuffer();
+}
+
ProducerChannel::ProducerChannel(BufferHubService* service, int channel_id,
uint32_t width, uint32_t height,
uint32_t layer_count, uint32_t format,
@@ -63,13 +87,16 @@
return;
}
+ *error = InitializeBuffer();
+}
+
+int ProducerChannel::InitializeBuffer() {
void* metadata_ptr = nullptr;
if (int ret = metadata_buffer_.Lock(BufferHubDefs::kMetadataUsage, /*x=*/0,
/*y=*/0, metadata_buf_size_,
/*height=*/1, &metadata_ptr)) {
ALOGE("ProducerChannel::ProducerChannel: Failed to lock metadata.");
- *error = -ret;
- return;
+ return ret;
}
metadata_header_ =
reinterpret_cast<BufferHubDefs::MetadataHeader*>(metadata_ptr);
@@ -85,15 +112,13 @@
release_fence_fd_.Reset(epoll_create1(EPOLL_CLOEXEC));
if (!acquire_fence_fd_ || !release_fence_fd_) {
ALOGE("ProducerChannel::ProducerChannel: Failed to create shared fences.");
- *error = -EIO;
- return;
+ return -EIO;
}
dummy_fence_fd_.Reset(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK));
if (!dummy_fence_fd_) {
ALOGE("ProducerChannel::ProducerChannel: Failed to create dummy fences.");
- *error = -EIO;
- return;
+ return EIO;
}
epoll_event event;
@@ -105,12 +130,25 @@
"ProducerChannel::ProducerChannel: Failed to modify the shared "
"release fence to include the dummy fence: %s",
strerror(errno));
- *error = -EIO;
- return;
+ return -EIO;
}
// Success.
- *error = 0;
+ return 0;
+}
+
+std::unique_ptr<ProducerChannel> ProducerChannel::Create(
+ BufferHubService* service, int buffer_id, int channel_id, IonBuffer buffer,
+ IonBuffer metadata_buffer, size_t user_metadata_size) {
+ int error = 0;
+ std::unique_ptr<ProducerChannel> producer(new ProducerChannel(
+ service, buffer_id, channel_id, std::move(buffer),
+ std::move(metadata_buffer), user_metadata_size, &error));
+
+ if (error < 0)
+ return nullptr;
+ else
+ return producer;
}
Status<std::shared_ptr<ProducerChannel>> ProducerChannel::Create(
diff --git a/services/vr/bufferhubd/producer_channel.h b/services/vr/bufferhubd/producer_channel.h
index de9ff31..67fdf15 100644
--- a/services/vr/bufferhubd/producer_channel.h
+++ b/services/vr/bufferhubd/producer_channel.h
@@ -30,6 +30,12 @@
template <typename T>
using BufferWrapper = pdx::rpc::BufferWrapper<T>;
+ static std::unique_ptr<ProducerChannel> Create(BufferHubService* service,
+ int buffer_id, int channel_id,
+ IonBuffer buffer,
+ IonBuffer metadata_buffer,
+ size_t user_metadata_size);
+
static pdx::Status<std::shared_ptr<ProducerChannel>> Create(
BufferHubService* service, int channel_id, uint32_t width,
uint32_t height, uint32_t layer_count, uint32_t format, uint64_t usage,
@@ -92,10 +98,14 @@
pdx::LocalHandle release_fence_fd_;
pdx::LocalHandle dummy_fence_fd_;
+ ProducerChannel(BufferHubService* service, int buffer_id, int channel_id,
+ IonBuffer buffer, IonBuffer metadata_buffer,
+ size_t user_metadata_size, int* error);
ProducerChannel(BufferHubService* service, int channel, uint32_t width,
uint32_t height, uint32_t layer_count, uint32_t format,
uint64_t usage, size_t user_metadata_size, int* error);
+ int InitializeBuffer();
pdx::Status<BufferDescription<BorrowedHandle>> OnGetBuffer(Message& message);
pdx::Status<void> OnProducerPost(Message& message, LocalFence acquire_fence);
pdx::Status<LocalFence> OnProducerGain(Message& message);