Remove the functionality of promoting a BufferHubBuffer to ProducerBuffer.
Test: buffer_hub-test buffer_hub_queue-test dvr_api-test
dvr_buffer_queue-test on marlin-eng
Bug: 77153033
Change-Id: I155fa5c5740243d84207f37e6a2fe37d6331628f
diff --git a/libs/gui/BufferHubProducer.cpp b/libs/gui/BufferHubProducer.cpp
index 3dac037..1a7c2d3 100644
--- a/libs/gui/BufferHubProducer.cpp
+++ b/libs/gui/BufferHubProducer.cpp
@@ -400,19 +400,8 @@
ALOGE("attachBuffer: DetachedBufferHandle cannot be NULL.");
return BAD_VALUE;
}
- auto detached_buffer = BufferHubBuffer::Import(std::move(detached_handle->handle()));
- if (detached_buffer == nullptr) {
- ALOGE("attachBuffer: BufferHubBuffer cannot be NULL.");
- return BAD_VALUE;
- }
- auto status_or_handle = detached_buffer->Promote();
- if (!status_or_handle.ok()) {
- ALOGE("attachBuffer: Failed to promote a BufferHubBuffer into a BufferProducer, error=%d.",
- status_or_handle.error());
- return BAD_VALUE;
- }
std::shared_ptr<BufferProducer> buffer_producer =
- BufferProducer::Import(status_or_handle.take());
+ BufferProducer::Import(std::move(detached_handle->handle()));
if (buffer_producer == nullptr) {
ALOGE("attachBuffer: Failed to import BufferProducer.");
return BAD_VALUE;
diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index 2696c6c..44a947e 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -174,26 +174,6 @@
return poll(&p, 1, timeoutMs);
}
-Status<LocalChannelHandle> BufferHubBuffer::Promote() {
- ATRACE_CALL();
-
- // TODO(b/112338294) remove after migrate producer buffer to binder
- ALOGW("BufferHubBuffer::Promote: not supported operation during migration");
- return {};
-
- ALOGD("BufferHubBuffer::Promote: id=%d.", mId);
-
- auto statusOrHandle = mClient.InvokeRemoteMethod<DetachedBufferRPC::Promote>();
- if (statusOrHandle.ok()) {
- // Invalidate the buffer.
- mBufferHandle = {};
- } else {
- ALOGE("BufferHubBuffer::Promote: Failed to promote buffer (id=%d): %s.", mId,
- statusOrHandle.GetErrorMessage().c_str());
- }
- return statusOrHandle;
-}
-
Status<LocalChannelHandle> BufferHubBuffer::Duplicate() {
ATRACE_CALL();
ALOGD("BufferHubBuffer::Duplicate: id=%d.", mId);
diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h
index d33be8c..cefe5b3 100644
--- a/libs/ui/include/ui/BufferHubBuffer.h
+++ b/libs/ui/include/ui/BufferHubBuffer.h
@@ -112,11 +112,6 @@
// Polls the fd for |timeoutMs| milliseconds (-1 for infinity).
int Poll(int timeoutMs);
- // Promotes a BufferHubBuffer to become a ProducerBuffer. Once promoted the BufferHubBuffer
- // channel will be closed automatically on successful IPC return. Further IPCs towards this
- // channel will return error.
- pdx::Status<pdx::LocalChannelHandle> Promote();
-
// Creates a BufferHubBuffer client from an existing one. The new client will
// share the same underlying gralloc buffer and ashmem region for metadata.
pdx::Status<pdx::LocalChannelHandle> Duplicate();
diff --git a/libs/ui/tests/BufferHubBuffer_test.cpp b/libs/ui/tests/BufferHubBuffer_test.cpp
index e71c99c..be06ad2 100644
--- a/libs/ui/tests/BufferHubBuffer_test.cpp
+++ b/libs/ui/tests/BufferHubBuffer_test.cpp
@@ -113,12 +113,6 @@
// TODO(b/112338294): rewrite test after migration
return;
-
- // Promote the detached buffer should fail as b1 is no longer the exclusive
- // owner of the buffer..
- statusOrHandle = b1->Promote();
- EXPECT_FALSE(statusOrHandle.ok());
- EXPECT_EQ(statusOrHandle.error(), EINVAL);
}
} // namespace android
diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp
index 571d382..8c6e7e2 100644
--- a/libs/vr/libbufferhub/buffer_hub-test.cpp
+++ b/libs/vr/libbufferhub/buffer_hub-test.cpp
@@ -794,43 +794,42 @@
EXPECT_EQ(d->id(), p_id);
}
-TEST_F(LibBufferHubTest, TestPromoteBufferHubBuffer) {
- // TODO(b/112338294) rewrite test after migration
- return;
+TEST_F(LibBufferHubTest, TestCreateBufferHubBufferFails) {
+ // Buffer Creation will fail: BLOB format requires height to be 1.
+ auto b1 = BufferHubBuffer::Create(kWidth, /*height=2*/ 2, kLayerCount,
+ /*format=*/HAL_PIXEL_FORMAT_BLOB, kUsage,
+ kUserMetadataSize);
- auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat,
- kUsage, kUserMetadataSize);
- int b1_id = b1->id();
- EXPECT_TRUE(b1->IsValid());
-
- auto status_or_handle = b1->Promote();
- EXPECT_TRUE(status_or_handle);
-
- // The detached buffer should have hangup.
- EXPECT_GT(RETRY_EINTR(b1->Poll(kPollTimeoutMs)), 0);
- auto status_or_int = b1->GetEventMask(POLLHUP);
- EXPECT_TRUE(status_or_int.ok());
- EXPECT_EQ(status_or_int.get(), POLLHUP);
-
- // The buffer client is still considered as connected but invalid.
- EXPECT_TRUE(b1->IsConnected());
+ EXPECT_FALSE(b1->IsConnected());
EXPECT_FALSE(b1->IsValid());
- // Gets the channel handle for the producer.
- LocalChannelHandle h1 = status_or_handle.take();
- EXPECT_TRUE(h1.valid());
+ // Buffer Creation will fail: user metadata size too large.
+ auto b2 = BufferHubBuffer::Create(
+ kWidth, kHeight, kLayerCount, kFormat, kUsage,
+ /*user_metadata_size=*/std::numeric_limits<size_t>::max());
- std::unique_ptr<ProducerBuffer> p1 = ProducerBuffer::Import(std::move(h1));
- EXPECT_FALSE(h1.valid());
- ASSERT_TRUE(p1 != nullptr);
- int p1_id = p1->id();
+ EXPECT_FALSE(b2->IsConnected());
+ EXPECT_FALSE(b2->IsValid());
- // A newly promoted ProducerBuffer should inherit the same buffer id.
- EXPECT_EQ(b1_id, p1_id);
- EXPECT_TRUE(IsBufferGained(p1->buffer_state()));
+ // Buffer Creation will fail: user metadata size too large.
+ auto b3 = BufferHubBuffer::Create(
+ kWidth, kHeight, kLayerCount, kFormat, kUsage,
+ /*user_metadata_size=*/std::numeric_limits<size_t>::max() -
+ kMetadataHeaderSize);
+
+ EXPECT_FALSE(b3->IsConnected());
+ EXPECT_FALSE(b3->IsValid());
}
-TEST_F(LibBufferHubTest, TestDetachThenPromote) {
+TEST_F(LibBufferHubTest, TestCreateBufferHubBuffer) {
+ auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat,
+ kUsage, kUserMetadataSize);
+ EXPECT_TRUE(b1->IsConnected());
+ EXPECT_TRUE(b1->IsValid());
+ EXPECT_NE(b1->id(), 0);
+}
+
+TEST_F(LibBufferHubTest, TestDetach) {
// TODO(b/112338294) rewrite test after migration
return;
@@ -852,24 +851,48 @@
EXPECT_TRUE(b1->IsValid());
int b1_id = b1->id();
EXPECT_EQ(b1_id, p1_id);
+}
- // Promote the detached buffer.
- status_or_handle = b1->Promote();
- // The buffer client is still considered as connected but invalid.
+TEST_F(LibBufferHubTest, TestDuplicateBufferHubBuffer) {
+ auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat,
+ kUsage, kUserMetadataSize);
+ int b1_id = b1->id();
+ EXPECT_TRUE(b1->IsValid());
+ EXPECT_EQ(b1->user_metadata_size(), kUserMetadataSize);
+
+ auto status_or_handle = b1->Duplicate();
+ EXPECT_TRUE(status_or_handle);
+
+ // The detached buffer should still be valid.
EXPECT_TRUE(b1->IsConnected());
- EXPECT_FALSE(b1->IsValid());
- EXPECT_TRUE(status_or_handle.ok());
+ EXPECT_TRUE(b1->IsValid());
- // Gets the channel handle for the producer.
+ // Gets the channel handle for the duplicated buffer.
LocalChannelHandle h2 = status_or_handle.take();
EXPECT_TRUE(h2.valid());
- std::unique_ptr<ProducerBuffer> p2 = ProducerBuffer::Import(std::move(h2));
+ std::unique_ptr<BufferHubBuffer> b2 = BufferHubBuffer::Import(std::move(h2));
EXPECT_FALSE(h2.valid());
- ASSERT_TRUE(p2 != nullptr);
- int p2_id = p2->id();
+ ASSERT_TRUE(b2 != nullptr);
+ EXPECT_TRUE(b2->IsValid());
+ EXPECT_EQ(b2->user_metadata_size(), kUserMetadataSize);
- // A newly promoted ProducerBuffer should inherit the same buffer id.
- EXPECT_EQ(b1_id, p2_id);
- EXPECT_TRUE(IsBufferGained(p2->buffer_state()));
+ int b2_id = b2->id();
+
+ // These two buffer instances are based on the same physical buffer under the
+ // hood, so they should share the same id.
+ EXPECT_EQ(b1_id, b2_id);
+ // We use buffer_state_bit() to tell those two instances apart.
+ EXPECT_NE(b1->buffer_state_bit(), b2->buffer_state_bit());
+ EXPECT_NE(b1->buffer_state_bit(), 0ULL);
+ EXPECT_NE(b2->buffer_state_bit(), 0ULL);
+ EXPECT_NE(b1->buffer_state_bit(), kProducerStateBit);
+ EXPECT_NE(b2->buffer_state_bit(), kProducerStateBit);
+
+ // Both buffer instances should be in gained state.
+ EXPECT_TRUE(IsBufferGained(b1->buffer_state()));
+ EXPECT_TRUE(IsBufferGained(b2->buffer_state()));
+
+ // TODO(b/112338294) rewrite test after migration
+ return;
}
diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
index 6d28d41..49f9c3e 100644
--- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
+++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
@@ -191,11 +191,6 @@
// Imports the given channel handle to a DetachedBuffer, taking ownership.
kOpImport,
- // Promotes a DetachedBuffer to become a ProducerBuffer. Once promoted the
- // DetachedBuffer channel will be closed automatically on successful IPC
- // return. Further IPCs towards this channel will return error.
- kOpPromote,
-
// Creates a DetachedBuffer client from an existing one. The new client will
// share the same underlying gralloc buffer and ashmem region for metadata.
kOpDuplicate,
@@ -212,10 +207,9 @@
uint32_t format, uint64_t usage,
size_t user_metadata_size));
PDX_REMOTE_METHOD(Import, kOpImport, BufferTraits<LocalHandle>(Void));
- PDX_REMOTE_METHOD(Promote, kOpPromote, LocalChannelHandle(Void));
PDX_REMOTE_METHOD(Duplicate, kOpDuplicate, LocalChannelHandle(Void));
- PDX_REMOTE_API(API, Create, Import, Promote, Duplicate);
+ PDX_REMOTE_API(API, Create, Import, Duplicate);
};
} // namespace dvr