From ea186fae038399fe8814543f62e9490c4854a960 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Thu, 9 Jan 2020 18:13:15 -0800 Subject: [PATCH] MediaMetrics: Add AudioAnalytics actions Underrun handling Latency handling Test: adb shell dumpsys media.metrics Bug: 138583596 Change-Id: I4c0a5ba0f4f4ce9209146dab99433e33646d959b --- .../include/MediaMetricsItem.h | 43 ++++++++++++ services/mediametrics/AnalyticsState.h | 18 +++++ services/mediametrics/AudioAnalytics.cpp | 66 +++++++++++++++++-- services/mediametrics/AudioAnalytics.h | 8 +++ 4 files changed, 130 insertions(+), 5 deletions(-) diff --git a/media/libmediametrics/include/MediaMetricsItem.h b/media/libmediametrics/include/MediaMetricsItem.h index 536c0e1aee..6141b3610a 100644 --- a/media/libmediametrics/include/MediaMetricsItem.h +++ b/media/libmediametrics/include/MediaMetricsItem.h @@ -35,6 +35,49 @@ namespace android { +/* + * MediaMetrics Keys and Properties for Audio. + * + * C/C++ friendly constants that ensure + * 1) Compilation error on misspelling + * 2) Consistent behavior and documentation. + * + * TODO: Move to separate header file. + */ + +// Taxonomy of audio keys + +// Key Prefixes are used for MediaMetrics Item Keys and ends with a ".". +// They must be appended with another value to make a key. +#define AMEDIAMETRICS_KEY_PREFIX_AUDIO "audio." + +// The AudioRecord key appends the "trackId" to the prefix. +#define AMEDIAMETRICS_KEY_PREFIX_AUDIO_RECORD AMEDIAMETRICS_KEY_PREFIX_AUDIO "record." + +// The AudioThread key appends the "threadId" to the prefix. +#define AMEDIAMETRICS_KEY_PREFIX_AUDIO_THREAD AMEDIAMETRICS_KEY_PREFIX_AUDIO "thread." + +// The AudioTrack key appends the "trackId" to the prefix. +#define AMEDIAMETRICS_KEY_PREFIX_AUDIO_TRACK AMEDIAMETRICS_KEY_PREFIX_AUDIO "track." + +// Keys are strings used for MediaMetrics Item Keys +#define AMEDIAMETRICS_KEY_AUDIO_FLINGER AMEDIAMETRICS_KEY_PREFIX_AUDIO "flinger" +#define AMEDIAMETRICS_KEY_AUDIO_POLICY AMEDIAMETRICS_KEY_PREFIX_AUDIO "policy" + +// Props are properties allowed for Mediametrics Items. +#define AMEDIAMETRICS_PROP_EVENT "event" // string value (often func name) +#define AMEDIAMETRICS_PROP_LATENCYMS "latencyMs" // double value +#define AMEDIAMETRICS_PROP_OUTPUTDEVICES "outputDevices" // string value +#define AMEDIAMETRICS_PROP_STARTUPMS "startupMs" // double value +#define AMEDIAMETRICS_PROP_THREADID "threadId" // int32 value io handle + +// Values are strings accepted for a given property. + +// An event is a general description, which often is a function name. +#define AMEDIAMETRICS_PROP_EVENT_VALUE_CTOR "ctor" +#define AMEDIAMETRICS_PROP_EVENT_VALUE_DTOR "dtor" +#define AMEDIAMETRICS_PROP_EVENT_VALUE_UNDERRUN "underrun" // from Thread + class IMediaMetricsService; class Parcel; diff --git a/services/mediametrics/AnalyticsState.h b/services/mediametrics/AnalyticsState.h index 4711bb62fa..290ed21306 100644 --- a/services/mediametrics/AnalyticsState.h +++ b/services/mediametrics/AnalyticsState.h @@ -55,6 +55,24 @@ public: return mTimeMachine.put(item, isTrusted) ?: mTransactionLog.put(item); } + /** + * Returns the TimeMachine. + * + * The TimeMachine object is internally locked, so access is safe and defined, + * but multiple threaded access may change results after calling. + */ + TimeMachine& timeMachine() { return mTimeMachine; } + const TimeMachine& timeMachine() const { return mTimeMachine; } + + /** + * Returns the TransactionLog. + * + * The TransactionLog object is internally locked, so access is safe and defined, + * but multiple threaded access may change results after calling. + */ + TransactionLog& transactionLog() { return mTransactionLog; } + const TransactionLog& transactionLog() const { return mTransactionLog; } + /** * Returns a pair consisting of the dump string, and the number of lines in the string. * diff --git a/services/mediametrics/AudioAnalytics.cpp b/services/mediametrics/AudioAnalytics.cpp index f7ee05117c..126e501238 100644 --- a/services/mediametrics/AudioAnalytics.cpp +++ b/services/mediametrics/AudioAnalytics.cpp @@ -32,16 +32,60 @@ AudioAnalytics::AudioAnalytics() // This triggers on an item of "audio.flinger" // with a property "event" set to "AudioFlinger" (the constructor). mActions.addAction( - "audio.flinger.event", - std::string("AudioFlinger"), + AMEDIAMETRICS_KEY_AUDIO_FLINGER "." AMEDIAMETRICS_PROP_EVENT, + std::string(AMEDIAMETRICS_PROP_EVENT_VALUE_CTOR), std::make_shared( - [this](const std::shared_ptr &){ - ALOGW("Audioflinger() constructor event detected"); + [this](const std::shared_ptr &item){ + ALOGW("(key=%s) Audioflinger constructor event detected", item->getKey().c_str()); mPreviousAnalyticsState.set(std::make_shared( *mAnalyticsState.get())); // Note: get returns shared_ptr temp, whose lifetime is extended // to end of full expression. mAnalyticsState->clear(); // TODO: filter the analytics state. + // Perhaps report this. + })); + + // Check underruns + mActions.addAction( + AMEDIAMETRICS_KEY_PREFIX_AUDIO_THREAD "*." AMEDIAMETRICS_PROP_EVENT, + std::string(AMEDIAMETRICS_PROP_EVENT_VALUE_UNDERRUN), + std::make_shared( + [this](const std::shared_ptr &item){ + std::string threadId = item->getKey().substr( + sizeof(AMEDIAMETRICS_KEY_PREFIX_AUDIO_THREAD) - 1); + std::string outputDevices; + mAnalyticsState->timeMachine().get( + item->getKey(), AMEDIAMETRICS_PROP_OUTPUTDEVICES, &outputDevices); + ALOGD("(key=%s) Thread underrun event detected on io handle:%s device:%s", + item->getKey().c_str(), threadId.c_str(), outputDevices.c_str()); + if (outputDevices.find("AUDIO_DEVICE_OUT_BLUETOOTH") != std::string::npos) { + // report this for Bluetooth + } + })); + + // Check latencies, playback and startup + mActions.addAction( + AMEDIAMETRICS_KEY_PREFIX_AUDIO_TRACK "*." AMEDIAMETRICS_PROP_LATENCYMS, + std::monostate{}, // accept any value + std::make_shared( + [this](const std::shared_ptr &item){ + double latencyMs{}; + double startupMs{}; + if (!item->get(AMEDIAMETRICS_PROP_LATENCYMS, &latencyMs) + || !item->get(AMEDIAMETRICS_PROP_STARTUPMS, &startupMs)) return; + + std::string trackId = item->getKey().substr( + sizeof(AMEDIAMETRICS_KEY_PREFIX_AUDIO_TRACK) - 1); + std::string thread = getThreadFromTrack(item->getKey()); + std::string outputDevices; + mAnalyticsState->timeMachine().get( + thread, AMEDIAMETRICS_PROP_OUTPUTDEVICES, &outputDevices); + ALOGD("(key=%s) Track latencyMs:%lf startupMs:%lf detected on port:%s device:%s", + item->getKey().c_str(), latencyMs, startupMs, + trackId.c_str(), outputDevices.c_str()); + if (outputDevices.find("AUDIO_DEVICE_OUT_BLUETOOTH") != std::string::npos) { + // report this for Bluetooth + } })); } @@ -53,7 +97,7 @@ AudioAnalytics::~AudioAnalytics() status_t AudioAnalytics::submit( const std::shared_ptr& item, bool isTrusted) { - if (!startsWith(item->getKey(), "audio.")) return BAD_VALUE; + if (!startsWith(item->getKey(), AMEDIAMETRICS_KEY_PREFIX_AUDIO)) return BAD_VALUE; status_t status = mAnalyticsState->submit(item, isTrusted); if (status != NO_ERROR) return status; // may not be permitted. @@ -94,4 +138,16 @@ void AudioAnalytics::checkActions(const std::shared_ptrtimeMachine().get( + track, AMEDIAMETRICS_PROP_THREADID, &threadId_int32) != NO_ERROR) { + return {}; + } + return std::string(AMEDIAMETRICS_KEY_PREFIX_AUDIO_THREAD) + std::to_string(threadId_int32); +} + } // namespace android diff --git a/services/mediametrics/AudioAnalytics.h b/services/mediametrics/AudioAnalytics.h index be885ec705..4a42e22566 100644 --- a/services/mediametrics/AudioAnalytics.h +++ b/services/mediametrics/AudioAnalytics.h @@ -72,6 +72,14 @@ private: */ void checkActions(const std::shared_ptr& item); + // HELPER METHODS + /** + * Return the audio thread associated with an audio track name. + * e.g. "audio.track.32" -> "audio.thread.10" if the associated + * threadId for the audio track is 10. + */ + std::string getThreadFromTrack(const std::string& track) const; + // Actions is individually locked AnalyticsActions mActions;