From: Jiwen 'Steve' Cai Date: Thu, 25 May 2017 06:16:54 +0000 (-0700) Subject: buffer_hub_queue_client: Batch allocate buffers X-Git-Tag: android-x86-9.0-r1~414^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=8fa4e107ab6cbc24c0e54d44db3341f006fe939a;p=android-x86%2Fframeworks-native.git buffer_hub_queue_client: Batch allocate buffers Bug: 36147743 Test: buffer_hub_queue-test, buffer_hub_queue_producer-test, dvr_api-test Change-Id: I40a9babfa8d28d4496e27a7ccecb7ae1b9bc7bd5 --- diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp index a53c8b001c..f0ef299c86 100644 --- a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp +++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp @@ -442,46 +442,78 @@ ProducerQueue::ProducerQueue(const ProducerQueueConfig& config, SetupQueue(status.get()); } -Status ProducerQueue::AllocateBuffer(uint32_t width, uint32_t height, - uint32_t layer_count, - uint32_t format, uint64_t usage, - size_t* out_slot) { - if (out_slot == nullptr) { - ALOGE("ProducerQueue::AllocateBuffer: Parameter out_slot cannot be null."); - return ErrorStatus(EINVAL); - } - - if (is_full()) { - ALOGE("ProducerQueue::AllocateBuffer queue is at maximum capacity: %zu", - capacity()); +Status> ProducerQueue::AllocateBuffers( + uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, + uint64_t usage, size_t buffer_count) { + if (capacity() + buffer_count > kMaxQueueCapacity) { + ALOGE( + "ProducerQueue::AllocateBuffers: queue is at capacity: %zu, cannot " + "allocate %zu more buffer(s).", + capacity(), buffer_count); return ErrorStatus(E2BIG); } - const size_t kBufferCount = 1u; Status>> status = InvokeRemoteMethod( - width, height, layer_count, format, usage, kBufferCount); + width, height, layer_count, format, usage, buffer_count); if (!status) { - ALOGE("ProducerQueue::AllocateBuffer failed to create producer buffer: %s", + ALOGE("ProducerQueue::AllocateBuffers: failed to allocate buffers: %s", status.GetErrorMessage().c_str()); return status.error_status(); } auto buffer_handle_slots = status.take(); - LOG_ALWAYS_FATAL_IF(buffer_handle_slots.size() != kBufferCount, + LOG_ALWAYS_FATAL_IF(buffer_handle_slots.size() != buffer_count, "BufferHubRPC::ProducerQueueAllocateBuffers should " - "return one and only one buffer handle."); + "return %zu buffer handle(s), but returned %zu instead.", + buffer_count, buffer_handle_slots.size()); + + std::vector buffer_slots; + buffer_slots.reserve(buffer_count); + + // Bookkeeping for each buffer. + for (auto& hs : buffer_handle_slots) { + auto& buffer_handle = hs.first; + size_t buffer_slot = hs.second; + + // Note that import might (though very unlikely) fail. If so, buffer_handle + // will be closed and included in returned buffer_slots. + if (AddBuffer(BufferProducer::Import(std::move(buffer_handle)), + buffer_slot)) { + ALOGD_IF(TRACE, "ProducerQueue::AllocateBuffers: new buffer at slot: %zu", + buffer_slot); + buffer_slots.push_back(buffer_slot); + } + } + if (buffer_slots.size() == 0) { + // Error out if no buffer is allocated and improted. + ALOGE(TRACE, "ProducerQueue::AllocateBuffers: no buffer allocated."); + ErrorStatus(ENOMEM); + } + + return {std::move(buffer_slots)}; +} + +Status ProducerQueue::AllocateBuffer(uint32_t width, uint32_t height, + uint32_t layer_count, + uint32_t format, uint64_t usage) { // We only allocate one buffer at a time. - auto& buffer_handle = buffer_handle_slots[0].first; - size_t buffer_slot = buffer_handle_slots[0].second; - ALOGD_IF(TRACE, - "ProducerQueue::AllocateBuffer, new buffer, channel_handle: %d", - buffer_handle.value()); + constexpr size_t buffer_count = 1; + auto status = + AllocateBuffers(width, height, layer_count, format, usage, buffer_count); + if (!status) { + ALOGE("ProducerQueue::AllocateBuffer: Failed to allocate buffer: %s", + status.GetErrorMessage().c_str()); + return status.error_status(); + } + + if (status.get().size() == 0) { + ALOGE(TRACE, "ProducerQueue::AllocateBuffer: no buffer allocated."); + ErrorStatus(ENOMEM); + } - *out_slot = buffer_slot; - return AddBuffer(BufferProducer::Import(std::move(buffer_handle)), - buffer_slot); + return {status.get()[0]}; } Status ProducerQueue::AddBuffer( diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp index fca5eca0d1..0f75a5c3d8 100644 --- a/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp +++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp @@ -612,9 +612,8 @@ status_t BufferHubQueueProducer::AllocateBuffer(uint32_t width, uint32_t height, uint32_t layer_count, PixelFormat format, uint64_t usage) { - size_t slot; auto status = - queue_->AllocateBuffer(width, height, layer_count, format, usage, &slot); + queue_->AllocateBuffer(width, height, layer_count, format, usage); if (!status) { ALOGE( "BufferHubQueueProducer::AllocateBuffer: Failed to allocate buffer: %s", @@ -622,6 +621,7 @@ status_t BufferHubQueueProducer::AllocateBuffer(uint32_t width, uint32_t height, return NO_MEMORY; } + size_t slot = status.get(); auto buffer_producer = queue_->GetBuffer(slot); LOG_ALWAYS_FATAL_IF(buffer_producer == nullptr, 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 912cee0309..d57c7af882 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 @@ -274,12 +274,20 @@ class ProducerQueue : public pdx::ClientBase { BufferHubQueue::GetBuffer(slot)); } + // Batch allocate buffers. Once allocated, producer buffers are automatically + // enqueue'd into the ProducerQueue and available to use (i.e. in GAINED + // state). Upon success, returns a list of slots for each buffer allocated. + pdx::Status> AllocateBuffers( + uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format, + uint64_t usage, size_t buffer_count); + // Allocate producer buffer to populate the queue. Once allocated, a producer // buffer is automatically enqueue'd into the ProducerQueue and available to - // use (i.e. in GAINED state). - pdx::Status AllocateBuffer(uint32_t width, uint32_t height, - uint32_t layer_count, uint32_t format, - uint64_t usage, size_t* out_slot); + // use (i.e. in GAINED state). Upon success, returns the slot number for the + // buffer allocated. + pdx::Status AllocateBuffer(uint32_t width, uint32_t height, + uint32_t layer_count, uint32_t format, + uint64_t usage); // Add a producer buffer to populate the queue. Once added, a producer buffer // is available to use (i.e. in GAINED state). diff --git a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp index 228212e37a..e0a4052ec2 100644 --- a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp +++ b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp @@ -46,12 +46,12 @@ class BufferHubQueueTest : public ::testing::Test { void AllocateBuffer(size_t* slot_out = nullptr) { // Create producer buffer. - size_t slot; auto status = producer_queue_->AllocateBuffer( kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat, - kBufferUsage, &slot); - ASSERT_TRUE(status.ok()); + kBufferUsage); + ASSERT_TRUE(status.ok()); + size_t slot = status.take(); if (slot_out) *slot_out = slot; } @@ -478,13 +478,13 @@ TEST_F(BufferHubQueueTest, TestUsageSetMask) { UsagePolicy{set_mask, 0, 0, 0})); // When allocation, leave out |set_mask| from usage bits on purpose. - size_t slot; auto status = producer_queue_->AllocateBuffer( kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat, - kBufferUsage & ~set_mask, &slot); + kBufferUsage & ~set_mask); ASSERT_TRUE(status.ok()); LocalHandle fence; + size_t slot; auto p1_status = producer_queue_->Dequeue(0, &slot, &fence); ASSERT_TRUE(p1_status.ok()); auto p1 = p1_status.take(); @@ -497,13 +497,13 @@ TEST_F(BufferHubQueueTest, TestUsageClearMask) { UsagePolicy{0, clear_mask, 0, 0})); // When allocation, add |clear_mask| into usage bits on purpose. - size_t slot; auto status = producer_queue_->AllocateBuffer( kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat, - kBufferUsage | clear_mask, &slot); + kBufferUsage | clear_mask); ASSERT_TRUE(status.ok()); LocalHandle fence; + size_t slot; auto p1_status = producer_queue_->Dequeue(0, &slot, &fence); ASSERT_TRUE(p1_status.ok()); auto p1 = p1_status.take(); @@ -517,16 +517,15 @@ TEST_F(BufferHubQueueTest, TestUsageDenySetMask) { // Now that |deny_set_mask| is illegal, allocation without those bits should // be able to succeed. - size_t slot; auto status = producer_queue_->AllocateBuffer( kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat, - kBufferUsage & ~deny_set_mask, &slot); + kBufferUsage & ~deny_set_mask); ASSERT_TRUE(status.ok()); // While allocation with those bits should fail. status = producer_queue_->AllocateBuffer(kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat, - kBufferUsage | deny_set_mask, &slot); + kBufferUsage | deny_set_mask); ASSERT_FALSE(status.ok()); ASSERT_EQ(EINVAL, status.error()); } @@ -538,16 +537,15 @@ TEST_F(BufferHubQueueTest, TestUsageDenyClearMask) { // Now that clearing |deny_clear_mask| is illegal (i.e. setting these bits are // mandatory), allocation with those bits should be able to succeed. - size_t slot; auto status = producer_queue_->AllocateBuffer( kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat, - kBufferUsage | deny_clear_mask, &slot); + kBufferUsage | deny_clear_mask); ASSERT_TRUE(status.ok()); // While allocation without those bits should fail. status = producer_queue_->AllocateBuffer( kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat, - kBufferUsage & ~deny_clear_mask, &slot); + kBufferUsage & ~deny_clear_mask); ASSERT_FALSE(status.ok()); ASSERT_EQ(EINVAL, status.error()); } diff --git a/libs/vr/libdisplay/display_client.cpp b/libs/vr/libdisplay/display_client.cpp index 6e7f5564c4..40c158a45a 100644 --- a/libs/vr/libdisplay/display_client.cpp +++ b/libs/vr/libdisplay/display_client.cpp @@ -145,20 +145,12 @@ Status> Surface::CreateQueue( auto producer_queue = status.take(); ALOGD_IF(TRACE, "Surface::CreateQueue: Allocating %zu buffers...", capacity); - for (size_t i = 0; i < capacity; i++) { - size_t slot; - auto allocate_status = producer_queue->AllocateBuffer( - width, height, layer_count, format, usage, &slot); - if (!allocate_status) { - ALOGE( - "Surface::CreateQueue: Failed to allocate buffer on queue_id=%d: %s", + auto allocate_status = producer_queue->AllocateBuffers( + width, height, layer_count, format, usage, capacity); + if (!allocate_status) { + ALOGE("Surface::CreateQueue: Failed to allocate buffer on queue_id=%d: %s", producer_queue->id(), allocate_status.GetErrorMessage().c_str()); - return allocate_status.error_status(); - } - ALOGD_IF( - TRACE, - "Surface::CreateQueue: Allocated buffer at slot=%zu of capacity=%zu", - slot, capacity); + return allocate_status.error_status(); } return {std::move(producer_queue)}; diff --git a/libs/vr/libdvr/dvr_buffer_queue.cpp b/libs/vr/libdvr/dvr_buffer_queue.cpp index 5d01c8f25e..aa2ed94eeb 100644 --- a/libs/vr/libdvr/dvr_buffer_queue.cpp +++ b/libs/vr/libdvr/dvr_buffer_queue.cpp @@ -122,7 +122,7 @@ int DvrWriteBufferQueue::Dequeue(int timeout, DvrWriteBuffer* write_buffer, } auto allocate_status = producer_queue_->AllocateBuffer( - width_, height_, old_layer_count, format_, old_usage, &slot); + width_, height_, old_layer_count, format_, old_usage); if (!allocate_status) { ALOGE("DvrWriteBufferQueue::Dequeue: Failed to allocate buffer: %s", allocate_status.GetErrorMessage().c_str()); diff --git a/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp index f2ae09ee8b..5158612260 100644 --- a/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp +++ b/libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp @@ -56,13 +56,10 @@ class DvrBufferQueueTest : public ::testing::Test { } void AllocateBuffers(size_t buffer_count) { - size_t out_slot; - for (size_t i = 0; i < buffer_count; i++) { - auto status = write_queue_->producer_queue()->AllocateBuffer( - kBufferWidth, kBufferHeight, kLayerCount, kBufferFormat, kBufferUsage, - &out_slot); - ASSERT_TRUE(status.ok()); - } + auto status = write_queue_->producer_queue()->AllocateBuffers( + kBufferWidth, kBufferHeight, kLayerCount, kBufferFormat, kBufferUsage, + buffer_count); + ASSERT_TRUE(status.ok()); } void HandleBufferAvailable() {