From 22c4350bd3122d4c0b22f1bb2e02533c449412ad Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Wed, 14 Aug 2019 13:15:32 -0700 Subject: [PATCH] Change video battery stats to better track actual use Reporting video on upon first queue/dequeue buffer activity, off after a period of inactivity (3 sec currently). bug: 138381810 test: Manual testing for now; unit tests will come after. With BatteryStatService instrumented to print out noteVideo*: Test case #1: Camera recording (using GCA): Camera creates encoder immediately when Video tab is entered, but recording won't start until button is pressed. After recording is stopped, a new encoder is again created to prepare for next session. So under old scheme, video is on as soon as we're in Video tab, and off/on even pairs are seen when video is stopped. Video stats is basically always On when we're in video tab. With this CL one should see: - There shouldn't be videoOn when Video tab is first entered - Start recording and there should be a videoOn. - Stop recording and there should be a videoOff, AND videoOn shouldn't immediately follow. - Kill camera app during active recording, video should be restored to Off. Test case #2: Playback of video clips in Photos with pause With old scheme pausing state will be videoOn because codec instance is still there. With new scheme, videoOff should come after video is paused for 3 sec, and video On should come again when video is resumed. Change-Id: Ifa8aa5d8c2c95fa25393fcb936e6e54578716783 --- media/libstagefright/MediaCodec.cpp | 87 ++++++++++++++++--- .../include/media/stagefright/MediaCodec.h | 15 +++- 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp index 4b52591aea..ae0fa3ca45 100644 --- a/media/libstagefright/MediaCodec.cpp +++ b/media/libstagefright/MediaCodec.cpp @@ -116,6 +116,7 @@ static bool isResourceError(status_t err) { static const int kMaxRetry = 2; static const int kMaxReclaimWaitTimeInUs = 500000; // 0.5s static const int kNumBuffersAlign = 16; +static const int kBatteryStatsTimeoutUs = 3000000ll; // 3 seconds //////////////////////////////////////////////////////////////////////////////// @@ -207,7 +208,17 @@ void MediaCodec::ResourceManagerServiceProxy::addResource( mService->addResource(mPid, mUid, clientId, client, resources); } -void MediaCodec::ResourceManagerServiceProxy::removeResource(int64_t clientId) { +void MediaCodec::ResourceManagerServiceProxy::removeResource( + int64_t clientId, + const Vector &resources) { + Mutex::Autolock _l(mLock); + if (mService == NULL) { + return; + } + mService->removeResource(mPid, clientId, resources); +} + +void MediaCodec::ResourceManagerServiceProxy::removeClient(int64_t clientId) { Mutex::Autolock _l(mLock); if (mService == NULL) { return; @@ -517,7 +528,6 @@ MediaCodec::MediaCodec(const sp &looper, pid_t pid, uid_t uid) mStickyError(OK), mSoftRenderer(NULL), mAnalyticsItem(NULL), - mBatteryStatNotified(false), mIsVideo(false), mVideoWidth(0), mVideoHeight(0), @@ -529,7 +539,10 @@ MediaCodec::MediaCodec(const sp &looper, pid_t pid, uid_t uid) mHaveInputSurface(false), mHavePendingInputBuffers(false), mCpuBoostRequested(false), - mLatencyUnknown(0) { + mLatencyUnknown(0), + mLastActivityTimeUs(-1ll), + mBatteryStatNotified(false), + mBatteryCheckerGeneration(0) { if (uid == kNoUid) { mUid = IPCThreadState::self()->getCallingUid(); } else { @@ -543,7 +556,7 @@ MediaCodec::MediaCodec(const sp &looper, pid_t pid, uid_t uid) MediaCodec::~MediaCodec() { CHECK_EQ(mState, UNINITIALIZED); - mResourceManagerService->removeResource(getId(mResourceManagerClient)); + mResourceManagerService->removeClient(getId(mResourceManagerClient)); flushAnalyticsItem(); } @@ -742,6 +755,8 @@ void MediaCodec::statsBufferSent(int64_t presentationUs) { return; } + scheduleBatteryCheckerIfNeeded(); + const int64_t nowNs = systemTime(SYSTEM_TIME_MONOTONIC); BufferFlightTiming_t startdata = { presentationUs, nowNs }; @@ -776,6 +791,8 @@ void MediaCodec::statsBufferReceived(int64_t presentationUs) { return; } + scheduleBatteryCheckerIfNeeded(); + BufferFlightTiming_t startdata; bool valid = false; while (mBuffersInFlight.size() > 0) { @@ -1221,6 +1238,13 @@ void MediaCodec::addResource( getId(mResourceManagerClient), mResourceManagerClient, resources); } +void MediaCodec::removeResource( + MediaResource::Type type, MediaResource::SubType subtype, uint64_t value) { + Vector resources; + resources.push_back(MediaResource(type, subtype, value)); + mResourceManagerService->removeResource(getId(mResourceManagerClient), resources); +} + status_t MediaCodec::start() { sp msg = new AMessage(kWhatStart, this); @@ -1682,6 +1706,45 @@ void MediaCodec::requestCpuBoostIfNeeded() { } } +void MediaCodec::scheduleBatteryCheckerIfNeeded() { + if (!mIsVideo || !isExecuting()) { + // ignore if not executing + return; + } + if (!mBatteryStatNotified) { + addResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); + mBatteryStatNotified = true; + sp msg = new AMessage(kWhatCheckBatteryStats, this); + msg->setInt32("generation", mBatteryCheckerGeneration); + + // post checker and clear last activity time + msg->post(kBatteryStatsTimeoutUs); + mLastActivityTimeUs = -1ll; + } else { + // update last activity time + mLastActivityTimeUs = ALooper::GetNowUs(); + } +} + +void MediaCodec::onBatteryChecker(const sp &msg) { + // ignore if this checker already expired because the client resource was removed + int32_t generation; + if (!msg->findInt32("generation", &generation) + || generation != mBatteryCheckerGeneration) { + return; + } + + if (mLastActivityTimeUs < 0ll) { + // timed out inactive, do not repost checker + removeResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); + mBatteryStatNotified = false; + } else { + // repost checker and clear last activity time + msg->post(kBatteryStatsTimeoutUs + mLastActivityTimeUs - ALooper::GetNowUs()); + mLastActivityTimeUs = -1ll; + } +} + //////////////////////////////////////////////////////////////////////////////// void MediaCodec::cancelPendingDequeueOperations() { @@ -1977,11 +2040,6 @@ void MediaCodec::onMessageReceived(const sp &msg) { if (mIsVideo) { // audio codec is currently ignored. addResource(resourceType, MediaResource::kVideoCodec, 1); - // TODO: track battery on/off by actual queueing/dequeueing - // For now, keep existing behavior and request battery on/off - // together with codec init/uninit. We'll improve the tracking - // later by adding/removing this based on queue/dequeue timing. - addResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); } (new AMessage)->postReply(mReplyID); @@ -2323,7 +2381,10 @@ void MediaCodec::onMessageReceived(const sp &msg) { mFlags &= ~kFlagIsComponentAllocated; - mResourceManagerService->removeResource(getId(mResourceManagerClient)); + // off since we're removing all resources including the battery on + mBatteryStatNotified = false; + mBatteryCheckerGeneration++; + mResourceManagerService->removeClient(getId(mResourceManagerClient)); (new AMessage)->postReply(mReplyID); break; @@ -3034,6 +3095,12 @@ void MediaCodec::onMessageReceived(const sp &msg) { break; } + case kWhatCheckBatteryStats: + { + onBatteryChecker(msg); + break; + } + default: TRESPASS(); } diff --git a/media/libstagefright/include/media/stagefright/MediaCodec.h b/media/libstagefright/include/media/stagefright/MediaCodec.h index 0218a88af7..462eb84e50 100644 --- a/media/libstagefright/include/media/stagefright/MediaCodec.h +++ b/media/libstagefright/include/media/stagefright/MediaCodec.h @@ -257,6 +257,7 @@ struct MediaCodec : public AHandler { kWhatSetCallback = 'setC', kWhatSetNotification = 'setN', kWhatDrmReleaseCrypto = 'rDrm', + kWhatCheckBatteryStats = 'chkB', }; enum { @@ -296,7 +297,11 @@ struct MediaCodec : public AHandler { const sp &client, const Vector &resources); - void removeResource(int64_t clientId); + void removeResource( + int64_t clientId, + const Vector &resources); + + void removeClient(int64_t clientId); bool reclaimResource(const Vector &resources); @@ -336,7 +341,6 @@ struct MediaCodec : public AHandler { sp mResourceManagerClient; sp mResourceManagerService; - bool mBatteryStatNotified; bool mIsVideo; int32_t mVideoWidth; int32_t mVideoHeight; @@ -430,6 +434,7 @@ struct MediaCodec : public AHandler { uint64_t getGraphicBufferSize(); void addResource(MediaResource::Type type, MediaResource::SubType subtype, uint64_t value); + void removeResource(MediaResource::Type type, MediaResource::SubType subtype, uint64_t value); void requestCpuBoostIfNeeded(); bool hasPendingBuffer(int portIndex); @@ -458,6 +463,12 @@ struct MediaCodec : public AHandler { Mutex mLatencyLock; int64_t mLatencyUnknown; // buffers for which we couldn't calculate latency + int64_t mLastActivityTimeUs; + bool mBatteryStatNotified; + int32_t mBatteryCheckerGeneration; + void onBatteryChecker(const sp& msg); + void scheduleBatteryCheckerIfNeeded(); + void statsBufferSent(int64_t presentationUs); void statsBufferReceived(int64_t presentationUs);