Improve summarized statistics

better management of how summarized records are managed.
Use differentiated names for the summations to avoid confusion with
individual records. Track values appropriate to calculating
min, max, mean, and standard deviation.

Bug: 65027360
Test: examination of dumpsys media.metrics -summary
Change-Id: I40d93d8a5a7cc2c188546574f59d557088d9c1e3
diff --git a/services/mediaanalytics/MetricsSummarizer.cpp b/services/mediaanalytics/MetricsSummarizer.cpp
index 3477f1f..93fe0ec 100644
--- a/services/mediaanalytics/MetricsSummarizer.cpp
+++ b/services/mediaanalytics/MetricsSummarizer.cpp
@@ -141,23 +141,23 @@
     List<MediaAnalyticsItem *>::iterator it = mSummaries->begin();
     for (; it != mSummaries->end(); it++) {
         bool good = sameAttributes((*it), item, getIgnorables());
-        ALOGV("Match against %s says %d",
-              (*it)->toString().c_str(), good);
+        ALOGV("Match against %s says %d", (*it)->toString().c_str(), good);
         if (good)
             break;
     }
     if (it == mSummaries->end()) {
             ALOGV("save new record");
-            item = item->dup();
-            if (item == NULL) {
+            MediaAnalyticsItem *nitem = item->dup();
+            if (nitem == NULL) {
                 ALOGE("unable to save MediaMetrics record");
             }
-            sortProps(item);
-            item->setInt32("count",1);
-            mSummaries->push_back(item);
+            sortProps(nitem);
+            nitem->setInt32("aggregated",1);
+            mergeRecord(*nitem, *item);
+            mSummaries->push_back(nitem);
     } else {
             ALOGV("increment existing record");
-            (*it)->addInt32("count",1);
+            (*it)->addInt32("aggregated",1);
             mergeRecord(*(*it), *item);
     }
 }
@@ -168,6 +168,71 @@
     return;
 }
 
+// keep some stats for things: sums, counts, standard deviation
+// the integer version -- all of these pieces are in 64 bits
+void MetricsSummarizer::minMaxVar64(MediaAnalyticsItem &summation, const char *key, int64_t value) {
+    if (key == NULL)
+        return;
+    int len = strlen(key) + 32;
+    char *tmpKey = (char *)malloc(len);
+
+    if (tmpKey == NULL) {
+        return;
+    }
+
+    // N - count of samples
+    snprintf(tmpKey, len, "%s.n", key);
+    summation.addInt64(tmpKey, 1);
+
+    // zero - count of samples that are zero
+    if (value == 0) {
+        snprintf(tmpKey, len, "%s.zero", key);
+        int64_t zero = 0;
+        (void) summation.getInt64(tmpKey,&zero);
+        zero++;
+        summation.setInt64(tmpKey, zero);
+    }
+
+    // min
+    snprintf(tmpKey, len, "%s.min", key);
+    int64_t min = value;
+    if (summation.getInt64(tmpKey,&min)) {
+        if (min > value) {
+            summation.setInt64(tmpKey, value);
+        }
+    } else {
+        summation.setInt64(tmpKey, value);
+    }
+
+    // max
+    snprintf(tmpKey, len, "%s.max", key);
+    int64_t max = value;
+    if (summation.getInt64(tmpKey,&max)) {
+        if (max < value) {
+            summation.setInt64(tmpKey, value);
+        }
+    } else {
+        summation.setInt64(tmpKey, value);
+    }
+
+    // components for mean, stddev;
+    // stddev = sqrt(1/4*(sumx2 - (2*sumx*sumx/n) + ((sumx/n)^2)))
+    // sum x
+    snprintf(tmpKey, len, "%s.sumX", key);
+    summation.addInt64(tmpKey, value);
+    // sum x^2
+    snprintf(tmpKey, len, "%s.sumX2", key);
+    summation.addInt64(tmpKey, value*value);
+
+
+    // last thing we do -- remove the base key from the summation
+    // record so we won't get confused about it having both individual
+    // and summary information in there.
+    summation.removeProp(key);
+
+    free(tmpKey);
+}
+
 
 //
 // Comparators
