Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ae9668c1 authored by Tianyu Jiang's avatar Tianyu Jiang
Browse files

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
parent 616aca2f
Loading
Loading
Loading
Loading
+5 −3
Original line number Original line Diff line number Diff line
@@ -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_fence_state = fence_state_->load(std::memory_order_acquire);
  uint64_t current_active_clients_bit_mask =
  uint64_t current_active_clients_bit_mask =
      active_clients_bit_mask_->load(std::memory_order_acquire);
      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
  // TODO(b/112007999) add an atomic variable in metadata header in shared
  // memory to indicate which client is the last producer of the buffer.
  // memory to indicate which client is the last producer of the buffer.
  // Currently, assume the first client is the only producer to the buffer.
  // Currently, assume the first client is the only producer to the buffer.
  if (current_fence_state & current_active_clients_bit_mask &
  if (current_fence_state & current_active_clients_bit_mask &
      (~BufferHubDefs::kFirstClientBitMask)) {
      (~BufferHubDefs::kFirstClientBitMask)) {
    *out_fence = shared_release_fence_.Duplicate();
    *out_fence = shared_release_fence_.Duplicate();
    out_meta->release_fence_mask =
    out_meta->release_fence_mask = current_fence_state &
        current_fence_state & current_active_clients_bit_mask;
                                   current_active_clients_bit_mask &
                                   (~BufferHubDefs::kFirstClientBitMask);
  }
  }


  return 0;
  return 0;
+11 −0
Original line number Original line Diff line number Diff line
@@ -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() {
void ConsumerChannel::OnProducerPosted() {
  acquired_ = false;
  acquired_ = false;
  released_ = false;
  released_ = false;
+1 −0
Original line number Original line Diff line number Diff line
@@ -26,6 +26,7 @@ class ConsumerChannel : public BufferHubChannel {
  uint64_t client_state_mask() const { return client_state_mask_; }
  uint64_t client_state_mask() const { return client_state_mask_; }
  BufferInfo GetBufferInfo() const override;
  BufferInfo GetBufferInfo() const override;


  void OnProducerGained();
  void OnProducerPosted();
  void OnProducerPosted();
  void OnProducerClosed();
  void OnProducerClosed();


+6 −3
Original line number Original line Diff line number Diff line
@@ -424,7 +424,7 @@ Status<void> ProducerChannel::OnProducerPost(Message&,
  // Signal any interested consumers. If there are none, the buffer will stay
  // Signal any interested consumers. If there are none, the buffer will stay
  // in posted state until a consumer comes online. This behavior guarantees
  // in posted state until a consumer comes online. This behavior guarantees
  // that no frame is silently dropped.
  // that no frame is silently dropped.
  for (auto consumer : consumer_channels_) {
  for (auto& consumer : consumer_channels_) {
    consumer->OnProducerPosted();
    consumer->OnProducerPosted();
  }
  }


@@ -437,6 +437,9 @@ Status<LocalFence> ProducerChannel::OnProducerGain(Message& /*message*/) {


  ClearAvailable();
  ClearAvailable();
  post_fence_.close();
  post_fence_.close();
  for (auto& consumer : consumer_channels_) {
    consumer->OnProducerGained();
  }
  return {std::move(returned_fence_)};
  return {std::move(returned_fence_)};
}
}


@@ -533,7 +536,7 @@ Status<void> ProducerChannel::OnConsumerRelease(Message&,


  uint64_t current_buffer_state =
  uint64_t current_buffer_state =
      buffer_state_->load(std::memory_order_acquire);
      buffer_state_->load(std::memory_order_acquire);
  if (BufferHubDefs::IsClientReleased(current_buffer_state,
  if (BufferHubDefs::IsBufferReleased(current_buffer_state &
                                      ~orphaned_consumer_bit_mask_)) {
                                      ~orphaned_consumer_bit_mask_)) {
    SignalAvailable();
    SignalAvailable();
    if (orphaned_consumer_bit_mask_) {
    if (orphaned_consumer_bit_mask_) {
@@ -560,7 +563,7 @@ void ProducerChannel::OnConsumerOrphaned(const uint64_t& consumer_state_mask) {


  uint64_t current_buffer_state =
  uint64_t current_buffer_state =
      buffer_state_->load(std::memory_order_acquire);
      buffer_state_->load(std::memory_order_acquire);
  if (BufferHubDefs::IsClientReleased(current_buffer_state,
  if (BufferHubDefs::IsBufferReleased(current_buffer_state &
                                      ~orphaned_consumer_bit_mask_)) {
                                      ~orphaned_consumer_bit_mask_)) {
    SignalAvailable();
    SignalAvailable();
  }
  }