Add shared memory based buffer metadata
This CLs reduces BufferHub CPU consumption by adding asynchronous
state transition so that out-of-process VR composition can run on 2016
pixel devices smoothly. In addition, this CL addresses a couple corner
cases in the existing bufferhub logic, which fixes various blackscreen
issues.
1/ Tracks buffer transition states (gained, posted, acquired, released)
from the client side via atomic shared memory and adds
PostAsync/AcquireAsync/ReleaseAsync/GainAsync with metadata and
fence support.
2/ Adds dequeue order guarantee for buffers enqueued with
dvrWriteBufferQueuePostBuffer.
3/ Synchronous BuffeHub operations are still supported.
4/ Bump up the bufferhubd's soft limit of open file descriptor.
5/ Handle orphaned consumer in acquired state. This is a corner case
that consumer process goes aways (most likely due to a crash) leaving
buffer stuck in acquired state with inconsistent buffer state.
6/ Fixes a race condition for released buffer to be Gain'ed and
Acquire'd when a new consumer is created in released state.
7/ Improve silent consumer queue efficiency: Silent queues no longer
import buffers or receive signals about new buffers and they are
limited to only spawning other consumers and notifications about
producers hanging up.
8/ Modify PDX/UDS channel event signaling to work around epoll
behavior. PDX UDS uses a combination of an eventfd and an epoll set
to simulate the original PDX transport channel events. An odd
behavior discovered in the kernel implementation of epoll was found
that causes the epoll fd to "unsignal" itself whenever epoll_wait()
is called on it, regardless of whether it should still be
pending. This breaks the edge triggerd behavior in nested epoll sets
that channel events depend on. Since this is unlikely to ever be
fixed in the kernel we work around the behavior by using the epoll
set only as a logical OR of two eventfds and never calling
epoll_wait() on it. When polling is required we use regluar poll()
with the eventfds and data fd to avoid the bad behavior in
epoll_wait().
9/ Keep reading data after PDX hangup signal. UDS will signal hangup
when the other end of the socket closes. However, data could still be
in the kerenl buffer and should be consumed. Fix an issue where the
service misses an impulse sent right before the socket is closed.
Bug: 65455724
Bug: 65458354
Bug: 65458312
Bug: 64027135
Bug: 67424527
Test: libpdx_uds_tests
bufferhub_tests
buffer_hub_queue-test
buffer_hub_queue_producer-test
dvr_api-test
Change-Id: Id07db1f206ccf4e06f7ee3c671193334408971ca
diff --git a/libs/vr/libdvr/dvr_buffer_queue.cpp b/libs/vr/libdvr/dvr_buffer_queue.cpp
index 035252d..09a49dd 100644
--- a/libs/vr/libdvr/dvr_buffer_queue.cpp
+++ b/libs/vr/libdvr/dvr_buffer_queue.cpp
@@ -27,15 +27,6 @@
format_(producer_queue->default_format()) {}
int DvrWriteBufferQueue::GetNativeWindow(ANativeWindow** out_window) {
- if (producer_queue_->metadata_size() != sizeof(DvrNativeBufferMetadata)) {
- ALOGE(
- "DvrWriteBufferQueue::GetNativeWindow: The size of buffer metadata "
- "(%zu) of the write queue does not match of size of "
- "DvrNativeBufferMetadata (%zu).",
- producer_queue_->metadata_size(), sizeof(DvrNativeBufferMetadata));
- return -EINVAL;
- }
-
if (native_window_ == nullptr) {
// Lazy creation of |native_window|, as not everyone is using
// DvrWriteBufferQueue as an external surface.
@@ -63,10 +54,27 @@
}
int DvrWriteBufferQueue::Dequeue(int timeout, DvrWriteBuffer* write_buffer,
- int* out_fence_fd, size_t* out_slot) {
+ int* out_fence_fd) {
+ DvrNativeBufferMetadata meta;
+ DvrWriteBuffer* buffer = nullptr;
+ int fence_fd = -1;
+ if (const int ret = GainBuffer(timeout, &buffer, &meta, &fence_fd))
+ return ret;
+ if (!buffer)
+ return -ENOMEM;
+
+ write_buffers_[buffer->slot].reset(buffer);
+ write_buffer->write_buffer = std::move(buffer->write_buffer);
+ *out_fence_fd = fence_fd;
+ return 0;
+}
+
+int DvrWriteBufferQueue::GainBuffer(int timeout,
+ DvrWriteBuffer** out_write_buffer,
+ DvrNativeBufferMetadata* out_meta,
+ int* out_fence_fd) {
size_t slot;
- pdx::LocalHandle fence;
- std::shared_ptr<BufferProducer> buffer_producer;
+ pdx::LocalHandle release_fence;
// Need to retry N+1 times, where N is total number of buffers in the queue.
// As in the worst case, we will dequeue all N buffers and reallocate them, on
@@ -75,15 +83,29 @@
size_t retry = 0;
for (; retry < max_retries; retry++) {
- auto buffer_status = producer_queue_->Dequeue(timeout, &slot, &fence);
+ auto buffer_status =
+ producer_queue_->Dequeue(timeout, &slot, out_meta, &release_fence);
if (!buffer_status) {
ALOGE_IF(buffer_status.error() != ETIMEDOUT,
- "DvrWriteBufferQueue::Dequeue: Failed to dequeue buffer: %s",
+ "DvrWriteBufferQueue::GainBuffer: Failed to dequeue buffer: %s",
buffer_status.GetErrorMessage().c_str());
return -buffer_status.error();
}
- buffer_producer = buffer_status.take();
+ if (write_buffers_[slot] == nullptr) {
+ // Lazy initialization of a write_buffers_ slot. Note that a slot will
+ // only be dynamically allocated once during the entire cycle life of a
+ // queue.
+ write_buffers_[slot] = std::make_unique<DvrWriteBuffer>();
+ write_buffers_[slot]->slot = slot;
+ }
+
+ LOG_ALWAYS_FATAL_IF(
+ write_buffers_[slot]->write_buffer,
+ "DvrWriteBufferQueue::GainBuffer: Buffer slot is not empty: %zu", slot);
+ write_buffers_[slot]->write_buffer = std::move(buffer_status.take());
+
+ const auto& buffer_producer = write_buffers_[slot]->write_buffer;
if (!buffer_producer)
return -ENOMEM;
@@ -122,6 +144,9 @@
remove_status.GetErrorMessage().c_str());
return -remove_status.error();
}
+ // Make sure that the previously allocated buffer is dereferenced from
+ // write_buffers_ array.
+ write_buffers_[slot]->write_buffer = nullptr;
auto allocate_status = producer_queue_->AllocateBuffer(
width_, height_, old_layer_count, format_, old_usage);
@@ -139,46 +164,8 @@
return -ENOMEM;
}
- write_buffer->write_buffer = std::move(buffer_producer);
- *out_fence_fd = fence.Release();
- if (out_slot) {
- // TODO(b/65469368): Remove this null check once dvrWriteBufferQueueDequeue
- // is deprecated.
- *out_slot = slot;
- }
- return 0;
-}
-
-int DvrWriteBufferQueue::GainBuffer(int timeout,
- DvrWriteBuffer** out_write_buffer,
- DvrNativeBufferMetadata* out_meta,
- int* out_fence_fd) {
- DvrWriteBuffer write_buffer;
- int fence_fd;
- size_t slot;
- const int ret = Dequeue(timeout, &write_buffer, &fence_fd, &slot);
- if (ret < 0) {
- ALOGE_IF(
- ret != -ETIMEDOUT,
- "DvrWriteBufferQueue::GainBuffer: Failed to dequeue buffer, ret=%d",
- ret);
- return ret;
- }
-
- if (write_buffers_[slot] == nullptr) {
- // Lazy initialization of a write_buffers_ slot. Note that a slot will only
- // be dynamically allocated once during the entire cycle life of a queue.
- write_buffers_[slot] = std::make_unique<DvrWriteBuffer>();
- write_buffers_[slot]->slot = slot;
- }
-
- LOG_ALWAYS_FATAL_IF(
- write_buffers_[slot]->write_buffer,
- "DvrWriteBufferQueue::GainBuffer: Buffer slot is not empty: %zu", slot);
- write_buffers_[slot]->write_buffer = std::move(write_buffer.write_buffer);
-
*out_write_buffer = write_buffers_[slot].release();
- *out_fence_fd = fence_fd;
+ *out_fence_fd = release_fence.Release();
return 0;
}
@@ -202,14 +189,16 @@
}
if (write_buffer->write_buffer->id() != producer_queue_->GetBufferId(slot)) {
ALOGE(
- "DvrWriteBufferQueue::PostBuffer: Buffer to be released does not "
- "belong to this buffer queue.");
+ "DvrWriteBufferQueue::PostBuffer: Buffer to be posted does not "
+ "belong to this buffer queue. Posting buffer: id=%d, buffer in "
+ "queue: id=%d",
+ write_buffer->write_buffer->id(), producer_queue_->GetBufferId(slot));
return -EINVAL;
}
+ write_buffer->write_buffer->SetQueueIndex(next_post_index_++);
pdx::LocalHandle fence(ready_fence_fd);
- // TODO(b/65455724): All BufferHub operations should be async.
- const int ret = write_buffer->write_buffer->Post(fence, meta, sizeof(*meta));
+ const int ret = write_buffer->write_buffer->PostAsync(meta, fence);
if (ret < 0) {
ALOGE("DvrWriteBufferQueue::PostBuffer: Failed to post buffer, ret=%d",
ret);
@@ -316,8 +305,7 @@
if (!write_queue || !write_buffer || !out_fence_fd)
return -EINVAL;
- // TODO(b/65469368): Deprecate this API once new GainBuffer API is in use.
- return write_queue->Dequeue(timeout, write_buffer, out_fence_fd, nullptr);
+ return write_queue->Dequeue(timeout, write_buffer, out_fence_fd);
}
int dvrWriteBufferQueueGainBuffer(DvrWriteBufferQueue* write_queue, int timeout,
@@ -370,8 +358,8 @@
}
int DvrReadBufferQueue::Dequeue(int timeout, DvrReadBuffer* read_buffer,
- int* out_fence_fd, size_t* out_slot,
- void* out_meta, size_t meta_size_bytes) {
+ int* out_fence_fd, void* out_meta,
+ size_t meta_size_bytes) {
if (meta_size_bytes != consumer_queue_->metadata_size()) {
ALOGE(
"DvrReadBufferQueue::Dequeue: Invalid metadata size, expected (%zu), "
@@ -394,11 +382,6 @@
read_buffer->read_buffer = buffer_status.take();
*out_fence_fd = acquire_fence.Release();
- if (out_slot) {
- // TODO(b/65469368): Remove this null check once dvrReadBufferQueueDequeue
- // is deprecated.
- *out_slot = slot;
- }
return 0;
}
@@ -406,17 +389,15 @@
DvrReadBuffer** out_read_buffer,
DvrNativeBufferMetadata* out_meta,
int* out_fence_fd) {
- DvrReadBuffer read_buffer;
- int fence_fd;
size_t slot;
- const int ret = Dequeue(timeout, &read_buffer, &fence_fd, &slot, out_meta,
- sizeof(*out_meta));
- if (ret < 0) {
- ALOGE_IF(
- ret != -ETIMEDOUT,
- "DvrReadBufferQueue::AcquireBuffer: Failed to dequeue buffer, error=%d",
- ret);
- return ret;
+ pdx::LocalHandle acquire_fence;
+ auto buffer_status =
+ consumer_queue_->Dequeue(timeout, &slot, out_meta, &acquire_fence);
+ if (!buffer_status) {
+ ALOGE_IF(buffer_status.error() != ETIMEDOUT,
+ "DvrReadBufferQueue::AcquireBuffer: Failed to dequeue buffer: %s",
+ buffer_status.GetErrorMessage().c_str());
+ return -buffer_status.error();
}
if (read_buffers_[slot] == nullptr) {
@@ -429,10 +410,10 @@
LOG_FATAL_IF(
read_buffers_[slot]->read_buffer,
"DvrReadBufferQueue::AcquireBuffer: Buffer slot is not empty: %zu", slot);
- read_buffers_[slot]->read_buffer = std::move(read_buffer.read_buffer);
+ read_buffers_[slot]->read_buffer = std::move(buffer_status.take());
*out_read_buffer = read_buffers_[slot].release();
- *out_fence_fd = fence_fd;
+ *out_fence_fd = acquire_fence.Release();
return 0;
}
@@ -457,20 +438,14 @@
if (read_buffer->read_buffer->id() != consumer_queue_->GetBufferId(slot)) {
ALOGE(
"DvrReadBufferQueue::ReleaseBuffer: Buffer to be released does not "
- "belong to this buffer queue.");
+ "belong to this buffer queue. Releasing buffer: id=%d, buffer in "
+ "queue: id=%d",
+ read_buffer->read_buffer->id(), consumer_queue_->GetBufferId(slot));
return -EINVAL;
}
pdx::LocalHandle fence(release_fence_fd);
- int ret = 0;
- if (fence) {
- ret = read_buffer->read_buffer->Release(fence);
- } else {
- // TODO(b/65458354): Send metadata back to producer once shared memory based
- // metadata is implemented.
- // TODO(b/65455724): All BufferHub operations should be async.
- ret = read_buffer->read_buffer->ReleaseAsync();
- }
+ int ret = read_buffer->read_buffer->ReleaseAsync(meta, fence);
if (ret < 0) {
ALOGE("DvrReadBufferQueue::ReleaseBuffer: Failed to release buffer, ret=%d",
ret);
@@ -559,9 +534,8 @@
if (meta_size_bytes != 0 && !out_meta)
return -EINVAL;
- // TODO(b/65469368): Deprecate this API once new AcquireBuffer API is in use.
- return read_queue->Dequeue(timeout, read_buffer, out_fence_fd, nullptr,
- out_meta, meta_size_bytes);
+ return read_queue->Dequeue(timeout, read_buffer, out_fence_fd, out_meta,
+ meta_size_bytes);
}
int dvrReadBufferQueueAcquireBuffer(DvrReadBufferQueue* read_queue, int timeout,
diff --git a/libs/vr/libdvr/dvr_buffer_queue_internal.h b/libs/vr/libdvr/dvr_buffer_queue_internal.h
index f9c0bfd..e53a686 100644
--- a/libs/vr/libdvr/dvr_buffer_queue_internal.h
+++ b/libs/vr/libdvr/dvr_buffer_queue_internal.h
@@ -42,8 +42,7 @@
int GetNativeWindow(ANativeWindow** out_window);
int CreateReadQueue(DvrReadBufferQueue** out_read_queue);
- int Dequeue(int timeout, DvrWriteBuffer* write_buffer, int* out_fence_fd,
- size_t* out_slot);
+ int Dequeue(int timeout, DvrWriteBuffer* write_buffer, int* out_fence_fd);
int GainBuffer(int timeout, DvrWriteBuffer** out_write_buffer,
DvrNativeBufferMetadata* out_meta, int* out_fence_fd);
int PostBuffer(DvrWriteBuffer* write_buffer,
@@ -55,6 +54,7 @@
std::array<std::unique_ptr<DvrWriteBuffer>, BufferHubQueue::kMaxQueueCapacity>
write_buffers_;
+ int64_t next_post_index_ = 0;
uint32_t width_;
uint32_t height_;
uint32_t format_;
@@ -75,7 +75,7 @@
int CreateReadQueue(DvrReadBufferQueue** out_read_queue);
int Dequeue(int timeout, DvrReadBuffer* read_buffer, int* out_fence_fd,
- size_t* out_slot, void* out_meta, size_t meta_size_bytes);
+ void* out_meta, size_t user_metadata_size);
int AcquireBuffer(int timeout, DvrReadBuffer** out_read_buffer,
DvrNativeBufferMetadata* out_meta, int* out_fence_fd);
int ReleaseBuffer(DvrReadBuffer* read_buffer,
diff --git a/libs/vr/libdvr/include/dvr/dvr_api.h b/libs/vr/libdvr/include/dvr/dvr_api.h
index 8f45ce7..499b7c1 100644
--- a/libs/vr/libdvr/include/dvr/dvr_api.h
+++ b/libs/vr/libdvr/include/dvr/dvr_api.h
@@ -15,6 +15,12 @@
extern "C" {
#endif
+#ifdef __GNUC__
+#define ALIGNED_DVR_STRUCT(x) __attribute__((packed, aligned(x)))
+#else
+#define ALIGNED_DVR_STRUCT(x)
+#endif
+
typedef struct ANativeWindow ANativeWindow;
typedef struct DvrPoseAsync DvrPoseAsync;
@@ -367,7 +373,24 @@
// existing data members. If new fields need to be added, please take extra care
// to make sure that new data field is padded properly the size of the struct
// stays same.
-struct DvrNativeBufferMetadata {
+struct ALIGNED_DVR_STRUCT(8) DvrNativeBufferMetadata {
+#ifdef __cplusplus
+ DvrNativeBufferMetadata()
+ : timestamp(0),
+ is_auto_timestamp(0),
+ dataspace(0),
+ crop_left(0),
+ crop_top(0),
+ crop_right(0),
+ crop_bottom(0),
+ scaling_mode(0),
+ transform(0),
+ index(0),
+ user_metadata_size(0),
+ user_metadata_ptr(0),
+ release_fence_mask(0),
+ reserved{0} {}
+#endif
// Timestamp of the frame.
int64_t timestamp;
@@ -391,10 +414,32 @@
// android/native_window.h
int32_t transform;
- // Reserved bytes for so that the struct is forward compatible.
- int32_t reserved[16];
+ // The index of the frame.
+ int64_t index;
+
+ // Size of additional metadata requested by user.
+ uint64_t user_metadata_size;
+
+ // Raw memory address of the additional user defined metadata. Only valid when
+ // user_metadata_size is non-zero.
+ uint64_t user_metadata_ptr;
+
+ // Only applicable for metadata retrieved from GainAsync. This indicates which
+ // consumer has pending fence that producer should epoll on.
+ uint64_t release_fence_mask;
+
+ // Reserved bytes for so that the struct is forward compatible and padding to
+ // 104 bytes so the size is a multiple of 8.
+ int32_t reserved[8];
};
+#ifdef __cplusplus
+// Warning: DvrNativeBufferMetadata is part of the DVR API and changing its size
+// will cause compatiblity issues between different DVR API releases.
+static_assert(sizeof(DvrNativeBufferMetadata) == 104,
+ "Unexpected size for DvrNativeBufferMetadata");
+#endif
+
struct DvrApi_v1 {
// Defines an API entry for V1 (no version suffix).
#define DVR_V1_API_ENTRY(name) Dvr##name##Ptr name
diff --git a/libs/vr/libdvr/tests/Android.bp b/libs/vr/libdvr/tests/Android.bp
index a9302a7..887766a 100644
--- a/libs/vr/libdvr/tests/Android.bp
+++ b/libs/vr/libdvr/tests/Android.bp
@@ -42,6 +42,7 @@
"dvr_named_buffer-test.cpp",
],
+ header_libs: ["libdvr_headers"],
static_libs: static_libraries,
shared_libs: shared_libraries,
cflags: [
diff --git a/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp
index f1c5e48..62cd8d4 100644
--- a/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp
+++ b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp
@@ -131,7 +131,7 @@
DvrWriteBuffer* wb = nullptr;
EXPECT_FALSE(dvrWriteBufferIsValid(wb));
- DvrNativeBufferMetadata meta = {0};
+ DvrNativeBufferMetadata meta;
int fence_fd = -1;
ret = dvrWriteBufferQueueGainBuffer(write_queue_, /*timeout=*/0, &wb, &meta,
&fence_fd);
@@ -150,8 +150,8 @@
DvrReadBufferQueue* read_queue = nullptr;
DvrReadBuffer* rb = nullptr;
DvrWriteBuffer* wb = nullptr;
- DvrNativeBufferMetadata meta1 = {0};
- DvrNativeBufferMetadata meta2 = {0};
+ DvrNativeBufferMetadata meta1;
+ DvrNativeBufferMetadata meta2;
int fence_fd = -1;
ret = dvrWriteBufferQueueCreateReadQueue(write_queue_, &read_queue);
@@ -180,7 +180,7 @@
wb = nullptr;
// Acquire buffer for reading.
- ret = dvrReadBufferQueueAcquireBuffer(read_queue, /*timeout=*/0, &rb, &meta2,
+ ret = dvrReadBufferQueueAcquireBuffer(read_queue, /*timeout=*/10, &rb, &meta2,
&fence_fd);
ASSERT_EQ(ret, 0);
ASSERT_NE(rb, nullptr);
@@ -245,7 +245,7 @@
int fence_fd = -1;
- DvrNativeBufferMetadata meta = {0};
+ DvrNativeBufferMetadata meta;
DvrReadBufferQueue* read_queue = nullptr;
DvrWriteBuffer* wb1 = nullptr;
DvrWriteBuffer* wb2 = nullptr;
@@ -400,7 +400,7 @@
// This test runs the following operations many many times. Thus we prefer to
// use ASSERT_XXX rather than EXPECT_XXX to avoid spamming the output.
std::function<void(size_t i)> Gain = [&](size_t i) {
- int ret = dvrWriteBufferQueueGainBuffer(write_queue_, /*timeout=*/0,
+ int ret = dvrWriteBufferQueueGainBuffer(write_queue_, /*timeout=*/10,
&wbs[i], &metas[i], &fence_fd);
ASSERT_EQ(ret, 0);
ASSERT_LT(fence_fd, 0); // expect invalid fence.
@@ -434,7 +434,7 @@
};
std::function<void(size_t i)> Acquire = [&](size_t i) {
- int ret = dvrReadBufferQueueAcquireBuffer(read_queue, /*timeout=*/0,
+ int ret = dvrReadBufferQueueAcquireBuffer(read_queue, /*timeout=*/10,
&rbs[i], &metas[i], &fence_fd);
ASSERT_EQ(ret, 0);
ASSERT_LT(fence_fd, 0); // expect invalid fence.