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

Commit 717c1069 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge "BufferHubQueueProducer reset buffers on disconnect" into oc-mr1-dev

parents 77dd2aaf 005f45d7
Loading
Loading
Loading
Loading
+22 −0
Original line number Diff line number Diff line
@@ -417,6 +417,28 @@ void BufferHubQueue::SetBufferRemovedCallback(BufferRemovedCallback callback) {
  on_buffer_removed_ = callback;
}

pdx::Status<void> BufferHubQueue::FreeAllBuffers() {
  // Clear all available buffers.
  available_buffers_.Clear();

  pdx::Status<void> last_error;  // No error.
  // Clear all buffers this producer queue is tracking.
  for (size_t slot = 0; slot < BufferHubQueue::kMaxQueueCapacity; slot++) {
    if (buffers_[slot] != nullptr) {
      auto status = RemoveBuffer(slot);
      if (!status) {
        ALOGE(
            "ProducerQueue::FreeAllBuffers: Failed to remove buffer at "
            "slot=%d.",
            slot);
        last_error = status.error_status();
      }
    }
  }

  return last_error;
}

ProducerQueue::ProducerQueue(LocalChannelHandle handle)
    : BASE(std::move(handle)) {
  auto status = ImportQueue();
+32 −5
Original line number Diff line number Diff line
@@ -206,11 +206,11 @@ status_t BufferHubQueueProducer::dequeueBuffer(
  // It's either in free state (if the buffer has never been used before) or
  // in queued state (if the buffer has been dequeued and queued back to
  // BufferHubQueue).
  // TODO(jwcai) Clean this up, make mBufferState compatible with BufferHub's
  // model.
  LOG_ALWAYS_FATAL_IF((!buffers_[slot].mBufferState.isFree() &&
  LOG_ALWAYS_FATAL_IF(
      (!buffers_[slot].mBufferState.isFree() &&
       !buffers_[slot].mBufferState.isQueued()),
                      "dequeueBuffer: slot %zu is not free or queued.", slot);
      "dequeueBuffer: slot %zu is not free or queued, actual state: %s.", slot,
      buffers_[slot].mBufferState.string());

  buffers_[slot].mBufferState.freeQueued();
  buffers_[slot].mBufferState.dequeue();
@@ -514,6 +514,7 @@ status_t BufferHubQueueProducer::disconnect(int api, DisconnectMode /*mode*/) {
    return BAD_VALUE;
  }

  FreeAllBuffers();
  connected_api_ = kNoConnectedApi;
  return NO_ERROR;
}
@@ -655,5 +656,31 @@ status_t BufferHubQueueProducer::RemoveBuffer(size_t slot) {
  return NO_ERROR;
}

status_t BufferHubQueueProducer::FreeAllBuffers() {
  for (size_t slot = 0; slot < BufferHubQueue::kMaxQueueCapacity; slot++) {
    // Reset in memory objects related the the buffer.
    buffers_[slot].mGraphicBuffer = nullptr;
    buffers_[slot].mBufferState.reset();
    buffers_[slot].mRequestBufferCalled = false;
    buffers_[slot].mBufferProducer = nullptr;
    buffers_[slot].mFence = Fence::NO_FENCE;
  }

  auto status = queue_->FreeAllBuffers();
  if (!status) {
    ALOGE(
        "BufferHubQueueProducer::FreeAllBuffers: Failed to free all buffers on "
        "the queue: %s",
        status.GetErrorMessage().c_str());
  }

  if (queue_->capacity() != 0 || queue_->count() != 0) {
    LOG_ALWAYS_FATAL(
        "BufferHubQueueProducer::FreeAllBuffers: Not all buffers are freed.");
  }

  return NO_ERROR;
}

}  // namespace dvr
}  // namespace android
+9 −0
Original line number Diff line number Diff line
@@ -122,6 +122,10 @@ class BufferHubQueue : public pdx::Client {
  // to deregister a buffer for epoll and internal bookkeeping.
  virtual pdx::Status<void> RemoveBuffer(size_t slot);

  // Free all buffers that belongs to this queue. Can only be called from
  // producer side.
  virtual pdx::Status<void> FreeAllBuffers();

  // Dequeue a buffer from the free queue, blocking until one is available. The
  // timeout argument specifies the number of milliseconds that |Dequeue()| will
  // block. Specifying a timeout of -1 causes Dequeue() to block indefinitely,
@@ -297,6 +301,11 @@ class ProducerQueue : public pdx::ClientBase<ProducerQueue, BufferHubQueue> {
  // Remove producer buffer from the queue.
  pdx::Status<void> RemoveBuffer(size_t slot) override;

  // Free all buffers on this producer queue.
  pdx::Status<void> FreeAllBuffers() override {
    return BufferHubQueue::FreeAllBuffers();
  }

  // Dequeue a producer buffer to write. The returned buffer in |Gain|'ed mode,
  // and caller should call Post() once it's done writing to release the buffer
  // to the consumer side.
+4 −0
Original line number Diff line number Diff line
@@ -135,6 +135,10 @@ class BufferHubQueueProducer : public BnGraphicBufferProducer {
  // Remove a buffer via BufferHubRPC.
  status_t RemoveBuffer(size_t slot);

  // Free all buffers which are owned by the prodcuer. Note that if graphic
  // buffers are acquired by the consumer, we can't .
  status_t FreeAllBuffers();

  // Concreate implementation backed by BufferHubBuffer.
  std::shared_ptr<ProducerQueue> queue_;

+97 −0
Original line number Diff line number Diff line
@@ -570,6 +570,103 @@ TEST_F(BufferHubQueueTest, TestQueueInfo) {
  EXPECT_EQ(consumer_queue_->is_async(), kIsAsync);
}

TEST_F(BufferHubQueueTest, TestFreeAllBuffers) {
  constexpr size_t kBufferCount = 2;

#define CHECK_NO_BUFFER_THEN_ALLOCATE(num_buffers)  \
  EXPECT_EQ(consumer_queue_->count(), 0U);          \
  EXPECT_EQ(consumer_queue_->capacity(), 0U);       \
  EXPECT_EQ(producer_queue_->count(), 0U);          \
  EXPECT_EQ(producer_queue_->capacity(), 0U);       \
  for (size_t i = 0; i < num_buffers; i++) {        \
    AllocateBuffer();                               \
  }                                                 \
  EXPECT_EQ(producer_queue_->count(), num_buffers); \
  EXPECT_EQ(producer_queue_->capacity(), num_buffers);

  size_t slot;
  uint64_t seq;
  LocalHandle fence;
  pdx::Status<void> status;
  pdx::Status<std::shared_ptr<BufferConsumer>> consumer_status;
  pdx::Status<std::shared_ptr<BufferProducer>> producer_status;
  std::shared_ptr<BufferConsumer> consumer_buffer;
  std::shared_ptr<BufferProducer> producer_buffer;

  ASSERT_TRUE(CreateQueues(config_builder_.SetMetadata<uint64_t>().Build(),
                           UsagePolicy{}));

  // Free all buffers when buffers are avaible for dequeue.
  CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
  status = producer_queue_->FreeAllBuffers();
  EXPECT_TRUE(status.ok());

  // Free all buffers when one buffer is dequeued.
  CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
  producer_status = producer_queue_->Dequeue(0, &slot, &fence);
  ASSERT_TRUE(producer_status.ok());
  status = producer_queue_->FreeAllBuffers();
  EXPECT_TRUE(status.ok());

  // Free all buffers when all buffers are dequeued.
  CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
  for (size_t i = 0; i < kBufferCount; i++) {
    producer_status = producer_queue_->Dequeue(0, &slot, &fence);
    ASSERT_TRUE(producer_status.ok());
  }
  status = producer_queue_->FreeAllBuffers();
  EXPECT_TRUE(status.ok());

  // Free all buffers when one buffer is posted.
  CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
  producer_status = producer_queue_->Dequeue(0, &slot, &fence);
  ASSERT_TRUE(producer_status.ok());
  producer_buffer = producer_status.take();
  ASSERT_NE(nullptr, producer_buffer);
  ASSERT_EQ(0, producer_buffer->Post(fence, &seq, sizeof(seq)));
  status = producer_queue_->FreeAllBuffers();
  EXPECT_TRUE(status.ok());

  // Free all buffers when all buffers are posted.
  CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
  for (size_t i = 0; i < kBufferCount; i++) {
    producer_status = producer_queue_->Dequeue(0, &slot, &fence);
    ASSERT_TRUE(producer_status.ok());
    producer_buffer = producer_status.take();
    ASSERT_NE(nullptr, producer_buffer);
    ASSERT_EQ(0, producer_buffer->Post(fence, &seq, sizeof(seq)));
  }
  status = producer_queue_->FreeAllBuffers();
  EXPECT_TRUE(status.ok());

  // Free all buffers when all buffers are acquired.
  CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
  for (size_t i = 0; i < kBufferCount; i++) {
    producer_status = producer_queue_->Dequeue(0, &slot, &fence);
    ASSERT_TRUE(producer_status.ok());
    producer_buffer = producer_status.take();
    ASSERT_NE(nullptr, producer_buffer);
    ASSERT_EQ(0, producer_buffer->Post(fence, &seq, sizeof(seq)));
    consumer_status = consumer_queue_->Dequeue(0, &slot, &seq, &fence);
    ASSERT_TRUE(consumer_status.ok());
  }

  status = producer_queue_->FreeAllBuffers();
  EXPECT_TRUE(status.ok());

  // In addition to FreeAllBuffers() from the queue, it is also required to
  // delete all references to the ProducerBuffer (i.e. the PDX client).
  producer_buffer = nullptr;

  // Crank consumer queue events to pickup EPOLLHUP events on the queue.
  consumer_queue_->HandleQueueEvents();

  // One last check.
  CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);

#undef CHECK_NO_BUFFER_THEN_ALLOCATE
}

}  // namespace

}  // namespace dvr
Loading