Skip to content

Commit

Permalink
Change video battery stats to better track actual use
Browse files Browse the repository at this point in the history 
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 GZOSP#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
  • Loading branch information
Chong Zhang committed Aug 21, 2019
1 parent fb092d3 commit 22c4350
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 12 deletions.
87 changes: 77 additions & 10 deletions media/libstagefright/MediaCodec.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

////////////////////////////////////////////////////////////////////////////////

Expand Down Expand Up @@ -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<MediaResource> &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;
Expand Down Expand Up @@ -517,7 +528,6 @@ MediaCodec::MediaCodec(const sp<ALooper> &looper, pid_t pid, uid_t uid)
mStickyError(OK),
mSoftRenderer(NULL),
mAnalyticsItem(NULL),
mBatteryStatNotified(false),
mIsVideo(false),
mVideoWidth(0),
mVideoHeight(0),
Expand All @@ -529,7 +539,10 @@ MediaCodec::MediaCodec(const sp<ALooper> &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 {
Expand All @@ -543,7 +556,7 @@ MediaCodec::MediaCodec(const sp<ALooper> &looper, pid_t pid, uid_t uid)

MediaCodec::~MediaCodec() {
CHECK_EQ(mState, UNINITIALIZED);
mResourceManagerService->removeResource(getId(mResourceManagerClient));
mResourceManagerService->removeClient(getId(mResourceManagerClient));

flushAnalyticsItem();
}
Expand Down Expand Up @@ -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 };

Expand Down Expand Up @@ -776,6 +791,8 @@ void MediaCodec::statsBufferReceived(int64_t presentationUs) {
return;
}

scheduleBatteryCheckerIfNeeded();

BufferFlightTiming_t startdata;
bool valid = false;
while (mBuffersInFlight.size() > 0) {
Expand Down Expand Up @@ -1221,6 +1238,13 @@ void MediaCodec::addResource(
getId(mResourceManagerClient), mResourceManagerClient, resources);
}

void MediaCodec::removeResource(
MediaResource::Type type, MediaResource::SubType subtype, uint64_t value) {
Vector<MediaResource> resources;
resources.push_back(MediaResource(type, subtype, value));
mResourceManagerService->removeResource(getId(mResourceManagerClient), resources);
}

status_t MediaCodec::start() {
sp<AMessage> msg = new AMessage(kWhatStart, this);

Expand Down Expand Up @@ -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<AMessage> 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<AMessage> &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() {
Expand Down Expand Up @@ -1977,11 +2040,6 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &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);
Expand Down Expand Up @@ -2323,7 +2381,10 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &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;
Expand Down Expand Up @@ -3034,6 +3095,12 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
break;
}

case kWhatCheckBatteryStats:
{
onBatteryChecker(msg);
break;
}

default:
TRESPASS();
}
Expand Down
15 changes: 13 additions & 2 deletions media/libstagefright/include/media/stagefright/MediaCodec.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ struct MediaCodec : public AHandler {
kWhatSetCallback = 'setC',
kWhatSetNotification = 'setN',
kWhatDrmReleaseCrypto = 'rDrm',
kWhatCheckBatteryStats = 'chkB',
};

enum {
Expand Down Expand Up @@ -296,7 +297,11 @@ struct MediaCodec : public AHandler {
const sp<IResourceManagerClient> &client,
const Vector<MediaResource> &resources);

void removeResource(int64_t clientId);
void removeResource(
int64_t clientId,
const Vector<MediaResource> &resources);

void removeClient(int64_t clientId);

bool reclaimResource(const Vector<MediaResource> &resources);

Expand Down Expand Up @@ -336,7 +341,6 @@ struct MediaCodec : public AHandler {
sp<IResourceManagerClient> mResourceManagerClient;
sp<ResourceManagerServiceProxy> mResourceManagerService;

bool mBatteryStatNotified;
bool mIsVideo;
int32_t mVideoWidth;
int32_t mVideoHeight;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<AMessage>& msg);
void scheduleBatteryCheckerIfNeeded();

void statsBufferSent(int64_t presentationUs);
void statsBufferReceived(int64_t presentationUs);

Expand Down

0 comments on commit 22c4350

Please sign in to comment.