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
diff --git a/services/vr/bufferhubd/buffer_channel.cpp b/services/vr/bufferhubd/buffer_channel.cpp
index dcc6ea4..cc9cf20 100644
--- a/services/vr/bufferhubd/buffer_channel.cpp
+++ b/services/vr/bufferhubd/buffer_channel.cpp
@@ -74,11 +74,6 @@
*this, &BufferChannel::OnDuplicate, message);
return true;
- case DetachedBufferRPC::Promote::Opcode:
- DispatchRemoteMethod<DetachedBufferRPC::Promote>(
- *this, &BufferChannel::OnPromote, message);
- return true;
-
default:
return false;
}
@@ -154,69 +149,5 @@
return status;
}
-Status<RemoteChannelHandle> BufferChannel::OnPromote(
- Message& message) {
- ATRACE_NAME("BufferChannel::OnPromote");
- ALOGD_IF(TRACE, "BufferChannel::OnPromote: buffer_id=%d", buffer_id());
-
- // Check whether this is the channel exclusive owner of the buffer_node_.
- if (buffer_state_bit_ != buffer_node_->active_buffer_bit_mask()) {
- ALOGE(
- "BufferChannel::OnPromote: Cannot promote this BufferChannel as its "
- "BufferNode is shared between multiple channels. This channel's state "
- "bit=0x%" PRIx64 ", acitve_buffer_bit_mask=0x%" PRIx64 ".",
- buffer_state_bit_, buffer_node_->active_buffer_bit_mask());
- return ErrorStatus(EINVAL);
- }
-
- // Note that the new ProducerChannel will have different channel_id, but
- // inherits the buffer_id from the DetachedBuffer.
- int channel_id;
- auto status = message.PushChannel(0, nullptr, &channel_id);
- if (!status) {
- ALOGE(
- "BufferChannel::OnPromote: Failed to push ProducerChannel: %s.",
- status.GetErrorMessage().c_str());
- return ErrorStatus(ENOMEM);
- }
-
- IonBuffer buffer = std::move(buffer_node_->buffer());
- IonBuffer metadata_buffer;
- if (int ret = metadata_buffer.Alloc(buffer_node_->metadata().metadata_size(),
- /*height=*/1,
- /*layer_count=*/1,
- BufferHubDefs::kMetadataFormat,
- BufferHubDefs::kMetadataUsage)) {
- ALOGE("BufferChannel::OnPromote: Failed to allocate metadata: %s",
- strerror(-ret));
- return ErrorStatus(EINVAL);
- }
-
- size_t user_metadata_size = buffer_node_->user_metadata_size();
-
- std::unique_ptr<ProducerChannel> channel = ProducerChannel::Create(
- service(), buffer_id(), channel_id, std::move(buffer),
- std::move(metadata_buffer), user_metadata_size);
- if (!channel) {
- ALOGE(
- "BufferChannel::OnPromote: Failed to create ProducerChannel from a "
- "BufferChannel, buffer_id=%d.",
- buffer_id());
- }
-
- const auto channel_status =
- service()->SetChannel(channel_id, std::move(channel));
- if (!channel_status) {
- // Technically, this should never fail, as we just pushed the channel. Note
- // that LOG_FATAL will be stripped out in non-debug build.
- LOG_FATAL(
- "BufferChannel::OnPromote: Failed to set new producer buffer channel: "
- "%s.",
- channel_status.GetErrorMessage().c_str());
- }
-
- return status;
-}
-
} // namespace dvr
} // namespace android
diff --git a/services/vr/bufferhubd/buffer_hub.cpp b/services/vr/bufferhubd/buffer_hub.cpp
index 15391da..6421a0b 100644
--- a/services/vr/bufferhubd/buffer_hub.cpp
+++ b/services/vr/bufferhubd/buffer_hub.cpp
@@ -265,14 +265,6 @@
SetChannel(channel->channel_id(), nullptr);
return {};
- case DetachedBufferRPC::Promote::Opcode:
- // In addition to the message handler in the BufferChannel's
- // HandleMessage method, we also need to invalid the channel. Note that
- // this has to be done after HandleMessage returns to make sure the IPC
- // request has went back to the client first.
- SetChannel(channel->channel_id(), nullptr);
- return {};
-
default:
return DefaultHandleMessage(message);
}
diff --git a/services/vr/bufferhubd/include/private/dvr/buffer_channel.h b/services/vr/bufferhubd/include/private/dvr/buffer_channel.h
index 1697251..0ce991a 100644
--- a/services/vr/bufferhubd/include/private/dvr/buffer_channel.h
+++ b/services/vr/bufferhubd/include/private/dvr/buffer_channel.h
@@ -50,7 +50,6 @@
pdx::Status<BufferTraits<pdx::BorrowedHandle>> OnImport(
pdx::Message& message);
pdx::Status<pdx::RemoteChannelHandle> OnDuplicate(pdx::Message& message);
- pdx::Status<pdx::RemoteChannelHandle> OnPromote(pdx::Message& message);
// The concrete implementation of the Buffer object.
std::shared_ptr<BufferNode> buffer_node_;