From ba8c48429d82d22737a84d2713fa1880a4654cbd Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Fri, 18 Jan 2019 11:35:33 -0800 Subject: [PATCH] Further work on libmediametrics stable API implement a missing method (for inflight metrics), enumerate exported symbols. Bug: 119675363 Test: build, CTS media/codec tests Change-Id: Id92f1b331babacc0de6c0005d6ba0c97c91c0291 --- media/libmediametrics/Android.bp | 21 +- .../IMediaAnalyticsService.cpp | 2 +- media/libmediametrics/MediaAnalyticsItem.cpp | 207 +++++++++++++++++- media/libmediametrics/MediaMetrics.cpp | 19 +- .../include/MediaAnalyticsItem.h | 15 +- media/libmediametrics/include/MediaMetrics.h | 10 +- media/libmediametrics/libmediametrics.map.txt | 29 +++ 7 files changed, 266 insertions(+), 37 deletions(-) create mode 100644 media/libmediametrics/libmediametrics.map.txt diff --git a/media/libmediametrics/Android.bp b/media/libmediametrics/Android.bp index e188e54fb3..15ea578fdb 100644 --- a/media/libmediametrics/Android.bp +++ b/media/libmediametrics/Android.bp @@ -1,6 +1,4 @@ -// TODO: change it back to cc_library_shared when there is a way to -// expose media metrics as stable API. -cc_library { +cc_library_shared { name: "libmediametrics", srcs: [ @@ -32,12 +30,13 @@ cc_library { cfi: true, }, - // enumerate the stable interface -// this would mean nobody can use the C++ interface. have to rework some things. -// stubs: { -// symbol_file: "libmediametrics.map.txt", -// versions: [ -// "1" , -// ] -// }, + // enumerate stable entry points, for apex use + stubs: { + symbol_file: "libmediametrics.map.txt", + versions: [ + "1" , + ] + }, } + + diff --git a/media/libmediametrics/IMediaAnalyticsService.cpp b/media/libmediametrics/IMediaAnalyticsService.cpp index 28a77462b2..9114927dac 100644 --- a/media/libmediametrics/IMediaAnalyticsService.cpp +++ b/media/libmediametrics/IMediaAnalyticsService.cpp @@ -142,7 +142,7 @@ status_t BnMediaAnalyticsService::onTransact( CHECK_INTERFACE(IMediaAnalyticsService, data, reply); bool forcenew; - MediaAnalyticsItem *item = new MediaAnalyticsItem; + MediaAnalyticsItem *item = MediaAnalyticsItem::create(); data.readBool(&forcenew); item->readFromParcel(data); diff --git a/media/libmediametrics/MediaAnalyticsItem.cpp b/media/libmediametrics/MediaAnalyticsItem.cpp index 448e2d9959..02c23b1231 100644 --- a/media/libmediametrics/MediaAnalyticsItem.cpp +++ b/media/libmediametrics/MediaAnalyticsItem.cpp @@ -52,6 +52,17 @@ const char * const MediaAnalyticsItem::EnabledProperty = "media.metrics.enabled const char * const MediaAnalyticsItem::EnabledPropertyPersist = "persist.media.metrics.enabled"; const int MediaAnalyticsItem::EnabledProperty_default = 1; +// So caller doesn't need to know size of allocated space +MediaAnalyticsItem *MediaAnalyticsItem::create() +{ + return MediaAnalyticsItem::create(kKeyNone); +} + +MediaAnalyticsItem *MediaAnalyticsItem::create(MediaAnalyticsItem::Key key) +{ + MediaAnalyticsItem *item = new MediaAnalyticsItem(key); + return item; +} // access functions for the class MediaAnalyticsItem::MediaAnalyticsItem() @@ -642,6 +653,19 @@ bool MediaAnalyticsItem::growProps(int increment) // int32_t MediaAnalyticsItem::readFromParcel(const Parcel& data) { + int32_t version = data.readInt32(); + + switch(version) { + case 0: + return readFromParcel0(data); + break; + default: + ALOGE("Unsupported MediaAnalyticsItem Parcel version: %d", version); + return -1; + } +} + +int32_t MediaAnalyticsItem::readFromParcel0(const Parcel& data) { // into 'this' object // .. we make a copy of the string to put away. mKey = data.readCString(); @@ -691,8 +715,23 @@ int32_t MediaAnalyticsItem::readFromParcel(const Parcel& data) { } int32_t MediaAnalyticsItem::writeToParcel(Parcel *data) { + if (data == NULL) return -1; + int32_t version = 0; + data->writeInt32(version); + + switch(version) { + case 0: + return writeToParcel0(data); + break; + default: + ALOGE("Unsupported MediaAnalyticsItem Parcel version: %d", version); + return -1; + } +} + +int32_t MediaAnalyticsItem::writeToParcel0(Parcel *data) { data->writeCString(mKey.c_str()); data->writeInt32(mPid); @@ -737,7 +776,6 @@ int32_t MediaAnalyticsItem::writeToParcel(Parcel *data) { return 0; } - const char *MediaAnalyticsItem::toCString() { return toCString(PROTO_LAST); } @@ -876,8 +914,6 @@ bool MediaAnalyticsItem::selfrecord(bool forcenew) { } return true; } else { - std::string p = this->toString(); - ALOGW("Unable to record: %s [forcenew=%d]", p.c_str(), forcenew); return false; } } @@ -1035,5 +1071,170 @@ bool MediaAnalyticsItem::merge(MediaAnalyticsItem *incoming) { return true; } +// a byte array; contents are +// overall length (uint32) including the length field itself +// encoding version (uint32) +// count of properties (uint32) +// N copies of: +// property name as length(int16), bytes +// the bytes WILL include the null terminator of the name +// type (uint8 -- 1 byte) +// size of value field (int16 -- 2 bytes) +// value (size based on type) +// int32, int64, double -- little endian 4/8/8 bytes respectively +// cstring -- N bytes of value [WITH terminator] + +enum { kInt32 = 0, kInt64, kDouble, kRate, kCString}; + +bool MediaAnalyticsItem::dumpAttributes(char **pbuffer, size_t *plength) { + + char *build = NULL; + + if (pbuffer == NULL || plength == NULL) + return false; + + // consistency for the caller, who owns whatever comes back in this pointer. + *pbuffer = NULL; + + // first, let's calculate sizes + int32_t goal = 0; + int32_t version = 0; + + goal += sizeof(uint32_t); // overall length, including the length field + goal += sizeof(uint32_t); // encoding version + goal += sizeof(uint32_t); // # properties + + int32_t count = mPropCount; + for (int i = 0 ; i < count; i++ ) { + Prop *prop = &mProps[i]; + goal += sizeof(uint16_t); // name length + goal += strlen(prop->mName) + 1; // string + null + goal += sizeof(uint8_t); // type + goal += sizeof(uint16_t); // size of value + switch (prop->mType) { + case MediaAnalyticsItem::kTypeInt32: + goal += sizeof(uint32_t); + break; + case MediaAnalyticsItem::kTypeInt64: + goal += sizeof(uint64_t); + break; + case MediaAnalyticsItem::kTypeDouble: + goal += sizeof(double); + break; + case MediaAnalyticsItem::kTypeRate: + goal += 2 * sizeof(uint64_t); + break; + case MediaAnalyticsItem::kTypeCString: + // length + actual string + null + goal += strlen(prop->u.CStringValue) + 1; + break; + default: + ALOGE("found bad Prop type: %d, idx %d, name %s", + prop->mType, i, prop->mName); + return false; + } + } + + // now that we have a size... let's allocate and fill + build = (char *)malloc(goal); + if (build == NULL) + return false; + + memset(build, 0, goal); + + char *filling = build; + +#define _INSERT(val, size) \ + { memcpy(filling, &(val), (size)); filling += (size);} +#define _INSERTSTRING(val, size) \ + { memcpy(filling, (val), (size)); filling += (size);} + + _INSERT(goal, sizeof(int32_t)); + _INSERT(version, sizeof(int32_t)); + _INSERT(count, sizeof(int32_t)); + + for (int i = 0 ; i < count; i++ ) { + Prop *prop = &mProps[i]; + int16_t attrNameLen = strlen(prop->mName) + 1; + _INSERT(attrNameLen, sizeof(int16_t)); + _INSERTSTRING(prop->mName, attrNameLen); // termination included + int8_t elemtype; + int16_t elemsize; + switch (prop->mType) { + case MediaAnalyticsItem::kTypeInt32: + { + elemtype = kInt32; + _INSERT(elemtype, sizeof(int8_t)); + elemsize = sizeof(int32_t); + _INSERT(elemsize, sizeof(int16_t)); + + _INSERT(prop->u.int32Value, sizeof(int32_t)); + break; + } + case MediaAnalyticsItem::kTypeInt64: + { + elemtype = kInt64; + _INSERT(elemtype, sizeof(int8_t)); + elemsize = sizeof(int64_t); + _INSERT(elemsize, sizeof(int16_t)); + + _INSERT(prop->u.int64Value, sizeof(int64_t)); + break; + } + case MediaAnalyticsItem::kTypeDouble: + { + elemtype = kDouble; + _INSERT(elemtype, sizeof(int8_t)); + elemsize = sizeof(double); + _INSERT(elemsize, sizeof(int16_t)); + + _INSERT(prop->u.doubleValue, sizeof(double)); + break; + } + case MediaAnalyticsItem::kTypeRate: + { + elemtype = kRate; + _INSERT(elemtype, sizeof(int8_t)); + elemsize = 2 * sizeof(uint64_t); + _INSERT(elemsize, sizeof(int16_t)); + + _INSERT(prop->u.rate.count, sizeof(uint64_t)); + _INSERT(prop->u.rate.duration, sizeof(uint64_t)); + break; + } + case MediaAnalyticsItem::kTypeCString: + { + elemtype = kCString; + _INSERT(elemtype, sizeof(int8_t)); + elemsize = strlen(prop->u.CStringValue) + 1; + _INSERT(elemsize, sizeof(int16_t)); + + _INSERTSTRING(prop->u.CStringValue, elemsize); + break; + } + default: + // error if can't encode; warning if can't decode + ALOGE("found bad Prop type: %d, idx %d, name %s", + prop->mType, i, prop->mName); + goto badness; + } + } + + if (build + goal != filling) { + ALOGE("problems populating; wrote=%d planned=%d", + (int)(filling-build), goal); + goto badness; + } + + *pbuffer = build; + *plength = goal; + + return true; + + badness: + free(build); + return false; +} + } // namespace android diff --git a/media/libmediametrics/MediaMetrics.cpp b/media/libmediametrics/MediaMetrics.cpp index 9b08aa74fd..61091900c0 100644 --- a/media/libmediametrics/MediaMetrics.cpp +++ b/media/libmediametrics/MediaMetrics.cpp @@ -34,7 +34,7 @@ // manage the overall record mediametrics_handle_t mediametrics_create(mediametricskey_t key) { - android::MediaAnalyticsItem *item = new android::MediaAnalyticsItem(key); + android::MediaAnalyticsItem *item = android::MediaAnalyticsItem::create(key); return (mediametrics_handle_t) item; } @@ -187,18 +187,9 @@ bool mediametrics_isEnabled() { return android::MediaAnalyticsItem::isEnabled(); } -#if 0 -// do not expose this as is. -// need to revisit (or redefine) how the android::Parcel parameter is handled -// so that it meets the stable-API criteria for updateable components. -// -int32_t mediametrics_writeToParcel(mediametrics_handle_t handle, android::Parcel *parcel) { +bool mediametrics_getAttributes(mediametrics_handle_t handle, char **buffer, size_t *length) { android::MediaAnalyticsItem *item = (android::MediaAnalyticsItem *) handle; - if (item == NULL) { - return -1; - } - return item->writeToParcel(parcel); -} -#endif - + if (item == NULL) return false; + return item->dumpAttributes(buffer, length); +} diff --git a/media/libmediametrics/include/MediaAnalyticsItem.h b/media/libmediametrics/include/MediaAnalyticsItem.h index b99cd91c1c..2f9e7c204f 100644 --- a/media/libmediametrics/include/MediaAnalyticsItem.h +++ b/media/libmediametrics/include/MediaAnalyticsItem.h @@ -17,9 +17,10 @@ #ifndef ANDROID_MEDIA_MEDIAANALYTICSITEM_H #define ANDROID_MEDIA_MEDIAANALYTICSITEM_H -#include #include #include + +#include #include #include #include @@ -84,6 +85,10 @@ class MediaAnalyticsItem { public: + // so clients do not need to know size details + static MediaAnalyticsItem* create(Key key); + static MediaAnalyticsItem* create(); + // access functions for the class MediaAnalyticsItem(); MediaAnalyticsItem(Key); @@ -175,6 +180,9 @@ class MediaAnalyticsItem { int32_t writeToParcel(Parcel *); int32_t readFromParcel(const Parcel&); + // supports the stable interface + bool dumpAttributes(char **pbuffer, size_t *plength); + std::string toString(); std::string toString(int version); const char *toCString(); @@ -183,6 +191,11 @@ class MediaAnalyticsItem { // are we collecting analytics data static bool isEnabled(); + private: + // handle Parcel version 0 + int32_t writeToParcel0(Parcel *); + int32_t readFromParcel0(const Parcel&); + protected: // merge fields from arg into this diff --git a/media/libmediametrics/include/MediaMetrics.h b/media/libmediametrics/include/MediaMetrics.h index 4d2f352984..a4e1ed2c13 100644 --- a/media/libmediametrics/include/MediaMetrics.h +++ b/media/libmediametrics/include/MediaMetrics.h @@ -85,13 +85,9 @@ const char *mediametrics_readable(mediametrics_handle_t handle); void mediametrics_setUid(mediametrics_handle_t handle, uid_t uid); bool mediametrics_isEnabled(); -#if 0 -// do not expose this as is. -// need to revisit (or redefine) how the android::Parcel parameter is handled -// so that it meets the stable-API criteria for updateable components. -// -int32_t mediametrics_writeToParcel(mediametrics_handle_t handle, android::Parcel *parcel); -#endif +// serialized copy of the attributes/values, mostly for upstream getMetrics() calls +// caller owns the buffer allocated as part of this call. +bool mediametrics_getAttributes(mediametrics_handle_t handle, char **buffer, size_t *length); __END_DECLS diff --git a/media/libmediametrics/libmediametrics.map.txt b/media/libmediametrics/libmediametrics.map.txt new file mode 100644 index 0000000000..c46281aa63 --- /dev/null +++ b/media/libmediametrics/libmediametrics.map.txt @@ -0,0 +1,29 @@ +LIBMEDIAMETRICS_1 { + global: + mediametrics_addDouble; # apex + mediametrics_addInt32; # apex + mediametrics_addInt64; # apex + mediametrics_addRate; # apex + mediametrics_count; # apex + mediametrics_create; # apex + mediametrics_delete; # apex + mediametrics_freeCString; # apex + mediametrics_getAttributes; # apex + mediametrics_getCString; # apex + mediametrics_getDouble; # apex + mediametrics_getInt32; # apex + mediametrics_getInt64; # apex + mediametrics_getKey; # apex + mediametrics_getRate; # apex + mediametrics_isEnabled; # apex + mediametrics_readable; # apex + mediametrics_selfRecord; # apex + mediametrics_setCString; # apex + mediametrics_setDouble; # apex + mediametrics_setInt32; # apex + mediametrics_setInt64; # apex + mediametrics_setRate; # apex + mediametrics_setUid; # apex + local: + *; +};