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

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

Fix minor corner cases in BufferHubQueueProducer

1/ Set reasonable return value BufferHubQueueProducer::query.
2/ Don't error out for setSharedBufferMode(false),
setAutoRefresh(false), as they are just setting default values again.
3/ Supports addition buffer metadata: transformt, crop,
data_format (a.k.a. color_format).
4/ This also changes BufferHubQueueProducer to be based of
BnInterface<IGraphicBufferProducer> so that itself can be passed via Binder.

Bug: 37342387
Bug: 36907193
Test: exoplayer_demo
Change-Id: Ie00bb79f6a249e09905ae52f85a85a67744cc90d
parent 693aa74d
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -8,17 +8,17 @@ namespace dvr {
/* static */
std::shared_ptr<BufferHubQueueCore> BufferHubQueueCore::Create() {
  auto core = std::shared_ptr<BufferHubQueueCore>(new BufferHubQueueCore());
  core->producer_ = ProducerQueue::Create<BufferMetadata>();
  core->producer_ = ProducerQueue::Create<NativeBufferMetadata>();
  return core;
}

/* static */
std::shared_ptr<BufferHubQueueCore> BufferHubQueueCore::Create(
    const std::shared_ptr<ProducerQueue>& producer) {
  if (producer->metadata_size() != sizeof(BufferMetadata)) {
  if (producer->metadata_size() != sizeof(NativeBufferMetadata)) {
    ALOGE(
        "BufferHubQueueCore::Create producer's metadata size is different than "
        "the size of BufferHubQueueCore::BufferMetadata");
        "the size of BufferHubQueueCore::NativeBufferMetadata");
    return nullptr;
  }

+60 −26
Original line number Diff line number Diff line
@@ -56,7 +56,8 @@ status_t BufferHubQueueProducer::setMaxDequeuedBufferCount(

  if (max_dequeued_buffers <= 0 ||
      max_dequeued_buffers >
          static_cast<int>(BufferHubQueue::kMaxQueueCapacity)) {
          static_cast<int>(BufferHubQueue::kMaxQueueCapacity -
                           BufferHubQueueCore::kDefaultUndequeuedBuffers)) {
    ALOGE("setMaxDequeuedBufferCount: %d out of range (0, %zu]",
          max_dequeued_buffers, BufferHubQueue::kMaxQueueCapacity);
    return BAD_VALUE;
@@ -121,7 +122,8 @@ status_t BufferHubQueueProducer::dequeueBuffer(
  }

  if (static_cast<int32_t>(core_->producer_->capacity()) <
      max_dequeued_buffer_count_) {
      max_dequeued_buffer_count_ +
          BufferHubQueueCore::kDefaultUndequeuedBuffers) {
    // Lazy allocation. When the capacity of |core_->producer_| has not reach
    // |max_dequeued_buffer_count_|, allocate new buffer.
    // TODO(jwcai) To save memory, the really reasonable thing to do is to go
@@ -232,16 +234,14 @@ status_t BufferHubQueueProducer::queueBuffer(int slot,
  }

  int64_t timestamp;
  int scaling_mode;
  sp<Fence> fence;
  Rect crop(Rect::EMPTY_RECT);

  // TODO(jwcai) The following attributes are ignored.
  bool is_auto_timestamp;
  android_dataspace data_space;
  android_dataspace dataspace;
  Rect crop(Rect::EMPTY_RECT);
  int scaling_mode;
  uint32_t transform;
  sp<Fence> fence;

  input.deflate(&timestamp, &is_auto_timestamp, &data_space, &crop,
  input.deflate(&timestamp, &is_auto_timestamp, &dataspace, &crop,
                &scaling_mode, &transform, &fence);

  // Check input scaling mode is valid.
@@ -302,7 +302,17 @@ status_t BufferHubQueueProducer::queueBuffer(int slot,

  LocalHandle fence_fd(fence->isValid() ? fence->dup() : -1);

  BufferHubQueueCore::BufferMetadata meta_data = {.timestamp = timestamp};
  BufferHubQueueCore::NativeBufferMetadata meta_data = {};
  meta_data.timestamp = timestamp;
  meta_data.is_auto_timestamp = static_cast<int32_t>(is_auto_timestamp);
  meta_data.dataspace = static_cast<int32_t>(dataspace);
  meta_data.crop_left = crop.left;
  meta_data.crop_top = crop.top;
  meta_data.crop_right = crop.right;
  meta_data.crop_bottom = crop.bottom;
  meta_data.scaling_mode = static_cast<int32_t>(scaling_mode);
  meta_data.transform = static_cast<int32_t>(transform);

  buffer_producer->Post(fence_fd, &meta_data, sizeof(meta_data));
  core_->buffers_[slot].mBufferState.queue();

@@ -369,7 +379,10 @@ status_t BufferHubQueueProducer::query(int what, int* out_value) {
  int value = 0;
  switch (what) {
    case NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS:
      value = 0;
      // TODO(b/36187402) This should be the maximum number of buffers that this
      // producer queue's consumer can acquire. Set to be at least one. Need to
      // find a way to set from the consumer side.
      value = BufferHubQueueCore::kDefaultUndequeuedBuffers;
      break;
    case NATIVE_WINDOW_BUFFER_AGE:
      value = 0;
@@ -394,11 +407,16 @@ status_t BufferHubQueueProducer::query(int what, int* out_value) {
      // IGraphicBufferConsumer parity.
      value = 0;
      break;
    // The following queries are currently considered as unsupported.
    // TODO(jwcai) Need to carefully check the whether they should be
    // supported after all.
    case NATIVE_WINDOW_STICKY_TRANSFORM:
    case NATIVE_WINDOW_DEFAULT_DATASPACE:
      // TODO(jwcai) Return the default value android::BufferQueue is using as
      // there is no way dvr::ConsumerQueue can set it.
      value = 0;  // HAL_DATASPACE_UNKNOWN
      break;
    case NATIVE_WINDOW_STICKY_TRANSFORM:
      // TODO(jwcai) Return the default value android::BufferQueue is using as
      // there is no way dvr::ConsumerQueue can set it.
      value = 0;
      break;
    default:
      return BAD_VALUE;
  }
@@ -432,7 +450,16 @@ status_t BufferHubQueueProducer::connect(
    case NATIVE_WINDOW_API_MEDIA:
    case NATIVE_WINDOW_API_CAMERA:
      core_->connected_api_ = api;
      // TODO(jwcai) Fill output.

      output->width = core_->producer_->default_width();
      output->height = core_->producer_->default_height();

      // default values, we don't use them yet.
      output->transformHint = 0;
      output->numPendingBuffers = 0;
      output->nextFrameNumber = 0;
      output->bufferReplaced = false;

      break;
    default:
      ALOGE("BufferHubQueueProducer::connect: unknow API %d", api);
@@ -503,17 +530,24 @@ String8 BufferHubQueueProducer::getConsumerName() const {
  return String8("BufferHubQueue::DummyConsumer");
}

status_t BufferHubQueueProducer::setSharedBufferMode(
    bool /* shared_buffer_mode */) {
  ALOGE("BufferHubQueueProducer::setSharedBufferMode not implemented.");
status_t BufferHubQueueProducer::setSharedBufferMode(bool shared_buffer_mode) {
  if (shared_buffer_mode) {
    ALOGE("BufferHubQueueProducer::setSharedBufferMode(true) is not supported.");
    // TODO(b/36373181) Front buffer mode for buffer hub queue as ANativeWindow.
    return INVALID_OPERATION;
  }
  // Setting to default should just work as a no-op.
  return NO_ERROR;
}

status_t BufferHubQueueProducer::setAutoRefresh(bool /* auto_refresh */) {
  ALOGE("BufferHubQueueProducer::setAutoRefresh not implemented.");
status_t BufferHubQueueProducer::setAutoRefresh(bool auto_refresh) {
  if (auto_refresh) {
    ALOGE("BufferHubQueueProducer::setAutoRefresh(true) is not supported.");
    return INVALID_OPERATION;
  }
  // Setting to default should just work as a no-op.
  return NO_ERROR;
}

status_t BufferHubQueueProducer::setDequeueTimeout(nsecs_t timeout) {
  ALOGD_IF(TRACE, __FUNCTION__);
@@ -545,8 +579,8 @@ status_t BufferHubQueueProducer::getUniqueId(uint64_t* out_id) const {
IBinder* BufferHubQueueProducer::onAsBinder() {
  // BufferHubQueueProducer is a non-binder implementation of
  // IGraphicBufferProducer.
  ALOGW("BufferHubQueueProducer::onAsBinder is not supported.");
  return nullptr;
  ALOGW("BufferHubQueueProducer::onAsBinder is not efficiently supported.");
  return this;
}

}  // namespace dvr
+2 −2
Original line number Diff line number Diff line
@@ -215,11 +215,11 @@ class BufferHubQueue : public pdx::Client {
  //    This is implemented by first detaching the buffer and then allocating a
  //    new buffer.
  // 3. During the same epoll_wait, Consumer queue's client side gets EPOLLIN
  //    event on the queue which indicates a new buffer is avaiable and the
  //    event on the queue which indicates a new buffer is available and the
  //    EPOLLHUP event for slot 0. Consumer handles these two events in order.
  // 4. Consumer client calls BufferHubRPC::ConsumerQueueImportBuffers and both
  //    slot 0 and (the new) slot 1 buffer will be imported. During the import
  //    of the buffer at slot 1, consuemr client detaches the old buffer so that
  //    of the buffer at slot 1, consumer client detaches the old buffer so that
  //    the new buffer can be registered. At the same time
  //    |epollhup_pending_[slot]| is marked to indicate that buffer at this slot
  //    was detached prior to EPOLLHUP event.
+40 −2
Original line number Diff line number Diff line
@@ -19,16 +19,54 @@ class BufferHubQueueCore {
 public:
  static constexpr int kNoConnectedApi = -1;

  // TODO(b/36187402) The actual implementation of BufferHubQueue's consumer
  // side logic doesn't limit the number of buffer it can acquire
  // simultaneously. We need a way for consumer logic to configure and enforce
  // that.
  static constexpr int kDefaultUndequeuedBuffers = 1;

  // Create a BufferHubQueueCore instance by creating a new producer queue.
  static std::shared_ptr<BufferHubQueueCore> Create();

  // Create a BufferHubQueueCore instance by importing an existing prodcuer queue.
  // Create a BufferHubQueueCore instance by importing an existing prodcuer
  // queue.
  static std::shared_ptr<BufferHubQueueCore> Create(
      const std::shared_ptr<ProducerQueue>& producer);

  struct BufferMetadata {
  // The buffer metadata that an Android Surface (a.k.a. ANativeWindow)
  // will populate. This must be aligned with the |DvrNativeBufferMetadata|
  // defined in |dvr_buffer_queue.h|. Please do not remove, modify, or reorder
  // existing data members. If new fields need to be added, please take extra
  // care to make sure that new data field is padded properly the size of the
  // struct stays same.
  // TODO(b/37578558) Move |dvr_api.h| into a header library so that this
  // structure won't be copied between |dvr_api.h| and |buffer_hub_qeue_core.h|.
  struct NativeBufferMetadata {
    // Timestamp of the frame.
    int64_t timestamp;

    // Whether the buffer is using auto timestamp.
    int32_t is_auto_timestamp;

    // Must be one of the HAL_DATASPACE_XXX value defined in system/graphics.h
    int32_t dataspace;

    // Crop extracted from an ACrop or android::Crop object.
    int32_t crop_left;
    int32_t crop_top;
    int32_t crop_right;
    int32_t crop_bottom;

    // Must be one of the NATIVE_WINDOW_SCALING_MODE_XXX value defined in
    // system/window.h.
    int32_t scaling_mode;

    // Must be one of the ANATIVEWINDOW_TRANSFORM_XXX value defined in
    // android/native_window.h
    int32_t transform;

    // Reserved bytes for so that the struct is forward compatible.
    int32_t reserved[16];
  };

  class NativeBuffer
+1 −1
Original line number Diff line number Diff line
@@ -8,7 +8,7 @@
namespace android {
namespace dvr {

class BufferHubQueueProducer : public IGraphicBufferProducer {
class BufferHubQueueProducer : public BnInterface<IGraphicBufferProducer> {
 public:
  BufferHubQueueProducer(const std::shared_ptr<BufferHubQueueCore>& core);

Loading