Simplify ProducerQueue::Create
This is a preparation CL that simplifies ProducerQueue's constructor,
Create(), and bufferhub_rpc::CreateProducerQueue prior to introducing
more queue attributes (is_async, width, height, max_capacity, etc) on
creation.
1/ Consolidate and/or remove unnecessary ProducerQueue::Create's
overloading (we had way too many overloads of the create function and I
figured it's awfully painful to introduce new attributes).
2/ Use UsagePolicy in ProducerQueue::Create. Also added default values
for UsagePolicy, so that empty uniform initialization gives us a default
policy. This helps us removing all ProducerQueue::Create overloading on
whether default usage policy is needed.
3/ Move |meta_size_bytes| into ProducerQueueConfig.
Bug: 38430974
Test: buffer_hub_queue_producer-test, buffer_hub_queue-test, dvr_api-test
Change-Id: Ieba9f4d1bce2162bd1e6063989985afc8d014dc7
diff --git a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
index ffdc9e2..58e4ace 100644
--- a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
+++ b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h
@@ -129,19 +129,26 @@
using LocalFence = FenceHandle<pdx::LocalHandle>;
using BorrowedFence = FenceHandle<pdx::BorrowedHandle>;
-struct QueueInfo {
+struct ProducerQueueConfig {
size_t meta_size_bytes;
+
+ private:
+ PDX_SERIALIZABLE_MEMBERS(ProducerQueueConfig, meta_size_bytes);
+};
+
+struct QueueInfo {
+ ProducerQueueConfig producer_config;
int id;
private:
- PDX_SERIALIZABLE_MEMBERS(QueueInfo, meta_size_bytes, id);
+ PDX_SERIALIZABLE_MEMBERS(QueueInfo, producer_config, id);
};
struct UsagePolicy {
- uint64_t usage_set_mask;
- uint64_t usage_clear_mask;
- uint64_t usage_deny_set_mask;
- uint64_t usage_deny_clear_mask;
+ uint64_t usage_set_mask{0};
+ uint64_t usage_clear_mask{0};
+ uint64_t usage_deny_set_mask{0};
+ uint64_t usage_deny_clear_mask{0};
private:
PDX_SERIALIZABLE_MEMBERS(UsagePolicy, usage_set_mask, usage_clear_mask,
diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
index 1978f41..a3479c1 100644
--- a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
+++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
@@ -10,7 +10,6 @@
#include <pdx/default_transport/client_channel.h>
#include <pdx/default_transport/client_channel_factory.h>
#include <pdx/file_handle.h>
-#include <private/dvr/bufferhub_rpc.h>
#define RETRY_EINTR(fnc_call) \
([&]() -> decltype(fnc_call) { \
@@ -106,7 +105,7 @@
status.GetErrorMessage().c_str());
return ErrorStatus(status.error());
} else {
- SetupQueue(status.get().meta_size_bytes, status.get().id);
+ SetupQueue(status.get().producer_config.meta_size_bytes, status.get().id);
return {};
}
}
@@ -396,9 +395,6 @@
return {std::move(buffer)};
}
-ProducerQueue::ProducerQueue(size_t meta_size)
- : ProducerQueue(meta_size, 0, 0, 0, 0) {}
-
ProducerQueue::ProducerQueue(LocalChannelHandle handle)
: BASE(std::move(handle)) {
auto status = ImportQueue();
@@ -409,14 +405,10 @@
}
}
-ProducerQueue::ProducerQueue(size_t meta_size, uint64_t usage_set_mask,
- uint64_t usage_clear_mask,
- uint64_t usage_deny_set_mask,
- uint64_t usage_deny_clear_mask)
+ProducerQueue::ProducerQueue(size_t meta_size, const UsagePolicy& usage)
: BASE(BufferHubRPC::kClientPath) {
- auto status = InvokeRemoteMethod<BufferHubRPC::CreateProducerQueue>(
- meta_size, UsagePolicy{usage_set_mask, usage_clear_mask,
- usage_deny_set_mask, usage_deny_clear_mask});
+ auto status =
+ InvokeRemoteMethod<BufferHubRPC::CreateProducerQueue>(meta_size, usage);
if (!status) {
ALOGE("ProducerQueue::ProducerQueue: Failed to create producer queue: %s",
status.GetErrorMessage().c_str());
@@ -424,7 +416,7 @@
return;
}
- SetupQueue(status.get().meta_size_bytes, status.get().id);
+ SetupQueue(status.get().producer_config.meta_size_bytes, status.get().id);
}
Status<void> ProducerQueue::AllocateBuffer(uint32_t width, uint32_t height,
diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp
index 932aa37..6d8d0a1 100644
--- a/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp
+++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp
@@ -11,7 +11,8 @@
/* static */
sp<BufferHubQueueProducer> BufferHubQueueProducer::Create() {
sp<BufferHubQueueProducer> producer = new BufferHubQueueProducer;
- producer->queue_ = ProducerQueue::Create<DvrNativeBufferMetadata>();
+ producer->queue_ =
+ ProducerQueue::Create<DvrNativeBufferMetadata>(UsagePolicy{});
return producer;
}
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 d8d326b..85cfaec 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
@@ -5,6 +5,7 @@
#include <pdx/client.h>
#include <pdx/status.h>
+#include <private/dvr/bufferhub_rpc.h>
#include <private/dvr/buffer_hub_client.h>
#include <private/dvr/epoll_file_descriptor.h>
#include <private/dvr/ring_buffer.h>
@@ -222,14 +223,6 @@
class ProducerQueue : public pdx::ClientBase<ProducerQueue, BufferHubQueue> {
public:
- template <typename Meta>
- static std::unique_ptr<ProducerQueue> Create() {
- return BASE::Create(sizeof(Meta));
- }
- static std::unique_ptr<ProducerQueue> Create(size_t meta_size_bytes) {
- return BASE::Create(meta_size_bytes);
- }
-
// Usage bits in |usage_set_mask| will be automatically masked on. Usage bits
// in |usage_clear_mask| will be automatically masked off. Note that
// |usage_set_mask| and |usage_clear_mask| may conflict with each other, but
@@ -242,20 +235,12 @@
// |usage_deny_clear_mask| shall not conflict with each other. Such
// configuration will be treated as invalid input on creation.
template <typename Meta>
- static std::unique_ptr<ProducerQueue> Create(uint32_t usage_set_mask,
- uint32_t usage_clear_mask,
- uint32_t usage_deny_set_mask,
- uint32_t usage_deny_clear_mask) {
- return BASE::Create(sizeof(Meta), usage_set_mask, usage_clear_mask,
- usage_deny_set_mask, usage_deny_clear_mask);
+ static std::unique_ptr<ProducerQueue> Create(const UsagePolicy& usage) {
+ return BASE::Create(sizeof(Meta), usage);
}
static std::unique_ptr<ProducerQueue> Create(size_t meta_size_bytes,
- uint32_t usage_set_mask,
- uint32_t usage_clear_mask,
- uint32_t usage_deny_set_mask,
- uint32_t usage_deny_clear_mask) {
- return BASE::Create(meta_size_bytes, usage_set_mask, usage_clear_mask,
- usage_deny_set_mask, usage_deny_clear_mask);
+ const UsagePolicy& usage) {
+ return BASE::Create(meta_size_bytes, usage);
}
// Import a ProducerQueue from a channel handle.
@@ -305,11 +290,8 @@
// Constructors are automatically exposed through ProducerQueue::Create(...)
// static template methods inherited from ClientBase, which take the same
// arguments as the constructors.
- explicit ProducerQueue(size_t meta_size);
explicit ProducerQueue(pdx::LocalChannelHandle handle);
- ProducerQueue(size_t meta_size, uint64_t usage_set_mask,
- uint64_t usage_clear_mask, uint64_t usage_deny_set_mask,
- uint64_t usage_deny_clear_mask);
+ ProducerQueue(size_t meta_size, const UsagePolicy& usage);
pdx::Status<Entry> OnBufferReady(
const std::shared_ptr<BufferHubBuffer>& buffer, size_t slot) override;
@@ -317,15 +299,9 @@
// Explicit specializations of ProducerQueue::Create for void metadata type.
template <>
-inline std::unique_ptr<ProducerQueue> ProducerQueue::Create<void>() {
- return ProducerQueue::Create(0);
-}
-template <>
inline std::unique_ptr<ProducerQueue> ProducerQueue::Create<void>(
- uint32_t usage_set_mask, uint32_t usage_clear_mask,
- uint32_t usage_deny_set_mask, uint32_t usage_deny_clear_mask) {
- return ProducerQueue::Create(0, usage_set_mask, usage_clear_mask,
- usage_deny_set_mask, usage_deny_clear_mask);
+ const UsagePolicy& usage) {
+ return ProducerQueue::Create(0, usage);
}
class ConsumerQueue : public BufferHubQueue {
diff --git a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
index ff2e146..60785b6 100644
--- a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
+++ b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
@@ -25,13 +25,8 @@
class BufferHubQueueTest : public ::testing::Test {
public:
template <typename Meta>
- bool CreateProducerQueue(uint64_t usage_set_mask = 0,
- uint64_t usage_clear_mask = 0,
- uint64_t usage_deny_set_mask = 0,
- uint64_t usage_deny_clear_mask = 0) {
- producer_queue_ =
- ProducerQueue::Create<Meta>(usage_set_mask, usage_clear_mask,
- usage_deny_set_mask, usage_deny_clear_mask);
+ bool CreateProducerQueue(const UsagePolicy& usage) {
+ producer_queue_ = ProducerQueue::Create<Meta>(usage);
return producer_queue_ != nullptr;
}
@@ -45,13 +40,8 @@
}
template <typename Meta>
- bool CreateQueues(int usage_set_mask = 0, int usage_clear_mask = 0,
- int usage_deny_set_mask = 0,
- int usage_deny_clear_mask = 0) {
- return CreateProducerQueue<Meta>(usage_set_mask, usage_clear_mask,
- usage_deny_set_mask,
- usage_deny_clear_mask) &&
- CreateConsumerQueue();
+ bool CreateQueues(const UsagePolicy& usage) {
+ return CreateProducerQueue<Meta>(usage) && CreateConsumerQueue();
}
void AllocateBuffer(size_t* slot_out = nullptr) {
@@ -74,7 +64,7 @@
TEST_F(BufferHubQueueTest, TestDequeue) {
const size_t nb_dequeue_times = 16;
- ASSERT_TRUE(CreateQueues<size_t>());
+ ASSERT_TRUE(CreateQueues<size_t>(UsagePolicy{}));
// Allocate only one buffer.
AllocateBuffer();
@@ -104,7 +94,7 @@
size_t slot;
uint64_t seq;
- ASSERT_TRUE(CreateQueues<uint64_t>());
+ ASSERT_TRUE(CreateQueues<uint64_t>(UsagePolicy{}));
for (size_t i = 0; i < kBufferCount; i++) {
AllocateBuffer();
@@ -175,7 +165,7 @@
}
TEST_F(BufferHubQueueTest, TestDetach) {
- ASSERT_TRUE(CreateProducerQueue<void>());
+ ASSERT_TRUE(CreateProducerQueue<void>(UsagePolicy{}));
// Allocate buffers.
const size_t kBufferCount = 4u;
@@ -278,7 +268,7 @@
}
TEST_F(BufferHubQueueTest, TestMultipleConsumers) {
- ASSERT_TRUE(CreateProducerQueue<void>());
+ ASSERT_TRUE(CreateProducerQueue<void>(UsagePolicy{}));
// Allocate buffers.
const size_t kBufferCount = 4u;
@@ -356,7 +346,7 @@
};
TEST_F(BufferHubQueueTest, TestMetadata) {
- ASSERT_TRUE(CreateQueues<TestMetadata>());
+ ASSERT_TRUE(CreateQueues<TestMetadata>(UsagePolicy{}));
AllocateBuffer();
std::vector<TestMetadata> ms = {
@@ -382,7 +372,7 @@
}
TEST_F(BufferHubQueueTest, TestMetadataMismatch) {
- ASSERT_TRUE(CreateQueues<int64_t>());
+ ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{}));
AllocateBuffer();
int64_t mi = 3;
@@ -401,7 +391,7 @@
}
TEST_F(BufferHubQueueTest, TestEnqueue) {
- ASSERT_TRUE(CreateQueues<int64_t>());
+ ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{}));
AllocateBuffer();
size_t slot;
@@ -418,7 +408,7 @@
}
TEST_F(BufferHubQueueTest, TestAllocateBuffer) {
- ASSERT_TRUE(CreateQueues<int64_t>());
+ ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{}));
size_t s1;
AllocateBuffer();
@@ -473,7 +463,7 @@
TEST_F(BufferHubQueueTest, TestUsageSetMask) {
const uint32_t set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;
- ASSERT_TRUE(CreateQueues<int64_t>(set_mask, 0, 0, 0));
+ ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{set_mask, 0, 0, 0}));
// When allocation, leave out |set_mask| from usage bits on purpose.
size_t slot;
@@ -491,7 +481,7 @@
TEST_F(BufferHubQueueTest, TestUsageClearMask) {
const uint32_t clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;
- ASSERT_TRUE(CreateQueues<int64_t>(0, clear_mask, 0, 0));
+ ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{0, clear_mask, 0, 0}));
// When allocation, add |clear_mask| into usage bits on purpose.
size_t slot;
@@ -509,7 +499,7 @@
TEST_F(BufferHubQueueTest, TestUsageDenySetMask) {
const uint32_t deny_set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;
- ASSERT_TRUE(CreateQueues<int64_t>(0, 0, deny_set_mask, 0));
+ ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{0, 0, deny_set_mask, 0}));
// Now that |deny_set_mask| is illegal, allocation without those bits should
// be able to succeed.
@@ -529,7 +519,7 @@
TEST_F(BufferHubQueueTest, TestUsageDenyClearMask) {
const uint32_t deny_clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;
- ASSERT_TRUE(CreateQueues<int64_t>(0, 0, 0, deny_clear_mask));
+ ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{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/libdvr/tests/dvr_buffer_queue-test.cpp b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp
index 5d12020..ded8277 100644
--- a/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp
+++ b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp
@@ -26,7 +26,7 @@
protected:
void SetUp() override {
write_queue_ = CreateDvrWriteBufferQueueFromProducerQueue(
- ProducerQueue::Create<TestMeta>(0, 0, 0, 0));
+ ProducerQueue::Create<TestMeta>(UsagePolicy{}));
ASSERT_NE(nullptr, write_queue_);
}
@@ -200,7 +200,7 @@
std::unique_ptr<DvrWriteBufferQueue, decltype(&dvrWriteBufferQueueDestroy)>
write_queue(
CreateDvrWriteBufferQueueFromProducerQueue(
- ProducerQueue::Create<DvrNativeBufferMetadata>(0, 0, 0, 0)),
+ ProducerQueue::Create<DvrNativeBufferMetadata>(UsagePolicy{})),
dvrWriteBufferQueueDestroy);
ASSERT_NE(nullptr, write_queue.get());
diff --git a/libs/vr/libvrflinger/display_surface.cpp b/libs/vr/libvrflinger/display_surface.cpp
index 6917b8c..4dd3e69 100644
--- a/libs/vr/libvrflinger/display_surface.cpp
+++ b/libs/vr/libvrflinger/display_surface.cpp
@@ -207,7 +207,7 @@
surface_id(), meta_size_bytes);
std::lock_guard<std::mutex> autolock(lock_);
- auto producer = ProducerQueue::Create(meta_size_bytes);
+ auto producer = ProducerQueue::Create(meta_size_bytes, UsagePolicy{});
if (!producer) {
ALOGE(
"ApplicationDisplaySurface::OnCreateQueue: Failed to create producer "
@@ -271,7 +271,8 @@
// Inject the hw composer usage flag to enable the display to read the
// buffers.
auto producer = ProducerQueue::Create(
- meta_size_bytes, GraphicBuffer::USAGE_HW_COMPOSER, 0, 0, 0);
+ meta_size_bytes,
+ UsagePolicy{GraphicBuffer::USAGE_HW_COMPOSER, 0, 0, 0});
if (!producer) {
ALOGE(
"DirectDisplaySurface::OnCreateQueue: Failed to create producer "