Update BufferHubQueue in preparation for VrFlinger refactor.
- Expose buffer hub queue id to the client. This helpful as an id
in queue enumeration in the DisplayManager API.
- Address log convention deviations.
- Clean up BufferHub service error paths.
- Clean up queue importing logic to query the service for queue
parameters to avoid accidental abuse.
- Make spawning consumer queues more efficient in RPC only cases.
- Clean up ConsumerQueue constructor logic; move to pdx::Client
base class (through BufferHubQueue) instead of pdx::ClientBase.
ConsumerQueue::Create doesn't make sense as a ConsumerQueue can
only be created from another queue.
Bug: 36401174
Test: build; run buffer_hub_queue-test
Change-Id: I0530f64ecb514d0f37bb13f9ce201b6ff51502b6
diff --git a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
index ed11551..7ed024f 100644
--- a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
+++ b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
@@ -119,6 +119,14 @@
using LocalFence = FenceHandle<pdx::LocalHandle>;
using BorrowedFence = FenceHandle<pdx::BorrowedHandle>;
+struct QueueInfo {
+ size_t meta_size_bytes;
+ int id;
+
+ private:
+ PDX_SERIALIZABLE_MEMBERS(QueueInfo, meta_size_bytes, id);
+};
+
// BufferHub Service RPC interface. Defines the endpoints, op codes, and method
// type signatures supported by bufferhubd.
struct BufferHubRPC {
@@ -151,6 +159,7 @@
kOpConsumerSetIgnore,
kOpCreateProducerQueue,
kOpCreateConsumerQueue,
+ kOpGetQueueInfo,
kOpProducerQueueAllocateBuffers,
kOpProducerQueueDetachBuffer,
kOpConsumerQueueImportBuffers,
@@ -192,18 +201,19 @@
// Buffer Queue Methods.
PDX_REMOTE_METHOD(CreateProducerQueue, kOpCreateProducerQueue,
- int(size_t meta_size_bytes, int usage_set_mask,
- int usage_clear_mask, int usage_deny_set_mask,
- int usage_deny_clear_mask));
+ QueueInfo(size_t meta_size_bytes, int usage_set_mask,
+ int usage_clear_mask, int usage_deny_set_mask,
+ int usage_deny_clear_mask));
PDX_REMOTE_METHOD(CreateConsumerQueue, kOpCreateConsumerQueue,
- std::pair<LocalChannelHandle, size_t>(Void));
+ LocalChannelHandle(Void));
+ PDX_REMOTE_METHOD(GetQueueInfo, kOpGetQueueInfo, QueueInfo(Void));
PDX_REMOTE_METHOD(ProducerQueueAllocateBuffers,
kOpProducerQueueAllocateBuffers,
std::vector<std::pair<LocalChannelHandle, size_t>>(
int width, int height, int format, int usage,
size_t slice_count, size_t buffer_count));
PDX_REMOTE_METHOD(ProducerQueueDetachBuffer, kOpProducerQueueDetachBuffer,
- int(size_t slot));
+ void(size_t slot));
PDX_REMOTE_METHOD(ConsumerQueueImportBuffers, kOpConsumerQueueImportBuffers,
std::vector<std::pair<LocalChannelHandle, size_t>>(Void));
};
diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
index 031401a..e491abc 100644
--- a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
+++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
@@ -11,34 +11,36 @@
#include <pdx/file_handle.h>
#include <private/dvr/bufferhub_rpc.h>
+using android::pdx::ErrorStatus;
+using android::pdx::LocalChannelHandle;
+using android::pdx::Status;
+
namespace android {
namespace dvr {
-BufferHubQueue::BufferHubQueue(LocalChannelHandle channel_handle,
- size_t meta_size)
+BufferHubQueue::BufferHubQueue(LocalChannelHandle channel_handle)
: Client{pdx::default_transport::ClientChannel::Create(
std::move(channel_handle))},
- meta_size_(meta_size),
- meta_buffer_tmp_(meta_size ? new uint8_t[meta_size] : nullptr),
+ meta_size_(0),
buffers_(BufferHubQueue::kMaxQueueCapacity),
epollhup_pending_(BufferHubQueue::kMaxQueueCapacity, false),
available_buffers_(BufferHubQueue::kMaxQueueCapacity),
fences_(BufferHubQueue::kMaxQueueCapacity),
- capacity_(0) {
+ capacity_(0),
+ id_(-1) {
Initialize();
}
-BufferHubQueue::BufferHubQueue(const std::string& endpoint_path,
- size_t meta_size)
+BufferHubQueue::BufferHubQueue(const std::string& endpoint_path)
: Client{pdx::default_transport::ClientChannelFactory::Create(
endpoint_path)},
- meta_size_(meta_size),
- meta_buffer_tmp_(meta_size ? new uint8_t[meta_size] : nullptr),
+ meta_size_(0),
buffers_(BufferHubQueue::kMaxQueueCapacity),
epollhup_pending_(BufferHubQueue::kMaxQueueCapacity, false),
available_buffers_(BufferHubQueue::kMaxQueueCapacity),
fences_(BufferHubQueue::kMaxQueueCapacity),
- capacity_(0) {
+ capacity_(0),
+ id_(-1) {
Initialize();
}
@@ -55,26 +57,47 @@
BufferHubQueue::kEpollQueueEventIndex)}};
ret = epoll_fd_.Control(EPOLL_CTL_ADD, event_fd(), &event);
if (ret < 0) {
- ALOGE("Failed to register ConsumerQueue into epoll event: %s",
+ ALOGE("BufferHubQueue::Initialize: Failed to add event fd to epoll set: %s",
strerror(-ret));
}
}
-std::unique_ptr<ConsumerQueue> BufferHubQueue::CreateConsumerQueue() {
- Status<std::pair<LocalChannelHandle, size_t>> status =
- InvokeRemoteMethod<BufferHubRPC::CreateConsumerQueue>();
-
+Status<void> BufferHubQueue::ImportQueue() {
+ auto status = InvokeRemoteMethod<BufferHubRPC::GetQueueInfo>();
if (!status) {
- ALOGE("Cannot create ConsumerQueue: %s", status.GetErrorMessage().c_str());
+ ALOGE("BufferHubQueue::ImportQueue: Failed to import queue: %s",
+ status.GetErrorMessage().c_str());
+ return ErrorStatus(status.error());
+ } else {
+ SetupQueue(status.get().meta_size_bytes, status.get().id);
+ return {};
+ }
+}
+
+void BufferHubQueue::SetupQueue(size_t meta_size_bytes, int id) {
+ meta_size_ = meta_size_bytes;
+ id_ = id;
+ meta_buffer_tmp_.reset(meta_size_ > 0 ? new uint8_t[meta_size_] : nullptr);
+}
+
+std::unique_ptr<ConsumerQueue> BufferHubQueue::CreateConsumerQueue() {
+ if (auto status = CreateConsumerQueueHandle())
+ return std::unique_ptr<ConsumerQueue>(new ConsumerQueue(status.take()));
+ else
return nullptr;
+}
+
+Status<LocalChannelHandle> BufferHubQueue::CreateConsumerQueueHandle() {
+ auto status = InvokeRemoteMethod<BufferHubRPC::CreateConsumerQueue>();
+ if (!status) {
+ ALOGE(
+ "BufferHubQueue::CreateConsumerQueue: Failed to create consumer queue: "
+ "%s",
+ status.GetErrorMessage().c_str());
+ return ErrorStatus(status.error());
}
- auto return_value = status.take();
-
- ALOGD_IF(TRACE, "BufferHubQueue::CreateConsumerQueue: meta_size_bytes=%zu",
- return_value.second);
- return ConsumerQueue::Create(std::move(return_value.first),
- return_value.second);
+ return status;
}
bool BufferHubQueue::WaitForBuffers(int timeout) {
@@ -89,7 +112,8 @@
}
if (ret < 0 && ret != -EINTR) {
- ALOGE("Failed to wait for buffers: %s", strerror(-ret));
+ ALOGE("BufferHubQueue::WaitForBuffers: Failed to wait for buffers: %s",
+ strerror(-ret));
return false;
}
@@ -108,7 +132,8 @@
} else if (is_queue_event_index(index)) {
HandleQueueEvent(events[i]);
} else {
- ALOGW("Unknown event index: %" PRId64, index);
+ ALOGW("BufferHubQueue::WaitForBuffers: Unknown event index: %" PRId64,
+ index);
}
}
}
@@ -134,7 +159,8 @@
if (events & EPOLLIN) {
int ret = OnBufferReady(buffer, &fences_[slot]);
if (ret < 0) {
- ALOGE("Failed to set buffer ready: %s", strerror(-ret));
+ ALOGE("BufferHubQueue::HandleBufferEvent: Failed to set buffer ready: %s",
+ strerror(-ret));
return;
}
Enqueue(buffer, slot);
@@ -144,8 +170,8 @@
// epoll FD is cleaned up when the replacement consumer client is imported,
// we shouldn't detach again if |epollhub_pending_[slot]| is set.
ALOGW(
- "Receives EPOLLHUP at slot: %zu, buffer event fd: %d, EPOLLHUP "
- "pending: %d",
+ "BufferHubQueue::HandleBufferEvent: Received EPOLLHUP at slot: %zu, "
+ "buffer event fd: %d, EPOLLHUP pending: %d",
slot, buffer->event_fd(), int{epollhup_pending_[slot]});
if (epollhup_pending_[slot]) {
epollhup_pending_[slot] = false;
@@ -153,7 +179,10 @@
DetachBuffer(slot);
}
} else {
- ALOGW("Unknown event, slot=%zu, epoll events=%d", slot, events);
+ ALOGW(
+ "BufferHubQueue::HandleBufferEvent: Unknown event, slot=%zu, epoll "
+ "events=%d",
+ slot, events);
}
}
@@ -169,12 +198,13 @@
if (events & EPOLLIN) {
// Note that after buffer imports, if |count()| still returns 0, epoll
// wait will be tried again to acquire the newly imported buffer.
- int ret = OnBufferAllocated();
- if (ret < 0) {
- ALOGE("Failed to import buffer: %s", strerror(-ret));
+ auto buffer_status = OnBufferAllocated();
+ if (!buffer_status) {
+ ALOGE("BufferHubQueue::HandleQueueEvent: Failed to import buffer: %s",
+ buffer_status.GetErrorMessage().c_str());
}
} else {
- ALOGW("Unknown epoll events=%d", events);
+ ALOGW("BufferHubQueue::HandleQueueEvent: Unknown epoll events=%d", events);
}
}
@@ -233,7 +263,7 @@
void BufferHubQueue::Enqueue(std::shared_ptr<BufferHubBuffer> buf,
size_t slot) {
if (count() == capacity_) {
- ALOGE("Buffer queue is full!");
+ ALOGE("BufferHubQueue::Enqueue: Buffer queue is full!");
return;
}
@@ -274,7 +304,7 @@
available_buffers_.PopFront();
if (!buf) {
- ALOGE("Dequeue: Buffer to be dequeued is nullptr");
+ ALOGE("BufferHubQueue::Dequeue: Buffer to be dequeued is nullptr");
return nullptr;
}
@@ -289,15 +319,22 @@
ProducerQueue::ProducerQueue(size_t meta_size)
: ProducerQueue(meta_size, 0, 0, 0, 0) {}
-ProducerQueue::ProducerQueue(LocalChannelHandle handle, size_t meta_size)
- : BASE(std::move(handle), meta_size) {}
+ProducerQueue::ProducerQueue(LocalChannelHandle handle)
+ : BASE(std::move(handle)) {
+ auto status = ImportQueue();
+ if (!status) {
+ ALOGE("ProducerQueue::ProducerQueue: Failed to import queue: %s",
+ status.GetErrorMessage().c_str());
+ Close(-status.error());
+ }
+}
ProducerQueue::ProducerQueue(size_t meta_size, int usage_set_mask,
int usage_clear_mask, int usage_deny_set_mask,
int usage_deny_clear_mask)
- : BASE(BufferHubRPC::kClientPath, meta_size) {
+ : BASE(BufferHubRPC::kClientPath) {
auto status = InvokeRemoteMethod<BufferHubRPC::CreateProducerQueue>(
- meta_size_, usage_set_mask, usage_clear_mask, usage_deny_set_mask,
+ meta_size, usage_set_mask, usage_clear_mask, usage_deny_set_mask,
usage_deny_clear_mask);
if (!status) {
ALOGE("ProducerQueue::ProducerQueue: Failed to create producer queue: %s",
@@ -305,12 +342,14 @@
Close(-status.error());
return;
}
+
+ SetupQueue(status.get().meta_size_bytes, status.get().id);
}
int ProducerQueue::AllocateBuffer(int width, int height, int format, int usage,
size_t slice_count, size_t* out_slot) {
if (out_slot == nullptr) {
- ALOGE("Parameter out_slot cannot be null.");
+ ALOGE("ProducerQueue::AllocateBuffer: Parameter out_slot cannot be null.");
return -EINVAL;
}
@@ -362,7 +401,7 @@
}
int ProducerQueue::DetachBuffer(size_t slot) {
- Status<int> status =
+ auto status =
InvokeRemoteMethod<BufferHubRPC::ProducerQueueDetachBuffer>(slot);
if (!status) {
ALOGE(
@@ -378,7 +417,9 @@
std::shared_ptr<BufferProducer> ProducerQueue::Dequeue(
int timeout, size_t* slot, LocalHandle* release_fence) {
if (slot == nullptr || release_fence == nullptr) {
- ALOGE("invalid parameter, slot=%p, release_fence=%p", slot, release_fence);
+ ALOGE(
+ "ProducerQueue::Dequeue: invalid parameter, slot=%p, release_fence=%p",
+ slot, release_fence);
return nullptr;
}
@@ -392,21 +433,27 @@
return buffer->Gain(release_fence);
}
-ConsumerQueue::ConsumerQueue(LocalChannelHandle handle, size_t meta_size)
- : BASE(std::move(handle), meta_size) {
- // TODO(b/34387835) Import consumer queue in case the ProducerQueue we are
+ConsumerQueue::ConsumerQueue(LocalChannelHandle handle)
+ : BufferHubQueue(std::move(handle)) {
+ auto status = ImportQueue();
+ if (!status) {
+ ALOGE("ConsumerQueue::ConsumerQueue: Failed to import queue: %s",
+ status.GetErrorMessage().c_str());
+ Close(-status.error());
+ }
+
+ // TODO(b/34387835) Import buffers in case the ProducerQueue we are
// based on was not empty.
}
-int ConsumerQueue::ImportBuffers() {
- Status<std::vector<std::pair<LocalChannelHandle, size_t>>> status =
- InvokeRemoteMethod<BufferHubRPC::ConsumerQueueImportBuffers>();
+Status<size_t> ConsumerQueue::ImportBuffers() {
+ auto status = InvokeRemoteMethod<BufferHubRPC::ConsumerQueueImportBuffers>();
if (!status) {
ALOGE(
"ConsumerQueue::ImportBuffers failed to import consumer buffer through "
"BufferBub, error: %s",
status.GetErrorMessage().c_str());
- return -status.error();
+ return ErrorStatus(status.error());
}
int last_error = 0;
@@ -431,7 +478,10 @@
}
}
- return imported_buffers > 0 ? imported_buffers : last_error;
+ if (imported_buffers > 0)
+ return {imported_buffers};
+ else
+ return ErrorStatus(-last_error);
}
int ConsumerQueue::AddBuffer(const std::shared_ptr<BufferConsumer>& buf,
@@ -445,15 +495,17 @@
LocalHandle* acquire_fence) {
if (meta_size != meta_size_) {
ALOGE(
- "metadata size (%zu) for the dequeuing buffer does not match metadata "
- "size (%zu) for the queue.",
+ "ConsumerQueue::Dequeue: Metadata size (%zu) for the dequeuing buffer "
+ "does not match metadata size (%zu) for the queue.",
meta_size, meta_size_);
return nullptr;
}
if (slot == nullptr || meta == nullptr || acquire_fence == nullptr) {
- ALOGE("invalid parameter, slot=%p, meta=%p, acquire_fence=%p", slot, meta,
- acquire_fence);
+ ALOGE(
+ "ConsumerQueue::Dequeue: Invalid parameter, slot=%p, meta=%p, "
+ "acquire_fence=%p",
+ slot, meta, acquire_fence);
return nullptr;
}
@@ -467,15 +519,19 @@
return buffer->Acquire(acquire_fence, meta_buffer_tmp_.get(), meta_size_);
}
-int ConsumerQueue::OnBufferAllocated() {
- const int ret = ImportBuffers();
- if (ret == 0) {
- ALOGW("No new buffer can be imported on buffer allocated event.");
- } else if (ret < 0) {
- ALOGE("Failed to import buffers on buffer allocated event.");
+Status<void> ConsumerQueue::OnBufferAllocated() {
+ auto status = ImportBuffers();
+ if (!status) {
+ ALOGE("ConsumerQueue::OnBufferAllocated: Failed to import buffers: %s",
+ status.GetErrorMessage().c_str());
+ return ErrorStatus(status.error());
+ } else if (status.get() == 0) {
+ ALOGW("ConsumerQueue::OnBufferAllocated: No new buffers allocated!");
+ return ErrorStatus(ENOBUFS);
+ } else {
+ ALOGD_IF(TRACE, "Imported %zu consumer buffers.", status.get());
+ return {};
}
- ALOGD_IF(TRACE, "Imported %d consumer buffers.", ret);
- return ret;
}
} // namespace dvr
diff --git a/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_client.h b/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_client.h
index a020dca..5ccd3a3 100644
--- a/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_client.h
+++ b/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_client.h
@@ -4,6 +4,7 @@
#include <gui/BufferQueueDefs.h>
#include <pdx/client.h>
+#include <pdx/status.h>
#include <private/dvr/buffer_hub_client.h>
#include <private/dvr/epoll_file_descriptor.h>
#include <private/dvr/ring_buffer.h>
@@ -41,6 +42,9 @@
// Return the default buffer format of this buffer queue.
int32_t default_format() const { return default_format_; }
+ // Create a new consumer in handle form for immediate transport over RPC.
+ Status<LocalChannelHandle> CreateConsumerQueueHandle();
+
// Return the number of buffers avaiable for dequeue.
size_t count() const { return available_buffers_.GetSize(); }
@@ -51,7 +55,7 @@
size_t metadata_size() const { return meta_size_; }
// Return whether the buffer queue is alrady full.
- bool is_full() const { return available_buffers_.IsFull(); }
+ bool is_full() const { return available_buffers_.IsFull(); }
explicit operator bool() const { return epoll_fd_.IsValid(); }
@@ -83,9 +87,18 @@
// timeout.
static constexpr int kNoTimeOut = -1;
+ int id() const { return id_; }
+
protected:
- BufferHubQueue(LocalChannelHandle channel, size_t meta_size);
- BufferHubQueue(const std::string& endpoint_path, size_t meta_size);
+ BufferHubQueue(LocalChannelHandle channel);
+ BufferHubQueue(const std::string& endpoint_path);
+
+ // Imports the queue parameters by querying BufferHub for the parameters for
+ // this channel.
+ Status<void> ImportQueue();
+
+ // Sets up the queue with the given parameters.
+ void SetupQueue(size_t meta_size_bytes_, int id);
// Called by ProducerQueue::AddBuffer and ConsumerQueue::AddBuffer only. to
// register a buffer for epoll and internal bookkeeping.
@@ -112,7 +125,7 @@
LocalHandle* fence) = 0;
// Called when a buffer is allocated remotely.
- virtual int OnBufferAllocated() = 0;
+ virtual Status<void> OnBufferAllocated() { return {}; }
// Data members to handle arbitrary metadata passed through BufferHub. It is
// fair to enforce that all buffers in the same queue share the same metadata
@@ -235,6 +248,9 @@
// Epoll fd used to wait for BufferHub events.
EpollFileDescriptor epoll_fd_;
+ // Global id for the queue that is consistent across processes.
+ int id_;
+
BufferHubQueue(const BufferHubQueue&) = delete;
void operator=(BufferHubQueue&) = delete;
};
@@ -267,9 +283,8 @@
}
// Import a |ProducerQueue| from a channel handle.
- template <typename Meta>
static std::unique_ptr<ProducerQueue> Import(LocalChannelHandle handle) {
- return BASE::Create(std::move(handle), sizeof(Meta));
+ return BASE::Create(std::move(handle));
}
// Get a buffer producer. Note that the method doesn't check whether the
@@ -311,18 +326,15 @@
// static template methods inherited from ClientBase, which take the same
// arguments as the constructors.
explicit ProducerQueue(size_t meta_size);
- ProducerQueue(LocalChannelHandle handle, size_t meta_size);
+ ProducerQueue(LocalChannelHandle handle);
ProducerQueue(size_t meta_size, int usage_set_mask, int usage_clear_mask,
int usage_deny_set_mask, int usage_deny_clear_mask);
int OnBufferReady(std::shared_ptr<BufferHubBuffer> buf,
LocalHandle* release_fence) override;
-
- // Producer buffer is always allocated from the client (i.e. local) side.
- int OnBufferAllocated() override { return 0; }
};
-class ConsumerQueue : public pdx::ClientBase<ConsumerQueue, BufferHubQueue> {
+class ConsumerQueue : public BufferHubQueue {
public:
// Get a buffer consumer. Note that the method doesn't check whether the
// buffer slot has a valid buffer that has been imported already. When no
@@ -333,10 +345,14 @@
BufferHubQueue::GetBuffer(slot));
}
+ // Import a |ConsumerQueue| from a channel handle.
+ static std::unique_ptr<ConsumerQueue> Import(LocalChannelHandle handle) {
+ return std::unique_ptr<ConsumerQueue>(new ConsumerQueue(std::move(handle)));
+ }
+
// Import newly created buffers from the service side.
- // Returns number of buffers successfully imported; or negative error code
- // when buffer import fails.
- int ImportBuffers();
+ // Returns number of buffers successfully imported or an error.
+ Status<size_t> ImportBuffers();
// Dequeue a consumer buffer to read. The returned buffer in |Acquired|'ed
// mode, and caller should call Releasse() once it's done writing to release
@@ -353,10 +369,11 @@
std::shared_ptr<BufferConsumer> Dequeue(int timeout, size_t* slot, void* meta,
size_t meta_size,
LocalHandle* acquire_fence);
- private:
- friend BASE;
- ConsumerQueue(LocalChannelHandle handle, size_t meta_size);
+ private:
+ friend BufferHubQueue;
+
+ ConsumerQueue(LocalChannelHandle handle);
// Add a consumer buffer to populate the queue. Once added, a consumer buffer
// is NOT available to use until the producer side |Post| it. |WaitForBuffers|
@@ -367,7 +384,7 @@
int OnBufferReady(std::shared_ptr<BufferHubBuffer> buf,
LocalHandle* acquire_fence) override;
- int OnBufferAllocated() override;
+ Status<void> OnBufferAllocated() override;
};
} // namespace dvr
diff --git a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
index 811543d..171577d 100644
--- a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
+++ b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
@@ -22,16 +22,20 @@
class BufferHubQueueTest : public ::testing::Test {
public:
template <typename Meta>
- void CreateQueues(int usage_set_mask = 0, int usage_clear_mask = 0,
+ bool CreateQueues(int usage_set_mask = 0, int usage_clear_mask = 0,
int usage_deny_set_mask = 0,
int usage_deny_clear_mask = 0) {
producer_queue_ =
ProducerQueue::Create<Meta>(usage_set_mask, usage_clear_mask,
usage_deny_set_mask, usage_deny_clear_mask);
- ASSERT_NE(nullptr, producer_queue_);
+ if (!producer_queue_)
+ return false;
consumer_queue_ = producer_queue_->CreateConsumerQueue();
- ASSERT_NE(nullptr, consumer_queue_);
+ if (!consumer_queue_)
+ return false;
+
+ return true;
}
void AllocateBuffer() {
@@ -51,7 +55,7 @@
TEST_F(BufferHubQueueTest, TestDequeue) {
const size_t nb_dequeue_times = 16;
- CreateQueues<size_t>();
+ ASSERT_TRUE(CreateQueues<size_t>());
// Allocate only one buffer.
AllocateBuffer();
@@ -77,7 +81,7 @@
size_t slot;
uint64_t seq;
- CreateQueues<uint64_t>();
+ ASSERT_TRUE(CreateQueues<uint64_t>());
for (size_t i = 0; i < nb_buffer; i++) {
AllocateBuffer();
@@ -127,7 +131,7 @@
};
TEST_F(BufferHubQueueTest, TestMetadata) {
- CreateQueues<TestMetadata>();
+ ASSERT_TRUE(CreateQueues<TestMetadata>());
AllocateBuffer();
std::vector<TestMetadata> ms = {
@@ -149,7 +153,7 @@
}
TEST_F(BufferHubQueueTest, TestMetadataMismatch) {
- CreateQueues<int64_t>();
+ ASSERT_TRUE(CreateQueues<int64_t>());
AllocateBuffer();
int64_t mi = 3;
@@ -166,7 +170,7 @@
}
TEST_F(BufferHubQueueTest, TestEnqueue) {
- CreateQueues<int64_t>();
+ ASSERT_TRUE(CreateQueues<int64_t>());
AllocateBuffer();
size_t slot;
@@ -181,7 +185,7 @@
}
TEST_F(BufferHubQueueTest, TestAllocateBuffer) {
- CreateQueues<int64_t>();
+ ASSERT_TRUE(CreateQueues<int64_t>());
size_t s1;
AllocateBuffer();
@@ -227,7 +231,7 @@
TEST_F(BufferHubQueueTest, TestUsageSetMask) {
const int set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;
- CreateQueues<int64_t>(set_mask, 0, 0, 0);
+ ASSERT_TRUE(CreateQueues<int64_t>(set_mask, 0, 0, 0));
// When allocation, leave out |set_mask| from usage bits on purpose.
size_t slot;
@@ -243,7 +247,7 @@
TEST_F(BufferHubQueueTest, TestUsageClearMask) {
const int clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;
- CreateQueues<int64_t>(0, clear_mask, 0, 0);
+ ASSERT_TRUE(CreateQueues<int64_t>(0, clear_mask, 0, 0));
// When allocation, add |clear_mask| into usage bits on purpose.
size_t slot;
@@ -259,7 +263,7 @@
TEST_F(BufferHubQueueTest, TestUsageDenySetMask) {
const int deny_set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;
- CreateQueues<int64_t>(0, 0, deny_set_mask, 0);
+ ASSERT_TRUE(CreateQueues<int64_t>(0, 0, deny_set_mask, 0));
// Now that |deny_set_mask| is illegal, allocation without those bits should
// be able to succeed.
@@ -278,7 +282,7 @@
TEST_F(BufferHubQueueTest, TestUsageDenyClearMask) {
const int deny_clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;
- CreateQueues<int64_t>(0, 0, 0, deny_clear_mask);
+ ASSERT_TRUE(CreateQueues<int64_t>(0, 0, 0, deny_clear_mask));
// Now that clearing |deny_clear_mask| is illegal (i.e. setting these bits are
// mandatory), allocation with those bits should be able to succeed.
diff --git a/libs/vr/libdisplay/display_client.cpp b/libs/vr/libdisplay/display_client.cpp
index 9952e59..6d39cdb 100644
--- a/libs/vr/libdisplay/display_client.cpp
+++ b/libs/vr/libdisplay/display_client.cpp
@@ -160,8 +160,7 @@
return nullptr;
}
- producer_queue_ =
- ProducerQueue::Import<DisplaySurfaceMetadata>(status.take());
+ producer_queue_ = ProducerQueue::Import(status.take());
}
return producer_queue_;
}
diff --git a/libs/vr/libdisplay/video_mesh_surface_client.cpp b/libs/vr/libdisplay/video_mesh_surface_client.cpp
index 04cc194..a2307e5 100644
--- a/libs/vr/libdisplay/video_mesh_surface_client.cpp
+++ b/libs/vr/libdisplay/video_mesh_surface_client.cpp
@@ -30,8 +30,7 @@
return nullptr;
}
- producer_queue_ =
- ProducerQueue::Import<VideoMeshSurfaceBufferMetadata>(status.take());
+ producer_queue_ = ProducerQueue::Import(status.take());
}
return producer_queue_;
}
diff --git a/services/vr/bufferhubd/buffer_hub.cpp b/services/vr/bufferhubd/buffer_hub.cpp
index de4950e..2ce60e5 100644
--- a/services/vr/bufferhubd/buffer_hub.cpp
+++ b/services/vr/bufferhubd/buffer_hub.cpp
@@ -16,7 +16,9 @@
#include "producer_queue_channel.h"
using android::pdx::Channel;
+using android::pdx::ErrorStatus;
using android::pdx::Message;
+using android::pdx::Status;
using android::pdx::rpc::DispatchRemoteMethod;
using android::pdx::default_transport::Endpoint;
@@ -28,9 +30,7 @@
BufferHubService::~BufferHubService() {}
-bool BufferHubService::IsInitialized() const {
- return BASE::IsInitialized();
-}
+bool BufferHubService::IsInitialized() const { return BASE::IsInitialized(); }
std::string BufferHubService::DumpState(size_t /*max_length*/) {
std::ostringstream stream;
@@ -374,7 +374,7 @@
}
}
-int BufferHubService::OnCreateProducerQueue(
+Status<QueueInfo> BufferHubService::OnCreateProducerQueue(
pdx::Message& message, size_t meta_size_bytes, int usage_set_mask,
int usage_clear_mask, int usage_deny_set_mask, int usage_deny_clear_mask) {
// Use the producer channel id as the global queue id.
@@ -386,7 +386,7 @@
if (const auto channel = message.GetChannel<BufferHubChannel>()) {
ALOGE("BufferHubService::OnCreateProducerQueue: already created: queue=%d",
queue_id);
- return -EALREADY;
+ return ErrorStatus(EALREADY);
}
int error;
@@ -394,10 +394,10 @@
this, queue_id, meta_size_bytes, usage_set_mask, usage_clear_mask,
usage_deny_set_mask, usage_deny_clear_mask, &error)) {
message.SetChannel(producer_channel);
- return 0;
+ return {{meta_size_bytes, queue_id}};
} else {
ALOGE("BufferHubService::OnCreateBuffer: Failed to create producer!!");
- return error;
+ return ErrorStatus(-error);
}
}
diff --git a/services/vr/bufferhubd/buffer_hub.h b/services/vr/bufferhubd/buffer_hub.h
index 8a7dca8..150e399 100644
--- a/services/vr/bufferhubd/buffer_hub.h
+++ b/services/vr/bufferhubd/buffer_hub.h
@@ -7,6 +7,7 @@
#include <hardware/gralloc.h>
#include <pdx/service.h>
+#include <private/dvr/bufferhub_rpc.h>
namespace android {
namespace dvr {
@@ -73,9 +74,9 @@
slice_count(slice_count),
name(name) {}
- BufferInfo(int id, size_t consumer_count, size_t capacity, int usage_set_mask,
- int usage_clear_mask, int usage_deny_set_mask,
- int usage_deny_clear_mask)
+ BufferInfo(int id, size_t consumer_count, size_t capacity,
+ int usage_set_mask, int usage_clear_mask,
+ int usage_deny_set_mask, int usage_deny_clear_mask)
: id(id),
type(kProducerQueueType),
consumer_count(consumer_count),
@@ -168,9 +169,9 @@
int format, int usage, size_t meta_size_bytes,
size_t slice_count);
int OnGetPersistentBuffer(pdx::Message& message, const std::string& name);
- int OnCreateProducerQueue(pdx::Message& message, size_t meta_size_bytes,
- int usage_set_mask, int usage_clear_mask,
- int usage_deny_set_mask, int usage_deny_clear_mask);
+ pdx::Status<QueueInfo> OnCreateProducerQueue(
+ pdx::Message& message, size_t meta_size_bytes, int usage_set_mask,
+ int usage_clear_mask, int usage_deny_set_mask, int usage_deny_clear_mask);
BufferHubService(const BufferHubService&) = delete;
void operator=(const BufferHubService&) = delete;
diff --git a/services/vr/bufferhubd/consumer_queue_channel.cpp b/services/vr/bufferhubd/consumer_queue_channel.cpp
index ae87acd..cc16f1f 100644
--- a/services/vr/bufferhubd/consumer_queue_channel.cpp
+++ b/services/vr/bufferhubd/consumer_queue_channel.cpp
@@ -40,6 +40,11 @@
*producer, &ProducerQueueChannel::OnCreateConsumerQueue, message);
return true;
+ case BufferHubRPC::GetQueueInfo::Opcode:
+ DispatchRemoteMethod<BufferHubRPC::GetQueueInfo>(
+ *producer, &ProducerQueueChannel::OnGetQueueInfo, message);
+ return true;
+
case BufferHubRPC::ConsumerQueueImportBuffers::Opcode:
DispatchRemoteMethod<BufferHubRPC::ConsumerQueueImportBuffers>(
*this, &ConsumerQueueChannel::OnConsumerQueueImportBuffers, message);
diff --git a/services/vr/bufferhubd/producer_queue_channel.cpp b/services/vr/bufferhubd/producer_queue_channel.cpp
index 7952642..7741058 100644
--- a/services/vr/bufferhubd/producer_queue_channel.cpp
+++ b/services/vr/bufferhubd/producer_queue_channel.cpp
@@ -3,6 +3,9 @@
#include "consumer_queue_channel.h"
#include "producer_channel.h"
+using android::pdx::ErrorStatus;
+using android::pdx::Message;
+using android::pdx::Status;
using android::pdx::RemoteChannelHandle;
using android::pdx::rpc::DispatchRemoteMethod;
@@ -58,6 +61,11 @@
*this, &ProducerQueueChannel::OnCreateConsumerQueue, message);
return true;
+ case BufferHubRPC::GetQueueInfo::Opcode:
+ DispatchRemoteMethod<BufferHubRPC::GetQueueInfo>(
+ *this, &ProducerQueueChannel::OnGetQueueInfo, message);
+ return true;
+
case BufferHubRPC::ProducerQueueAllocateBuffers::Opcode:
DispatchRemoteMethod<BufferHubRPC::ProducerQueueAllocateBuffers>(
*this, &ProducerQueueChannel::OnProducerQueueAllocateBuffers,
@@ -84,8 +92,8 @@
usage_deny_clear_mask_);
}
-std::pair<RemoteChannelHandle, size_t>
-ProducerQueueChannel::OnCreateConsumerQueue(Message& message) {
+Status<RemoteChannelHandle> ProducerQueueChannel::OnCreateConsumerQueue(
+ Message& message) {
ATRACE_NAME("ProducerQueueChannel::OnCreateConsumerQueue");
ALOGD_IF(TRACE, "ProducerQueueChannel::OnCreateConsumerQueue: channel_id=%d",
channel_id());
@@ -97,7 +105,7 @@
"ProducerQueueChannel::OnCreateConsumerQueue: failed to push consumer "
"channel: %s",
status.GetErrorMessage().c_str());
- REPLY_ERROR_RETURN(message, status.error(), {});
+ return ErrorStatus(ENOMEM);
}
const auto channel_status = service()->SetChannel(
@@ -108,13 +116,17 @@
"ProducerQueueChannel::OnCreateConsumerQueue: failed to set new "
"consumer channel: %s",
channel_status.GetErrorMessage().c_str());
- REPLY_ERROR_RETURN(message, channel_status.error(), {});
+ return ErrorStatus(ENOMEM);
}
- return std::make_pair(status.take(), meta_size_bytes_);
+ return {status.take()};
}
-std::vector<std::pair<RemoteChannelHandle, size_t>>
+Status<QueueInfo> ProducerQueueChannel::OnGetQueueInfo(Message&) {
+ return {{meta_size_bytes_, buffer_id()}};
+}
+
+Status<std::vector<std::pair<RemoteChannelHandle, size_t>>>
ProducerQueueChannel::OnProducerQueueAllocateBuffers(Message& message,
int width, int height,
int format, int usage,
@@ -135,7 +147,7 @@
"not permitted. Violating usage_deny_set_mask, the following bits "
"shall not be set: %d.",
usage, usage_deny_set_mask_);
- REPLY_ERROR_RETURN(message, EINVAL, buffer_handles);
+ return ErrorStatus(EINVAL);
}
if (~usage & usage_deny_clear_mask_) {
@@ -144,7 +156,7 @@
"not permitted. Violating usage_deny_clear_mask, the following bits "
"must be set: %d.",
usage, usage_deny_clear_mask_);
- REPLY_ERROR_RETURN(message, EINVAL, buffer_handles);
+ return ErrorStatus(EINVAL);
}
// Force set mask and clear mask. Note that |usage_set_mask_| takes precedence
@@ -152,24 +164,24 @@
int effective_usage = (usage & ~usage_clear_mask_) | usage_set_mask_;
for (size_t i = 0; i < buffer_count; i++) {
- auto buffer_handle_slot = AllocateBuffer(message, width, height, format,
- effective_usage, slice_count);
- if (!buffer_handle_slot.first) {
+ auto status = AllocateBuffer(message, width, height, format,
+ effective_usage, slice_count);
+ if (!status) {
ALOGE(
- "ProducerQueueChannel::OnProducerQueueAllocateBuffers: failed to "
+ "ProducerQueueChannel::OnProducerQueueAllocateBuffers: Failed to "
"allocate new buffer.");
- REPLY_ERROR_RETURN(message, ENOMEM, buffer_handles);
+ return ErrorStatus(status.error());
}
- buffer_handles.emplace_back(std::move(buffer_handle_slot.first),
- buffer_handle_slot.second);
+ buffer_handles.push_back(status.take());
}
- return buffer_handles;
+ return {std::move(buffer_handles)};
}
-std::pair<RemoteChannelHandle, size_t> ProducerQueueChannel::AllocateBuffer(
- Message& message, int width, int height, int format, int usage,
- size_t slice_count) {
+Status<std::pair<RemoteChannelHandle, size_t>>
+ProducerQueueChannel::AllocateBuffer(Message& message, int width, int height,
+ int format, int usage,
+ size_t slice_count) {
ATRACE_NAME("ProducerQueueChannel::AllocateBuffer");
ALOGD_IF(TRACE,
"ProducerQueueChannel::AllocateBuffer: producer_channel_id=%d",
@@ -177,7 +189,7 @@
if (capacity_ >= BufferHubRPC::kMaxQueueCapacity) {
ALOGE("ProducerQueueChannel::AllocateBuffer: reaches kMaxQueueCapacity.");
- return {};
+ return ErrorStatus(E2BIG);
}
// Here we are creating a new BufferHubBuffer, initialize the producer
@@ -189,7 +201,7 @@
if (!status) {
ALOGE("ProducerQueueChannel::AllocateBuffer: failed to push channel: %s",
status.GetErrorMessage().c_str());
- return {};
+ return ErrorStatus(status.error());
}
ALOGD_IF(TRACE,
@@ -199,14 +211,14 @@
auto buffer_handle = status.take();
int error;
- const auto producer_channel = ProducerChannel::Create(
- service(), buffer_id, width, height, format, usage,
- meta_size_bytes_, slice_count, &error);
+ const auto producer_channel =
+ ProducerChannel::Create(service(), buffer_id, width, height, format,
+ usage, meta_size_bytes_, slice_count, &error);
if (!producer_channel) {
ALOGE(
"ProducerQueueChannel::AllocateBuffer: Failed to create "
"BufferHubBuffer producer!!");
- return {};
+ return ErrorStatus(ENOMEM);
}
ALOGD_IF(
@@ -221,7 +233,7 @@
"ProducerQueueChannel::AllocateBuffer: failed to set producer channel "
"for new BufferHubBuffer: %s",
channel_status.GetErrorMessage().c_str());
- return {};
+ return ErrorStatus(ENOMEM);
}
// Register the newly allocated buffer's channel_id into the first empty
@@ -235,7 +247,7 @@
ALOGE(
"ProducerQueueChannel::AllocateBuffer: Cannot find empty slot for new "
"buffer allocation.");
- return {};
+ return ErrorStatus(E2BIG);
}
buffers_[slot] = producer_channel;
@@ -250,29 +262,29 @@
consumer_channel->RegisterNewBuffer(producer_channel, slot);
}
- return {std::move(buffer_handle), slot};
+ return {{std::move(buffer_handle), slot}};
}
-int ProducerQueueChannel::OnProducerQueueDetachBuffer(Message& /*message*/,
- size_t slot) {
+Status<void> ProducerQueueChannel::OnProducerQueueDetachBuffer(
+ Message& /*message*/, size_t slot) {
if (buffers_[slot].expired()) {
ALOGE(
"ProducerQueueChannel::OnProducerQueueDetachBuffer: trying to detach "
"an invalid buffer producer at slot %zu",
slot);
- return -EINVAL;
+ return ErrorStatus(EINVAL);
}
if (capacity_ == 0) {
ALOGE(
"ProducerQueueChannel::OnProducerQueueDetachBuffer: trying to detach a "
"buffer producer while the queue's capacity is already zero.");
- return -EINVAL;
+ return ErrorStatus(EINVAL);
}
buffers_[slot].reset();
capacity_--;
- return 0;
+ return {};
}
void ProducerQueueChannel::AddConsumer(ConsumerQueueChannel* channel) {
diff --git a/services/vr/bufferhubd/producer_queue_channel.h b/services/vr/bufferhubd/producer_queue_channel.h
index 49611d4..a12a37d 100644
--- a/services/vr/bufferhubd/producer_queue_channel.h
+++ b/services/vr/bufferhubd/producer_queue_channel.h
@@ -3,6 +3,7 @@
#include "buffer_hub.h"
+#include <pdx/status.h>
#include <private/dvr/bufferhub_rpc.h>
namespace android {
@@ -10,17 +11,14 @@
class ProducerQueueChannel : public BufferHubChannel {
public:
- using Message = pdx::Message;
- using RemoteChannelHandle = pdx::RemoteChannelHandle;
-
static std::shared_ptr<ProducerQueueChannel> Create(
BufferHubService* service, int channel_id, size_t meta_size_bytes,
int usage_set_mask, int usage_clear_mask, int usage_deny_set_mask,
int usage_deny_clear_mask, int* error);
~ProducerQueueChannel() override;
- bool HandleMessage(Message& message) override;
- void HandleImpulse(Message& message) override;
+ bool HandleMessage(pdx::Message& message) override;
+ void HandleImpulse(pdx::Message& message) override;
BufferInfo GetBufferInfo() const override;
@@ -28,19 +26,22 @@
// producer queue.
// Returns a handle for the service channel, as well as the size of the
// metadata associated with the queue.
- std::pair<RemoteChannelHandle, size_t> OnCreateConsumerQueue(
- Message& message);
+ pdx::Status<pdx::RemoteChannelHandle> OnCreateConsumerQueue(
+ pdx::Message& message);
+
+ pdx::Status<QueueInfo> OnGetQueueInfo(pdx::Message& message);
// Allocate a new BufferHubProducer according to the input spec. Client may
// handle this as if a new producer is created through kOpCreateBuffer.
- std::vector<std::pair<RemoteChannelHandle, size_t>>
- OnProducerQueueAllocateBuffers(Message& message, int width, int height,
+ pdx::Status<std::vector<std::pair<pdx::RemoteChannelHandle, size_t>>>
+ OnProducerQueueAllocateBuffers(pdx::Message& message, int width, int height,
int format, int usage, size_t slice_count,
size_t buffer_count);
// Detach a BufferHubProducer indicated by |slot|. Note that the buffer must
// be in Gain'ed state for the producer queue to detach.
- int OnProducerQueueDetachBuffer(Message& message, size_t slot);
+ pdx::Status<void> OnProducerQueueDetachBuffer(pdx::Message& message,
+ size_t slot);
void AddConsumer(ConsumerQueueChannel* channel);
void RemoveConsumer(ConsumerQueueChannel* channel);
@@ -56,10 +57,9 @@
// and our return type is a RemoteChannelHandle.
// Returns the remote channdel handle and the slot number for the newly
// allocated buffer.
- std::pair<RemoteChannelHandle, size_t> AllocateBuffer(Message& message,
- int width, int height,
- int format, int usage,
- size_t slice_count);
+ pdx::Status<std::pair<pdx::RemoteChannelHandle, size_t>> AllocateBuffer(
+ pdx::Message& message, int width, int height, int format, int usage,
+ size_t slice_count);
// Size of the meta data associated with all the buffers allocated from the
// queue. Now we assume the metadata size is immutable once the queue is