MediaMetrics: Convert to AIDL
Test: adb shell dumpsys media.metrics
Test: atest mediametrics_tests
Bug: 173831888
Change-Id: I3ae121c7cd03719e9a9732bca5f545189b9e47a4
diff --git a/media/libmediametrics/Android.bp b/media/libmediametrics/Android.bp
index c2e1dc9..3dfd850 100644
--- a/media/libmediametrics/Android.bp
+++ b/media/libmediametrics/Android.bp
@@ -7,7 +7,6 @@
name: "libmediametrics",
srcs: [
- "IMediaMetricsService.cpp",
"MediaMetricsItem.cpp",
"MediaMetrics.cpp",
],
@@ -17,6 +16,7 @@
"libcutils",
"liblog",
"libutils",
+ "mediametricsservice-aidl-unstable-cpp",
],
export_include_dirs: ["include"],
@@ -58,3 +58,21 @@
"//frameworks/base/media/jni",
],
}
+
+aidl_interface {
+ name: "mediametricsservice-aidl",
+ unstable: true,
+ local_include_dir: "aidl",
+ vendor_available: true,
+ srcs: [
+ "aidl/android/media/IMediaMetricsService.aidl",
+ ],
+ double_loadable: true,
+ backend: {
+ cpp: {
+ apex_available: [
+ "//apex_available:platform",
+ ],
+ },
+ },
+}
diff --git a/media/libmediametrics/MediaMetricsItem.cpp b/media/libmediametrics/MediaMetricsItem.cpp
index 7cdbe5f..f4371fd 100644
--- a/media/libmediametrics/MediaMetricsItem.cpp
+++ b/media/libmediametrics/MediaMetricsItem.cpp
@@ -31,8 +31,9 @@
#include <utils/SortedVector.h>
#include <utils/threads.h>
+#include <android/media/BnMediaMetricsService.h> // for direct Binder access
+#include <android/media/IMediaMetricsService.h>
#include <binder/IServiceManager.h>
-#include <media/IMediaMetricsService.h>
#include <media/MediaMetricsItem.h>
#include <private/android_filesystem_config.h>
@@ -278,17 +279,18 @@
// calls the appropriate daemon
bool mediametrics::Item::selfrecord() {
ALOGD_IF(DEBUG_API, "%s: delivering %s", __func__, this->toString().c_str());
- sp<IMediaMetricsService> svc = getService();
- if (svc != NULL) {
- status_t status = svc->submit(this);
- if (status != NO_ERROR) {
- ALOGW("%s: failed to record: %s", __func__, this->toString().c_str());
- return false;
- }
- return true;
- } else {
+
+ char *str;
+ size_t size;
+ status_t status = writeToByteString(&str, &size);
+ if (status == NO_ERROR) {
+ status = submitBuffer(str, size);
+ }
+ if (status != NO_ERROR) {
+ ALOGW("%s: failed to record: %s", __func__, this->toString().c_str());
return false;
}
+ return true;
}
//static
@@ -327,7 +329,7 @@
static sp<MediaMetricsDeathNotifier> sNotifier;
// static
-sp<IMediaMetricsService> BaseItem::sMediaMetricsService;
+sp<media::IMediaMetricsService> BaseItem::sMediaMetricsService;
static std::mutex sServiceMutex;
static int sRemainingBindAttempts = SVC_TRIES;
@@ -339,29 +341,67 @@
}
// static
-bool BaseItem::submitBuffer(const char *buffer, size_t size) {
-/*
- mediametrics::Item item;
- status_t status = item.readFromByteString(buffer, size);
- ALOGD("%s: status:%d, size:%zu, item:%s", __func__, status, size, item.toString().c_str());
- return item.selfrecord();
- */
-
+status_t BaseItem::submitBuffer(const char *buffer, size_t size) {
ALOGD_IF(DEBUG_API, "%s: delivering %zu bytes", __func__, size);
- sp<IMediaMetricsService> svc = getService();
- if (svc != nullptr) {
- const status_t status = svc->submitBuffer(buffer, size);
- if (status != NO_ERROR) {
- ALOGW("%s: failed(%d) to record: %zu bytes", __func__, status, size);
- return false;
- }
- return true;
+
+ // Validate size
+ if (size > std::numeric_limits<int32_t>::max()) return BAD_VALUE;
+
+ // Do we have the service available?
+ sp<media::IMediaMetricsService> svc = getService();
+ if (svc == nullptr) return NO_INIT;
+
+ ::android::status_t status = NO_ERROR;
+ if constexpr (/* DISABLES CODE */ (false)) {
+ // THIS PATH IS FOR REFERENCE ONLY.
+ // It is compiled so that any changes to IMediaMetricsService::submitBuffer()
+ // will lead here. If this code is changed, the else branch must
+ // be changed as well.
+ //
+ // Use the AIDL calling interface - this is a bit slower as a byte vector must be
+ // constructed. As the call is one-way, the only a transaction error occurs.
+ status = svc->submitBuffer({buffer, buffer + size}).transactionError();
+ } else {
+ // Use the Binder calling interface - this direct implementation avoids
+ // malloc/copy/free for the vector and reduces the overhead for logging.
+ // We based this off of the AIDL generated file:
+ // out/soong/.intermediates/frameworks/av/media/libmediametrics/mediametricsservice-aidl-unstable-cpp-source/gen/android/media/IMediaMetricsService.cpp
+ // TODO: Create an AIDL C++ back end optimized form of vector writing.
+ ::android::Parcel _aidl_data;
+ ::android::Parcel _aidl_reply; // we don't care about this as it is one-way.
+
+ status = _aidl_data.writeInterfaceToken(svc->getInterfaceDescriptor());
+ if (status != ::android::OK) goto _aidl_error;
+
+ status = _aidl_data.writeInt32(static_cast<int32_t>(size));
+ if (status != ::android::OK) goto _aidl_error;
+
+ status = _aidl_data.write(buffer, static_cast<int32_t>(size));
+ if (status != ::android::OK) goto _aidl_error;
+
+ status = ::android::IInterface::asBinder(svc)->transact(
+ ::android::media::BnMediaMetricsService::TRANSACTION_submitBuffer,
+ _aidl_data, &_aidl_reply, ::android::IBinder::FLAG_ONEWAY);
+
+ // AIDL permits setting a default implementation for additional functionality.
+ // See go/aog/713984. This is not used here.
+ // if (status == ::android::UNKNOWN_TRANSACTION
+ // && ::android::media::IMediaMetricsService::getDefaultImpl()) {
+ // status = ::android::media::IMediaMetricsService::getDefaultImpl()
+ // ->submitBuffer(immutableByteVectorFromBuffer(buffer, size))
+ // .transactionError();
+ // }
}
- return false;
+
+ if (status == NO_ERROR) return NO_ERROR;
+
+ _aidl_error:
+ ALOGW("%s: failed(%d) to record: %zu bytes", __func__, status, size);
+ return status;
}
//static
-sp<IMediaMetricsService> BaseItem::getService() {
+sp<media::IMediaMetricsService> BaseItem::getService() {
static const char *servicename = "media.metrics";
static const bool enabled = isEnabled(); // singleton initialized
@@ -379,7 +419,7 @@
if (sm != nullptr) {
sp<IBinder> binder = sm->getService(String16(servicename));
if (binder != nullptr) {
- sMediaMetricsService = interface_cast<IMediaMetricsService>(binder);
+ sMediaMetricsService = interface_cast<media::IMediaMetricsService>(binder);
sNotifier = new MediaMetricsDeathNotifier();
binder->linkToDeath(sNotifier);
} else {
diff --git a/media/libmediametrics/aidl/android/media/IMediaMetricsService.aidl b/media/libmediametrics/aidl/android/media/IMediaMetricsService.aidl
new file mode 100644
index 0000000..b14962d
--- /dev/null
+++ b/media/libmediametrics/aidl/android/media/IMediaMetricsService.aidl
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.media;
+
+/**
+ * MediaMetrics service interface
+ *
+ * {@hide}
+ */
+interface IMediaMetricsService {
+ oneway void submitBuffer(in byte[] buffer);
+}
diff --git a/media/libmediametrics/include/media/IMediaMetricsService.h b/media/libmediametrics/include/media/IMediaMetricsService.h
deleted file mode 100644
index d6871ec..0000000
--- a/media/libmediametrics/include/media/IMediaMetricsService.h
+++ /dev/null
@@ -1,73 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef ANDROID_IMEDIAANALYTICSSERVICE_H
-#define ANDROID_IMEDIAANALYTICSSERVICE_H
-
-#include <utils/String8.h>
-#include <binder/IInterface.h>
-#include <binder/Parcel.h>
-
-#include <sys/types.h>
-#include <utils/Errors.h>
-#include <utils/Log.h>
-#include <utils/RefBase.h>
-#include <utils/List.h>
-
-#include <binder/IServiceManager.h>
-
-#include <media/MediaMetricsItem.h>
-
-namespace android {
-
-class IMediaMetricsService: public IInterface
-{
-public:
- DECLARE_META_INTERFACE(MediaMetricsService);
-
- /**
- * Submits the indicated record to the mediaanalytics service, where
- * it will be merged (if appropriate) with incomplete records that
- * share the same key and sessionID.
- *
- * \param item the item to submit.
- * \return status which is negative if an error is detected (some errors
- may be silent and return 0 - success).
- */
- virtual status_t submit(mediametrics::Item *item) = 0;
-
- virtual status_t submitBuffer(const char *buffer, size_t length) = 0;
-};
-
-// ----------------------------------------------------------------------------
-
-class BnMediaMetricsService: public BnInterface<IMediaMetricsService>
-{
-public:
- status_t onTransact(uint32_t code,
- const Parcel& data,
- Parcel* reply,
- uint32_t flags = 0) override;
-
-protected:
- // Internal call where release is true if the service is to delete the item.
- virtual status_t submitInternal(
- mediametrics::Item *item, bool release) = 0;
-};
-
-}; // namespace android
-
-#endif // ANDROID_IMEDIASTATISTICSSERVICE_H
diff --git a/media/libmediametrics/include/media/MediaMetricsItem.h b/media/libmediametrics/include/media/MediaMetricsItem.h
index 303343f..428992c 100644
--- a/media/libmediametrics/include/media/MediaMetricsItem.h
+++ b/media/libmediametrics/include/media/MediaMetricsItem.h
@@ -32,7 +32,8 @@
namespace android {
-class IMediaMetricsService;
+namespace media { class IMediaMetricsService; }
+
class Parcel;
/*
@@ -239,7 +240,10 @@
public:
// are we collecting metrics data
static bool isEnabled();
- static sp<IMediaMetricsService> getService();
+ // returns the MediaMetrics service if active.
+ static sp<media::IMediaMetricsService> getService();
+ // submits a raw buffer directly to the MediaMetrics service - this is highly optimized.
+ static status_t submitBuffer(const char *buffer, size_t len);
protected:
static constexpr const char * const EnabledProperty = "media.metrics.enabled";
@@ -247,10 +251,9 @@
static const int EnabledProperty_default = 1;
// let's reuse a binder connection
- static sp<IMediaMetricsService> sMediaMetricsService;
+ static sp<media::IMediaMetricsService> sMediaMetricsService;
static void dropInstance();
- static bool submitBuffer(const char *buffer, size_t len);
template <typename T>
struct is_item_type {
@@ -573,7 +576,7 @@
bool record() {
return updateHeader()
- && BaseItem::submitBuffer(getBuffer(), getLength());
+ && BaseItem::submitBuffer(getBuffer(), getLength()) == OK;
}
bool isValid () const {
diff --git a/services/mediametrics/Android.bp b/services/mediametrics/Android.bp
index 3bb70f1..b64f726 100644
--- a/services/mediametrics/Android.bp
+++ b/services/mediametrics/Android.bp
@@ -100,6 +100,7 @@
"libmediametricsservice",
"libmediautils",
"libutils",
+ "mediametricsservice-aidl-unstable-cpp",
],
header_libs: [
"libaudioutils_headers",
@@ -142,6 +143,7 @@
},
shared_libs: [
+ "mediametricsservice-aidl-unstable-cpp",
"libbase", // android logging
"libbinder",
"libcutils",
diff --git a/services/mediametrics/MediaMetricsService.cpp b/services/mediametrics/MediaMetricsService.cpp
index bf6e428..9d380ec 100644
--- a/services/mediametrics/MediaMetricsService.cpp
+++ b/services/mediametrics/MediaMetricsService.cpp
@@ -25,6 +25,7 @@
#include <android/content/pm/IPackageManagerNative.h> // package info
#include <audio_utils/clock.h> // clock conversions
#include <binder/IPCThreadState.h> // get calling uid
+#include <binder/IServiceManager.h> // checkCallingPermission
#include <cutils/properties.h> // for property_get
#include <mediautils/MemoryLeakTrackUtil.h>
#include <memunreachable/memunreachable.h>
diff --git a/services/mediametrics/MediaMetricsService.h b/services/mediametrics/MediaMetricsService.h
index 792b7f0..bcae397 100644
--- a/services/mediametrics/MediaMetricsService.h
+++ b/services/mediametrics/MediaMetricsService.h
@@ -24,7 +24,7 @@
// IMediaMetricsService must include Vector, String16, Errors
#include <android-base/thread_annotations.h>
-#include <media/IMediaMetricsService.h>
+#include <android/media/BnMediaMetricsService.h>
#include <mediautils/ServiceUtilities.h>
#include <utils/String8.h>
@@ -32,12 +32,18 @@
namespace android {
-class MediaMetricsService : public BnMediaMetricsService
+class MediaMetricsService : public media::BnMediaMetricsService
{
public:
MediaMetricsService();
~MediaMetricsService() override;
+ // AIDL interface
+ binder::Status submitBuffer(const std::vector<uint8_t>& buffer) override {
+ status_t status = submitBuffer((char *)buffer.data(), buffer.size());
+ return binder::Status::fromStatusT(status);
+ }
+
/**
* Submits the indicated record to the mediaanalytics service.
*
@@ -45,11 +51,11 @@
* \return status failure, which is negative on binder transaction failure.
* As the transaction is one-way, remote failures will not be reported.
*/
- status_t submit(mediametrics::Item *item) override {
+ status_t submit(mediametrics::Item *item) {
return submitInternal(item, false /* release */);
}
- status_t submitBuffer(const char *buffer, size_t length) override {
+ status_t submitBuffer(const char *buffer, size_t length) {
mediametrics::Item *item = new mediametrics::Item();
return item->readFromByteString(buffer, length)
?: submitInternal(item, true /* release */);
@@ -81,7 +87,7 @@
// Internal call where release is true if ownership of item is transferred
// to the service (that is, the service will eventually delete the item).
- status_t submitInternal(mediametrics::Item *item, bool release) override;
+ status_t submitInternal(mediametrics::Item *item, bool release);
private:
void processExpirations();
diff --git a/services/mediametrics/fuzzer/Android.bp b/services/mediametrics/fuzzer/Android.bp
index df4c867..6ac9d20 100644
--- a/services/mediametrics/fuzzer/Android.bp
+++ b/services/mediametrics/fuzzer/Android.bp
@@ -43,6 +43,7 @@
"libstagefright",
"libstatslog",
"libutils",
+ "mediametricsservice-aidl-unstable-cpp",
],
include_dirs: [
diff --git a/services/mediametrics/tests/Android.bp b/services/mediametrics/tests/Android.bp
index c2e0759..94112b0 100644
--- a/services/mediametrics/tests/Android.bp
+++ b/services/mediametrics/tests/Android.bp
@@ -19,6 +19,7 @@
"libmediametricsservice",
"libmediautils",
"libutils",
+ "mediametricsservice-aidl-unstable-cpp",
],
header_libs: [