Update BufferHubQueueProducer
This is the second step to make BufferHub parcelable. There will be one
last CL actually use the parcelable BufferHubQueueProducer in
android::Surface and android::view::Surface to wrap thigns up.
1/ Remove BufferHubQueueProducer::Create(), as there is no real use case
for that at all.
2/ Add support for taking out underlying dvr::ProducerQueueParcelable
with proper safety check.
3/ Update tests to cover the end-to-end case of sending BufferHub-backed
IGBP over binder.
Bug: 37517761
Bug: 63909629
Test: buffer_hub_queue_producer-test
Change-Id: I06ca04b6a8ac1fb15d61ce7884990c3aca1c29ed
diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp
index 221bc4f..ace01a6 100644
--- a/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp
+++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp
@@ -9,16 +9,6 @@
namespace dvr {
/* static */
-sp<BufferHubQueueProducer> BufferHubQueueProducer::Create() {
- sp<BufferHubQueueProducer> producer = new BufferHubQueueProducer;
- auto config = ProducerQueueConfigBuilder()
- .SetMetadata<DvrNativeBufferMetadata>()
- .Build();
- producer->queue_ = ProducerQueue::Create(config, UsagePolicy{});
- return producer;
-}
-
-/* static */
sp<BufferHubQueueProducer> BufferHubQueueProducer::Create(
const std::shared_ptr<ProducerQueue>& queue) {
if (queue->metadata_size() != sizeof(DvrNativeBufferMetadata)) {
@@ -33,6 +23,19 @@
return producer;
}
+/* static */
+sp<BufferHubQueueProducer> BufferHubQueueProducer::Create(
+ ProducerQueueParcelable parcelable) {
+ if (!parcelable.IsValid()) {
+ ALOGE("BufferHubQueueProducer::Create: Invalid producer parcelable.");
+ return nullptr;
+ }
+
+ sp<BufferHubQueueProducer> producer = new BufferHubQueueProducer;
+ producer->queue_ = ProducerQueue::Import(parcelable.TakeChannelHandle());
+ return producer;
+}
+
status_t BufferHubQueueProducer::requestBuffer(int slot,
sp<GraphicBuffer>* buf) {
ALOGD_IF(TRACE, "requestBuffer: slot=%d", slot);
@@ -475,6 +478,13 @@
return BAD_VALUE;
}
+ if (!queue_->is_connected()) {
+ ALOGE(
+ "BufferHubQueueProducer::connect: This BufferHubQueueProducer is not "
+ "connected to bufferhud. Has it been taken out as a parcelable?");
+ return BAD_VALUE;
+ }
+
switch (api) {
case NATIVE_WINDOW_API_EGL:
case NATIVE_WINDOW_API_CPU:
@@ -617,6 +627,39 @@
return NO_ERROR;
}
+status_t BufferHubQueueProducer::TakeAsParcelable(
+ ProducerQueueParcelable* out_parcelable) {
+ if (!out_parcelable || out_parcelable->IsValid())
+ return BAD_VALUE;
+
+ if (connected_api_ != kNoConnectedApi) {
+ ALOGE(
+ "BufferHubQueueProducer::TakeAsParcelable: BufferHubQueueProducer has "
+ "connected client. Must disconnect first.");
+ return BAD_VALUE;
+ }
+
+ if (!queue_->is_connected()) {
+ ALOGE(
+ "BufferHubQueueProducer::TakeAsParcelable: This BufferHubQueueProducer "
+ "is not connected to bufferhud. Has it been taken out as a "
+ "parcelable?");
+ return BAD_VALUE;
+ }
+
+ auto status = queue_->TakeAsParcelable();
+ if (!status) {
+ ALOGE(
+ "BufferHubQueueProducer::TakeAsParcelable: Failed to take out "
+ "ProducuerQueueParcelable from the producer queue, error: %s.",
+ status.GetErrorMessage().c_str());
+ return BAD_VALUE;
+ }
+
+ *out_parcelable = status.take();
+ return NO_ERROR;
+}
+
status_t BufferHubQueueProducer::AllocateBuffer(uint32_t width, uint32_t height,
uint32_t layer_count,
PixelFormat format,
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 c8e31c7..5b320a4 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
@@ -74,7 +74,8 @@
return available_buffers_.size() >= kMaxQueueCapacity;
}
- explicit operator bool() const { return epoll_fd_.IsValid(); }
+ // Returns whether the buffer queue is connected to bufferhubd.
+ bool is_connected() const { return !!GetChannel(); }
int GetBufferId(size_t slot) const {
return (slot < buffers_.size() && buffers_[slot]) ? buffers_[slot]->id()
diff --git a/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_producer.h b/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_producer.h
index 7ed55fb..9c85048 100644
--- a/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_producer.h
+++ b/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_producer.h
@@ -3,6 +3,7 @@
#include <gui/IGraphicBufferProducer.h>
#include <private/dvr/buffer_hub_queue_client.h>
+#include <private/dvr/buffer_hub_queue_parcelable.h>
namespace android {
namespace dvr {
@@ -17,14 +18,16 @@
// that.
static constexpr int kDefaultUndequeuedBuffers = 1;
- // Create a BufferHubQueueProducer instance by creating a new producer queue.
- static sp<BufferHubQueueProducer> Create();
-
- // Create a BufferHubQueueProducer instance by importing an existing prodcuer
+ // Creates a BufferHubQueueProducer instance by importing an existing prodcuer
// queue.
static sp<BufferHubQueueProducer> Create(
const std::shared_ptr<ProducerQueue>& producer);
+ // Creates a BufferHubQueueProducer instance by importing an existing prodcuer
+ // parcelable. Note that this call takes the ownership of the parcelable
+ // object and is guaranteed to succeed if parcelable object is valid.
+ static sp<BufferHubQueueProducer> Create(ProducerQueueParcelable parcelable);
+
// See |IGraphicBufferProducer::requestBuffer|
status_t requestBuffer(int slot, sp<GraphicBuffer>* buf) override;
@@ -115,6 +118,14 @@
// See |IGraphicBufferProducer::getConsumerUsage|
status_t getConsumerUsage(uint64_t* out_usage) const override;
+ // Takes out the current producer as a binder parcelable object. Note that the
+ // producer must be disconnected to be exportable. After successful export,
+ // the producer queue can no longer be connected again. Returns NO_ERROR when
+ // takeout is successful and out_parcelable will hold the new parcelable
+ // object. Also note that out_parcelable cannot be NULL and must points to an
+ // invalid parcelable.
+ status_t TakeAsParcelable(ProducerQueueParcelable* out_parcelable);
+
private:
using LocalHandle = pdx::LocalHandle;
diff --git a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue_producer-test.cpp b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue_producer-test.cpp
index 28cd63a..96f5404 100644
--- a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue_producer-test.cpp
+++ b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue_producer-test.cpp
@@ -3,12 +3,15 @@
#include <base/logging.h>
#include <gui/IProducerListener.h>
#include <gui/Surface.h>
+#include <pdx/default_transport/channel_parcelable.h>
#include <gtest/gtest.h>
namespace android {
namespace dvr {
+using pdx::LocalHandle;
+
namespace {
// Default dimensions before setDefaultBufferSize is called by the consumer.
@@ -92,7 +95,13 @@
ALOGD_IF(TRACE, "Begin test: %s.%s", testInfo->test_case_name(),
testInfo->name());
- mProducer = BufferHubQueueProducer::Create();
+ auto config = ProducerQueueConfigBuilder()
+ .SetMetadata<DvrNativeBufferMetadata>()
+ .Build();
+ auto queue = ProducerQueue::Create(config, UsagePolicy{});
+ ASSERT_TRUE(queue != nullptr);
+
+ mProducer = BufferHubQueueProducer::Create(std::move(queue));
ASSERT_TRUE(mProducer != nullptr);
mSurface = new Surface(mProducer, true);
ASSERT_TRUE(mSurface != nullptr);
@@ -546,6 +555,55 @@
EXPECT_EQ(NO_ERROR, mProducer->disconnect(kTestApi));
}
+TEST_F(BufferHubQueueProducerTest, TakeAsParcelable) {
+ // Connected producer cannot be taken out as a parcelable.
+ EXPECT_NO_FATAL_FAILURE(ConnectProducer());
+ ProducerQueueParcelable producer_parcelable;
+ EXPECT_EQ(mProducer->TakeAsParcelable(&producer_parcelable), BAD_VALUE);
+
+ // Create a valid dummy producer parcelable.
+ auto dummy_channel_parcelable =
+ std::make_unique<pdx::default_transport::ChannelParcelable>(
+ LocalHandle(0), LocalHandle(0), LocalHandle(0));
+ EXPECT_TRUE(dummy_channel_parcelable->IsValid());
+ ProducerQueueParcelable dummy_producer_parcelable(
+ std::move(dummy_channel_parcelable));
+ EXPECT_TRUE(dummy_producer_parcelable.IsValid());
+
+ // Disconnect producer can be taken out, but only to an invalid parcelable.
+ ASSERT_EQ(mProducer->disconnect(kTestApi), NO_ERROR);
+ EXPECT_EQ(mProducer->TakeAsParcelable(&dummy_producer_parcelable), BAD_VALUE);
+ EXPECT_FALSE(producer_parcelable.IsValid());
+ EXPECT_EQ(mProducer->TakeAsParcelable(&producer_parcelable), NO_ERROR);
+ EXPECT_TRUE(producer_parcelable.IsValid());
+
+ // Should still be able to query buffer dimension after disconnect.
+ int32_t value = -1;
+ EXPECT_EQ(NO_ERROR, mProducer->query(NATIVE_WINDOW_WIDTH, &value));
+ EXPECT_EQ(static_cast<uint32_t>(value), kDefaultWidth);
+
+ EXPECT_EQ(mProducer->query(NATIVE_WINDOW_HEIGHT, &value), NO_ERROR);
+ EXPECT_EQ(static_cast<uint32_t>(value), kDefaultHeight);
+
+ EXPECT_EQ(mProducer->query(NATIVE_WINDOW_FORMAT, &value), NO_ERROR);
+ EXPECT_EQ(value, kDefaultFormat);
+
+ // But connect to API will fail.
+ IGraphicBufferProducer::QueueBufferOutput output;
+ EXPECT_EQ(mProducer->connect(kDummyListener, kTestApi, kTestControlledByApp,
+ &output),
+ BAD_VALUE);
+
+ // Create a new producer from the parcelable and connect to kTestApi should
+ // succeed.
+ sp<BufferHubQueueProducer> new_producer =
+ BufferHubQueueProducer::Create(std::move(producer_parcelable));
+ ASSERT_TRUE(new_producer != nullptr);
+ EXPECT_EQ(new_producer->connect(kDummyListener, kTestApi,
+ kTestControlledByApp, &output),
+ NO_ERROR);
+}
+
} // namespace
} // namespace dvr