MediaTranscodingService: Simplify TranscodingClientManager.
Change the singleton to return reference instead of sp<>;
Bug: 145233472
Test: Unit test.
Change-Id: Ie5b8631ec9e917d80805f63c77618e24720f53bc
diff --git a/media/libmediatranscoding/TranscodingClientManager.cpp b/media/libmediatranscoding/TranscodingClientManager.cpp
index e26dbaa..7252437 100644
--- a/media/libmediatranscoding/TranscodingClientManager.cpp
+++ b/media/libmediatranscoding/TranscodingClientManager.cpp
@@ -26,9 +26,9 @@
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 @@
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()
diff --git a/media/libmediatranscoding/include/media/TranscodingClientManager.h b/media/libmediatranscoding/include/media/TranscodingClientManager.h
index 74a000f..eec120a 100644
--- a/media/libmediatranscoding/include/media/TranscodingClientManager.h
+++ b/media/libmediatranscoding/include/media/TranscodingClientManager.h
@@ -45,7 +45,7 @@
* 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 @@
friend class TranscodingClientManagerTest;
/** Get the singleton instance of the TranscodingClientManager. */
- static sp<TranscodingClientManager> getInstance();
+ static TranscodingClientManager& getInstance();
TranscodingClientManager();
diff --git a/media/libmediatranscoding/tests/TranscodingClientManager_tests.cpp b/media/libmediatranscoding/tests/TranscodingClientManager_tests.cpp
index 97c8919..5d2419d 100644
--- a/media/libmediatranscoding/tests/TranscodingClientManager_tests.cpp
+++ b/media/libmediatranscoding/tests/TranscodingClientManager_tests.cpp
@@ -87,15 +87,11 @@
class TranscodingClientManagerTest : public ::testing::Test {
public:
- TranscodingClientManagerTest() { ALOGD("TranscodingClientManagerTest created"); }
+ TranscodingClientManagerTest() : mClientManager(TranscodingClientManager::getInstance()) {
+ ALOGD("TranscodingClientManagerTest created");
+ }
void SetUp() override {
- mClientManager = TranscodingClientManager::getInstance();
- if (mClientManager == nullptr) {
- ALOGE("Failed to acquire TranscodingClientManager.");
- return;
- }
-
::ndk::SpAIBinder binder(AServiceManager_getService("media.transcoding"));
mService = IMediaTranscodingService::fromBinder(binder);
if (mService == nullptr) {
@@ -108,13 +104,12 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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);
}
diff --git a/services/mediatranscoding/MediaTranscodingService.cpp b/services/mediatranscoding/MediaTranscodingService.cpp
index 480844e..82d4161 100644
--- a/services/mediatranscoding/MediaTranscodingService.cpp
+++ b/services/mediatranscoding/MediaTranscodingService.cpp
@@ -45,9 +45,9 @@
}
}
-MediaTranscodingService::MediaTranscodingService() {
+MediaTranscodingService::MediaTranscodingService()
+ : mTranscodingClientManager(TranscodingClientManager::getInstance()) {
ALOGV("MediaTranscodingService is created");
- mTranscodingClientManager = TranscodingClientManager::getInstance();
}
MediaTranscodingService::~MediaTranscodingService() {
@@ -64,7 +64,7 @@
write(fd, result.string(), result.size());
Vector<String16> args;
- mTranscodingClientManager->dumpAllClients(fd, args);
+ mTranscodingClientManager.dumpAllClients(fd, args);
return OK;
}
@@ -124,7 +124,7 @@
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 @@
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 @@
}
}
- *_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();
}
diff --git a/services/mediatranscoding/MediaTranscodingService.h b/services/mediatranscoding/MediaTranscodingService.h
index 4bdb078..cc69727 100644
--- a/services/mediatranscoding/MediaTranscodingService.h
+++ b/services/mediatranscoding/MediaTranscodingService.h
@@ -64,7 +64,7 @@
mutable std::mutex mServiceLock;
- sp<TranscodingClientManager> mTranscodingClientManager;
+ TranscodingClientManager& mTranscodingClientManager;
};
} // namespace android