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

Commit 6b4b1e5b authored by Fan Xu's avatar Fan Xu Committed by Tianyu Jiang
Browse files

Change bufferhub's bufferId to int 

After discussion we decided that our new system should still use int as
type of buffer id. Now the Id generator will generate int ids >= 0
instead of uint32_t ids > 0.

Remove redundant log in destructor of BufferNode, now only log when
freeId failed.

Update BufferHubIdGenerator_test.cpp to fit in current API. Add code to
cleanup generated ids in TestGenerateUniqueIncrementalID.

Test: BufferHubServer_test
Change-Id: I2018bfd009a3c311a99b9114762d0f4fbf1e3fe2
Fix: 118844348
parent e8f65978

services/bufferhub/BufferHubIdGenerator.cpp

+7 −9
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
 */



#include <bufferhub/BufferHubIdGenerator.h>

#include <log/log.h>



namespace android {

namespace frameworks {
@@ -22,20 +23,18 @@ namespace bufferhub { 
namespace V1_0 {

namespace implementation {



constexpr uint32_t BufferHubIdGenerator::kInvalidId;



BufferHubIdGenerator& BufferHubIdGenerator::getInstance() {

    static BufferHubIdGenerator generator;



    return generator;

}



uint32_t BufferHubIdGenerator::getId() {

int BufferHubIdGenerator::getId() {

    std::lock_guard<std::mutex> lock(mIdsInUseMutex);



    do {

        if (++mLastId >= std::numeric_limits<uint32_t>::max()) {

            mLastId = kInvalidId + 1;

        if (++mLastId >= std::numeric_limits<int>::max()) {

            mLastId = 0;

        }

    } while (mIdsInUse.find(mLastId) != mIdsInUse.end());
@@ -43,15 +42,14 @@ uint32_t BufferHubIdGenerator::getId() { 
    return mLastId;

}



bool BufferHubIdGenerator::freeId(uint32_t id) {

void BufferHubIdGenerator::freeId(int id) {

    std::lock_guard<std::mutex> lock(mIdsInUseMutex);

    auto iter = mIdsInUse.find(id);

    if (iter != mIdsInUse.end()) {

        mIdsInUse.erase(iter);

        return true;

    } else {

        ALOGW("%s: Cannot free nonexistent id #%d", __FUNCTION__, id);

    }



    return false;

}



} // namespace implementation

services/bufferhub/BufferHubService.cpp

+2 −2
Original line number Diff line number Diff line
@@ -343,15 +343,15 @@ void BufferHubService::onClientClosed(const BufferClient* client) { 


// Implementation of this function should be consistent with the definition of bufferInfo handle in

// ui/BufferHubDefs.h.

hidl_handle BufferHubService::buildBufferInfo(uint32_t bufferId, uint32_t clientBitMask,

hidl_handle BufferHubService::buildBufferInfo(int bufferId, uint32_t clientBitMask,

                                              uint32_t userMetadataSize, const int metadataFd) {

    native_handle_t* infoHandle = native_handle_create(BufferHubDefs::kBufferInfoNumFds,

                                                       BufferHubDefs::kBufferInfoNumInts);



    infoHandle->data[0] = dup(metadataFd);

    infoHandle->data[1] = bufferId;

    // Use memcpy to convert to int without missing digit.

    // TOOD(b/121345852): use bit_cast to unpack bufferInfo when C++20 becomes available.

    memcpy(&infoHandle->data[1], &bufferId, sizeof(bufferId));

    memcpy(&infoHandle->data[2], &clientBitMask, sizeof(clientBitMask));

    memcpy(&infoHandle->data[3], &userMetadataSize, sizeof(userMetadataSize));

services/bufferhub/BufferNode.cpp

+3 −7
Original line number Diff line number Diff line
@@ -30,7 +30,7 @@ void BufferNode::InitializeMetadata() { 


// Allocates a new BufferNode.

BufferNode::BufferNode(uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format,

                       uint64_t usage, size_t user_metadata_size, uint32_t id)

                       uint64_t usage, size_t user_metadata_size, int id)

      : mId(id) {

    uint32_t out_stride = 0;

    // graphicBufferId is not used in GraphicBufferAllocator::allocate
@@ -73,12 +73,8 @@ BufferNode::~BufferNode() { 
    }



    // Free the id, if valid

    if (id() != BufferHubIdGenerator::kInvalidId) {

        if (BufferHubIdGenerator::getInstance().freeId(id())) {

            ALOGI("%s: id #%u is freed.", __FUNCTION__, id());

        } else {

            ALOGE("%s: Cannot free nonexistent id #%u", __FUNCTION__, id());

        }

    if (mId >= 0) {

        BufferHubIdGenerator::getInstance().freeId(mId);

    }

}

services/bufferhub/include/bufferhub/BufferHubIdGenerator.h

+10 −12
Original line number Diff line number Diff line
@@ -28,30 +28,28 @@ namespace bufferhub { 
namespace V1_0 {

namespace implementation {



// A thread-safe incremental uint32_t id generator.

// A thread-safe, non-negative, incremental, int id generator.

class BufferHubIdGenerator {

public:

    // 0 is considered invalid

    static constexpr uint32_t kInvalidId = 0U;



    // Get the singleton instance of this class

    static BufferHubIdGenerator& getInstance();



    // Gets next available id. If next id is greater than std::numeric_limits<uint32_t>::max() (2 ^

    // 32 - 1), it will try to get an id start from 1 again.

    uint32_t getId();

    // Gets next available id. If next id is greater than std::numeric_limits<int32_t>::max(), it

    // will try to get an id start from 0 again.

    int getId();



    // Free a specific id. Return true on freed, false on not found.

    bool freeId(uint32_t id);

    // Free a specific id.

    void freeId(int id);



private:

    BufferHubIdGenerator() = default;

    ~BufferHubIdGenerator() = default;



    // Start from -1 so all valid ids will be >= 0

    int mLastId = -1;



    std::mutex mIdsInUseMutex;

    // Start from kInvalidID to avoid generating it.

    uint32_t mLastId = kInvalidId;

    std::set<uint32_t> mIdsInUse GUARDED_BY(mIdsInUseMutex);

    std::set<int> mIdsInUse GUARDED_BY(mIdsInUseMutex);

};



} // namespace implementation

services/bufferhub/include/bufferhub/BufferHubService.h

+2 −2
Original line number Diff line number Diff line
@@ -58,8 +58,8 @@ public: 


private:

    // Helper function to build BufferTraits.bufferInfo handle

    hidl_handle buildBufferInfo(uint32_t bufferId, uint32_t clientBitMask,

                                uint32_t userMetadataSize, const int metadataFd);

    hidl_handle buildBufferInfo(int bufferId, uint32_t clientBitMask, uint32_t userMetadataSize,

                                const int metadataFd);



    // Helper function to remove all the token belongs to a specific client.

    void removeTokenByClient(const BufferClient* client);