Merge "Explicitly clears signal upon OnProducerGained in consumer channels"

4 files changed, 23 insertions, 6 deletions

diff --git a/libs/vr/libbufferhub/producer_buffer.cpp b/libs/vr/libbufferhub/producer_buffer.cpp
index de03fad01f..cd92b625d5 100644
--- a/libs/vr/libbufferhub/producer_buffer.cpp
+++ b/libs/vr/libbufferhub/producer_buffer.cpp
@@ -224,15 +224,17 @@ int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta,
   uint64_t current_fence_state = fence_state_->load(std::memory_order_acquire);
   uint64_t current_active_clients_bit_mask =
       active_clients_bit_mask_->load(std::memory_order_acquire);
-  // If there is an release fence from consumer, we need to return it.
+  // If there are release fence(s) from consumer(s), we need to return it to the
+  // consumer(s).
   // TODO(b/112007999) add an atomic variable in metadata header in shared
   // memory to indicate which client is the last producer of the buffer.
   // Currently, assume the first client is the only producer to the buffer.
   if (current_fence_state & current_active_clients_bit_mask &
       (~BufferHubDefs::kFirstClientBitMask)) {
     *out_fence = shared_release_fence_.Duplicate();
-    out_meta->release_fence_mask =
-        current_fence_state & current_active_clients_bit_mask;
+    out_meta->release_fence_mask = current_fence_state &
+                                   current_active_clients_bit_mask &
+                                   (~BufferHubDefs::kFirstClientBitMask);
   }
 
   return 0;
diff --git a/services/vr/bufferhubd/consumer_channel.cpp b/services/vr/bufferhubd/consumer_channel.cpp
index f936e95d38..158ef9ceaa 100644
--- a/services/vr/bufferhubd/consumer_channel.cpp
+++ b/services/vr/bufferhubd/consumer_channel.cpp
@@ -153,6 +153,17 @@ Status<void> ConsumerChannel::OnConsumerRelease(Message& message,
   }
 }
 
+void ConsumerChannel::OnProducerGained() {
+  // Clear the signal if exist. There is a possiblity that the signal still
+  // exist in consumer client when producer gains the buffer, e.g. newly added
+  // consumer fail to acquire the previous posted buffer in time. Then, when
+  // producer gains back the buffer, posts the buffer again and signal the
+  // consumer later, there won't be an signal change in eventfd, and thus,
+  // consumer will miss the posted buffer later. Thus, we need to clear the
+  // signal in consumer clients if the signal exist.
+  ClearAvailable();
+}
+
 void ConsumerChannel::OnProducerPosted() {
   acquired_ = false;
   released_ = false;
diff --git a/services/vr/bufferhubd/include/private/dvr/consumer_channel.h b/services/vr/bufferhubd/include/private/dvr/consumer_channel.h
index de0f23c5d0..5fb4ec1725 100644
--- a/services/vr/bufferhubd/include/private/dvr/consumer_channel.h
+++ b/services/vr/bufferhubd/include/private/dvr/consumer_channel.h
@@ -26,6 +26,7 @@ class ConsumerChannel : public BufferHubChannel {
   uint64_t client_state_mask() const { return client_state_mask_; }
   BufferInfo GetBufferInfo() const override;
 
+  void OnProducerGained();
   void OnProducerPosted();
   void OnProducerClosed();
 
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index ab1d94c3cd..b541fb3963 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -424,7 +424,7 @@ Status<void> ProducerChannel::OnProducerPost(Message&,
   // Signal any interested consumers. If there are none, the buffer will stay
   // in posted state until a consumer comes online. This behavior guarantees
   // that no frame is silently dropped.
-  for (auto consumer : consumer_channels_) {
+  for (auto& consumer : consumer_channels_) {
     consumer->OnProducerPosted();
   }
 
@@ -437,6 +437,9 @@ Status<LocalFence> ProducerChannel::OnProducerGain(Message& /*message*/) {
 
   ClearAvailable();
   post_fence_.close();
+  for (auto& consumer : consumer_channels_) {
+    consumer->OnProducerGained();
+  }
   return {std::move(returned_fence_)};
 }
 
@@ -533,7 +536,7 @@ Status<void> ProducerChannel::OnConsumerRelease(Message&,
 
   uint64_t current_buffer_state =
       buffer_state_->load(std::memory_order_acquire);
-  if (BufferHubDefs::IsClientReleased(current_buffer_state,
+  if (BufferHubDefs::IsBufferReleased(current_buffer_state &
                                       ~orphaned_consumer_bit_mask_)) {
     SignalAvailable();
     if (orphaned_consumer_bit_mask_) {
@@ -560,7 +563,7 @@ void ProducerChannel::OnConsumerOrphaned(const uint64_t& consumer_state_mask) {
 
   uint64_t current_buffer_state =
       buffer_state_->load(std::memory_order_acquire);
-  if (BufferHubDefs::IsClientReleased(current_buffer_state,
+  if (BufferHubDefs::IsBufferReleased(current_buffer_state &
                                       ~orphaned_consumer_bit_mask_)) {
     SignalAvailable();
   }