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

Commit b53da4fd authored by Jiwen 'Steve' Cai's avatar Jiwen 'Steve' Cai
Browse files

dvr_read_buffer_queue: don't stop releasing buffer 

We used to abort ReleaseBuffer logic if we cannot verify that the buffer
being released is in the target queue. However, in certain edge case,
this could be true and we still want to release the buffer. One example
is when a ProducerQueue closes and triggers its ConsumerQueue to remove
all its buffers. In this case, ConsumerQueue::ReleaseBuffer() should
still function properly.

Bug: 77982072
Test: dvr_buffer_queue-test
Change-Id: I6d8e2525470c8e3d78862d3f0753c8735d528b53
parent 0c7c5d6c

libs/vr/libdvr/dvr_buffer.cpp

+4 −2
Original line number Diff line number Diff line
@@ -48,7 +48,8 @@ void dvrWriteBufferDestroy(DvrWriteBuffer* write_buffer) { 
    ALOGW_IF(

        write_buffer->slot != -1,

        "dvrWriteBufferDestroy: Destroying a buffer associated with a valid "

        "buffer queue slot. This may indicate possible leaks.");

        "buffer queue slot. This may indicate possible leaks, buffer_id=%d.",

        dvrWriteBufferGetId(write_buffer));

    delete write_buffer;

  }

}
@@ -118,7 +119,8 @@ void dvrReadBufferDestroy(DvrReadBuffer* read_buffer) { 
    ALOGW_IF(

        read_buffer->slot != -1,

        "dvrReadBufferDestroy: Destroying a buffer associated with a valid "

        "buffer queue slot. This may indicate possible leaks.");

        "buffer queue slot. This may indicate possible leaks, buffer_id=%d.",

        dvrReadBufferGetId(read_buffer));

    delete read_buffer;

  }

}

libs/vr/libdvr/dvr_buffer_queue.cpp

+16 −6
Original line number Diff line number Diff line
@@ -434,12 +434,22 @@ int DvrReadBufferQueue::ReleaseBuffer(DvrReadBuffer* read_buffer, 
    return -EINVAL;

  }

  if (read_buffer->read_buffer->id() != consumer_queue_->GetBufferId(slot)) {

    if (consumer_queue_->GetBufferId(slot) > 0) {

      ALOGE(

        "DvrReadBufferQueue::ReleaseBuffer: Buffer to be released does not "

        "belong to this buffer queue. Releasing buffer: id=%d, buffer in "

        "queue: id=%d",

        read_buffer->read_buffer->id(), consumer_queue_->GetBufferId(slot));

    return -EINVAL;

          "DvrReadBufferQueue::ReleaseBuffer: Buffer to be released may not "

          "belong to this queue (queue_id=%d): attempting to release buffer "

          "(buffer_id=%d) at slot %d which holds a different buffer "

          "(buffer_id=%d).",

          consumer_queue_->id(), read_buffer->read_buffer->id(),

          static_cast<int>(slot), consumer_queue_->GetBufferId(slot));

    } else {

      ALOGI(

          "DvrReadBufferQueue::ReleaseBuffer: Buffer to be released may not "

          "belong to this queue (queue_id=%d): attempting to release buffer "

          "(buffer_id=%d) at slot %d which is empty.",

          consumer_queue_->id(), read_buffer->read_buffer->id(),

          static_cast<int>(slot));

    }

  }



  pdx::LocalHandle fence(release_fence_fd);

libs/vr/libdvr/tests/dvr_buffer_queue-test.cpp

+51 −0
Original line number Diff line number Diff line
@@ -525,4 +525,55 @@ TEST_F(DvrBufferQueueTest, StableBufferIdAndHardwareBuffer) { 
  }

}



TEST_F(DvrBufferQueueTest, ConsumerReleaseAfterProducerDestroy) {

  int ret = api_.WriteBufferQueueCreate(

      kBufferWidth, kBufferHeight, kBufferFormat, kLayerCount, kBufferUsage,

      kQueueCapacity, sizeof(DvrNativeBufferMetadata), &write_queue_);

  ASSERT_EQ(ret, 0);



  DvrReadBufferQueue* read_queue = nullptr;

  DvrReadBuffer* rb = nullptr;

  DvrWriteBuffer* wb = nullptr;

  DvrNativeBufferMetadata meta1;

  DvrNativeBufferMetadata meta2;

  int fence_fd = -1;



  ret = api_.WriteBufferQueueCreateReadQueue(write_queue_, &read_queue);

  ASSERT_EQ(ret, 0);



  api_.ReadBufferQueueSetBufferAvailableCallback(

      read_queue, &BufferAvailableCallback, this);



  // Gain buffer for writing.

  ret = api_.WriteBufferQueueGainBuffer(write_queue_, /*timeout=*/0, &wb,

                                        &meta1, &fence_fd);

  ASSERT_EQ(ret, 0);

  close(fence_fd);



  // Post buffer to the read_queue.

  ret = api_.WriteBufferQueuePostBuffer(write_queue_, wb, &meta1, /*fence=*/-1);

  ASSERT_EQ(ret, 0);

  wb = nullptr;



  // Acquire buffer for reading.

  ret = api_.ReadBufferQueueAcquireBuffer(read_queue, /*timeout=*/10, &rb,

                                          &meta2, &fence_fd);

  ASSERT_EQ(ret, 0);

  close(fence_fd);



  // Destroy the write buffer queue and make sure the reader queue is picking

  // these events up.

  api_.WriteBufferQueueDestroy(write_queue_);

  ret = api_.ReadBufferQueueHandleEvents(read_queue);

  ASSERT_EQ(0, ret);



  // Release buffer to the write_queue.

  ret = api_.ReadBufferQueueReleaseBuffer(read_queue, rb, &meta2,

                                          /*release_fence_fd=*/-1);

  ASSERT_EQ(ret, 0);

  rb = nullptr;



  api_.ReadBufferQueueDestroy(read_queue);

}



}  // namespace