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

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

Simplify ProducerQueue::Create 

This is a preparation CL that simplifies ProducerQueue's constructor,
Create(), and bufferhub_rpc::CreateProducerQueue prior to introducing
more queue attributes (is_async, width, height, max_capacity, etc) on
creation.

1/ Consolidate and/or remove unnecessary ProducerQueue::Create's
overloading (we had way too many overloads of the create function and I
figured it's awfully painful to introduce new attributes).
2/ Use UsagePolicy in ProducerQueue::Create. Also added default values
for UsagePolicy, so that empty uniform initialization gives us a default
policy. This helps us removing all ProducerQueue::Create overloading on
whether default usage policy is needed.
3/ Move |meta_size_bytes| into ProducerQueueConfig.

Bug: 38430974
Test: buffer_hub_queue_producer-test, buffer_hub_queue-test, dvr_api-test
Change-Id: Ieba9f4d1bce2162bd1e6063989985afc8d014dc7
parent 1f76a0da

libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h

+13 −6
Original line number Original line Diff line number Diff line
@@ -129,19 +129,26 @@ class FenceHandle { 
using LocalFence = FenceHandle<pdx::LocalHandle>;

using LocalFence = FenceHandle<pdx::LocalHandle>;

using BorrowedFence = FenceHandle<pdx::BorrowedHandle>;

using BorrowedFence = FenceHandle<pdx::BorrowedHandle>;





struct QueueInfo {

struct ProducerQueueConfig {

  size_t meta_size_bytes;

  size_t meta_size_bytes;



 private:

  PDX_SERIALIZABLE_MEMBERS(ProducerQueueConfig, meta_size_bytes);

};



struct QueueInfo {

  ProducerQueueConfig producer_config;

  int id;

  int id;





 private:

 private:

  PDX_SERIALIZABLE_MEMBERS(QueueInfo, meta_size_bytes, id);

  PDX_SERIALIZABLE_MEMBERS(QueueInfo, producer_config, id);

};

};





struct UsagePolicy {

struct UsagePolicy {

  uint64_t usage_set_mask;

  uint64_t usage_set_mask{0};

  uint64_t usage_clear_mask;

  uint64_t usage_clear_mask{0};

  uint64_t usage_deny_set_mask;

  uint64_t usage_deny_set_mask{0};

  uint64_t usage_deny_clear_mask;

  uint64_t usage_deny_clear_mask{0};





 private:

 private:

  PDX_SERIALIZABLE_MEMBERS(UsagePolicy, usage_set_mask, usage_clear_mask,

  PDX_SERIALIZABLE_MEMBERS(UsagePolicy, usage_set_mask, usage_clear_mask,

libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp

+5 −13
Original line number Original line Diff line number Diff line
@@ -10,7 +10,6 @@ 
#include <pdx/default_transport/client_channel.h>

#include <pdx/default_transport/client_channel.h>

#include <pdx/default_transport/client_channel_factory.h>

#include <pdx/default_transport/client_channel_factory.h>

#include <pdx/file_handle.h>

#include <pdx/file_handle.h>

#include <private/dvr/bufferhub_rpc.h>





#define RETRY_EINTR(fnc_call)                 \

#define RETRY_EINTR(fnc_call)                 \

  ([&]() -> decltype(fnc_call) {              \

  ([&]() -> decltype(fnc_call) {              \
@@ -106,7 +105,7 @@ Status<void> BufferHubQueue::ImportQueue() { 
          status.GetErrorMessage().c_str());

          status.GetErrorMessage().c_str());

    return ErrorStatus(status.error());

    return ErrorStatus(status.error());

  } else {

  } else {

    SetupQueue(status.get().meta_size_bytes, status.get().id);

    SetupQueue(status.get().producer_config.meta_size_bytes, status.get().id);

    return {};

    return {};

  }

  }

}

}
@@ -396,9 +395,6 @@ Status<std::shared_ptr<BufferHubBuffer>> BufferHubQueue::Dequeue( 
  return {std::move(buffer)};

  return {std::move(buffer)};

}

}





ProducerQueue::ProducerQueue(size_t meta_size)

    : ProducerQueue(meta_size, 0, 0, 0, 0) {}



ProducerQueue::ProducerQueue(LocalChannelHandle handle)

ProducerQueue::ProducerQueue(LocalChannelHandle handle)

    : BASE(std::move(handle)) {

    : BASE(std::move(handle)) {

  auto status = ImportQueue();

  auto status = ImportQueue();
@@ -409,14 +405,10 @@ ProducerQueue::ProducerQueue(LocalChannelHandle handle) 
  }

  }

}

}





ProducerQueue::ProducerQueue(size_t meta_size, uint64_t usage_set_mask,

ProducerQueue::ProducerQueue(size_t meta_size, const UsagePolicy& usage)

                             uint64_t usage_clear_mask,

                             uint64_t usage_deny_set_mask,

                             uint64_t usage_deny_clear_mask)

    : BASE(BufferHubRPC::kClientPath) {

    : BASE(BufferHubRPC::kClientPath) {

  auto status = InvokeRemoteMethod<BufferHubRPC::CreateProducerQueue>(

  auto status =

      meta_size, UsagePolicy{usage_set_mask, usage_clear_mask,

      InvokeRemoteMethod<BufferHubRPC::CreateProducerQueue>(meta_size, usage);

                             usage_deny_set_mask, usage_deny_clear_mask});

  if (!status) {

  if (!status) {

    ALOGE("ProducerQueue::ProducerQueue: Failed to create producer queue: %s",

    ALOGE("ProducerQueue::ProducerQueue: Failed to create producer queue: %s",

          status.GetErrorMessage().c_str());

          status.GetErrorMessage().c_str());
@@ -424,7 +416,7 @@ ProducerQueue::ProducerQueue(size_t meta_size, uint64_t usage_set_mask, 
    return;

    return;

  }

  }





  SetupQueue(status.get().meta_size_bytes, status.get().id);

  SetupQueue(status.get().producer_config.meta_size_bytes, status.get().id);

}

}





Status<void> ProducerQueue::AllocateBuffer(uint32_t width, uint32_t height,

Status<void> ProducerQueue::AllocateBuffer(uint32_t width, uint32_t height,

libs/vr/libbufferhubqueue/buffer_hub_queue_producer.cpp

+2 −1
Original line number Original line Diff line number Diff line
@@ -11,7 +11,8 @@ namespace dvr { 
/* static */

/* static */

sp<BufferHubQueueProducer> BufferHubQueueProducer::Create() {

sp<BufferHubQueueProducer> BufferHubQueueProducer::Create() {

  sp<BufferHubQueueProducer> producer = new BufferHubQueueProducer;

  sp<BufferHubQueueProducer> producer = new BufferHubQueueProducer;

  producer->queue_ = ProducerQueue::Create<DvrNativeBufferMetadata>();

  producer->queue_ =

      ProducerQueue::Create<DvrNativeBufferMetadata>(UsagePolicy{});

  return producer;

  return producer;

}

}

libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_client.h

+8 −32
Original line number Original line Diff line number Diff line
@@ -5,6 +5,7 @@ 




#include <pdx/client.h>

#include <pdx/client.h>

#include <pdx/status.h>

#include <pdx/status.h>

#include <private/dvr/bufferhub_rpc.h>

#include <private/dvr/buffer_hub_client.h>

#include <private/dvr/buffer_hub_client.h>

#include <private/dvr/epoll_file_descriptor.h>

#include <private/dvr/epoll_file_descriptor.h>

#include <private/dvr/ring_buffer.h>

#include <private/dvr/ring_buffer.h>
@@ -222,14 +223,6 @@ class BufferHubQueue : public pdx::Client { 




class ProducerQueue : public pdx::ClientBase<ProducerQueue, BufferHubQueue> {

class ProducerQueue : public pdx::ClientBase<ProducerQueue, BufferHubQueue> {

 public:

 public:

  template <typename Meta>

  static std::unique_ptr<ProducerQueue> Create() {

    return BASE::Create(sizeof(Meta));

  }

  static std::unique_ptr<ProducerQueue> Create(size_t meta_size_bytes) {

    return BASE::Create(meta_size_bytes);

  }



  // Usage bits in |usage_set_mask| will be automatically masked on. Usage bits

  // Usage bits in |usage_set_mask| will be automatically masked on. Usage bits

  // in |usage_clear_mask| will be automatically masked off. Note that

  // in |usage_clear_mask| will be automatically masked off. Note that

  // |usage_set_mask| and |usage_clear_mask| may conflict with each other, but

  // |usage_set_mask| and |usage_clear_mask| may conflict with each other, but
@@ -242,20 +235,12 @@ class ProducerQueue : public pdx::ClientBase<ProducerQueue, BufferHubQueue> { 
  // |usage_deny_clear_mask| shall not conflict with each other. Such

  // |usage_deny_clear_mask| shall not conflict with each other. Such

  // configuration will be treated as invalid input on creation.

  // configuration will be treated as invalid input on creation.

  template <typename Meta>

  template <typename Meta>

  static std::unique_ptr<ProducerQueue> Create(uint32_t usage_set_mask,

  static std::unique_ptr<ProducerQueue> Create(const UsagePolicy& usage) {

                                               uint32_t usage_clear_mask,

    return BASE::Create(sizeof(Meta), usage);

                                               uint32_t usage_deny_set_mask,

                                               uint32_t usage_deny_clear_mask) {

    return BASE::Create(sizeof(Meta), usage_set_mask, usage_clear_mask,

                        usage_deny_set_mask, usage_deny_clear_mask);

  }

  }

  static std::unique_ptr<ProducerQueue> Create(size_t meta_size_bytes,

  static std::unique_ptr<ProducerQueue> Create(size_t meta_size_bytes,

                                               uint32_t usage_set_mask,

                                               const UsagePolicy& usage) {

                                               uint32_t usage_clear_mask,

    return BASE::Create(meta_size_bytes, usage);

                                               uint32_t usage_deny_set_mask,

                                               uint32_t usage_deny_clear_mask) {

    return BASE::Create(meta_size_bytes, usage_set_mask, usage_clear_mask,

                        usage_deny_set_mask, usage_deny_clear_mask);

  }

  }





  // Import a ProducerQueue from a channel handle.

  // Import a ProducerQueue from a channel handle.
@@ -305,11 +290,8 @@ class ProducerQueue : public pdx::ClientBase<ProducerQueue, BufferHubQueue> { 
  // Constructors are automatically exposed through ProducerQueue::Create(...)

  // Constructors are automatically exposed through ProducerQueue::Create(...)

  // static template methods inherited from ClientBase, which take the same

  // static template methods inherited from ClientBase, which take the same

  // arguments as the constructors.

  // arguments as the constructors.

  explicit ProducerQueue(size_t meta_size);

  explicit ProducerQueue(pdx::LocalChannelHandle handle);

  explicit ProducerQueue(pdx::LocalChannelHandle handle);

  ProducerQueue(size_t meta_size, uint64_t usage_set_mask,

  ProducerQueue(size_t meta_size, const UsagePolicy& usage);

                uint64_t usage_clear_mask, uint64_t usage_deny_set_mask,

                uint64_t usage_deny_clear_mask);





  pdx::Status<Entry> OnBufferReady(

  pdx::Status<Entry> OnBufferReady(

      const std::shared_ptr<BufferHubBuffer>& buffer, size_t slot) override;

      const std::shared_ptr<BufferHubBuffer>& buffer, size_t slot) override;
@@ -317,15 +299,9 @@ class ProducerQueue : public pdx::ClientBase<ProducerQueue, BufferHubQueue> { 




// Explicit specializations of ProducerQueue::Create for void metadata type.

// Explicit specializations of ProducerQueue::Create for void metadata type.

template <>

template <>

inline std::unique_ptr<ProducerQueue> ProducerQueue::Create<void>() {

  return ProducerQueue::Create(0);

}

template <>

inline std::unique_ptr<ProducerQueue> ProducerQueue::Create<void>(

inline std::unique_ptr<ProducerQueue> ProducerQueue::Create<void>(

    uint32_t usage_set_mask, uint32_t usage_clear_mask,

    const UsagePolicy& usage) {

    uint32_t usage_deny_set_mask, uint32_t usage_deny_clear_mask) {

  return ProducerQueue::Create(0, usage);

  return ProducerQueue::Create(0, usage_set_mask, usage_clear_mask,

                               usage_deny_set_mask, usage_deny_clear_mask);

}

}





class ConsumerQueue : public BufferHubQueue {

class ConsumerQueue : public BufferHubQueue {

libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp

+16 −26
Original line number Original line Diff line number Diff line
@@ -25,13 +25,8 @@ constexpr int kBufferUsage = GRALLOC_USAGE_SW_READ_RARELY; 
class BufferHubQueueTest : public ::testing::Test {

class BufferHubQueueTest : public ::testing::Test {

 public:

 public:

  template <typename Meta>

  template <typename Meta>

  bool CreateProducerQueue(uint64_t usage_set_mask = 0,

  bool CreateProducerQueue(const UsagePolicy& usage) {

                           uint64_t usage_clear_mask = 0,

    producer_queue_ = ProducerQueue::Create<Meta>(usage);

                           uint64_t usage_deny_set_mask = 0,

                           uint64_t usage_deny_clear_mask = 0) {

    producer_queue_ =

        ProducerQueue::Create<Meta>(usage_set_mask, usage_clear_mask,

                                    usage_deny_set_mask, usage_deny_clear_mask);

    return producer_queue_ != nullptr;

    return producer_queue_ != nullptr;

  }

  }
@@ -45,13 +40,8 @@ class BufferHubQueueTest : public ::testing::Test { 
  }

  }





  template <typename Meta>

  template <typename Meta>

  bool CreateQueues(int usage_set_mask = 0, int usage_clear_mask = 0,

  bool CreateQueues(const UsagePolicy& usage) {

                    int usage_deny_set_mask = 0,

    return CreateProducerQueue<Meta>(usage) && CreateConsumerQueue();

                    int usage_deny_clear_mask = 0) {

    return CreateProducerQueue<Meta>(usage_set_mask, usage_clear_mask,

                                     usage_deny_set_mask,

                                     usage_deny_clear_mask) &&

           CreateConsumerQueue();

  }

  }





  void AllocateBuffer(size_t* slot_out = nullptr) {

  void AllocateBuffer(size_t* slot_out = nullptr) {
@@ -74,7 +64,7 @@ class BufferHubQueueTest : public ::testing::Test { 
TEST_F(BufferHubQueueTest, TestDequeue) {

TEST_F(BufferHubQueueTest, TestDequeue) {

  const size_t nb_dequeue_times = 16;

  const size_t nb_dequeue_times = 16;





  ASSERT_TRUE(CreateQueues<size_t>());

  ASSERT_TRUE(CreateQueues<size_t>(UsagePolicy{}));





  // Allocate only one buffer.

  // Allocate only one buffer.

  AllocateBuffer();

  AllocateBuffer();
@@ -104,7 +94,7 @@ TEST_F(BufferHubQueueTest, TestProducerConsumer) { 
  size_t slot;

  size_t slot;

  uint64_t seq;

  uint64_t seq;





  ASSERT_TRUE(CreateQueues<uint64_t>());

  ASSERT_TRUE(CreateQueues<uint64_t>(UsagePolicy{}));





  for (size_t i = 0; i < kBufferCount; i++) {

  for (size_t i = 0; i < kBufferCount; i++) {

    AllocateBuffer();

    AllocateBuffer();
@@ -175,7 +165,7 @@ TEST_F(BufferHubQueueTest, TestProducerConsumer) { 
}

}





TEST_F(BufferHubQueueTest, TestDetach) {

TEST_F(BufferHubQueueTest, TestDetach) {

  ASSERT_TRUE(CreateProducerQueue<void>());

  ASSERT_TRUE(CreateProducerQueue<void>(UsagePolicy{}));





  // Allocate buffers.

  // Allocate buffers.

  const size_t kBufferCount = 4u;

  const size_t kBufferCount = 4u;
@@ -278,7 +268,7 @@ TEST_F(BufferHubQueueTest, TestDetach) { 
}

}





TEST_F(BufferHubQueueTest, TestMultipleConsumers) {

TEST_F(BufferHubQueueTest, TestMultipleConsumers) {

  ASSERT_TRUE(CreateProducerQueue<void>());

  ASSERT_TRUE(CreateProducerQueue<void>(UsagePolicy{}));





  // Allocate buffers.

  // Allocate buffers.

  const size_t kBufferCount = 4u;

  const size_t kBufferCount = 4u;
@@ -356,7 +346,7 @@ struct TestMetadata { 
};

};





TEST_F(BufferHubQueueTest, TestMetadata) {

TEST_F(BufferHubQueueTest, TestMetadata) {

  ASSERT_TRUE(CreateQueues<TestMetadata>());

  ASSERT_TRUE(CreateQueues<TestMetadata>(UsagePolicy{}));

  AllocateBuffer();

  AllocateBuffer();





  std::vector<TestMetadata> ms = {

  std::vector<TestMetadata> ms = {
@@ -382,7 +372,7 @@ TEST_F(BufferHubQueueTest, TestMetadata) { 
}

}





TEST_F(BufferHubQueueTest, TestMetadataMismatch) {

TEST_F(BufferHubQueueTest, TestMetadataMismatch) {

  ASSERT_TRUE(CreateQueues<int64_t>());

  ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{}));

  AllocateBuffer();

  AllocateBuffer();





  int64_t mi = 3;

  int64_t mi = 3;
@@ -401,7 +391,7 @@ TEST_F(BufferHubQueueTest, TestMetadataMismatch) { 
}

}





TEST_F(BufferHubQueueTest, TestEnqueue) {

TEST_F(BufferHubQueueTest, TestEnqueue) {

  ASSERT_TRUE(CreateQueues<int64_t>());

  ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{}));

  AllocateBuffer();

  AllocateBuffer();





  size_t slot;

  size_t slot;
@@ -418,7 +408,7 @@ TEST_F(BufferHubQueueTest, TestEnqueue) { 
}

}





TEST_F(BufferHubQueueTest, TestAllocateBuffer) {

TEST_F(BufferHubQueueTest, TestAllocateBuffer) {

  ASSERT_TRUE(CreateQueues<int64_t>());

  ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{}));





  size_t s1;

  size_t s1;

  AllocateBuffer();

  AllocateBuffer();
@@ -473,7 +463,7 @@ TEST_F(BufferHubQueueTest, TestAllocateBuffer) { 




TEST_F(BufferHubQueueTest, TestUsageSetMask) {

TEST_F(BufferHubQueueTest, TestUsageSetMask) {

  const uint32_t set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;

  const uint32_t set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;

  ASSERT_TRUE(CreateQueues<int64_t>(set_mask, 0, 0, 0));

  ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{set_mask, 0, 0, 0}));





  // When allocation, leave out |set_mask| from usage bits on purpose.

  // When allocation, leave out |set_mask| from usage bits on purpose.

  size_t slot;

  size_t slot;
@@ -491,7 +481,7 @@ TEST_F(BufferHubQueueTest, TestUsageSetMask) { 




TEST_F(BufferHubQueueTest, TestUsageClearMask) {

TEST_F(BufferHubQueueTest, TestUsageClearMask) {

  const uint32_t clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;

  const uint32_t clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;

  ASSERT_TRUE(CreateQueues<int64_t>(0, clear_mask, 0, 0));

  ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{0, clear_mask, 0, 0}));





  // When allocation, add |clear_mask| into usage bits on purpose.

  // When allocation, add |clear_mask| into usage bits on purpose.

  size_t slot;

  size_t slot;
@@ -509,7 +499,7 @@ TEST_F(BufferHubQueueTest, TestUsageClearMask) { 




TEST_F(BufferHubQueueTest, TestUsageDenySetMask) {

TEST_F(BufferHubQueueTest, TestUsageDenySetMask) {

  const uint32_t deny_set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;

  const uint32_t deny_set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;

  ASSERT_TRUE(CreateQueues<int64_t>(0, 0, deny_set_mask, 0));

  ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{0, 0, deny_set_mask, 0}));





  // Now that |deny_set_mask| is illegal, allocation without those bits should

  // Now that |deny_set_mask| is illegal, allocation without those bits should

  // be able to succeed.

  // be able to succeed.
@@ -529,7 +519,7 @@ TEST_F(BufferHubQueueTest, TestUsageDenySetMask) { 




TEST_F(BufferHubQueueTest, TestUsageDenyClearMask) {

TEST_F(BufferHubQueueTest, TestUsageDenyClearMask) {

  const uint32_t deny_clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;

  const uint32_t deny_clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;

  ASSERT_TRUE(CreateQueues<int64_t>(0, 0, 0, deny_clear_mask));

  ASSERT_TRUE(CreateQueues<int64_t>(UsagePolicy{0, 0, 0, deny_clear_mask}));





  // Now that clearing |deny_clear_mask| is illegal (i.e. setting these bits are

  // Now that clearing |deny_clear_mask| is illegal (i.e. setting these bits are

  // mandatory), allocation with those bits should be able to succeed.

  // mandatory), allocation with those bits should be able to succeed.