summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Zim <zezeozue@google.com> 2021-01-26 18:30:53 +0000
committer Manish Singh <psych@google.com> 2021-01-28 23:23:10 +0000
commit801d9858bc5e9cb12ab373da91e8ecc66ac9865f (patch)
tree0f4c9a3bac6b73a4d9784c6f7ce710fc91d58bf2
parent6609f75742d26ff8406533c5afb222873280c6c4 (diff)
Improve transcoding metrics logging
Populated these new fields 1. video_duration 2. capture_framerate 3. transcode_reason transcode_reason is particularly complicated because we decide the transcode_reason at FUSE_LOOKUP, but we only log the metric at FUSE_READ. Hence, a new field was introduced on struct node#transforms_reason. The transforms_reason is an opaque value like transforms and it should not be parsed by the native FUSE daemon but should just be sent along during the transform() call. Refactored shouldTranscode() to return the transcode reason enum int value or 0 if should not transcode Lastly, fixed the metrics logging of read_cache and read_direct. Previously, we were logging in shouldTranscode which is called very often in FUSE_LOOKUP. Now, we log during open(2). This is much better, but is still not technically correct since an app might open but never read. Handle IllegalStateException during transcoding metrics logging TranscodeHelper#onFileOpen might hit an IllegalStateException if the volume is not ready for any reason. Now we handle the exception instead of crashing the MediaProvider Test: atest TranscodeTest, atest com.android.devicehealthckecks.SystemAppCheck#system_app_crash Bug: 178711878, 178447612 Change-Id: I517410ae2e127e9577d16c8dce9ef3de689e4398
-rwxr-xr-xjni/FuseDaemon.cpp15
-rw-r--r--jni/MediaProviderWrapper.cpp26
-rw-r--r--jni/MediaProviderWrapper.h14
-rw-r--r--jni/node-inl.h24
-rw-r--r--jni/node_test.cpp12
-rw-r--r--src/com/android/providers/media/FileLookupResult.java6
-rw-r--r--src/com/android/providers/media/MediaProvider.java65
-rw-r--r--src/com/android/providers/media/TranscodeHelper.java234
-rw-r--r--tests/src/com/android/providers/media/MediaProviderForFuseTest.java4
9 files changed, 252 insertions, 148 deletions
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index 7a9932275..c4eab9382 100755
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -438,6 +438,7 @@ static node* make_node_entry(fuse_req_t req, node* parent, const string& name, c
bool should_invalidate = false;
bool transforms_complete = true;
int transforms = 0;
+ int transforms_reason = 0;
string io_path;
if (S_ISREG(e->attr.st_mode)) {
@@ -455,6 +456,7 @@ static node* make_node_entry(fuse_req_t req, node* parent, const string& name, c
transforms = file_lookup_result->transforms;
io_path = file_lookup_result->io_path;
transforms_complete = file_lookup_result->transforms_complete;
+ transforms_reason = file_lookup_result->transforms_reason;
// Invalidate if the inode supports transforms so that we always get a lookup into userspace
should_invalidate = file_lookup_result->transforms_supported;
@@ -468,7 +470,7 @@ static node* make_node_entry(fuse_req_t req, node* parent, const string& name, c
node = parent->LookupChildByName(name, true /* acquire */, transforms);
if (!node) {
node = ::node::Create(parent, name, io_path, should_invalidate, transforms_complete,
- transforms, &fuse->lock, &fuse->tracker);
+ transforms, transforms_reason, &fuse->lock, &fuse->tracker);
} else if (!mediaprovider::fuse::containsMount(path, std::to_string(getuid() / PER_USER_RANGE))) {
// Only invalidate a path if it does not contain mount.
// Invalidate both names to ensure there's no dentry left in the kernel after the following
@@ -728,7 +730,8 @@ static void pf_setattr(fuse_req_t req,
} else {
const struct fuse_ctx* ctx = fuse_req_ctx(req);
std::unique_ptr<FileOpenResult> result = fuse->mp->OnFileOpen(
- path, path, ctx->uid, ctx->pid, true /* for_write */, false /* redact */);
+ path, path, ctx->uid, ctx->pid, node->GetTransformsReason(), true /* for_write */,
+ false /* redact */, false /* log_transforms_metrics */);
if (!result) {
fuse_reply_err(req, EFAULT);
@@ -1125,7 +1128,8 @@ static void pf_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info* fi) {
bool for_write = is_requesting_write(fi->flags);
// We don't redact if the caller was granted write permission for this file
std::unique_ptr<FileOpenResult> result = fuse->mp->OnFileOpen(
- build_path, io_path, ctx->uid, ctx->pid, for_write, !for_write /* redact */);
+ build_path, io_path, ctx->uid, ctx->pid, node->GetTransformsReason(), for_write,
+ !for_write /* redact */, true /* log_transforms_metrics */);
if (!result) {
fuse_reply_err(req, EFAULT);
return;
@@ -1258,7 +1262,7 @@ static void pf_read(fuse_req_t req, fuse_ino_t ino, size_t size, off_t off,
if (!node->IsTransformsComplete()) {
if (!fuse->mp->Transform(node->BuildPath(), node->GetIoPath(), node->GetTransforms(),
- h->uid)) {
+ node->GetTransformsReason(), h->uid)) {
fuse_reply_err(req, EFAULT);
return;
}
@@ -1633,7 +1637,8 @@ static void pf_access(fuse_req_t req, fuse_ino_t ino, int mask) {
}
std::unique_ptr<FileOpenResult> result = fuse->mp->OnFileOpen(
- path, path, req->ctx.uid, req->ctx.pid, for_write, false /* redact */);
+ path, path, req->ctx.uid, req->ctx.pid, node->GetTransformsReason(), for_write,
+ false /* redact */, false /* log_transforms_metrics */);
if (!result) {
status = EFAULT;
}
diff --git a/jni/MediaProviderWrapper.cpp b/jni/MediaProviderWrapper.cpp
index c0f84b7eb..c049ddbba 100644
--- a/jni/MediaProviderWrapper.cpp
+++ b/jni/MediaProviderWrapper.cpp
@@ -232,7 +232,7 @@ MediaProviderWrapper::MediaProviderWrapper(JNIEnv* env, jobject media_provider)
/*is_static*/ false);
mid_delete_file_ = CacheMethod(env, "deleteFile", "(Ljava/lang/String;I)I", /*is_static*/ false);
mid_on_file_open_ = CacheMethod(env, "onFileOpen",
- "(Ljava/lang/String;Ljava/lang/String;IIZZ)Lcom/android/"
+ "(Ljava/lang/String;Ljava/lang/String;IIIZZZ)Lcom/android/"
"providers/media/FileOpenResult;",
/*is_static*/ false);
mid_is_mkdir_or_rmdir_allowed_ = CacheMethod(env, "isDirectoryCreationOrDeletionAllowed",
@@ -253,7 +253,7 @@ MediaProviderWrapper::MediaProviderWrapper(JNIEnv* env, jobject media_provider)
/*is_static*/ false);
mid_is_app_clone_user_ = CacheMethod(env, "isAppCloneUser", "(I)Z",
/*is_static*/ false);
- mid_transform_ = CacheMethod(env, "transform", "(Ljava/lang/String;Ljava/lang/String;II)Z",
+ mid_transform_ = CacheMethod(env, "transform", "(Ljava/lang/String;Ljava/lang/String;III)Z",
/*is_static*/ false);
mid_file_lookup_ =
CacheMethod(env, "onFileLookup",
@@ -268,6 +268,8 @@ MediaProviderWrapper::MediaProviderWrapper(JNIEnv* env, jobject media_provider)
file_lookup_result_class_ =
reinterpret_cast<jclass>(env->NewGlobalRef(file_lookup_result_class_));
fid_file_lookup_transforms_ = CacheField(env, file_lookup_result_class_, "transforms", "I");
+ fid_file_lookup_transforms_reason_ =
+ CacheField(env, file_lookup_result_class_, "transformsReason", "I");
fid_file_lookup_uid_ = CacheField(env, file_lookup_result_class_, "uid", "I");
fid_file_lookup_transforms_complete_ =
CacheField(env, file_lookup_result_class_, "transformsComplete", "Z");
@@ -315,8 +317,9 @@ int MediaProviderWrapper::DeleteFile(const string& path, uid_t uid) {
std::unique_ptr<FileOpenResult> MediaProviderWrapper::OnFileOpen(const string& path,
const string& io_path, uid_t uid,
- pid_t tid, bool for_write,
- bool redact) {
+ pid_t tid, int transforms_reason,
+ bool for_write, bool redact,
+ bool log_transforms_metrics) {
JNIEnv* env = MaybeAttachCurrentThread();
if (shouldBypassMediaProvider(uid)) {
return std::make_unique<FileOpenResult>(0, uid, new RedactionInfo());
@@ -326,7 +329,8 @@ std::unique_ptr<FileOpenResult> MediaProviderWrapper::OnFileOpen(const string& p
ScopedLocalRef<jstring> j_io_path(env, env->NewStringUTF(io_path.c_str()));
ScopedLocalRef<jobject> j_res_file_open_object(
env, env->CallObjectMethod(media_provider_object_, mid_on_file_open_, j_path.get(),
- j_io_path.get(), uid, tid, for_write, redact));
+ j_io_path.get(), uid, tid, transforms_reason, for_write,
+ redact, log_transforms_metrics));
if (CheckForJniException(env)) {
return nullptr;
@@ -480,6 +484,8 @@ std::unique_ptr<FileLookupResult> MediaProviderWrapper::FileLookup(const std::st
}
int transforms = env->GetIntField(j_res_file_lookup_object.get(), fid_file_lookup_transforms_);
+ int transforms_reason =
+ env->GetIntField(j_res_file_lookup_object.get(), fid_file_lookup_transforms_reason_);
int original_uid = env->GetIntField(j_res_file_lookup_object.get(), fid_file_lookup_uid_);
bool transforms_complete = env->GetBooleanField(j_res_file_lookup_object.get(),
fid_file_lookup_transforms_complete_);
@@ -490,20 +496,20 @@ std::unique_ptr<FileLookupResult> MediaProviderWrapper::FileLookup(const std::st
(jstring)env->GetObjectField(j_res_file_lookup_object.get(), fid_file_lookup_io_path_));
ScopedUtfChars j_io_path_utf(env, j_io_path.get());
- std::unique_ptr<FileLookupResult> file_lookup_result =
- std::make_unique<FileLookupResult>(transforms, original_uid, transforms_complete,
- transforms_supported, string(j_io_path_utf.c_str()));
+ std::unique_ptr<FileLookupResult> file_lookup_result = std::make_unique<FileLookupResult>(
+ transforms, transforms_reason, original_uid, transforms_complete, transforms_supported,
+ string(j_io_path_utf.c_str()));
return file_lookup_result;
}
bool MediaProviderWrapper::Transform(const std::string& src, const std::string& dst, int transforms,
- uid_t uid) {
+ int transforms_reason, uid_t uid) {
JNIEnv* env = MaybeAttachCurrentThread();
ScopedLocalRef<jstring> j_src(env, env->NewStringUTF(src.c_str()));
ScopedLocalRef<jstring> j_dst(env, env->NewStringUTF(dst.c_str()));
bool res = env->CallBooleanMethod(media_provider_object_, mid_transform_, j_src.get(),
- j_dst.get(), transforms, uid);
+ j_dst.get(), transforms, transforms_reason, uid);
if (CheckForJniException(env)) {
return false;
diff --git a/jni/MediaProviderWrapper.h b/jni/MediaProviderWrapper.h
index b3248cb41..c33ee62ec 100644
--- a/jni/MediaProviderWrapper.h
+++ b/jni/MediaProviderWrapper.h
@@ -51,9 +51,10 @@ struct FileOpenResult {
* status and the ioPath. Provided by MediaProvider.java via a JNI call.
*/
struct FileLookupResult {
- FileLookupResult(int transforms, uid_t uid, bool transforms_complete, bool transforms_supported,
- const std::string& io_path)
+ FileLookupResult(int transforms, int transforms_reason, uid_t uid, bool transforms_complete,
+ bool transforms_supported, const std::string& io_path)
: transforms(transforms),
+ transforms_reason(transforms_reason),
uid(uid),
transforms_complete(transforms_complete),
transforms_supported(transforms_supported),
@@ -68,6 +69,7 @@ struct FileLookupResult {
* via a JNI call.
*/
const int transforms;
+ const int transforms_reason;
const uid_t uid;
const bool transforms_complete;
const bool transforms_supported;
@@ -146,7 +148,9 @@ class MediaProviderWrapper final {
* @return FileOpenResult containing status, uid and redaction_info
*/
std::unique_ptr<FileOpenResult> OnFileOpen(const std::string& path, const std::string& io_path,
- uid_t uid, pid_t tid, bool for_write, bool redact);
+ uid_t uid, pid_t tid, int transforms_reason,
+ bool for_write, bool redact,
+ bool log_transforms_metrics);
/**
* Determines if the given UID is allowed to create a directory with the given path.
@@ -216,7 +220,8 @@ class MediaProviderWrapper final {
std::unique_ptr<FileLookupResult> FileLookup(const std::string& path, uid_t uid, pid_t tid);
/** Transforms from src to dst file */
- bool Transform(const std::string& src, const std::string& dst, int transforms, uid_t uid);
+ bool Transform(const std::string& src, const std::string& dst, int transforms,
+ int transforms_reason, uid_t uid);
/**
* Determines if to allow FUSE_LOOKUP for uid. Might allow uids that don't belong to the
@@ -264,6 +269,7 @@ class MediaProviderWrapper final {
jmethodID mid_file_lookup_;
/** Cached FileLookupResult field IDs **/
jfieldID fid_file_lookup_transforms_;
+ jfieldID fid_file_lookup_transforms_reason_;
jfieldID fid_file_lookup_uid_;
jfieldID fid_file_lookup_transforms_complete_;
jfieldID fid_file_lookup_transforms_supported_;
diff --git a/jni/node-inl.h b/jni/node-inl.h
index 2b577bf4a..99960ed30 100644
--- a/jni/node-inl.h
+++ b/jni/node-inl.h
@@ -120,13 +120,14 @@ class node {
// Creates a new node with the specified parent, name and lock.
static node* Create(node* parent, const std::string& name, const std::string& io_path,
bool should_invalidate, bool transforms_complete, const int transforms,
- std::recursive_mutex* lock, NodeTracker* tracker) {
+ const int transforms_reason, std::recursive_mutex* lock,
+ NodeTracker* tracker) {
// Place the entire constructor under a critical section to make sure
// node creation, tracking (if enabled) and the addition to a parent are
// atomic.
std::lock_guard<std::recursive_mutex> guard(*lock);
return new node(parent, name, io_path, should_invalidate, transforms_complete, transforms,
- lock, tracker);
+ transforms_reason, lock, tracker);
}
// Creates a new root node. Root nodes have no parents by definition
@@ -135,7 +136,8 @@ class node {
NodeTracker* tracker) {
std::lock_guard<std::recursive_mutex> guard(*lock);
node* root = new node(nullptr, path, path, false /* should_invalidate */,
- true /* transforms_complete */, 0, lock, tracker);
+ true /* transforms_complete */, 0 /* transforms */,
+ 0 /* transforms_reason */, lock, tracker);
// The root always has one extra reference to avoid it being
// accidentally collected.
@@ -269,6 +271,8 @@ class node {
int GetTransforms() const { return transforms_; }
+ int GetTransformsReason() const { return transforms_reason_; }
+
bool IsTransformsComplete() const {
return transforms_complete_.load(std::memory_order_acquire);
}
@@ -350,11 +354,12 @@ class node {
private:
node(node* parent, const std::string& name, const std::string& io_path,
const bool should_invalidate, const bool transforms_complete, const int transforms,
- std::recursive_mutex* lock, NodeTracker* tracker)
+ const int transforms_reason, std::recursive_mutex* lock, NodeTracker* tracker)
: name_(name),
io_path_(io_path),
transforms_complete_(transforms_complete),
transforms_(transforms),
+ transforms_reason_(transforms_reason),
refcount_(0),
parent_(nullptr),
has_redacted_cache_(false),
@@ -490,10 +495,15 @@ class node {
// Whether any transforms required on |io_path_| are complete.
// If false, might need to call a node transform function with |transforms| below
std::atomic_bool transforms_complete_;
- // Opaque flags that determine the 'supported' and 'required' transforms to perform on node
- // before IO. These flags should not be interpreted in native but should be passed as part
- // of a transform function and if successful, |transforms_complete_| should be set to true
+ // Opaque flags that determines the 'required' transforms to perform on node
+ // before IO. These flags should not be interpreted in native but should be passed to the
+ // MediaProvider as part of a transform function and if successful, |transforms_complete_|
+ // should be set to true
const int transforms_;
+ // Opaque value indicating the reason why transforms are required.
+ // This value should not be interpreted in native but should be passed to the MediaProvider
+ // as part of a transform function
+ const int transforms_reason_;
// The reference count for this node. Guarded by |lock_|.
uint32_t refcount_;
// Set of children of this node. All of them contain a back reference
diff --git a/jni/node_test.cpp b/jni/node_test.cpp
index 6496f8084..ddb15ceb6 100644
--- a/jni/node_test.cpp
+++ b/jni/node_test.cpp
@@ -33,7 +33,7 @@ class NodeTest : public ::testing::Test {
unique_node_ptr CreateNode(node* parent, const std::string& path, const int transforms = 0) {
return unique_node_ptr(
- node::Create(parent, path, "", true, true, transforms, &lock_, &tracker_),
+ node::Create(parent, path, "", true, true, transforms, 0, &lock_, &tracker_),
&NodeTest::destroy);
}
@@ -68,7 +68,7 @@ TEST_F(NodeTest, TestCreate_withParent) {
}
TEST_F(NodeTest, TestRelease) {
- node* node = node::Create(nullptr, "/path", "", false, true, 0, &lock_, &tracker_);
+ node* node = node::Create(nullptr, "/path", "", false, true, 0, 0, &lock_, &tracker_);
acquire(node);
acquire(node);
ASSERT_EQ(3, GetRefCount(node));
@@ -278,10 +278,10 @@ TEST_F(NodeTest, DeleteTree) {
unique_node_ptr parent = CreateNode(nullptr, "/path");
// This is the tree that we intend to delete.
- node* child = node::Create(parent.get(), "subdir", "", false, true, 0, &lock_, &tracker_);
- node::Create(child, "s1", "", false, true, 0, &lock_, &tracker_);
- node* subchild2 = node::Create(child, "s2", "", false, true, 0, &lock_, &tracker_);
- node::Create(subchild2, "sc2", "", false, true, 0, &lock_, &tracker_);
+ node* child = node::Create(parent.get(), "subdir", "", false, true, 0, 0, &lock_, &tracker_);
+ node::Create(child, "s1", "", false, true, 0, 0, &lock_, &tracker_);
+ node* subchild2 = node::Create(child, "s2", "", false, true, 0, 0, &lock_, &tracker_);
+ node::Create(subchild2, "sc2", "", false, true, 0, 0, &lock_, &tracker_);
ASSERT_EQ(child, parent->LookupChildByName("subdir", false /* acquire */));
node::DeleteTree(child);
diff --git a/src/com/android/providers/media/FileLookupResult.java b/src/com/android/providers/media/FileLookupResult.java
index 4dc4d034c..f0e6bfbe9 100644
--- a/src/com/android/providers/media/FileLookupResult.java
+++ b/src/com/android/providers/media/FileLookupResult.java
@@ -22,14 +22,16 @@ package com.android.providers.media;
*/
public final class FileLookupResult {
public final int transforms;
+ public final int transformsReason;
public final int uid;
public final boolean transformsComplete;
public final boolean transformsSupported;
public final String ioPath;
- public FileLookupResult(int transforms, int uid, boolean transformsComplete,
- boolean transformsSupported, String ioPath) {
+ public FileLookupResult(int transforms, int transformsReason, int uid,
+ boolean transformsComplete, boolean transformsSupported, String ioPath) {
this.transforms = transforms;
+ this.transformsReason = transformsReason;
this.uid = uid;
this.transformsComplete = transformsComplete;
this.transformsSupported = transformsSupported;
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 8ff0c2a6d..d13868d6d 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -1363,13 +1363,14 @@ public class MediaProvider extends ContentProvider {
* Called from JNI in jni/MediaProviderWrapper.cpp
*/
@Keep
- public boolean transformForFuse(String src, String dst, int transforms, int uid) {
+ public boolean transformForFuse(String src, String dst, int transforms, int transformsReason,
+ int uid) {
if ((transforms & FLAG_TRANSFORM_TRANSCODING) != 0) {
if (mTranscodeHelper.isTranscodeFileCached(uid, src, dst)) {
Log.d(TAG, "Using transcode cache for " + src);
return true;
}
- return mTranscodeHelper.transcode(src, dst, uid);
+ return mTranscodeHelper.transcode(src, dst, uid, transformsReason);
}
return true;
}
@@ -1393,30 +1394,30 @@ public class MediaProvider extends ContentProvider {
boolean transformsComplete = true;
boolean transformsSupported = mTranscodeHelper.supportsTranscode(path);
int transforms = 0;
+ int transformsReason = 0;
if (transformsSupported) {
- boolean shouldTranscode = false;
PendingOpenInfo info = null;
synchronized (mPendingOpenInfo) {
info = mPendingOpenInfo.get(tid);
}
if (info != null && info.uid == uid) {
- shouldTranscode = info.shouldTranscode;
+ transformsReason = info.transcodeReason;
} else {
- shouldTranscode = mTranscodeHelper.shouldTranscode(path, uid,
+ transformsReason = mTranscodeHelper.shouldTranscode(path, uid,
null /* bundle */);
}
- if (shouldTranscode) {
+ if (transformsReason > 0) {
ioPath = mTranscodeHelper.getIoPath(path, uid);
transformsComplete = false;
transforms = FLAG_TRANSFORM_TRANSCODING;
}
}
- return new FileLookupResult(transforms, uid, transformsComplete, transformsSupported,
- ioPath);
+ return new FileLookupResult(transforms, transformsReason, uid, transformsComplete,
+ transformsSupported, ioPath);
}
public int getBinderUidForFuse(int uid, int tid) {
@@ -5977,6 +5978,18 @@ public class MediaProvider extends ContentProvider {
});
}
+ private void notifyTranscodeHelperOnFileOpen(String path, String ioPath, int uid,
+ int transformsReason) {
+ BackgroundThread.getExecutor().execute(() -> {
+ final LocalCallingIdentity token = clearLocalCallingIdentity();
+ try {
+ mTranscodeHelper.onFileOpen(path, ioPath, uid, transformsReason);
+ } finally {
+ restoreLocalCallingIdentity(token);
+ }
+ });
+ }
+
/**
* Update row(s) that match {@code userWhere} in MediaProvider database with {@code values}.
* Treats update as replace for updates with conflicts.
@@ -6648,13 +6661,14 @@ public class MediaProvider extends ContentProvider {
}
private ParcelFileDescriptor openWithFuse(String filePath, int uid, int modeBits,
- boolean shouldRedact, boolean shouldTranscode) throws FileNotFoundException {
+ boolean shouldRedact, boolean shouldTranscode, int transcodeReason)
+ throws FileNotFoundException {
Log.d(TAG, "Open with FUSE. FilePath: " + filePath + ". Uid: " + uid
+ ". ShouldRedact: " + shouldRedact + ". ShouldTranscode: " + shouldTranscode);
int tid = android.os.Process.myTid();
synchronized (mPendingOpenInfo) {
- mPendingOpenInfo.put(tid, new PendingOpenInfo(uid, shouldRedact, shouldTranscode));
+ mPendingOpenInfo.put(tid, new PendingOpenInfo(uid, shouldRedact, transcodeReason));
}
try {
@@ -6795,15 +6809,16 @@ public class MediaProvider extends ContentProvider {
final ParcelFileDescriptor pfd;
final String filePath = file.getPath();
final int uid = Binder.getCallingUid();
- boolean shouldTranscode = mTranscodeHelper.shouldTranscode(filePath, uid, opts);
+ int transcodeReason = mTranscodeHelper.shouldTranscode(filePath, uid, opts);
+ boolean shouldTranscode = transcodeReason > 0;
if (redactionInfo.redactionRanges.length > 0) {
// If fuse is enabled, we can provide an fd that points to the fuse
// file system and handle redaction in the fuse handler when the caller reads.
pfd = openWithFuse(filePath, uid, modeBits, true /* shouldRedact */,
- shouldTranscode);
+ shouldTranscode, transcodeReason);
} else if (shouldTranscode) {
pfd = openWithFuse(filePath, uid, modeBits, false /* shouldRedact */,
- shouldTranscode);
+ shouldTranscode, transcodeReason);
} else {
FuseDaemon daemon = null;
try {
@@ -6822,7 +6837,7 @@ public class MediaProvider extends ContentProvider {
// resulting from cache inconsistencies between the upper and lower
// filesystem caches
pfd = openWithFuse(filePath, uid, modeBits, false /* shouldRedact */,
- shouldTranscode);
+ shouldTranscode, transcodeReason);
try {
lowerFsFd.close();
} catch (IOException e) {
@@ -7091,11 +7106,11 @@ public class MediaProvider extends ContentProvider {
private static final class PendingOpenInfo {
public final int uid;
public final boolean shouldRedact;
- public final boolean shouldTranscode;
- public PendingOpenInfo(int uid, boolean shouldRedact, boolean shouldTranscode) {
+ public final int transcodeReason;
+ public PendingOpenInfo(int uid, boolean shouldRedact, int transcodeReason) {
this.uid = uid;
this.shouldRedact = shouldRedact;
- this.shouldTranscode = shouldTranscode;
+ this.transcodeReason = transcodeReason;
}
}
@@ -7256,20 +7271,21 @@ public class MediaProvider extends ContentProvider {
* @param path the path of the file to be opened
* @param uid UID of the app requesting to open the file
* @param forWrite specifies if the file is to be opened for write
- * @return 0 upon success. {@link OsConstants#EACCES} if the operation is illegal or not
- * permitted for the given {@code uid} or if the calling package is a legacy app that doesn't
- * have right storage permission.
+ * @return {@link FileOpenResult} with {@code status} {@code 0} upon success and
+ * {@link FileOpenResult} with {@code status} {@link OsConstants#EACCES} if the operation is
+ * illegal or not permitted for the given {@code uid} or if the calling package is a legacy app
+ * that doesn't have right storage permission.
*
* Called from JNI in jni/MediaProviderWrapper.cpp
*/
@Keep
public FileOpenResult onFileOpenForFuse(String path, String ioPath, int uid, int tid,
- boolean forWrite, boolean redact) {
+ int transformsReason, boolean forWrite, boolean redact, boolean logTransformsMetrics) {
int original_uid = getBinderUidForFuse(uid, tid);
final LocalCallingIdentity token =
clearLocalCallingIdentity(getCachedCallingIdentityForFuse(uid));
-
+ boolean isSuccess = false;
try {
if (isPrivatePackagePathNotAccessibleByCaller(path)) {
Log.e(TAG, "Can't open a file in another app's external directory!");
@@ -7277,6 +7293,7 @@ public class MediaProvider extends ContentProvider {
}
if (shouldBypassFuseRestrictions(forWrite, path)) {
+ isSuccess = true;
return new FileOpenResult(0 /* status */, uid,
redact ? getRedactionRangesForFuse(path, ioPath, original_uid, uid, tid) :
new long[0]);
@@ -7331,6 +7348,7 @@ public class MediaProvider extends ContentProvider {
throw e;
}
}
+ isSuccess = true;
return new FileOpenResult(0 /* status */, uid,
redact ? getRedactionRangesForFuse(path, ioPath, original_uid, uid, tid) :
new long[0]);
@@ -7345,6 +7363,9 @@ public class MediaProvider extends ContentProvider {
Log.e(TAG, "Permission to access file: " + path + " is denied");
return new FileOpenResult(OsConstants.EACCES /* status */, uid, new long[0]);
} finally {
+ if (isSuccess && logTransformsMetrics) {
+ notifyTranscodeHelperOnFileOpen(path, ioPath, original_uid, transformsReason);
+ }
restoreLocalCallingIdentity(token);
}
}
diff --git a/src/com/android/providers/media/TranscodeHelper.java b/src/com/android/providers/media/TranscodeHelper.java
index c8ff43f0e..e789032d4 100644
--- a/src/com/android/providers/media/TranscodeHelper.java
+++ b/src/com/android/providers/media/TranscodeHelper.java
@@ -60,6 +60,7 @@ import android.os.SystemProperties;
import android.os.UserHandle;
import android.provider.MediaStore;
import android.provider.MediaStore.Files.FileColumns;
+import android.provider.MediaStore.MediaColumns;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;
@@ -72,6 +73,7 @@ import androidx.annotation.Nullable;
import androidx.core.app.NotificationCompat;
import androidx.core.app.NotificationManagerCompat;
+import com.android.providers.media.util.BackgroundThread;
import com.android.providers.media.util.FileUtils;
import com.android.providers.media.util.ForegroundThread;
import com.android.providers.media.util.SQLiteQueryBuilder;
@@ -263,26 +265,29 @@ public class TranscodeHelper {
return name;
}
- private void reportTranscodingResult(int uid, boolean success, long durationMillis) {
- if (!isTranscodeEnabled()) {
- return;
- }
-
- MediaProviderStatsLog.write(
- TRANSCODING_DATA,
- getNameForUid(uid),
- MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_TYPE__READ_TRANSCODE,
- -1, // file size
- success ? TRANSCODING_DATA__TRANSCODE_RESULT__SUCCESS :
- TRANSCODING_DATA__TRANSCODE_RESULT__FAIL,
- durationMillis,
- -1, // file_duration_millis
- -1, // file_framerate_fps
- -1 // access_reason
- );
+ private void reportTranscodingResult(int uid, boolean success, long transcodingDurationMs,
+ int transcodingReason, String src, String dst) {
+ BackgroundThread.getExecutor().execute(() -> {
+ try (Cursor c = queryFileForTranscode(src,
+ new String[] {MediaColumns.DURATION, MediaColumns.CAPTURE_FRAMERATE})) {
+ if (c != null && c.moveToNext()) {
+ MediaProviderStatsLog.write(
+ TRANSCODING_DATA,
+ getNameForUid(uid),
+ MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_TYPE__READ_TRANSCODE,
+ success ? new File(dst).length() : -1,
+ success ? TRANSCODING_DATA__TRANSCODE_RESULT__SUCCESS :
+ TRANSCODING_DATA__TRANSCODE_RESULT__FAIL,
+ transcodingDurationMs,
+ c.getLong(0) /* video_duration */,
+ c.getLong(1) /* capture_framerate */,
+ transcodingReason);
+ }
+ }
+ });
}
- public boolean transcode(String src, String dst, int uid) {
+ public boolean transcode(String src, String dst, int uid, int reason) {
StorageTranscodingSession storageSession = null;
TranscodingSession transcodingSession = null;
CountDownLatch latch = null;
@@ -324,7 +329,8 @@ public class TranscodeHelper {
finishTranscodingResult(uid, src, transcodingSession, latch);
}
} finally {
- reportTranscodingResult(uid, result, SystemClock.elapsedRealtime() - startTime);
+ reportTranscodingResult(uid, result, SystemClock.elapsedRealtime() - startTime, reason,
+ src, dst);
}
return result;
}
@@ -375,32 +381,27 @@ public class TranscodeHelper {
return transcodePath;
}
- private void reportTranscodingDirectAccess(int uid) {
- if (!isTranscodeEnabled()) {
- return;
- }
-
- MediaProviderStatsLog.write(
- TRANSCODING_DATA,
- getNameForUid(uid),
- MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_TYPE__READ_DIRECT,
- -1, // file size
- TRANSCODING_DATA__TRANSCODE_RESULT__UNDEFINED,
- -1, // transcoding duration
- -1, // file_duration_millis
- -1, // file_framerate_fps
- -1 // access_reason
- );
- }
-
// TODO(b/173491972): Generalize to consider other file/app media capabilities beyond hevc
- public boolean shouldTranscode(String path, int uid, Bundle bundle) {
+ /**
+ * @return 0 or >0 representing whether we should transcode or not.
+ * 0 means we should not transcode, otherwise we should transcode and the value is the
+ * reason that will be logged to westworld as a transcode reason. Possible values are:
+ * <ul>
+ * <li>MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__SYSTEM_DEFAULT=1
+ * <li>MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__SYSTEM_CONFIG=2
+ * <li>MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__APP_MANIFEST=3
+ * <li>MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__APP_COMPAT=4
+ * <li>MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__APP_EXTRA=5
+ * </ul>
+ *
+ */
+ public int shouldTranscode(String path, int uid, Bundle bundle) {
boolean isTranscodeEnabled = isTranscodeEnabled();
updateConfigs(isTranscodeEnabled);
if (!isTranscodeEnabled) {
logVerbose("Transcode not enabled");
- return false;
+ return 0;
}
logVerbose("Checking shouldTranscode for: " + path + ". Uid: " + uid);
@@ -412,7 +413,7 @@ public class TranscodeHelper {
// 2. Uid is from native process on device
// 3. Uid is ourselves, which can happen when we are opening a file via FUSE for
// redaction on behalf of another app via ContentResolver
- return false;
+ return 0;
}
// Transcode only if file needs transcoding
@@ -420,33 +421,32 @@ public class TranscodeHelper {
if (fileFlags == 0) {
// Nothing to transcode
- return false;
+ return 0;
}
- boolean transcodeNeeded = doesAppNeedTranscoding(uid, bundle, fileFlags);
- if (!transcodeNeeded) {
- reportTranscodingDirectAccess(uid);
- }
- return transcodeNeeded;
+ return doesAppNeedTranscoding(uid, bundle, fileFlags);
}
- private boolean doesAppNeedTranscoding(int uid, Bundle bundle, int fileFlags) {
+ private int doesAppNeedTranscoding(int uid, Bundle bundle, int fileFlags) {
// Check explicit Bundle provided
if (bundle != null) {
if (bundle.getBoolean(MediaStore.EXTRA_ACCEPT_ORIGINAL_MEDIA_FORMAT, false)) {
logVerbose("Original format requested");
- return false;
+ return 0;
}
ApplicationMediaCapabilities capabilities =
bundle.getParcelable(MediaStore.EXTRA_MEDIA_CAPABILITIES);
if (capabilities != null) {
- Optional<Boolean> result = checkAppMediaSupport(
+ Optional<Boolean> appExtraResult = checkAppMediaSupport(
capabilitiesToSupportedFlags(capabilities),
capabilitiesToUnsupportedFlags(capabilities), fileFlags,
- "app_media_capabilities");
- if (result.isPresent()) {
- return result.get();
+ "app_extra");
+ if (appExtraResult.isPresent()) {
+ if (appExtraResult.get()) {
+ return MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__APP_EXTRA;
+ }
+ return 0;
}
// Bundle didn't have enough information to make decision, continue
}
@@ -455,7 +455,10 @@ public class TranscodeHelper {
// Check app compat support
Optional<Boolean> appCompatResult = checkAppCompatSupport(uid, fileFlags);
if (appCompatResult.isPresent()) {
- return appCompatResult.get();
+ if (appCompatResult.get()) {
+ return MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__APP_COMPAT;
+ }
+ return 0;
}
// App compat didn't have enough information to make decision, continue
@@ -466,10 +469,13 @@ public class TranscodeHelper {
// Check app manifest support
for (String callingPackage : callingPackages) {
- Optional<Boolean> manifestResult = checkManifestSupport(callingPackage, identity,
+ Optional<Boolean> appManifestResult = checkManifestSupport(callingPackage, identity,
fileFlags);
- if (manifestResult.isPresent()) {
- return manifestResult.get();
+ if (appManifestResult.isPresent()) {
+ if (appManifestResult.get()) {
+ return MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__APP_MANIFEST;
+ }
+ return 0;
}
// App manifest didn't have enough information to make decision, continue
@@ -482,10 +488,13 @@ public class TranscodeHelper {
int supportedFlags = configCompatFlags;
int unsupportedFlags = ~configCompatFlags & MEDIA_FORMAT_FLAG_MASK;
- Optional<Boolean> configCompatResult = checkAppMediaSupport(supportedFlags,
- unsupportedFlags, fileFlags, "config_compat_manifest");
- if (configCompatResult.isPresent()) {
- return configCompatResult.get();
+ Optional<Boolean> systemConfigResult = checkAppMediaSupport(supportedFlags,
+ unsupportedFlags, fileFlags, "system_config");
+ if (systemConfigResult.isPresent()) {
+ if (systemConfigResult.get()) {
+ return MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__SYSTEM_CONFIG;
+ }
+ return 0;
}
// Should never get here because the supported & unsupported flags should span
// the entire universe of file flags
@@ -496,10 +505,10 @@ public class TranscodeHelper {
// TODO: Need to add transcode_default as flags
if (shouldTranscodeDefault()) {
logVerbose("Default behavior should transcode");
- return true;
+ return MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_REASON__SYSTEM_DEFAULT;
} else {
logVerbose("Default behavior should not transcode");
- return false;
+ return 0;
}
}
@@ -563,7 +572,7 @@ public class TranscodeHelper {
return 0;
}
- if (MediaFormat.MIMETYPE_VIDEO_HEVC.equalsIgnoreCase(cursor.getString(0))) {
+ if (isHevc(cursor.getString(0))) {
return FLAG_HEVC;
} else {
logVerbose("File is not HEVC");
@@ -572,6 +581,10 @@ public class TranscodeHelper {
}
}
+ private boolean isHevc(String mimeType) {
+ return MediaFormat.MIMETYPE_VIDEO_HEVC.equalsIgnoreCase(mimeType);
+ }
+
public boolean supportsTranscode(String path) {
File file = new File(path);
String name = file.getName();
@@ -706,43 +719,85 @@ public class TranscodeHelper {
FileColumns._VIDEO_CODEC_TYPE,
FileColumns.SIZE,
FileColumns.OWNER_PACKAGE_NAME,
- FileColumns.DATA},
+ FileColumns.DATA,
+ MediaColumns.DURATION,
+ MediaColumns.CAPTURE_FRAMERATE
+ },
null, null, null)) {
- if (supportsTranscode(c.getString(3)) &&
- MediaFormat.MIMETYPE_VIDEO_HEVC.equalsIgnoreCase(c.getString(0))) {
- MediaProviderStatsLog.write(
- TRANSCODING_DATA,
- c.getString(2),
- MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_TYPE__HEVC_WRITE,
- c.getLong(1),
- TRANSCODING_DATA__TRANSCODE_RESULT__UNDEFINED,
- -1, // transcoding duration
- -1, // file_duration_millis
- -1, // file_framerate_fps
- -1 // access_reason
- );
+ if (supportsTranscode(c.getString(3))) {
+ if (isHevc(c.getString(0))) {
+ MediaProviderStatsLog.write(
+ TRANSCODING_DATA,
+ c.getString(2) /* owner_package_name */,
+ MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_TYPE__HEVC_WRITE,
+ c.getLong(1) /* file size */,
+ TRANSCODING_DATA__TRANSCODE_RESULT__UNDEFINED,
+ -1 /* transcoding_duration */,
+ c.getLong(4) /* video_duration */,
+ c.getLong(5) /* capture_framerate */,
+ -1 /* transcode_reason */);
+
+ } else {
+ MediaProviderStatsLog.write(
+ TRANSCODING_DATA,
+ c.getString(2) /* owner_package_name */,
+ MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_TYPE__AVC_WRITE,
+ c.getLong(1) /* file size */,
+ TRANSCODING_DATA__TRANSCODE_RESULT__UNDEFINED,
+ -1 /* transcoding_duration */,
+ c.getLong(4) /* video_duration */,
+ c.getLong(5) /* capture_framerate */,
+ -1 /* transcode_reason */);
+ }
}
} catch (Exception e) {
Log.w(TAG, "Couldn't get cursor for scanned file", e);
}
}
- private void reportTranscodingCachedAccess(int uid) {
+ void onFileOpen(String path, String ioPath, int uid, int transformsReason) {
if (!isTranscodeEnabled()) {
return;
}
- MediaProviderStatsLog.write(
- TRANSCODING_DATA,
- getNameForUid(uid),
- MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_TYPE__READ_CACHE,
- -1, // file size
- TRANSCODING_DATA__TRANSCODE_RESULT__UNDEFINED,
- -1, // transcoding duration
- -1, // file_duration_millis
- -1, // file_framerate_fps
- -1 // access_reason
- );
+ String[] resolverInfoProjection = new String[] {
+ FileColumns._VIDEO_CODEC_TYPE,
+ FileColumns.SIZE,
+ MediaColumns.DURATION,
+ MediaColumns.CAPTURE_FRAMERATE
+ };
+
+ try (Cursor c = queryFileForTranscode(path, resolverInfoProjection)) {
+ if (c != null && c.moveToNext()) {
+ if (isHevc(c.getString(0)) && supportsTranscode(path)) {
+ if (transformsReason == 0) {
+ MediaProviderStatsLog.write(
+ TRANSCODING_DATA,
+ getNameForUid(uid) /* owner_package_name */,
+ MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_TYPE__READ_DIRECT,
+ c.getLong(1) /* file size */,
+ TRANSCODING_DATA__TRANSCODE_RESULT__UNDEFINED,
+ -1 /* transcoding_duration */,
+ c.getLong(2) /* video_duration */,
+ c.getLong(3) /* capture_framerate */,
+ -1 /* transcode_reason */);
+ } else if (isTranscodeFileCached(uid, path, ioPath)) {
+ MediaProviderStatsLog.write(
+ TRANSCODING_DATA,
+ getNameForUid(uid) /* owner_package_name */,
+ MediaProviderStatsLog.TRANSCODING_DATA__ACCESS_TYPE__READ_CACHE,
+ c.getLong(1) /* file size */,
+ TRANSCODING_DATA__TRANSCODE_RESULT__UNDEFINED,
+ -1 /* transcoding_duration */,
+ c.getLong(2) /* video_duration */,
+ c.getLong(3) /* capture_framerate */,
+ transformsReason /* transcode_reason */);
+ } // else if file is not in cache, we'll log at read(2) when we transcode
+ }
+ }
+ } catch (IllegalStateException e) {
+ Log.w(TAG, "Unable to log metrics on file open", e);
+ }
}
public boolean isTranscodeFileCached(int uid, String path, String transcodePath) {
@@ -760,7 +815,6 @@ public class TranscodeHelper {
new File(transcodePath).exists();
if (result) {
logEvent("Transcode cache hit: " + path, null /* session */);
- reportTranscodingCachedAccess(uid);
}
return result;
}
diff --git a/tests/src/com/android/providers/media/MediaProviderForFuseTest.java b/tests/src/com/android/providers/media/MediaProviderForFuseTest.java
index a6c028809..afc57be6d 100644
--- a/tests/src/com/android/providers/media/MediaProviderForFuseTest.java
+++ b/tests/src/com/android/providers/media/MediaProviderForFuseTest.java
@@ -96,8 +96,8 @@ public class MediaProviderForFuseTest {
// We can write our file
FileOpenResult result = sMediaProvider.onFileOpenForFuse(
- file.getPath(), file.getPath(), sTestUid, 0 /* tid */, true /* forWrite */,
- false /* redact */);
+ file.getPath(), file.getPath(), sTestUid, 0 /* tid */, 0 /* transforms_reason */,
+ true /* forWrite */, false /* redact */, false /* transcode_metrics */);
Truth.assertThat(result.status).isEqualTo(0);
Truth.assertThat(result.redactionRanges).isEqualTo(new long[0]);