dvrReadBufferQueueDequeue: allow empty metadata
Currently, when there is no metadata on the read buffer queue, we have
to pass in dummy out_metadata; otherwise the API always return
-EINVAL. This CL allows (nullptr, 0) as legit input and add test to
cover that.
Bug: 62958474
Test: Ran the updated DvrBufferQueueTest
Change-Id: I0ad666006a5d93b67a48b9c34f27e325dfa9a419
diff --git a/libs/vr/libdvr/dvr_buffer_queue.cpp b/libs/vr/libdvr/dvr_buffer_queue.cpp
index aa2ed94..2e1655f 100644
--- a/libs/vr/libdvr/dvr_buffer_queue.cpp
+++ b/libs/vr/libdvr/dvr_buffer_queue.cpp
@@ -313,7 +313,10 @@
int dvrReadBufferQueueDequeue(DvrReadBufferQueue* read_queue, int timeout,
DvrReadBuffer* read_buffer, int* out_fence_fd,
void* out_meta, size_t meta_size_bytes) {
- if (!read_queue || !read_buffer || !out_fence_fd || !out_meta)
+ if (!read_queue || !read_buffer || !out_fence_fd)
+ return -EINVAL;
+
+ if (meta_size_bytes != 0 && !out_meta)
return -EINVAL;
return read_queue->Dequeue(timeout, read_buffer, out_fence_fd, out_meta,
diff --git a/libs/vr/libdvr/include/dvr/dvr_buffer_queue.h b/libs/vr/libdvr/include/dvr/dvr_buffer_queue.h
index caf208d..95c04f1 100644
--- a/libs/vr/libdvr/include/dvr/dvr_buffer_queue.h
+++ b/libs/vr/libdvr/include/dvr/dvr_buffer_queue.h
@@ -124,6 +124,8 @@
// signals the release of underlying buffer. The consumer should wait until
// this fence clears before reading data from it.
// @param out_meta The memory area where a metadata object will be filled.
+// Can be nullptr iff |meta_size_bytes| is zero (i.e., there is no
+// metadata).
// @param meta_size_bytes Size of the metadata object caller expects. If it
// doesn't match the size of actually metadata transported by the buffer
// queue, the method returns -EINVAL.
diff --git a/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp
index 5158612..497b1cb 100644
--- a/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp
+++ b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp
@@ -37,15 +37,11 @@
protected:
void SetUp() override {
- auto config = ProducerQueueConfigBuilder()
- .SetDefaultWidth(kBufferWidth)
- .SetDefaultHeight(kBufferHeight)
- .SetDefaultFormat(kBufferFormat)
- .SetMetadata<TestMeta>()
- .Build();
- write_queue_ =
- new DvrWriteBufferQueue(ProducerQueue::Create(config, UsagePolicy{}));
- ASSERT_NE(nullptr, write_queue_);
+ config_builder_ = ProducerQueueConfigBuilder()
+ .SetDefaultWidth(kBufferWidth)
+ .SetDefaultHeight(kBufferHeight)
+ .SetDefaultFormat(kBufferFormat)
+ .SetMetadata<TestMeta>();
}
void TearDown() override {
@@ -55,6 +51,12 @@
}
}
+ void CreateWriteBufferQueue() {
+ write_queue_ = new DvrWriteBufferQueue(
+ ProducerQueue::Create(config_builder_.Build(), UsagePolicy{}));
+ ASSERT_NE(nullptr, write_queue_);
+ }
+
void AllocateBuffers(size_t buffer_count) {
auto status = write_queue_->producer_queue()->AllocateBuffers(
kBufferWidth, kBufferHeight, kLayerCount, kBufferFormat, kBufferUsage,
@@ -73,18 +75,23 @@
buffer_removed_count_);
}
+ ProducerQueueConfigBuilder config_builder_;
DvrWriteBufferQueue* write_queue_{nullptr};
int buffer_available_count_{0};
int buffer_removed_count_{0};
};
-TEST_F(DvrBufferQueueTest, TestWrite_QueueDestroy) {
+TEST_F(DvrBufferQueueTest, TestWrite_QueueCreateDestroy) {
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+
dvrWriteBufferQueueDestroy(write_queue_);
write_queue_ = nullptr;
}
TEST_F(DvrBufferQueueTest, TestWrite_QueueGetCapacity) {
- AllocateBuffers(kQueueCapacity);
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+ ASSERT_NO_FATAL_FAILURE(AllocateBuffers(kQueueCapacity));
+
size_t capacity = dvrWriteBufferQueueGetCapacity(write_queue_);
ALOGD_IF(TRACE, "TestWrite_QueueGetCapacity, capacity=%zu", capacity);
@@ -92,6 +99,8 @@
}
TEST_F(DvrBufferQueueTest, TestCreateReadQueueFromWriteQueue) {
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+
DvrReadBufferQueue* read_queue = nullptr;
int ret = dvrWriteBufferQueueCreateReadQueue(write_queue_, &read_queue);
@@ -102,6 +111,8 @@
}
TEST_F(DvrBufferQueueTest, TestCreateReadQueueFromReadQueue) {
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+
DvrReadBufferQueue* read_queue1 = nullptr;
DvrReadBufferQueue* read_queue2 = nullptr;
int ret = dvrWriteBufferQueueCreateReadQueue(write_queue_, &read_queue1);
@@ -119,7 +130,8 @@
}
TEST_F(DvrBufferQueueTest, CreateEmptyBuffer) {
- AllocateBuffers(3);
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+ ASSERT_NO_FATAL_FAILURE(AllocateBuffers(3));
DvrReadBuffer* read_buffer = nullptr;
DvrWriteBuffer* write_buffer = nullptr;
@@ -152,6 +164,9 @@
}
TEST_F(DvrBufferQueueTest, TestDequeuePostDequeueRelease) {
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+ ASSERT_NO_FATAL_FAILURE(AllocateBuffers(kQueueCapacity));
+
static constexpr int kTimeout = 0;
DvrReadBufferQueue* read_queue = nullptr;
DvrReadBuffer* rb = nullptr;
@@ -172,8 +187,6 @@
dvrReadBufferCreateEmpty(&rb);
ASSERT_NE(nullptr, rb);
- AllocateBuffers(kQueueCapacity);
-
// Gain buffer for writing.
ret = dvrWriteBufferQueueDequeue(write_queue_, kTimeout, wb, &fence_fd);
ASSERT_EQ(0, ret);
@@ -221,6 +234,8 @@
}
TEST_F(DvrBufferQueueTest, TestGetExternalSurface) {
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+
ANativeWindow* window = nullptr;
// The |write_queue_| doesn't have proper metadata (must be
@@ -251,6 +266,9 @@
// Before each dequeue operation, we resize the buffer queue and expect the
// queue always return buffer with desired dimension.
TEST_F(DvrBufferQueueTest, TestResizeBuffer) {
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+ ASSERT_NO_FATAL_FAILURE(AllocateBuffers(kQueueCapacity));
+
static constexpr int kTimeout = 0;
int fence_fd = -1;
@@ -278,8 +296,6 @@
dvrWriteBufferCreateEmpty(&wb3);
ASSERT_NE(nullptr, wb3);
- AllocateBuffers(kQueueCapacity);
-
// Handle all pending events on the read queue.
ret = dvrReadBufferQueueHandleEvents(read_queue);
ASSERT_EQ(0, ret);
@@ -369,6 +385,71 @@
dvrReadBufferQueueDestroy(read_queue);
}
+TEST_F(DvrBufferQueueTest, DequeueEmptyMetadata) {
+ // Overrides default queue parameters: Empty metadata.
+ config_builder_.SetMetadata<void>();
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+ ASSERT_NO_FATAL_FAILURE(AllocateBuffers(1));
+
+ DvrReadBuffer* rb = nullptr;
+ DvrWriteBuffer* wb = nullptr;
+ dvrReadBufferCreateEmpty(&rb);
+ dvrWriteBufferCreateEmpty(&wb);
+
+ DvrReadBufferQueue* read_queue = nullptr;
+ EXPECT_EQ(0, dvrWriteBufferQueueCreateReadQueue(write_queue_, &read_queue));
+
+ const int kTimeoutMs = 0;
+ int fence_fd = -1;
+ EXPECT_EQ(0, dvrWriteBufferQueueDequeue(write_queue_, 0, wb, &fence_fd));
+
+ EXPECT_EQ(0, dvrWriteBufferPost(wb, /*fence=*/-1, nullptr, 0));
+ EXPECT_EQ(0, dvrWriteBufferClear(wb));
+ dvrWriteBufferDestroy(wb);
+ wb = nullptr;
+
+ // When acquire buffer, it's legit to pass nullptr as out_meta iff metadata
+ // size is Zero.
+ EXPECT_EQ(0, dvrReadBufferQueueDequeue(read_queue, kTimeoutMs, rb, &fence_fd,
+ nullptr, 0));
+ EXPECT_TRUE(dvrReadBufferIsValid(rb));
+}
+
+TEST_F(DvrBufferQueueTest, DequeueMismatchMetadata) {
+ ASSERT_NO_FATAL_FAILURE(CreateWriteBufferQueue());
+ ASSERT_NO_FATAL_FAILURE(AllocateBuffers(1));
+
+ DvrReadBuffer* rb = nullptr;
+ DvrWriteBuffer* wb = nullptr;
+ dvrReadBufferCreateEmpty(&rb);
+ dvrWriteBufferCreateEmpty(&wb);
+
+ DvrReadBufferQueue* read_queue = nullptr;
+ EXPECT_EQ(0, dvrWriteBufferQueueCreateReadQueue(write_queue_, &read_queue));
+
+ const int kTimeoutMs = 0;
+ int fence_fd = -1;
+ EXPECT_EQ(0, dvrWriteBufferQueueDequeue(write_queue_, 0, wb, &fence_fd));
+
+ TestMeta seq = 42U;
+ EXPECT_EQ(0, dvrWriteBufferPost(wb, /*fence=*/-1, &seq, sizeof(seq)));
+ EXPECT_EQ(0, dvrWriteBufferClear(wb));
+ dvrWriteBufferDestroy(wb);
+ wb = nullptr;
+
+ // Dequeue with wrong metadata will cause EINVAL.
+ int8_t wrong_metadata;
+ EXPECT_EQ(-EINVAL,
+ dvrReadBufferQueueDequeue(read_queue, kTimeoutMs, rb, &fence_fd,
+ &wrong_metadata, sizeof(wrong_metadata)));
+ EXPECT_FALSE(dvrReadBufferIsValid(rb));
+
+ // Dequeue with empty metadata will cause EINVAL.
+ EXPECT_EQ(-EINVAL, dvrReadBufferQueueDequeue(read_queue, kTimeoutMs, rb,
+ &fence_fd, nullptr, 0));
+ EXPECT_FALSE(dvrReadBufferIsValid(rb));
+}
+
} // namespace
} // namespace dvr