@@ -186,20 +251,23 @@
     ALOGV("MetricsSummarizer::sameAttributes(): summ %s", summ->toString().c_str());
     ALOGV("MetricsSummarizer::sameAttributes(): single %s", single->toString().c_str());
 
+    // keep different sources/users separate
+    if (single->mUid != summ->mUid) {
+        return false;
+    }
+
     // this can be made better.
     for(size_t i=0;i<single->mPropCount;i++) {
         MediaAnalyticsItem::Prop *prop1 = &(single->mProps[i]);
         const char *attrName = prop1->mName;
-        ALOGV("compare on attr '%s'", attrName);
 
         // is it something we should ignore
         if (ignorable != NULL) {
             const char **ig = ignorable;
-            while (*ig) {
+            for (;*ig; ig++) {
                 if (strcmp(*ig, attrName) == 0) {
                     break;
                 }
-                ig++;
             }
             if (*ig) {
                 ALOGV("we don't mind that it has attr '%s'", attrName);
@@ -218,29 +286,42 @@
         }
         switch (prop1->mType) {
             case MediaAnalyticsItem::kTypeInt32:
-                if (prop1->u.int32Value != prop2->u.int32Value)
+                if (prop1->u.int32Value != prop2->u.int32Value) {
+                    ALOGV("mismatch values");
                     return false;
+                }
                 break;
             case MediaAnalyticsItem::kTypeInt64:
-                if (prop1->u.int64Value != prop2->u.int64Value)
+                if (prop1->u.int64Value != prop2->u.int64Value) {
+                    ALOGV("mismatch values");
                     return false;
+                }
                 break;
             case MediaAnalyticsItem::kTypeDouble:
                 // XXX: watch out for floating point comparisons!
-                if (prop1->u.doubleValue != prop2->u.doubleValue)
+                if (prop1->u.doubleValue != prop2->u.doubleValue) {
+                    ALOGV("mismatch values");
                     return false;
+                }
                 break;
             case MediaAnalyticsItem::kTypeCString:
-                if (strcmp(prop1->u.CStringValue, prop2->u.CStringValue) != 0)
+                if (strcmp(prop1->u.CStringValue, prop2->u.CStringValue) != 0) {
+                    ALOGV("mismatch values");
                     return false;
+                }
                 break;
             case MediaAnalyticsItem::kTypeRate:
-                if (prop1->u.rate.count != prop2->u.rate.count)
+                if (prop1->u.rate.count != prop2->u.rate.count) {
+                    ALOGV("mismatch values");
                     return false;
-                if (prop1->u.rate.duration != prop2->u.rate.duration)
+                }
+                if (prop1->u.rate.duration != prop2->u.rate.duration) {
+                    ALOGV("mismatch values");
                     return false;
+                }
                 break;
             default:
+                ALOGV("mismatch values in default type");
                 return false;
         }
     }
@@ -248,15 +329,6 @@
     return true;
 }
 
-bool MetricsSummarizer::sameAttributesId(MediaAnalyticsItem *summ, MediaAnalyticsItem *single, const char **ignorable) {
-
-    // verify same user
-    if (summ->mPid != single->mPid)
-        return false;
-
-    // and finally do the more expensive validation of the attributes
-    return sameAttributes(summ, single, ignorable);
-}
 
 int MetricsSummarizer::PropSorter(const void *a, const void *b) {
     MediaAnalyticsItem::Prop *ai = (MediaAnalyticsItem::Prop *)a;
@@ -267,14 +339,8 @@
 // we sort in the summaries so that it looks pretty in the dumpsys
 void MetricsSummarizer::sortProps(MediaAnalyticsItem *item) {
     if (item->mPropCount != 0) {
-        if (DEBUG_SORT) {
-            ALOGD("sortProps(pre): %s", item->toString().c_str());
-        }
         qsort(item->mProps, item->mPropCount,
               sizeof(MediaAnalyticsItem::Prop), MetricsSummarizer::PropSorter);
-        if (DEBUG_SORT) {
-            ALOGD("sortProps(pst): %s", item->toString().c_str());
-        }
     }
 }
 
diff --git a/services/mediaanalytics/MetricsSummarizer.h b/services/mediaanalytics/MetricsSummarizer.h
index 0b64eac..a9f0786 100644
--- a/services/mediaanalytics/MetricsSummarizer.h
+++ b/services/mediaanalytics/MetricsSummarizer.h
@@ -59,10 +59,9 @@
 
     // various comparators
     // "do these records have same attributes and values in those attrs"
-    // ditto, but watch for "error" fields
     bool sameAttributes(MediaAnalyticsItem *summ, MediaAnalyticsItem *single, const char **ignoreables);
-    // attributes + from the same app/userid
-    bool sameAttributesId(MediaAnalyticsItem *summ, MediaAnalyticsItem *single, const char **ignoreables);
+
+    void minMaxVar64(MediaAnalyticsItem &summ, const char *key, int64_t value);
 
     static int PropSorter(const void *a, const void *b);
     void sortProps(MediaAnalyticsItem *item);
diff --git a/services/mediaanalytics/MetricsSummarizerPlayer.cpp b/services/mediaanalytics/MetricsSummarizerPlayer.cpp
index 5162059..f882cb9 100644
--- a/services/mediaanalytics/MetricsSummarizerPlayer.cpp
+++ b/services/mediaanalytics/MetricsSummarizerPlayer.cpp
@@ -51,37 +51,43 @@
     setIgnorables(player_ignorable);
 }
 
+// NB: this is also called for the first time -- so summation == item
+// Not sure if we need a flag for that or not.
+// In this particular mergeRecord() code -- we're' ok for this.
 void MetricsSummarizerPlayer::mergeRecord(MediaAnalyticsItem &summation, MediaAnalyticsItem &item) {
 
     ALOGV("MetricsSummarizerPlayer::mergeRecord()");
 
-    //
-    // we sum time & frames.
-    // be careful about our special "-1" values that indicate 'unknown'
-    // treat those as 0 [basically, not summing them into the totals].
+
     int64_t duration = 0;
     if (item.getInt64("android.media.mediaplayer.durationMs", &duration)) {
         ALOGV("found durationMs of %" PRId64, duration);
-        summation.addInt64("android.media.mediaplayer.durationMs",duration);
+        minMaxVar64(summation, "android.media.mediaplayer.durationMs", duration);
     }
+
     int64_t playing = 0;
-    if (item.getInt64("android.media.mediaplayer.playingMs", &playing))
+    if (item.getInt64("android.media.mediaplayer.playingMs", &playing)) {
         ALOGV("found playingMs of %" PRId64, playing);
-        if (playing >= 0) {
-            summation.addInt64("android.media.mediaplayer.playingMs",playing);
-        }
+    }
+    if (playing >= 0) {
+        minMaxVar64(summation,"android.media.mediaplayer.playingMs",playing);
+    }
+
     int64_t frames = 0;
-    if (item.getInt64("android.media.mediaplayer.frames", &frames))
+    if (item.getInt64("android.media.mediaplayer.frames", &frames)) {
         ALOGV("found framess of %" PRId64, frames);
-        if (frames >= 0) {
-            summation.addInt64("android.media.mediaplayer.frames",frames);
-        }
+    }
+    if (frames >= 0) {
+        minMaxVar64(summation,"android.media.mediaplayer.frames",frames);
+    }
+
     int64_t dropped = 0;
-    if (item.getInt64("android.media.mediaplayer.dropped", &dropped))
+    if (item.getInt64("android.media.mediaplayer.dropped", &dropped)) {
         ALOGV("found dropped of %" PRId64, dropped);
-        if (dropped >= 0) {
-            summation.addInt64("android.media.mediaplayer.dropped",dropped);
-        }
+    }
+    if (dropped >= 0) {
+        minMaxVar64(summation,"android.media.mediaplayer.dropped",dropped);
+    }
 }
 
 } // namespace android