Integrate feedback from MediaMetrics API reviews

observations about missing action in close().
javadoc on some KEY_* fields.
Implement PlaybackStateEvent within BundleSession as example.

Bug: 220129161
Test: atest CtsMediaHostTestCases  (with corresponding CLs for cts)
Change-Id: I09a8be6d3f2780ca97b6b08cd33ccf35b02c245f

10 files changed, 70 insertions, 1 deletions

diff --git a/core/api/current.txt b/core/api/current.txt
index 37022d0ab402..71383235f55b 100644
--- a/core/api/current.txt
+++ b/core/api/current.txt
@@ -24461,6 +24461,7 @@ package android.media.metrics {
     method public void close();
     method @NonNull public android.media.metrics.LogSessionId getSessionId();
     method public void reportBundleMetrics(@NonNull android.os.PersistableBundle);
+    field public static final String KEY_STATSD_ATOM = "bundlesession-statsd-atom";
   }
 
   public final class EditingSession implements java.lang.AutoCloseable {
@@ -24484,6 +24485,7 @@ package android.media.metrics {
     method @NonNull public android.media.metrics.PlaybackSession createPlaybackSession();
     method @NonNull public android.media.metrics.RecordingSession createRecordingSession();
     method @NonNull public android.media.metrics.TranscodingSession createTranscodingSession();
+    method @NonNull public void releaseSessionId(@NonNull String);
     field public static final long INVALID_TIMESTAMP = -1L; // 0xffffffffffffffffL
   }
 
diff --git a/media/java/android/media/metrics/BundleSession.java b/media/java/android/media/metrics/BundleSession.java
index 743d2338d69a..0f0fdfe1ce81 100644
--- a/media/java/android/media/metrics/BundleSession.java
+++ b/media/java/android/media/metrics/BundleSession.java
@@ -32,6 +32,15 @@ public final class BundleSession implements AutoCloseable {
     private final @NonNull MediaMetricsManager mManager;
     private final @NonNull LogSessionId mLogSessionId;
 
+    /**
+     * A key describing the statsd atom into which to place this bundle's other contents.
+     * The associated value is an integer.
+     *
+     * @see #reportBundleMetrics
+     */
+
+    public static final String KEY_STATSD_ATOM = "bundlesession-statsd-atom";
+
     /** @hide */
     public BundleSession(@NonNull String id, @NonNull MediaMetricsManager manager) {
         mId = id;
@@ -44,6 +53,10 @@ public final class BundleSession implements AutoCloseable {
     /**
      * Reports metrics via bundle.
      *
+     * The key {@link #KEY_STATSD_ATOM} references an integer value that
+     * indicates the statsd atom for the data in this bundle. Other keys
+     * and their types are defined on a per-atom basis.
+     *
      */
     public void reportBundleMetrics(@NonNull PersistableBundle metrics) {
         mManager.reportBundleMetrics(mId, metrics);
@@ -68,5 +81,6 @@ public final class BundleSession implements AutoCloseable {
 
     @Override
     public void close() {
+        mManager.releaseSessionId(mLogSessionId.getStringId());
     }
 }
diff --git a/media/java/android/media/metrics/EditingSession.java b/media/java/android/media/metrics/EditingSession.java
index 2a48a728a2f0..2ddf623b1ed3 100644
--- a/media/java/android/media/metrics/EditingSession.java
+++ b/media/java/android/media/metrics/EditingSession.java
@@ -59,5 +59,6 @@ public final class EditingSession implements AutoCloseable {
 
     @Override
     public void close() {
+        mManager.releaseSessionId(mLogSessionId.getStringId());
     }
 }
diff --git a/media/java/android/media/metrics/IMediaMetricsManager.aidl b/media/java/android/media/metrics/IMediaMetricsManager.aidl
index a774403b4835..51b1cc27c8a7 100644
--- a/media/java/android/media/metrics/IMediaMetricsManager.aidl
+++ b/media/java/android/media/metrics/IMediaMetricsManager.aidl
@@ -41,4 +41,6 @@ interface IMediaMetricsManager {
     String getEditingSessionId(int userId);
     String getBundleSessionId(int userId);
     void reportBundleMetrics(in String sessionId, in PersistableBundle metrics, int userId);
+
+    void releaseSessionId(in String sessionId, int userId);
 }
diff --git a/media/java/android/media/metrics/MediaMetricsManager.java b/media/java/android/media/metrics/MediaMetricsManager.java
index 7229c471c3b6..0898874c2f65 100644
--- a/media/java/android/media/metrics/MediaMetricsManager.java
+++ b/media/java/android/media/metrics/MediaMetricsManager.java
@@ -171,6 +171,18 @@ public final class MediaMetricsManager {
     }
 
     /**
+     * Creates a generic bundle session.
+     */
+    @NonNull
+    public void releaseSessionId(@NonNull String sessionId) {
+        try {
+            mService.releaseSessionId(sessionId, mUserId);
+        } catch (RemoteException e) {
+            throw e.rethrowFromSystemServer();
+        }
+    }
+
+    /**
      * Reports error event.
      * @hide
      */
diff --git a/media/java/android/media/metrics/PlaybackSession.java b/media/java/android/media/metrics/PlaybackSession.java
index aad510e87a7e..f8dd756c5f9c 100644
--- a/media/java/android/media/metrics/PlaybackSession.java
+++ b/media/java/android/media/metrics/PlaybackSession.java
@@ -100,5 +100,6 @@ public final class PlaybackSession implements AutoCloseable {
     @Override
     public void close() {
         mClosed = true;
+        mManager.releaseSessionId(mLogSessionId.getStringId());
     }
 }
diff --git a/media/java/android/media/metrics/PlaybackStateEvent.java b/media/java/android/media/metrics/PlaybackStateEvent.java
index 8e7482534677..cfe0ff81520f 100644
--- a/media/java/android/media/metrics/PlaybackStateEvent.java
+++ b/media/java/android/media/metrics/PlaybackStateEvent.java
@@ -31,6 +31,11 @@ import java.util.Objects;
  * Playback state event.
  */
 public final class PlaybackStateEvent extends Event implements Parcelable {
+
+        // RBE -- we should be getting these values from the proto, not doing them
+        // hand-coded values here.
+        // frameorks/proto_logging/stats/message/mediametrics_message.pb.h
+        // package libstatslog?
     /** Playback has not started (initial state) */
     public static final int STATE_NOT_STARTED = 0;
     /** Playback is buffering in the background for initial playback start */
diff --git a/media/java/android/media/metrics/RecordingSession.java b/media/java/android/media/metrics/RecordingSession.java
index d38835149cd8..87cc2501dd70 100644
--- a/media/java/android/media/metrics/RecordingSession.java
+++ b/media/java/android/media/metrics/RecordingSession.java
@@ -61,5 +61,6 @@ public final class RecordingSession implements AutoCloseable {
     @Override
     public void close() {
         mClosed = true;
+        mManager.releaseSessionId(mLogSessionId.getStringId());
     }
 }
diff --git a/media/java/android/media/metrics/TranscodingSession.java b/media/java/android/media/metrics/TranscodingSession.java
index e6d359aec35d..352138e304c1 100644
--- a/media/java/android/media/metrics/TranscodingSession.java
+++ b/media/java/android/media/metrics/TranscodingSession.java
@@ -59,5 +59,6 @@ public final class TranscodingSession implements AutoCloseable {
 
     @Override
     public void close() {
+        mManager.releaseSessionId(mLogSessionId.getStringId());
     }
 }
diff --git a/services/core/java/com/android/server/media/metrics/MediaMetricsManagerService.java b/services/core/java/com/android/server/media/metrics/MediaMetricsManagerService.java
index 1ae7ac07594b..df612e63927f 100644
--- a/services/core/java/com/android/server/media/metrics/MediaMetricsManagerService.java
+++ b/services/core/java/com/android/server/media/metrics/MediaMetricsManagerService.java
@@ -19,6 +19,7 @@ package com.android.server.media.metrics;
 import android.content.Context;
 import android.content.pm.PackageManager;
 import android.media.MediaMetrics;
+import android.media.metrics.BundleSession;
 import android.media.metrics.IMediaMetricsManager;
 import android.media.metrics.NetworkEvent;
 import android.media.metrics.PlaybackErrorEvent;
@@ -187,11 +188,33 @@ public final class MediaMetricsManagerService extends SystemService {
                 return;
             }
 
-            int atomid = metrics.getInt("KEY_STATSD_ATOM_NUMBER");
+            int atomid = metrics.getInt(BundleSession.KEY_STATSD_ATOM);
             switch (atomid) {
                 default:
                     return;
                 // to be extended as we define statsd atoms
+                case 322: // MediaPlaybackStateEvent
+                    // pattern for the keys:
+                    // <statsd event> - <fieldname>
+                    // match types to what stats will want
+                    String _sessionId = metrics.getString("playbackstateevent-sessionid");
+                    int _state = metrics.getInt("playbackstateevent-state", -1);
+                    long _lifetime = metrics.getLong("playbackstateevent-lifetime", -1);
+                    if (_sessionId == null || _state < 0 || _lifetime < 0) {
+                        Slog.d(TAG, "dropping incomplete data for atom 322: _sessionId: "
+                                        + _sessionId + " _state: " + _state
+                                        + " _lifetime: " + _lifetime);
+                        return;
+                    }
+                    StatsEvent statsEvent = StatsEvent.newBuilder()
+                            .setAtomId(322)
+                            .writeString(_sessionId)
+                            .writeInt(_state)
+                            .writeLong(_lifetime)
+                            .usePooledBuffer()
+                            .build();
+                    StatsLog.write(statsEvent);
+                    return;
             }
         }
 
@@ -227,6 +250,13 @@ public final class MediaMetricsManagerService extends SystemService {
         }
 
         @Override
+        public void releaseSessionId(String sessionId, int userId) {
+            // De-authorize this session-id in the native mediametrics service.
+            // TODO: plumbing to the native mediametrics service
+            Slog.v(TAG, "Releasing sessionId " + sessionId + " for userId " + userId + " [NOP]");
+        }
+
+        @Override
         public String getPlaybackSessionId(int userId) {
             return getSessionIdInternal(userId);
         }