stagefright: put battery update under a separate lock
Battery update doesn't need to lock the main lock for
mediaplayerservice. Battery update sometimes is called
from lower level componenet (eg. StagefrightRecorder),
locking the main lock here has potential to deadlock.
Puting the battery update in a separate class so that
it's clear this is not to be mixed with the rest of
the mediaplayerservice states.
bug: 38230347
Change-Id: Idf5f26f2b07ad6303775763ce283dad0679843d5
diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp
index cba5cf5..cc0ea1e 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -275,20 +275,6 @@
ALOGV("MediaPlayerService created");
mNextConnId = 1;
- mBatteryAudio.refCount = 0;
- for (int i = 0; i < NUM_AUDIO_DEVICES; i++) {
- mBatteryAudio.deviceOn[i] = 0;
- mBatteryAudio.lastTime[i] = 0;
- mBatteryAudio.totalTime[i] = 0;
- }
- // speaker is on by default
- mBatteryAudio.deviceOn[SPEAKER] = 1;
-
- // reset battery stats
- // if the mediaserver has crashed, battery stats could be left
- // in bad state, reset the state upon service start.
- BatteryNotifier::getInstance().noteResetVideo();
-
MediaPlayerFactory::registerBuiltinFactories();
}
@@ -2485,7 +2471,31 @@
////////////////////////////////////////////////////////////////////////////////
-void MediaPlayerService::addBatteryData(uint32_t params)
+void MediaPlayerService::addBatteryData(uint32_t params) {
+ mBatteryTracker.addBatteryData(params);
+}
+
+status_t MediaPlayerService::pullBatteryData(Parcel* reply) {
+ return mBatteryTracker.pullBatteryData(reply);
+}
+
+MediaPlayerService::BatteryTracker::BatteryTracker() {
+ mBatteryAudio.refCount = 0;
+ for (int i = 0; i < NUM_AUDIO_DEVICES; i++) {
+ mBatteryAudio.deviceOn[i] = 0;
+ mBatteryAudio.lastTime[i] = 0;
+ mBatteryAudio.totalTime[i] = 0;
+ }
+ // speaker is on by default
+ mBatteryAudio.deviceOn[SPEAKER] = 1;
+
+ // reset battery stats
+ // if the mediaserver has crashed, battery stats could be left
+ // in bad state, reset the state upon service start.
+ BatteryNotifier::getInstance().noteResetVideo();
+}
+
+void MediaPlayerService::BatteryTracker::addBatteryData(uint32_t params)
{
Mutex::Autolock lock(mLock);
@@ -2625,7 +2635,7 @@
}
}
-status_t MediaPlayerService::pullBatteryData(Parcel* reply) {
+status_t MediaPlayerService::BatteryTracker::pullBatteryData(Parcel* reply) {
Mutex::Autolock lock(mLock);
// audio output devices usage
diff --git a/media/libmediaplayerservice/MediaPlayerService.h b/media/libmediaplayerservice/MediaPlayerService.h
index 009fe73..06b9cad 100644
--- a/media/libmediaplayerservice/MediaPlayerService.h
+++ b/media/libmediaplayerservice/MediaPlayerService.h
@@ -246,51 +246,62 @@
CAMERA_PROCESS_DEATH = 4
};
- // For battery usage tracking purpose
- struct BatteryUsageInfo {
- // how many streams are being played by one UID
- int refCount;
- // a temp variable to store the duration(ms) of audio codecs
- // when we start a audio codec, we minus the system time from audioLastTime
- // when we pause it, we add the system time back to the audioLastTime
- // so after the pause, audioLastTime = pause time - start time
- // if multiple audio streams are played (or recorded), then audioLastTime
- // = the total playing time of all the streams
- int32_t audioLastTime;
- // when all the audio streams are being paused, we assign audioLastTime to
- // this variable, so this value could be provided to the battery app
- // in the next pullBatteryData call
- int32_t audioTotalTime;
-
- int32_t videoLastTime;
- int32_t videoTotalTime;
- };
- KeyedVector<int, BatteryUsageInfo> mBatteryData;
-
- enum {
- SPEAKER,
- OTHER_AUDIO_DEVICE,
- SPEAKER_AND_OTHER,
- NUM_AUDIO_DEVICES
- };
-
- struct BatteryAudioFlingerUsageInfo {
- int refCount; // how many audio streams are being played
- int deviceOn[NUM_AUDIO_DEVICES]; // whether the device is currently used
- int32_t lastTime[NUM_AUDIO_DEVICES]; // in ms
- // totalTime[]: total time of audio output devices usage
- int32_t totalTime[NUM_AUDIO_DEVICES]; // in ms
- };
-
- // This varialble is used to record the usage of audio output device
- // for battery app
- BatteryAudioFlingerUsageInfo mBatteryAudio;
-
// Collect info of the codec usage from media player and media recorder
virtual void addBatteryData(uint32_t params);
// API for the Battery app to pull the data of codecs usage
virtual status_t pullBatteryData(Parcel* reply);
private:
+ struct BatteryTracker {
+ BatteryTracker();
+ // Collect info of the codec usage from media player and media recorder
+ void addBatteryData(uint32_t params);
+ // API for the Battery app to pull the data of codecs usage
+ status_t pullBatteryData(Parcel* reply);
+
+ private:
+ // For battery usage tracking purpose
+ struct BatteryUsageInfo {
+ // how many streams are being played by one UID
+ int refCount;
+ // a temp variable to store the duration(ms) of audio codecs
+ // when we start a audio codec, we minus the system time from audioLastTime
+ // when we pause it, we add the system time back to the audioLastTime
+ // so after the pause, audioLastTime = pause time - start time
+ // if multiple audio streams are played (or recorded), then audioLastTime
+ // = the total playing time of all the streams
+ int32_t audioLastTime;
+ // when all the audio streams are being paused, we assign audioLastTime to
+ // this variable, so this value could be provided to the battery app
+ // in the next pullBatteryData call
+ int32_t audioTotalTime;
+
+ int32_t videoLastTime;
+ int32_t videoTotalTime;
+ };
+ KeyedVector<int, BatteryUsageInfo> mBatteryData;
+
+ enum {
+ SPEAKER,
+ OTHER_AUDIO_DEVICE,
+ SPEAKER_AND_OTHER,
+ NUM_AUDIO_DEVICES
+ };
+
+ struct BatteryAudioFlingerUsageInfo {
+ int refCount; // how many audio streams are being played
+ int deviceOn[NUM_AUDIO_DEVICES]; // whether the device is currently used
+ int32_t lastTime[NUM_AUDIO_DEVICES]; // in ms
+ // totalTime[]: total time of audio output devices usage
+ int32_t totalTime[NUM_AUDIO_DEVICES]; // in ms
+ };
+
+ // This varialble is used to record the usage of audio output device
+ // for battery app
+ BatteryAudioFlingerUsageInfo mBatteryAudio;
+
+ mutable Mutex mLock;
+ };
+ BatteryTracker mBatteryTracker;
class Client : public BnMediaPlayer {
// IMediaPlayer interface