Explicitly clears signal upon OnProducerGained in consumer channels
And fix some unreadable code around buffer state
and a wrong-but-no-effect-yet fence return.
Bug: 120569702
Test: patch ag/4833259 and not see washing machine
Test: AHardwareBufferTest BufferHubBuffer_test
buffer_hub_queue_producer-test buffer_hub-test buffer_hub_queue-test
dvr_buffer_queue-test dvr_api-test libdvrtracking-test
Change-Id: I63b18a78ea05d2084b2ed29fbcaacbb652d48846
diff --git a/libs/vr/libbufferhub/producer_buffer.cpp b/libs/vr/libbufferhub/producer_buffer.cpp
index de03fad..cd92b62 100644
--- a/libs/vr/libbufferhub/producer_buffer.cpp
+++ b/libs/vr/libbufferhub/producer_buffer.cpp
@@ -224,15 +224,17 @@
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 f936e95..158ef9c 100644
--- a/services/vr/bufferhubd/consumer_channel.cpp
+++ b/services/vr/bufferhubd/consumer_channel.cpp
@@ -153,6 +153,17 @@
}
}
+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 de0f23c..5fb4ec1 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 @@
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 ab1d94c..b541fb3 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -424,7 +424,7 @@
// 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 @@
ClearAvailable();
post_fence_.close();
+ for (auto& consumer : consumer_channels_) {
+ consumer->OnProducerGained();
+ }
return {std::move(returned_fence_)};
}
@@ -533,7 +536,7 @@
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 @@
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();
}