Replace the use of helper function IsBufferReleased to member function

IsReleased() or is_released().

Fix: 122854791
Test: BufferHub_test BufferHubServer_test VtsHalBufferHubV1_0TargetTest
buffer_hub-test buffer_hub_queue-test dvr_buffer_queue-test
on walleye_xr
Change-Id: I2431a4ddd78cb2eef9bdeafc0d9048571f7a0c61

9 files changed, 50 insertions, 53 deletions

diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index 4b3d3ba06e..4b20772b75 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -318,6 +318,11 @@ int BufferHubBuffer::Release() {
     return 0;
 }
 
+bool BufferHubBuffer::IsReleased() const {
+    return (buffer_state_->load(std::memory_order_acquire) &
+            active_clients_bit_mask_->load(std::memory_order_acquire)) == 0;
+}
+
 bool BufferHubBuffer::IsValid() const {
     return mBufferHandle.getNativeHandle() != nullptr && mId >= 0 && mClientStateMask != 0U &&
             mEventFd.get() >= 0 && mMetadata.IsValid() && mBufferClient != nullptr;
diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h
index 42d9320545..0b6d75a9cb 100644
--- a/libs/ui/include/ui/BufferHubBuffer.h
+++ b/libs/ui/include/ui/BufferHubBuffer.h
@@ -59,9 +59,7 @@ public:
     const BufferHubEventFd& eventFd() const { return mEventFd; }
 
     // Returns the current value of MetadataHeader::buffer_state.
-    uint32_t buffer_state() {
-        return mMetadata.metadata_header()->buffer_state.load(std::memory_order_acquire);
-    }
+    uint32_t buffer_state() const { return buffer_state_->load(std::memory_order_acquire); }
 
     // A state mask which is unique to a buffer hub client among all its siblings sharing the same
     // concrete graphic buffer.
@@ -97,6 +95,9 @@ public:
     // current cycle of the usage of the buffer.
     int Release();
 
+    // Returns whether the buffer is released by all active clients or not.
+    bool IsReleased() const;
+
     // Creates a token that stands for this BufferHubBuffer client and could be used for Import to
     // create another BufferHubBuffer. The new BufferHubBuffer will share the same underlying
     // gralloc buffer and ashmem region for metadata. Note that the caller owns the token and
diff --git a/libs/ui/include/ui/BufferHubDefs.h b/libs/ui/include/ui/BufferHubDefs.h
index 43d900c034..ff970cbd64 100644
--- a/libs/ui/include/ui/BufferHubDefs.h
+++ b/libs/ui/include/ui/BufferHubDefs.h
@@ -106,11 +106,6 @@ static inline bool IsClientAcquired(uint32_t state, uint32_t client_bit_mask) {
     return high_bits == 0U;
 }
 
-// Returns true if all clients are in released state.
-static inline bool IsBufferReleased(uint32_t state) {
-    return state == 0U;
-}
-
 // Returns true if the input client is in released state.
 static inline bool IsClientReleased(uint32_t state, uint32_t client_bit_mask) {
     return (state & client_bit_mask) == 0U;
diff --git a/libs/ui/tests/BufferHubBuffer_test.cpp b/libs/ui/tests/BufferHubBuffer_test.cpp
index 58965c1859..3bcd9353fc 100644
--- a/libs/ui/tests/BufferHubBuffer_test.cpp
+++ b/libs/ui/tests/BufferHubBuffer_test.cpp
@@ -35,7 +35,6 @@ namespace {
 using ::android::BufferHubDefs::AnyClientAcquired;
 using ::android::BufferHubDefs::AnyClientGained;
 using ::android::BufferHubDefs::AnyClientPosted;
-using ::android::BufferHubDefs::IsBufferReleased;
 using ::android::BufferHubDefs::IsClientAcquired;
 using ::android::BufferHubDefs::IsClientGained;
 using ::android::BufferHubDefs::IsClientPosted;
@@ -162,8 +161,8 @@ TEST_F(BufferHubBufferTest, DuplicateAndImportBuffer) {
     EXPECT_NE(b1->client_state_mask(), b2->client_state_mask());
 
     // Both buffer instances should be in released state currently.
-    EXPECT_TRUE(IsBufferReleased(b1->buffer_state()));
-    EXPECT_TRUE(IsBufferReleased(b2->buffer_state()));
+    EXPECT_TRUE(b1->IsReleased());
+    EXPECT_TRUE(b2->IsReleased());
 
     // The event fd should behave like duped event fds.
     const BufferHubEventFd& eventFd1 = b1->eventFd();
@@ -230,7 +229,7 @@ TEST_F(BufferHubBufferTest, ImportInvalidToken) {
 }
 
 TEST_F(BufferHubBufferStateTransitionTest, GainBuffer_fromReleasedState) {
-    ASSERT_TRUE(IsBufferReleased(b1->buffer_state()));
+    ASSERT_TRUE(b1->IsReleased());
 
     // Successful gaining the buffer should change the buffer state bit of b1 to
     // gained state, other client state bits to released state.
@@ -319,7 +318,7 @@ TEST_F(BufferHubBufferStateTransitionTest, PostBuffer_fromAcquiredState) {
 }
 
 TEST_F(BufferHubBufferStateTransitionTest, PostBuffer_fromReleasedState) {
-    ASSERT_TRUE(IsBufferReleased(b1->buffer_state()));
+    ASSERT_TRUE(b1->IsReleased());
 
     // Posting from released state should fail.
     EXPECT_EQ(b1->Post(), -EBUSY);
@@ -357,7 +356,7 @@ TEST_F(BufferHubBufferStateTransitionTest, AcquireBuffer_fromSelfInAcquiredState
 }
 
 TEST_F(BufferHubBufferStateTransitionTest, AcquireBuffer_fromReleasedState) {
-    ASSERT_TRUE(IsBufferReleased(b1->buffer_state()));
+    ASSERT_TRUE(b1->IsReleased());
 
     // Acquiring form released state should fail.
     EXPECT_EQ(b1->Acquire(), -EBUSY);
@@ -374,13 +373,13 @@ TEST_F(BufferHubBufferStateTransitionTest, AcquireBuffer_fromGainedState) {
 }
 
 TEST_F(BufferHubBufferStateTransitionTest, ReleaseBuffer_fromSelfInReleasedState) {
-    ASSERT_TRUE(IsBufferReleased(b1->buffer_state()));
+    ASSERT_TRUE(b1->IsReleased());
 
     EXPECT_EQ(b1->Release(), 0);
 }
 
 TEST_F(BufferHubBufferStateTransitionTest, ReleaseBuffer_fromSelfInGainedState) {
-    ASSERT_TRUE(IsBufferReleased(b1->buffer_state()));
+    ASSERT_TRUE(b1->IsReleased());
     ASSERT_EQ(b1->Gain(), 0);
     ASSERT_TRUE(AnyClientGained(b1->buffer_state()));
 
diff --git a/libs/ui/tests/BufferHubMetadata_test.cpp b/libs/ui/tests/BufferHubMetadata_test.cpp
index 11f8e57adc..b7f0b4b140 100644
--- a/libs/ui/tests/BufferHubMetadata_test.cpp
+++ b/libs/ui/tests/BufferHubMetadata_test.cpp
@@ -17,8 +17,6 @@
 #include <gtest/gtest.h>
 #include <ui/BufferHubMetadata.h>
 
-using android::BufferHubDefs::IsBufferReleased;
-
 namespace android {
 namespace dvr {
 
@@ -52,13 +50,17 @@ TEST_F(BufferHubMetadataTest, Import_Success) {
   BufferHubDefs::MetadataHeader* mh1 = m1.metadata_header();
   EXPECT_NE(mh1, nullptr);
 
-  EXPECT_TRUE(IsBufferReleased(mh1->buffer_state.load()));
+  // Check if the newly allocated buffer is initialized in released state (i.e.
+  // state equals to 0U).
+  EXPECT_TRUE(mh1->buffer_state.load() == 0U);
 
   EXPECT_TRUE(m2.IsValid());
   BufferHubDefs::MetadataHeader* mh2 = m2.metadata_header();
   EXPECT_NE(mh2, nullptr);
 
-  EXPECT_TRUE(IsBufferReleased(mh2->buffer_state.load()));
+  // Check if the newly allocated buffer is initialized in released state (i.e.
+  // state equals to 0U).
+  EXPECT_TRUE(mh2->buffer_state.load() == 0U);
 }
 
 TEST_F(BufferHubMetadataTest, MoveMetadataInvalidatesOldOne) {
diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp
index ed5a992f3c..a47a98f3b6 100644
--- a/libs/vr/libbufferhub/buffer_hub-test.cpp
+++ b/libs/vr/libbufferhub/buffer_hub-test.cpp
@@ -22,7 +22,6 @@
 using android::BufferHubDefs::AnyClientAcquired;
 using android::BufferHubDefs::AnyClientGained;
 using android::BufferHubDefs::AnyClientPosted;
-using android::BufferHubDefs::IsBufferReleased;
 using android::BufferHubDefs::IsClientAcquired;
 using android::BufferHubDefs::IsClientPosted;
 using android::BufferHubDefs::IsClientReleased;
@@ -284,7 +283,7 @@ TEST_F(LibBufferHubTest, TestAsyncStateTransitions) {
   EXPECT_EQ(0, c->ReleaseAsync(&metadata, invalid_fence));
   EXPECT_LT(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
   EXPECT_EQ(p->buffer_state(), c->buffer_state());
-  EXPECT_TRUE(IsBufferReleased(p->buffer_state()));
+  EXPECT_TRUE(p->is_released());
 
   // Acquire and post in released state should fail.
   EXPECT_EQ(-EBUSY, c->AcquireAsync(&metadata, &invalid_fence));
@@ -356,7 +355,7 @@ TEST_F(LibBufferHubTest, TestGainPostedBuffer_noConsumer) {
   // Producer state bit is in released state after post, other clients shall be
   // in posted state although there is no consumer of this buffer yet.
   ASSERT_TRUE(IsClientReleased(p->buffer_state(), p->client_state_mask()));
-  ASSERT_FALSE(IsBufferReleased(p->buffer_state()));
+  ASSERT_TRUE(p->is_released());
   ASSERT_TRUE(AnyClientPosted(p->buffer_state()));
 
   // Gain in released state should succeed.
@@ -374,7 +373,7 @@ TEST_F(LibBufferHubTest, TestMaxConsumers) {
   for (size_t i = 0; i < kMaxConsumerCount; ++i) {
     cs[i] = ConsumerBuffer::Import(p->CreateConsumer());
     ASSERT_TRUE(cs[i].get() != nullptr);
-    EXPECT_TRUE(IsBufferReleased(cs[i]->buffer_state()));
+    EXPECT_TRUE(cs[i]->is_released());
     EXPECT_NE(producer_state_mask, cs[i]->client_state_mask());
   }
 
@@ -397,12 +396,12 @@ TEST_F(LibBufferHubTest, TestMaxConsumers) {
   // All consumers have to release before the buffer is considered to be
   // released.
   for (size_t i = 0; i < kMaxConsumerCount; i++) {
-    EXPECT_FALSE(IsBufferReleased(p->buffer_state()));
+    EXPECT_FALSE(p->is_released());
     EXPECT_EQ(0, cs[i]->ReleaseAsync(&metadata, invalid_fence));
   }
 
   EXPECT_LT(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
-  EXPECT_TRUE(IsBufferReleased(p->buffer_state()));
+  EXPECT_TRUE(p->is_released());
 
   // Buffer state cross all clients must be consistent.
   for (size_t i = 0; i < kMaxConsumerCount; i++) {
@@ -445,7 +444,7 @@ TEST_F(LibBufferHubTest, TestCreateTheFirstConsumerAfterPostingBuffer) {
 
   // Post the gained buffer before any consumer gets created.
   EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
-  EXPECT_FALSE(IsBufferReleased(p->buffer_state()));
+  EXPECT_TRUE(p->is_released());
   EXPECT_EQ(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
 
   // Newly created consumer will be signalled for the posted buffer although it
@@ -481,7 +480,7 @@ TEST_F(LibBufferHubTest, TestCreateConsumerWhenBufferReleased) {
   // need to wait until the releasd is confirmed before creating another
   // consumer.
   EXPECT_LT(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
-  EXPECT_TRUE(IsBufferReleased(p->buffer_state()));
+  EXPECT_TRUE(p->is_released());
 
   // Create another consumer immediately after the release, should not make the
   // buffer un-released.
@@ -489,7 +488,7 @@ TEST_F(LibBufferHubTest, TestCreateConsumerWhenBufferReleased) {
       ConsumerBuffer::Import(p->CreateConsumer());
   ASSERT_TRUE(c2.get() != nullptr);
 
-  EXPECT_TRUE(IsBufferReleased(p->buffer_state()));
+  EXPECT_TRUE(p->is_released());
   EXPECT_EQ(0, p->GainAsync(&metadata, &invalid_fence));
   EXPECT_TRUE(AnyClientGained(p->buffer_state()));
 }
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 f5761d5a04..bab7367caa 100644
--- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
+++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
@@ -51,10 +51,6 @@ static inline bool IsClientAcquired(uint32_t state, uint32_t client_bit_mask) {
   return android::BufferHubDefs::IsClientAcquired(state, client_bit_mask);
 }
 
-static inline bool IsBufferReleased(uint32_t state) {
-  return android::BufferHubDefs::IsBufferReleased(state);
-}
-
 static inline bool IsClientReleased(uint32_t state, uint32_t client_bit_mask) {
   return android::BufferHubDefs::IsClientReleased(state, client_bit_mask);
 }
diff --git a/services/vr/bufferhubd/include/private/dvr/producer_channel.h b/services/vr/bufferhubd/include/private/dvr/producer_channel.h
index 96ef1a20a0..45593ad955 100644
--- a/services/vr/bufferhubd/include/private/dvr/producer_channel.h
+++ b/services/vr/bufferhubd/include/private/dvr/producer_channel.h
@@ -69,7 +69,7 @@ class ProducerChannel : public BufferHubChannel {
 
   bool CheckParameters(uint32_t width, uint32_t height, uint32_t layer_count,
                        uint32_t format, uint64_t usage,
-                       size_t user_metadata_size);
+                       size_t user_metadata_size) const;
 
  private:
   std::vector<ConsumerChannel*> consumer_channels_;
@@ -112,6 +112,10 @@ class ProducerChannel : public BufferHubChannel {
   // This function is used for clean up for failures in CreateConsumer method.
   void RemoveConsumerClientMask(uint32_t consumer_state_mask);
 
+  // Checks whether the buffer is released by all active clients, excluding
+  // orphaned consumers.
+  bool IsBufferReleasedByAllActiveClientsExceptForOrphans() const;
+
   ProducerChannel(const ProducerChannel&) = delete;
   void operator=(const ProducerChannel&) = delete;
 };
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index 895dee0701..164d9e6b19 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -333,7 +333,10 @@ Status<RemoteChannelHandle> ProducerChannel::CreateConsumer(
 
   uint32_t current_buffer_state =
       buffer_state_->load(std::memory_order_acquire);
-  if (BufferHubDefs::IsBufferReleased(current_buffer_state) ||
+  // Return the consumer channel handle without signal when adding the new
+  // consumer to a buffer that is available to producer (a.k.a a fully-released
+  // buffer) or a gained buffer.
+  if (current_buffer_state == 0U ||
       BufferHubDefs::AnyClientGained(current_buffer_state)) {
     return {status.take()};
   }
@@ -356,7 +359,7 @@ Status<RemoteChannelHandle> ProducerChannel::CreateConsumer(
           ". About to try again if the buffer is still not gained nor fully "
           "released.",
           __FUNCTION__, current_buffer_state, updated_buffer_state);
-      if (BufferHubDefs::IsBufferReleased(current_buffer_state) ||
+      if (current_buffer_state == 0U ||
           BufferHubDefs::AnyClientGained(current_buffer_state)) {
         ALOGI("%s: buffer is gained or fully released, state=%" PRIx32 ".",
               __FUNCTION__, current_buffer_state);
@@ -536,14 +539,7 @@ Status<void> ProducerChannel::OnConsumerRelease(Message&,
     }
   }
 
-  uint32_t current_buffer_state =
-      buffer_state_->load(std::memory_order_acquire);
-  uint32_t current_active_clients_bit_mask =
-      active_clients_bit_mask_->load(std::memory_order_acquire);
-  // Signal producer if all current active consumers have released the buffer.
-  if (BufferHubDefs::IsBufferReleased(current_buffer_state &
-                                      ~orphaned_consumer_bit_mask_ &
-                                      current_active_clients_bit_mask)) {
+  if (IsBufferReleasedByAllActiveClientsExceptForOrphans()) {
     buffer_state_->store(0U);
     SignalAvailable();
     if (orphaned_consumer_bit_mask_) {
@@ -568,14 +564,7 @@ void ProducerChannel::OnConsumerOrphaned(const uint32_t& consumer_state_mask) {
            __FUNCTION__, consumer_state_mask);
   orphaned_consumer_bit_mask_ |= consumer_state_mask;
 
-  uint32_t current_buffer_state =
-      buffer_state_->load(std::memory_order_acquire);
-  uint32_t current_active_clients_bit_mask =
-      active_clients_bit_mask_->load(std::memory_order_acquire);
-  // Signal producer if all current active consumers have released the buffer.
-  if (BufferHubDefs::IsBufferReleased(current_buffer_state &
-                                      ~orphaned_consumer_bit_mask_ &
-                                      current_active_clients_bit_mask)) {
+  if (IsBufferReleasedByAllActiveClientsExceptForOrphans()) {
     buffer_state_->store(0U);
     SignalAvailable();
   }
@@ -667,12 +656,19 @@ void ProducerChannel::RemoveConsumer(ConsumerChannel* channel) {
 bool ProducerChannel::CheckParameters(uint32_t width, uint32_t height,
                                       uint32_t layer_count, uint32_t format,
                                       uint64_t usage,
-                                      size_t user_metadata_size) {
+                                      size_t user_metadata_size) const {
   return user_metadata_size == user_metadata_size_ &&
          buffer_.width() == width && buffer_.height() == height &&
          buffer_.layer_count() == layer_count && buffer_.format() == format &&
          buffer_.usage() == usage;
 }
 
+bool ProducerChannel::IsBufferReleasedByAllActiveClientsExceptForOrphans()
+    const {
+  return (buffer_state_->load(std::memory_order_acquire) &
+          ~orphaned_consumer_bit_mask_ &
+          active_clients_bit_mask_->load(std::memory_order_acquire)) == 0U;
+}
+
 }  // namespace dvr
 }  // namespace android