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