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

Commit 5172cabc authored by hkuang's avatar hkuang Committed by Hangyu Kuang
Browse files

MediaTranscodingService: Simplify TranscodingClientManager. 

Change the singleton to return reference instead of sp<>;

Bug: 145233472
Test: Unit test.
Change-Id: Ie5b8631ec9e917d80805f63c77618e24720f53bc
parent 3f4a68a0

media/libmediatranscoding/TranscodingClientManager.cpp

+5 −5
Original line number Diff line number Diff line
@@ -26,9 +26,9 @@ namespace android { 
using Status = ::ndk::ScopedAStatus;



// static

sp<TranscodingClientManager> TranscodingClientManager::getInstance() {

    static sp<TranscodingClientManager> sInstance = new TranscodingClientManager();

    return sInstance;

TranscodingClientManager& TranscodingClientManager::getInstance() {

    static TranscodingClientManager gInstance{};

    return gInstance;

}



// static
@@ -36,8 +36,8 @@ void TranscodingClientManager::BinderDiedCallback(void* cookie) { 
    int32_t clientId = static_cast<int32_t>(reinterpret_cast<intptr_t>(cookie));

    ALOGD("Client %" PRId32 " is dead", clientId);

    // Don't check for pid validity since we know it's already dead.

    sp<TranscodingClientManager> manager = TranscodingClientManager::getInstance();

    manager->removeClient(clientId);

    TranscodingClientManager& manager = TranscodingClientManager::getInstance();

    manager.removeClient(clientId);

}



TranscodingClientManager::TranscodingClientManager()

media/libmediatranscoding/include/media/TranscodingClientManager.h

+2 −2
Original line number Diff line number Diff line
@@ -45,7 +45,7 @@ class MediaTranscodingService; 
 * TODO(hkuang): Hook up with ResourceManager for resource management.

 * TODO(hkuang): Hook up with MediaMetrics to log all the transactions.

 */

class TranscodingClientManager : public RefBase {

class TranscodingClientManager {

   public:

    virtual ~TranscodingClientManager();
@@ -115,7 +115,7 @@ class TranscodingClientManager : public RefBase { 
    friend class TranscodingClientManagerTest;



    /** Get the singleton instance of the TranscodingClientManager. */

    static sp<TranscodingClientManager> getInstance();

    static TranscodingClientManager& getInstance();



    TranscodingClientManager();

media/libmediatranscoding/tests/TranscodingClientManager_tests.cpp

+26 −31
Original line number Diff line number Diff line
@@ -87,15 +87,11 @@ struct TestClient : public BnTranscodingServiceClient { 


class TranscodingClientManagerTest : public ::testing::Test {

   public:

    TranscodingClientManagerTest() { ALOGD("TranscodingClientManagerTest created"); }



    void SetUp() override {

        mClientManager = TranscodingClientManager::getInstance();

        if (mClientManager == nullptr) {

            ALOGE("Failed to acquire TranscodingClientManager.");

            return;

    TranscodingClientManagerTest() : mClientManager(TranscodingClientManager::getInstance()) {

        ALOGD("TranscodingClientManagerTest created");

    }



    void SetUp() override {

        ::ndk::SpAIBinder binder(AServiceManager_getService("media.transcoding"));

        mService = IMediaTranscodingService::fromBinder(binder);

        if (mService == nullptr) {
@@ -108,13 +104,12 @@ class TranscodingClientManagerTest : public ::testing::Test { 


    void TearDown() override {

        ALOGI("TranscodingClientManagerTest tear down");

        mClientManager = nullptr;

        mService = nullptr;

    }



    ~TranscodingClientManagerTest() { ALOGD("TranscodingClientManagerTest destroyed"); }



    sp<TranscodingClientManager> mClientManager = nullptr;

    TranscodingClientManager& mClientManager;

    std::shared_ptr<ITranscodingServiceClient> mTestClient = nullptr;

    std::shared_ptr<IMediaTranscodingService> mService = nullptr;

};
@@ -129,7 +124,7 @@ TEST_F(TranscodingClientManagerTest, TestAddingWithInvalidClientId) { 
                    client, kInvalidClientId, kClientPid, kClientUid, kClientOpPackageName);



    // Add the client to the manager and expect failure.

    status_t err = mClientManager->addClient(std::move(clientInfo));

    status_t err = mClientManager.addClient(std::move(clientInfo));

    EXPECT_TRUE(err != OK);

}
@@ -143,7 +138,7 @@ TEST_F(TranscodingClientManagerTest, TestAddingWithInvalidClientPid) { 
                    client, kClientId, kInvalidClientPid, kClientUid, kClientOpPackageName);



    // Add the client to the manager and expect failure.

    status_t err = mClientManager->addClient(std::move(clientInfo));

    status_t err = mClientManager.addClient(std::move(clientInfo));

    EXPECT_TRUE(err != OK);

}
@@ -157,7 +152,7 @@ TEST_F(TranscodingClientManagerTest, TestAddingWithInvalidClientUid) { 
                    client, kClientId, kClientPid, kInvalidClientUid, kClientOpPackageName);



    // Add the client to the manager and expect failure.

    status_t err = mClientManager->addClient(std::move(clientInfo));

    status_t err = mClientManager.addClient(std::move(clientInfo));

    EXPECT_TRUE(err != OK);

}
@@ -171,7 +166,7 @@ TEST_F(TranscodingClientManagerTest, TestAddingWithInvalidClientPackageName) { 
                    client, kClientId, kClientPid, kClientUid, kInvalidClientOpPackageName);



    // Add the client to the manager and expect failure.

    status_t err = mClientManager->addClient(std::move(clientInfo));

    status_t err = mClientManager.addClient(std::move(clientInfo));

    EXPECT_TRUE(err != OK);

}
@@ -183,13 +178,13 @@ TEST_F(TranscodingClientManagerTest, TestAddingValidClient) { 
            std::make_unique<TranscodingClientManager::ClientInfo>(

                    client1, kClientId, kClientPid, kClientUid, kClientOpPackageName);



    status_t err = mClientManager->addClient(std::move(clientInfo));

    status_t err = mClientManager.addClient(std::move(clientInfo));

    EXPECT_TRUE(err == OK);



    size_t numOfClients = mClientManager->getNumOfClients();

    size_t numOfClients = mClientManager.getNumOfClients();

    EXPECT_EQ(numOfClients, 1);



    err = mClientManager->removeClient(kClientId);

    err = mClientManager.removeClient(kClientId);

    EXPECT_TRUE(err == OK);

}
@@ -201,13 +196,13 @@ TEST_F(TranscodingClientManagerTest, TestAddingDupliacteClient) { 
            std::make_unique<TranscodingClientManager::ClientInfo>(

                    client1, kClientId, kClientPid, kClientUid, kClientOpPackageName);



    status_t err = mClientManager->addClient(std::move(clientInfo));

    status_t err = mClientManager.addClient(std::move(clientInfo));

    EXPECT_TRUE(err == OK);



    err = mClientManager->addClient(std::move(clientInfo));

    err = mClientManager.addClient(std::move(clientInfo));

    EXPECT_TRUE(err != OK);



    err = mClientManager->removeClient(kClientId);

    err = mClientManager.removeClient(kClientId);

    EXPECT_TRUE(err == OK);

}
@@ -219,7 +214,7 @@ TEST_F(TranscodingClientManagerTest, TestAddingMultipleClient) { 
            std::make_unique<TranscodingClientManager::ClientInfo>(

                    client1, kClientId, kClientPid, kClientUid, kClientOpPackageName);



    status_t err = mClientManager->addClient(std::move(clientInfo1));

    status_t err = mClientManager.addClient(std::move(clientInfo1));

    EXPECT_TRUE(err == OK);



    std::shared_ptr<ITranscodingServiceClient> client2 =
@@ -229,7 +224,7 @@ TEST_F(TranscodingClientManagerTest, TestAddingMultipleClient) { 
            std::make_unique<TranscodingClientManager::ClientInfo>(

                    client2, kClientId + 1, kClientPid, kClientUid, kClientOpPackageName);



    err = mClientManager->addClient(std::move(clientInfo2));

    err = mClientManager.addClient(std::move(clientInfo2));

    EXPECT_TRUE(err == OK);



    std::shared_ptr<ITranscodingServiceClient> client3 =
@@ -240,27 +235,27 @@ TEST_F(TranscodingClientManagerTest, TestAddingMultipleClient) { 
            std::make_unique<TranscodingClientManager::ClientInfo>(

                    client3, kClientId + 2, kClientPid, kClientUid, kClientOpPackageName);



    err = mClientManager->addClient(std::move(clientInfo3));

    err = mClientManager.addClient(std::move(clientInfo3));

    EXPECT_TRUE(err == OK);



    size_t numOfClients = mClientManager->getNumOfClients();

    size_t numOfClients = mClientManager.getNumOfClients();

    EXPECT_EQ(numOfClients, 3);



    err = mClientManager->removeClient(kClientId);

    err = mClientManager.removeClient(kClientId);

    EXPECT_TRUE(err == OK);



    err = mClientManager->removeClient(kClientId + 1);

    err = mClientManager.removeClient(kClientId + 1);

    EXPECT_TRUE(err == OK);



    err = mClientManager->removeClient(kClientId + 2);

    err = mClientManager.removeClient(kClientId + 2);

    EXPECT_TRUE(err == OK);

}



TEST_F(TranscodingClientManagerTest, TestRemovingNonExistClient) {

    status_t err = mClientManager->removeClient(kInvalidClientId);

    status_t err = mClientManager.removeClient(kInvalidClientId);

    EXPECT_TRUE(err != OK);



    err = mClientManager->removeClient(1000 /* clientId */);

    err = mClientManager.removeClient(1000 /* clientId */);

    EXPECT_TRUE(err != OK);

}
@@ -272,13 +267,13 @@ TEST_F(TranscodingClientManagerTest, TestCheckClientWithClientId) { 
            std::make_unique<TranscodingClientManager::ClientInfo>(

                    client, kClientId, kClientPid, kClientUid, kClientOpPackageName);



    status_t err = mClientManager->addClient(std::move(clientInfo));

    status_t err = mClientManager.addClient(std::move(clientInfo));

    EXPECT_TRUE(err == OK);



    bool res = mClientManager->isClientIdRegistered(kClientId);

    bool res = mClientManager.isClientIdRegistered(kClientId);

    EXPECT_TRUE(res);



    res = mClientManager->isClientIdRegistered(kInvalidClientId);

    res = mClientManager.isClientIdRegistered(kInvalidClientId);

    EXPECT_FALSE(res);

}

services/mediatranscoding/MediaTranscodingService.cpp

+7 −7
Original line number Diff line number Diff line
@@ -45,9 +45,9 @@ static bool isTrustedCallingUid(uid_t uid) { 
    }

}



MediaTranscodingService::MediaTranscodingService() {

MediaTranscodingService::MediaTranscodingService()

      : mTranscodingClientManager(TranscodingClientManager::getInstance()) {

    ALOGV("MediaTranscodingService is created");

    mTranscodingClientManager = TranscodingClientManager::getInstance();

}



MediaTranscodingService::~MediaTranscodingService() {
@@ -64,7 +64,7 @@ binder_status_t MediaTranscodingService::dump(int fd, const char** /*args*/, uin 
    write(fd, result.string(), result.size());



    Vector<String16> args;

    mTranscodingClientManager->dumpAllClients(fd, args);

    mTranscodingClientManager.dumpAllClients(fd, args);

    return OK;

}
@@ -124,7 +124,7 @@ Status MediaTranscodingService::registerClient( 
    int32_t clientId = in_clientPid;



    // Checks if the client already registers.

    if (mTranscodingClientManager->isClientIdRegistered(clientId)) {

    if (mTranscodingClientManager.isClientIdRegistered(clientId)) {

        return Status::fromServiceSpecificError(ERROR_ALREADY_EXISTS);

    }
@@ -132,7 +132,7 @@ Status MediaTranscodingService::registerClient( 
    std::unique_ptr<TranscodingClientManager::ClientInfo> newClient =

            std::make_unique<TranscodingClientManager::ClientInfo>(

                    in_client, clientId, in_clientPid, in_clientUid, in_opPackageName);

    status_t err = mTranscodingClientManager->addClient(std::move(newClient));

    status_t err = mTranscodingClientManager.addClient(std::move(newClient));

    if (err != OK) {

        *_aidl_return = kInvalidClientId;

        return STATUS_ERROR_FMT(err, "Failed to add client to TranscodingClientManager");
@@ -164,13 +164,13 @@ Status MediaTranscodingService::unregisterClient(int32_t clientId, bool* _aidl_r 
        }

    }



    *_aidl_return = (mTranscodingClientManager->removeClient(clientId) == OK);

    *_aidl_return = (mTranscodingClientManager.removeClient(clientId) == OK);

    return Status::ok();

}



Status MediaTranscodingService::getNumOfClients(int32_t* _aidl_return) {

    ALOGD("MediaTranscodingService::getNumOfClients");

    *_aidl_return = mTranscodingClientManager->getNumOfClients();

    *_aidl_return = mTranscodingClientManager.getNumOfClients();

    return Status::ok();

}

services/mediatranscoding/MediaTranscodingService.h

+1 −1
Original line number Diff line number Diff line
@@ -64,7 +64,7 @@ private: 


    mutable std::mutex mServiceLock;



    sp<TranscodingClientManager> mTranscodingClientManager;

    TranscodingClientManager& mTranscodingClientManager;

};



}  // namespace android