Merge "Move freeze trigger into processFreeze method" into main
diff --git a/media/libstagefright/VideoRenderQualityTracker.cpp b/media/libstagefright/VideoRenderQualityTracker.cpp
index 3bd236a..9b792b3 100644
--- a/media/libstagefright/VideoRenderQualityTracker.cpp
+++ b/media/libstagefright/VideoRenderQualityTracker.cpp
@@ -161,7 +161,6 @@
getFlag(traceTriggerEnabled, "trace_trigger_enabled");
getFlag(traceTriggerThrottleMs, "trace_trigger_throttle_ms");
getFlag(traceMinFreezeDurationMs, "trace_minimum_freeze_duration_ms");
- getFlag(traceMaxFreezeDurationMs, "trace_maximum_freeze_duration_ms");
#undef getFlag
return c;
}
@@ -208,7 +207,6 @@
"ro.build.type", "user") != "user"; // Enabled for non-user builds for debugging.
traceTriggerThrottleMs = 5 * 60 * 1000; // 5 mins.
traceMinFreezeDurationMs = 400;
- traceMaxFreezeDurationMs = 1500;
}
VideoRenderQualityTracker::VideoRenderQualityTracker()
@@ -300,15 +298,7 @@
int64_t actualRenderTimeUs = actualRenderTimeNs / 1000;
if (mLastRenderTimeUs != -1) {
- int64_t frameRenderDurationMs = (actualRenderTimeUs - mLastRenderTimeUs) / 1000;
- mRenderDurationMs += frameRenderDurationMs;
- if (mConfiguration.traceTriggerEnabled
- // Threshold for visible video freeze.
- && frameRenderDurationMs >= mConfiguration.traceMinFreezeDurationMs
- // Threshold for removing long render durations which could be video pause.
- && frameRenderDurationMs < mConfiguration.traceMaxFreezeDurationMs) {
- triggerTraceWithThrottle(mTraceTriggerFn, mConfiguration, actualRenderTimeUs);
- }
+ mRenderDurationMs += (actualRenderTimeUs - mLastRenderTimeUs) / 1000;
}
// Now that a frame has been rendered, the previously skipped frames can be processed as skipped
@@ -549,7 +539,7 @@
}
if (!isLikelyCatchingUpAfterPause) {
processFreeze(actualRenderTimeUs, mLastRenderTimeUs, mLastFreezeEndTimeUs, mFreezeEvent,
- mMetrics, mConfiguration);
+ mMetrics, mConfiguration, mTraceTriggerFn);
mLastFreezeEndTimeUs = actualRenderTimeUs;
}
}
@@ -575,8 +565,8 @@
void VideoRenderQualityTracker::processFreeze(int64_t actualRenderTimeUs, int64_t lastRenderTimeUs,
int64_t lastFreezeEndTimeUs, FreezeEvent &e,
- VideoRenderQualityMetrics &m,
- const Configuration &c) {
+ VideoRenderQualityMetrics &m, const Configuration &c,
+ const TraceTriggerFn traceTriggerFn) {
int32_t durationMs = int32_t((actualRenderTimeUs - lastRenderTimeUs) / 1000);
m.freezeDurationMsHistogram.insert(durationMs);
int32_t distanceMs = -1;
@@ -612,6 +602,11 @@
e.details.distanceMs.push_back(distanceMs); // -1 for first detail in the first event
}
}
+
+ if (c.traceTriggerEnabled && durationMs >= c.traceMinFreezeDurationMs) {
+ ALOGI("Video freezed %lld ms", (long long) durationMs);
+ triggerTraceWithThrottle(traceTriggerFn, c, actualRenderTimeUs);
+ }
}
void VideoRenderQualityTracker::maybeCaptureFreezeEvent(int64_t actualRenderTimeUs,
@@ -818,17 +813,20 @@
static int64_t lastTriggerUs = -1;
static Mutex updateLastTriggerLock;
- Mutex::Autolock autoLock(updateLastTriggerLock);
- if (lastTriggerUs != -1) {
- int32_t sinceLastTriggerMs = int32_t((triggerTimeUs - lastTriggerUs) / 1000);
- // Throttle the trace trigger calls to reduce continuous PID fork calls in a short time
- // to impact device performance, and reduce spamming trace reports.
- if (sinceLastTriggerMs < c.traceTriggerThrottleMs) {
- ALOGI("Not triggering trace - not enough time since last trigger");
- return;
+ {
+ Mutex::Autolock autoLock(updateLastTriggerLock);
+ if (lastTriggerUs != -1) {
+ int32_t sinceLastTriggerMs = int32_t((triggerTimeUs - lastTriggerUs) / 1000);
+ // Throttle the trace trigger calls to reduce continuous PID fork calls in a short time
+ // to impact device performance, and reduce spamming trace reports.
+ if (sinceLastTriggerMs < c.traceTriggerThrottleMs) {
+ ALOGI("Not triggering trace - not enough time since last trigger");
+ return;
+ }
}
+ lastTriggerUs = triggerTimeUs;
}
- lastTriggerUs = triggerTimeUs;
+
(*traceTriggerFn)();
}
diff --git a/media/libstagefright/include/media/stagefright/VideoRenderQualityTracker.h b/media/libstagefright/include/media/stagefright/VideoRenderQualityTracker.h
index d58dfad..7139deb 100644
--- a/media/libstagefright/include/media/stagefright/VideoRenderQualityTracker.h
+++ b/media/libstagefright/include/media/stagefright/VideoRenderQualityTracker.h
@@ -220,11 +220,6 @@
//
// The minimum frame render duration to recognize video freeze event to collect trace.
int32_t traceMinFreezeDurationMs;
- //
- // The maximum frame render duration to recognize video freeze event. A frame render
- // duration that is larger than the max duration would not trigger trace collection for
- // video freeze because it's highly possible a video pause.
- int32_t traceMaxFreezeDurationMs;
};
struct FreezeEvent {
@@ -380,7 +375,8 @@
// Process a frame freeze.
static void processFreeze(int64_t actualRenderTimeUs, int64_t lastRenderTimeUs,
int64_t lastFreezeEndTimeUs, FreezeEvent &e,
- VideoRenderQualityMetrics &m, const Configuration &c);
+ VideoRenderQualityMetrics &m, const Configuration &c,
+ const TraceTriggerFn traceTriggerFn);
// Retrieve a freeze event if an event just finished.
static void maybeCaptureFreezeEvent(int64_t actualRenderTimeUs, int64_t lastFreezeEndTimeUs,
diff --git a/media/libstagefright/tests/VideoRenderQualityTracker_test.cpp b/media/libstagefright/tests/VideoRenderQualityTracker_test.cpp
index 9b6315c..16f8294 100644
--- a/media/libstagefright/tests/VideoRenderQualityTracker_test.cpp
+++ b/media/libstagefright/tests/VideoRenderQualityTracker_test.cpp
@@ -156,7 +156,6 @@
EXPECT_EQ(c.traceTriggerEnabled, d.traceTriggerEnabled);
EXPECT_EQ(c.traceTriggerThrottleMs, d.traceTriggerThrottleMs);
EXPECT_EQ(c.traceMinFreezeDurationMs, d.traceMinFreezeDurationMs);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, d.traceMaxFreezeDurationMs);
}
TEST_F(VideoRenderQualityTrackerTest, getFromServerConfigurableFlags_withEmpty) {
@@ -188,7 +187,6 @@
EXPECT_EQ(c.traceTriggerEnabled, d.traceTriggerEnabled);
EXPECT_EQ(c.traceTriggerThrottleMs, d.traceTriggerThrottleMs);
EXPECT_EQ(c.traceMinFreezeDurationMs, d.traceMinFreezeDurationMs);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, d.traceMaxFreezeDurationMs);
}
TEST_F(VideoRenderQualityTrackerTest, getFromServerConfigurableFlags_withInvalid) {
@@ -220,7 +218,6 @@
EXPECT_EQ(c.traceTriggerEnabled, d.traceTriggerEnabled);
EXPECT_EQ(c.traceTriggerThrottleMs, d.traceTriggerThrottleMs);
EXPECT_EQ(c.traceMinFreezeDurationMs, d.traceMinFreezeDurationMs);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, d.traceMaxFreezeDurationMs);
}
TEST_F(VideoRenderQualityTrackerTest, getFromServerConfigurableFlags_withAlmostValid) {
@@ -297,7 +294,6 @@
EXPECT_EQ(c.traceTriggerEnabled, d.traceTriggerEnabled);
EXPECT_EQ(c.traceTriggerThrottleMs, d.traceTriggerThrottleMs);
EXPECT_EQ(c.traceMinFreezeDurationMs, d.traceMinFreezeDurationMs);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, d.traceMaxFreezeDurationMs);
}
TEST_F(VideoRenderQualityTrackerTest, getFromServerConfigurableFlags_withValid) {
@@ -412,7 +408,6 @@
EXPECT_EQ(c.traceTriggerEnabled, true);
EXPECT_EQ(c.traceTriggerThrottleMs, 50000);
EXPECT_EQ(c.traceMinFreezeDurationMs, 1000);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, 5000);
}
TEST_F(VideoRenderQualityTrackerTest, countsReleasedFrames) {
@@ -1126,53 +1121,34 @@
c.enabled = true;
c.traceTriggerEnabled = true; // The trigger is enabled, so traces should be triggered.
// The value of traceTriggerThrottleMs must be larger than traceMinFreezeDurationMs. Otherwise,
- // the throttle does work.
+ // the throttle does not work.
c.traceTriggerThrottleMs = 200;
- c.traceMinFreezeDurationMs = 40;
- int32_t freeze = c.traceMinFreezeDurationMs;
+ c.traceMinFreezeDurationMs = 4 * 20; // 4 frames.
Helper h(20, c);
- // Freeze triggers separated by 80ms which is less than the threshold.
- h.render({
- freeze, // Freeze duration does not check trace trigger.
- 20, // Trace triggered.
- 20, // Throttle time: 20/200ms
- 20, // Throttle time: 40/200ms
- freeze, // Throttle time: 80/200ms
- 20, // Throttle time: 100/200ms (Trace not triggered)
- });
+ // Freeze triggers separated by 100ms which is less than the threshold.
+ h.render(1); // Video start.
+ h.drop(3); // Freeze.
+ h.render(1); // Trace triggered.
+ h.render(1); // Throttle time: 20/200ms
+ h.drop(3); // Throttle time: 80/200ms
+ h.render(1); // Throttle time: 100/200ms (Trace not triggered)
EXPECT_EQ(h.getTraceTriggeredCount(), 1);
// Next freeze trigger is separated by 200ms which breaks the throttle threshold.
- h.render({
- 20, // Throttle time: 120/200ms
- 20, // Throttle time: 140/200ms
- 20, // Throttle time: 160/200ms
- freeze, // Throttle time: 200/200ms
- 20, // Trace triggered.
- });
+ h.render(1); // Throttle time: 120/200ms
+ h.drop(3); // Throttle time: 180/200ms
+ h.render(1); // Throttle time: 200/200ms (Trace triggered)
EXPECT_EQ(h.getTraceTriggeredCount(), 2);
- // Next freeze trigger is separated by 80ms which is less than the threshold.
- h.render({
- 20, // Throttle time: 20/200ms
- 20, // Throttle time: 40/200ms
- freeze, // Throttle time: 80/200ms
- 20, // Throttle time: 100/200ms (Trace not triggered)
- });
+ // Next freeze trigger is separated by 100ms which is less than the threshold.
+ h.render(1); // Throttle time: 20/200ms
+ h.drop(3); // Throttle time: 80/200ms
+ h.render(1); // Throttle time: 100/200ms (Trace not triggered)
EXPECT_EQ(h.getTraceTriggeredCount(), 2);
-}
-
-TEST_F(VideoRenderQualityTrackerTest, freezeForTraceDuration_triggersTrace) {
- Configuration c;
- c.enabled = true;
- c.traceTriggerEnabled = true; // The trigger is enabled, so traces should be triggered.
- c.traceTriggerThrottleMs = 0; // Disable throttle in the test case.
- int32_t freeze1 = c.traceMinFreezeDurationMs;
- int32_t freeze2 = c.traceMaxFreezeDurationMs - 1;
- int32_t couldBeAPause = c.traceMaxFreezeDurationMs + 1;
-
- Helper h(20, c);
- h.render({freeze1, 20, freeze2, 20, couldBeAPause, 20});
-
+ // Freeze duration is less than traceMinFreezeDurationMs and throttle ends.
+ h.render(1); // Throttle time: 120/200ms
+ h.render(1); // Throttle time: 140/200ms
+ h.drop(2); // Throttle time: 180/200ms
+ h.render(1); // Throttle time: 200/200ms (Trace not triggered, freeze duration = 60ms)
EXPECT_EQ(h.getTraceTriggeredCount(), 2);
}
@@ -1182,12 +1158,14 @@
c.enabled = true;
c.traceTriggerEnabled = false; // The trigger is disabled, so no traces should be triggered.
c.traceTriggerThrottleMs = 0; // Disable throttle in the test case.
- int32_t freeze1 = c.traceMinFreezeDurationMs;
- int32_t freeze2 = c.traceMaxFreezeDurationMs - 1;
- int32_t couldBeAPause = c.traceMaxFreezeDurationMs + 1;
+ c.traceMinFreezeDurationMs = 4 * 20; // 4 frames.
Helper h(20, c);
- h.render({freeze1, 20, freeze2, 20, couldBeAPause, 20});
+ h.render(1);
+ h.drop(3);
+ h.render(1); // Render duration is 80 ms.
+ h.drop(4);
+ h.render(1); // Render duration is 100 ms.
EXPECT_EQ(h.getTraceTriggeredCount(), 0);
}