Merge "Improve logging of child commands." into nyc-dev
diff --git a/cmds/atrace/atrace.cpp b/cmds/atrace/atrace.cpp
index 87f637c..23954ea 100644
--- a/cmds/atrace/atrace.cpp
+++ b/cmds/atrace/atrace.cpp
@@ -42,6 +42,8 @@
using namespace android;
+#define LOG_TAG "atrace"
+
#define NELEM(x) ((int) (sizeof(x) / sizeof((x)[0])))
enum { MAX_SYS_FILES = 10 };
@@ -769,6 +771,7 @@
// Read the current kernel trace and write it to stdout.
static void dumpTrace()
{
+ ALOGE("Dumping trace");
int traceFD = open(k_tracePath, O_RDWR);
if (traceFD == -1) {
fprintf(stderr, "error opening %s: %s (%d)\n", k_tracePath,
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index d7e5f17..5898b41 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -606,7 +606,7 @@
run_command("LIBRANK", 10, SU_PATH, "root", "librank", NULL);
run_command("PRINTENV", 10, "printenv", NULL);
- run_command("NETSTAT", 10, "netstat", NULL);
+ run_command("NETSTAT", 10, "netstat", "-n", NULL);
run_command("LSMOD", 10, "lsmod", NULL);
do_dmesg();
@@ -1170,8 +1170,8 @@
log_path.c_str(), tmp_path.c_str(), screenshot_path.c_str());
if (do_zip_file) {
- MYLOGD("Creating initial .zip file\n");
path = bugreport_dir + "/" + base_name + "-" + suffix + ".zip";
+ MYLOGD("Creating initial .zip file (%s)\n", path.c_str());
create_parent_dirs(path.c_str());
zip_file.reset(fopen(path.c_str(), "wb"));
if (!zip_file) {
diff --git a/cmds/installd/Android.mk b/cmds/installd/Android.mk
index 65bcf39..d35099a 100644
--- a/cmds/installd/Android.mk
+++ b/cmds/installd/Android.mk
@@ -80,6 +80,36 @@
LOCAL_CLANG := true
include $(BUILD_EXECUTABLE)
+# OTA chroot tool
+
+include $(CLEAR_VARS)
+LOCAL_MODULE := otapreopt_chroot
+LOCAL_MODULE_TAGS := optional
+LOCAL_CFLAGS := $(common_cflags)
+
+LOCAL_SRC_FILES := otapreopt_chroot.cpp
+LOCAL_SHARED_LIBRARIES := \
+ libbase \
+ liblog \
+
+LOCAL_ADDITIONAL_DEPENDENCIES += $(LOCAL_PATH)/Android.mk
+LOCAL_CLANG := true
+include $(BUILD_EXECUTABLE)
+
+# OTA postinstall script
+
+include $(CLEAR_VARS)
+LOCAL_MODULE:= otapreopt_script
+LOCAL_MODULE_TAGS := optional
+LOCAL_MODULE_CLASS := EXECUTABLES
+LOCAL_SRC_FILES := otapreopt_script.sh
+
+# Let this depend on otapreopt and the chroot tool, so we just have to mention one in a
+# configuration.
+LOCAL_REQUIRED_MODULES := otapreopt otapreopt_chroot
+
+include $(BUILD_PREBUILT)
+
# Tests.
include $(LOCAL_PATH)/tests/Android.mk
\ No newline at end of file
diff --git a/cmds/installd/commands.cpp b/cmds/installd/commands.cpp
index ac2ee30..77bcfc2 100644
--- a/cmds/installd/commands.cpp
+++ b/cmds/installd/commands.cpp
@@ -826,7 +826,9 @@
strcpy(dex2oat_compiler_filter_arg, "--compiler-filter=interpret-only");
have_dex2oat_compiler_filter_flag = true;
} else if (extract_only) {
- strcpy(dex2oat_compiler_filter_arg, "--compiler-filter=verify-at-runtime");
+ // Temporarily make extract-only mean interpret-only, so extracted files will be verified.
+ // b/26833007
+ strcpy(dex2oat_compiler_filter_arg, "--compiler-filter=interpret-only");
have_dex2oat_compiler_filter_flag = true;
} else if (have_dex2oat_compiler_filter_flag) {
sprintf(dex2oat_compiler_filter_arg, "--compiler-filter=%s", dex2oat_compiler_filter_flag);
@@ -1852,7 +1854,8 @@
{
struct stat s;
if (stat(b_path.c_str(), &s) != 0) {
- LOG(ERROR) << "Can't find A/B artifact at " << b_path;
+ // Silently ignore for now. The service calling this isn't smart enough to understand
+ // lack of artifacts at the moment.
return -1;
}
if (!S_ISREG(s.st_mode)) {
diff --git a/cmds/installd/file_parsing.h b/cmds/installd/file_parsing.h
new file mode 100644
index 0000000..3e2f815
--- /dev/null
+++ b/cmds/installd/file_parsing.h
@@ -0,0 +1,60 @@
+/*
+ * 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 OTAPREOPT_FILE_PARSING_H_
+#define OTAPREOPT_FILE_PARSING_H_
+
+#include <fstream>
+#include <functional>
+#include <string>
+
+namespace android {
+namespace installd {
+
+bool ParseFile(const std::string& strFile, std::function<bool (const std::string&)> parse) {
+ std::ifstream input_stream(strFile);
+
+ if (!input_stream.is_open()) {
+ return false;
+ }
+
+ while (!input_stream.eof()) {
+ // Read the next line.
+ std::string line;
+ getline(input_stream, line);
+
+ // Is the line empty? Simplifies the next check.
+ if (line.empty()) {
+ continue;
+ }
+
+ // Is this a comment (starts with pound)?
+ if (line[0] == '#') {
+ continue;
+ }
+
+ if (!parse(line)) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
+} // namespace installd
+} // namespace android
+
+#endif // OTAPREOPT_FILE_PARSING_H_
diff --git a/cmds/installd/installd.cpp b/cmds/installd/installd.cpp
index 7ca7527..2bf27a2 100644
--- a/cmds/installd/installd.cpp
+++ b/cmds/installd/installd.cpp
@@ -128,13 +128,13 @@
bool create_cache_path(char path[PKG_PATH_MAX],
const char *src,
const char *instruction_set) {
- size_t srclen = strlen(src);
-
- /* demand that we are an absolute path */
- if ((src == 0) || (src[0] != '/') || strstr(src,"..")) {
+ /* demand that we are an absolute path */
+ if ((src == nullptr) || (src[0] != '/') || strstr(src,"..")) {
return false;
}
+ size_t srclen = strlen(src);
+
if (srclen > PKG_PATH_MAX) { // XXX: PKG_NAME_MAX?
return false;
}
@@ -216,29 +216,40 @@
return destroy_app_data(parse_null(arg[0]), arg[1], atoi(arg[2]), atoi(arg[3]));
}
-static int do_ota_dexopt(char **arg, char reply[REPLY_MAX] ATTRIBUTE_UNUSED) {
- // Time to fork and run otapreopt.
- pid_t pid = fork();
- if (pid == 0) {
- const char* argv[1 + 9 + 1];
- argv[0] = "/system/bin/otapreopt";
- for (size_t i = 1; i <= 9; ++i) {
- argv[i] = arg[i - 1];
- }
- argv[10] = nullptr;
+// We use otapreopt_chroot to get into the chroot.
+static constexpr const char* kOtaPreopt = "/system/bin/otapreopt_chroot";
- execv(argv[0], (char * const *)argv);
- ALOGE("execv(OTAPREOPT) failed: %s\n", strerror(errno));
- exit(99);
- } else {
- int res = wait_child(pid);
- if (res == 0) {
- ALOGV("DexInv: --- END OTAPREOPT (success) ---\n");
- } else {
- ALOGE("DexInv: --- END OTAPREOPT --- status=0x%04x, process failed\n", res);
+static int do_ota_dexopt(char **arg, char reply[REPLY_MAX] ATTRIBUTE_UNUSED) {
+ // Time to fork and run otapreopt.
+
+ // Check that the tool exists.
+ struct stat s;
+ if (stat(kOtaPreopt, &s) != 0) {
+ LOG(ERROR) << "Otapreopt chroot tool not found.";
+ return -1;
}
- return res;
- }
+
+ pid_t pid = fork();
+ if (pid == 0) {
+ const char* argv[1 + 9 + 1];
+ argv[0] = kOtaPreopt;
+ for (size_t i = 1; i <= 9; ++i) {
+ argv[i] = arg[i - 1];
+ }
+ argv[10] = nullptr;
+
+ execv(argv[0], (char * const *)argv);
+ PLOG(ERROR) << "execv(OTAPREOPT_CHROOT) failed";
+ exit(99);
+ } else {
+ int res = wait_child(pid);
+ if (res == 0) {
+ ALOGV("DexInv: --- END OTAPREOPT (success) ---\n");
+ } else {
+ ALOGE("DexInv: --- END OTAPREOPT --- status=0x%04x, process failed\n", res);
+ }
+ return res;
+ }
}
static int do_dexopt(char **arg, char reply[REPLY_MAX])
diff --git a/cmds/installd/otapreopt.cpp b/cmds/installd/otapreopt.cpp
index 3aac48b..89a4225 100644
--- a/cmds/installd/otapreopt.cpp
+++ b/cmds/installd/otapreopt.cpp
@@ -17,6 +17,7 @@
#include <algorithm>
#include <inttypes.h>
#include <random>
+#include <regex>
#include <selinux/android.h>
#include <selinux/avc.h>
#include <stdlib.h>
@@ -35,6 +36,7 @@
#include <private/android_filesystem_config.h>
#include <commands.h>
+#include <file_parsing.h>
#include <globals.h>
#include <installd_deps.h> // Need to fill in requirements of commands.
#include <string_helpers.h>
@@ -54,8 +56,8 @@
namespace android {
namespace installd {
-static constexpr const char* kBootClassPathPropertyName = "env.BOOTCLASSPATH";
-static constexpr const char* kAndroidRootPathPropertyName = "env.ANDROID_ROOT";
+static constexpr const char* kBootClassPathPropertyName = "BOOTCLASSPATH";
+static constexpr const char* kAndroidRootPathPropertyName = "ANDROID_ROOT";
static constexpr const char* kOTARootDirectory = "/system-b";
static constexpr size_t kISAIndex = 3;
@@ -131,41 +133,55 @@
private:
bool ReadSystemProperties() {
- // TODO(agampe): What to do about the things in default.prop? It's only heap sizes, so it's easy
- // to emulate for now, but has issues (e.g., vendors modifying the boot classpath
- // may require larger values here - revisit). That's why this goes first, so that
- // if those dummy values are overridden in build.prop, that's what we'll get.
- //
- // Note: It seems we'll get access to the B root partition, so we should read the default.prop
- // file.
- // if (!system_properties_.Load(b_mount_path_ + "/default.prop") {
- // return false;
- // }
- system_properties_.SetProperty("dalvik.vm.image-dex2oat-Xms", "64m");
- system_properties_.SetProperty("dalvik.vm.image-dex2oat-Xmx", "64m");
- system_properties_.SetProperty("dalvik.vm.dex2oat-Xms", "64m");
- system_properties_.SetProperty("dalvik.vm.dex2oat-Xmx", "512m");
+ static constexpr const char* kPropertyFiles[] = {
+ "/default.prop", "/system/build.prop"
+ };
- // TODO(agampe): Do this properly/test.
- return system_properties_.Load(b_mount_path_ + "/system/build.prop");
+ for (size_t i = 0; i < arraysize(kPropertyFiles); ++i) {
+ if (!system_properties_.Load(kPropertyFiles[i])) {
+ return false;
+ }
+ }
+
+ return true;
}
bool ReadEnvironment() {
- // Read important environment variables. For simplicity, store them as
- // system properties.
- // TODO(agampe): We'll have to parse init.environ.rc for BOOTCLASSPATH.
- // For now, just the A version.
- const char* boot_classpath = getenv("BOOTCLASSPATH");
- if (boot_classpath == nullptr) {
- return false;
- }
- system_properties_.SetProperty(kBootClassPathPropertyName, boot_classpath);
+ // Parse the environment variables from init.environ.rc, which have the form
+ // export NAME VALUE
+ // For simplicity, don't respect string quotation. The values we are interested in can be
+ // encoded without them.
+ std::regex export_regex("\\s*export\\s+(\\S+)\\s+(\\S+)");
+ bool parse_result = ParseFile("/init.environ.rc", [&](const std::string& line) {
+ std::smatch export_match;
+ if (!std::regex_match(line, export_match, export_regex)) {
+ return true;
+ }
- const char* root_path = getenv("ANDROID_ROOT");
- if (root_path == nullptr) {
+ if (export_match.size() != 3) {
+ return true;
+ }
+
+ std::string name = export_match[1].str();
+ std::string value = export_match[2].str();
+
+ system_properties_.SetProperty(name, value);
+
+ return true;
+ });
+ if (!parse_result) {
return false;
}
- system_properties_.SetProperty(kAndroidRootPathPropertyName, b_mount_path_ + root_path);
+
+ // Check that we found important properties.
+ constexpr const char* kRequiredProperties[] = {
+ kBootClassPathPropertyName, kAndroidRootPathPropertyName
+ };
+ for (size_t i = 0; i < arraysize(kRequiredProperties); ++i) {
+ if (system_properties_.GetProperty(kRequiredProperties[i]) == nullptr) {
+ return false;
+ }
+ }
return true;
}
@@ -239,9 +255,7 @@
// TODO: Delete files, just for a blank slate.
const std::string& boot_cp = *system_properties_.GetProperty(kBootClassPathPropertyName);
- std::string preopted_boot_art_path = StringPrintf("%s/system/framework/%s/boot.art",
- b_mount_path_.c_str(),
- isa);
+ std::string preopted_boot_art_path = StringPrintf("/system/framework/%s/boot.art", isa);
if (access(preopted_boot_art_path.c_str(), F_OK) == 0) {
return PatchoatBootImage(art_path, isa);
} else {
@@ -254,7 +268,7 @@
// This needs to be kept in sync with ART, see art/runtime/gc/space/image_space.cc.
std::vector<std::string> cmd;
- cmd.push_back(b_mount_path_ + "/system/bin/patchoat");
+ cmd.push_back("/system/bin/patchoat");
cmd.push_back("--input-image-location=/system/framework/boot.art");
cmd.push_back(StringPrintf("--output-image-file=%s", art_path.c_str()));
@@ -279,7 +293,7 @@
const char* isa) {
// This needs to be kept in sync with ART, see art/runtime/gc/space/image_space.cc.
std::vector<std::string> cmd;
- cmd.push_back(b_mount_path_ + "/system/bin/dex2oat");
+ cmd.push_back("/system/bin/dex2oat");
cmd.push_back(StringPrintf("--image=%s", art_path.c_str()));
for (const std::string& boot_part : Split(boot_cp, ':')) {
cmd.push_back(StringPrintf("--dex-file=%s", boot_part.c_str()));
@@ -305,8 +319,7 @@
"--compiler-filter=",
false,
cmd);
- cmd.push_back(StringPrintf("--image-classes=%s/system/etc/preloaded-classes",
- b_mount_path_.c_str()));
+ cmd.push_back("--image-classes=/system/etc/preloaded-classes");
// TODO: Compiled-classes.
const std::string* extra_opts =
system_properties_.GetProperty("dalvik.vm.image-dex2oat-flags");
@@ -468,10 +481,6 @@
}
}
- // The path where the B partitions are mounted.
- // TODO(agampe): If we're running this *inside* the change-root, we wouldn't need this.
- std::string b_mount_path_;
-
// Stores the system properties read out of the B partition. We need to use these properties
// to compile, instead of the A properties we could get from init/get_property.
SystemProperties system_properties_;
diff --git a/cmds/installd/otapreopt_chroot.cpp b/cmds/installd/otapreopt_chroot.cpp
new file mode 100644
index 0000000..f7f69a9
--- /dev/null
+++ b/cmds/installd/otapreopt_chroot.cpp
@@ -0,0 +1,99 @@
+/*
+ ** Copyright 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.
+ */
+
+#include <linux/unistd.h>
+#include <sys/mount.h>
+#include <sys/wait.h>
+
+#include <android-base/logging.h>
+#include <android-base/macros.h>
+#include <android-base/stringprintf.h>
+
+#ifndef LOG_TAG
+#define LOG_TAG "otapreopt"
+#endif
+
+using android::base::StringPrintf;
+
+namespace android {
+namespace installd {
+
+static int otapreopt_chroot(const int argc, char **arg) {
+ // We need to run the otapreopt tool from the postinstall partition. As such, set up a
+ // mount namespace and change root.
+
+ // Create our own mount namespace.
+ if (unshare(CLONE_NEWNS) != 0) {
+ PLOG(ERROR) << "Failed to unshare() for otapreopt.";
+ exit(200);
+ }
+
+ // Make postinstall private, so that our changes don't propagate.
+ if (mount("", "/postinstall", nullptr, MS_PRIVATE, nullptr) != 0) {
+ PLOG(ERROR) << "Failed to mount private.";
+ exit(201);
+ }
+
+ // Bind mount necessary directories.
+ constexpr const char* kBindMounts[] = {
+ "/data", "/dev", "/proc", "/sys"
+ };
+ for (size_t i = 0; i < arraysize(kBindMounts); ++i) {
+ std::string trg = StringPrintf("/postinstall%s", kBindMounts[i]);
+ if (mount(kBindMounts[i], trg.c_str(), nullptr, MS_BIND, nullptr) != 0) {
+ PLOG(ERROR) << "Failed to bind-mount " << kBindMounts[i];
+ exit(202);
+ }
+ }
+
+ // Chdir into /postinstall.
+ if (chdir("/postinstall") != 0) {
+ PLOG(ERROR) << "Unable to chdir into /postinstall.";
+ exit(203);
+ }
+
+ // Make /postinstall the root in our mount namespace.
+ if (chroot(".") != 0) {
+ PLOG(ERROR) << "Failed to chroot";
+ exit(204);
+ }
+
+ if (chdir("/") != 0) {
+ PLOG(ERROR) << "Unable to chdir into /.";
+ exit(205);
+ }
+
+ // Now go on and run otapreopt.
+
+ const char* argv[1 + 9 + 1];
+ CHECK_EQ(argc, 10);
+ argv[0] = "/system/bin/otapreopt";
+ for (size_t i = 1; i <= 9; ++i) {
+ argv[i] = arg[i];
+ }
+ argv[10] = nullptr;
+
+ execv(argv[0], (char * const *)argv);
+ PLOG(ERROR) << "execv(OTAPREOPT) failed.";
+ exit(99);
+}
+
+} // namespace installd
+} // namespace android
+
+int main(const int argc, char *argv[]) {
+ return android::installd::otapreopt_chroot(argc, argv);
+}
diff --git a/cmds/installd/otapreopt_script.sh b/cmds/installd/otapreopt_script.sh
new file mode 100644
index 0000000..a31734a
--- /dev/null
+++ b/cmds/installd/otapreopt_script.sh
@@ -0,0 +1,50 @@
+#!/system/bin/sh
+
+#
+# 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.
+#
+
+# This script will run as a postinstall step to drive otapreopt.
+
+# Maximum number of packages/steps.
+MAXIMUM_PACKAGES=1000
+
+PREPARE=$(cmd otadexopt prepare)
+if [ "$PREPARE" != "Success" ] ; then
+ echo "Failed to prepare."
+ exit 1
+fi
+
+i=0
+while ((i<MAXIMUM_PACKAGES)) ; do
+ cmd otadexopt step
+ DONE=$(cmd otadexopt done)
+ if [ "$DONE" = "OTA complete." ] ; then
+ break
+ fi
+ sleep 1
+ i=$((i+1))
+done
+
+DONE=$(cmd otadexopt done)
+if [ "$DONE" = "OTA incomplete." ] ; then
+ echo "Incomplete."
+else
+ echo "Complete or error."
+fi
+
+cmd otadexopt cleanup
+
+exit 0
diff --git a/cmds/installd/system_properties.h b/cmds/installd/system_properties.h
index 1b5fb3a..2d940a3 100644
--- a/cmds/installd/system_properties.h
+++ b/cmds/installd/system_properties.h
@@ -21,6 +21,8 @@
#include <string>
#include <unordered_map>
+#include <file_parsing.h>
+
namespace android {
namespace installd {
@@ -28,31 +30,11 @@
class SystemProperties {
public:
bool Load(const std::string& strFile) {
- std::ifstream input_stream(strFile);
-
- if (!input_stream.is_open()) {
- return false;
- }
-
- while (!input_stream.eof()) {
- // Read the next line.
- std::string line;
- getline(input_stream, line);
-
- // Is the line empty? Simplifies the next check.
- if (line.empty()) {
- continue;
- }
-
- // Is this a comment (starts with pound)?
- if (line[0] == '#') {
- continue;
- }
-
+ return ParseFile(strFile, [&](const std::string& line) {
size_t equals_pos = line.find('=');
if (equals_pos == std::string::npos || equals_pos == 0) {
// Did not find equals sign, or it's the first character - isn't a valid line.
- continue;
+ return true;
}
std::string key = line.substr(0, equals_pos);
@@ -60,9 +42,9 @@
line.length() - equals_pos + 1);
properties_.insert(std::make_pair(key, value));
- }
- return true;
+ return true;
+ });
}
// Look up the key in the map. Returns null if the key isn't mapped.
diff --git a/cmds/servicemanager/binder.c b/cmds/servicemanager/binder.c
index 9e99085..01218c9 100644
--- a/cmds/servicemanager/binder.c
+++ b/cmds/servicemanager/binder.c
@@ -167,20 +167,27 @@
return res;
}
-void binder_send_reply(struct binder_state *bs,
- struct binder_io *reply,
- binder_uintptr_t buffer_to_free,
- int status)
+void binder_free_buffer(struct binder_state *bs,
+ binder_uintptr_t buffer_to_free)
{
struct {
uint32_t cmd_free;
binder_uintptr_t buffer;
+ } __attribute__((packed)) data;
+ data.cmd_free = BC_FREE_BUFFER;
+ data.buffer = buffer_to_free;
+ binder_write(bs, &data, sizeof(data));
+}
+
+void binder_send_reply(struct binder_state *bs,
+ struct binder_io *reply,
+ int status)
+{
+ struct {
uint32_t cmd_reply;
struct binder_transaction_data txn;
} __attribute__((packed)) data;
- data.cmd_free = BC_FREE_BUFFER;
- data.buffer = buffer_to_free;
data.cmd_reply = BC_REPLY;
data.txn.target.ptr = 0;
data.txn.cookie = 0;
@@ -243,7 +250,9 @@
bio_init(&reply, rdata, sizeof(rdata), 4);
bio_init_from_txn(&msg, txn);
res = func(bs, txn, &msg, &reply);
- binder_send_reply(bs, &reply, txn->data.ptr.buffer, res);
+ binder_free_buffer(bs, txn->data.ptr.buffer);
+ if ((txn->flags & TF_ONE_WAY) == 0)
+ binder_send_reply(bs, &reply, res);
}
ptr += sizeof(*txn);
break;
diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h
index 1b63552..9307a26 100644
--- a/include/gui/ConsumerBase.h
+++ b/include/gui/ConsumerBase.h
@@ -26,8 +26,6 @@
#include <utils/threads.h>
#include <gui/IConsumerListener.h>
-#include <queue>
-
namespace android {
// ----------------------------------------------------------------------------
@@ -110,18 +108,18 @@
// from the derived class.
virtual void onLastStrongRef(const void* id);
- // Handlers for the IConsumerListener interface, these will be called from
- // the message queue thread. These calls are used to notify the ConsumerBase
- // of asynchronous events in the BufferQueue. The onFrameAvailableHandler,
- // onFrameReplacedHandler, and onBuffersReleasedHandler methods should not
- // need to be overridden by derived classes, but if they are overridden the
- // ConsumerBase implementation must be called from the derived class. The
- // ConsumerBase version of onSidebandStreamChangedHandler does nothing and
- // can be overriden by derived classes if they want the notification.
- virtual void onFrameAvailableHandler(const BufferItem& item);
- virtual void onFrameReplacedHandler(const BufferItem& item);
- virtual void onBuffersReleasedHandler();
- virtual void onSidebandStreamChangedHandler();
+ // Implementation of the IConsumerListener interface. These
+ // calls are used to notify the ConsumerBase of asynchronous events in the
+ // BufferQueue. The onFrameAvailable, onFrameReplaced, and
+ // onBuffersReleased methods should not need to be overridden by derived
+ // classes, but if they are overridden the ConsumerBase implementation must
+ // be called from the derived class. The ConsumerBase version of
+ // onSidebandStreamChanged does nothing and can be overriden by derived
+ // classes if they want the notification.
+ virtual void onFrameAvailable(const BufferItem& item) override;
+ virtual void onFrameReplaced(const BufferItem& item) override;
+ virtual void onBuffersReleased() override;
+ virtual void onSidebandStreamChanged() override;
// freeBufferLocked frees up the given buffer slot. If the slot has been
// initialized this will release the reference to the GraphicBuffer in that
@@ -246,35 +244,6 @@
//
// This mutex is intended to be locked by derived classes.
mutable Mutex mMutex;
-
- // Implements the ConsumerListener interface
- virtual void onFrameAvailable(const BufferItem& item) override;
- virtual void onFrameReplaced(const BufferItem& item) override;
- virtual void onBuffersReleased() override;
- virtual void onSidebandStreamChanged() override;
-
- enum MessageType {
- ON_FRAME_AVAILABLE,
- ON_FRAME_REPLACED,
- ON_BUFFERS_RELEASED,
- ON_SIDEBAND_STREAM_CHANGED,
- EXIT,
- };
-
- mutable Mutex mMessageQueueLock;
- Condition mMessageAvailable;
- std::queue<std::pair<MessageType, BufferItem>> mMessageQueue;
-
- class MessageThread : public Thread {
- public:
- MessageThread(ConsumerBase* consumerBase) :
- mConsumerBase(consumerBase) {};
- protected:
- virtual bool threadLoop() override;
- ConsumerBase* mConsumerBase;
- };
-
- sp<MessageThread> mMessageThread;
};
// ----------------------------------------------------------------------------
diff --git a/include/media/hardware/HardwareAPI.h b/include/media/hardware/HardwareAPI.h
index 77ce563..654773d 100644
--- a/include/media/hardware/HardwareAPI.h
+++ b/include/media/hardware/HardwareAPI.h
@@ -315,53 +315,108 @@
OMX_PTR pSidebandWindow; // OUT
};
-// Color description parameters. This is passed via OMX_SetConfig or OMX_GetConfig
-// to video encoders and decoders when the
-// 'OMX.google.android.index.describeColorAspects' extension is given.
+// Color space description (aspects) parameters.
+// This is passed via OMX_SetConfig or OMX_GetConfig to video encoders and decoders when the
+// 'OMX.google.android.index.describeColorAspects' extension is given. Component SHALL behave
+// as described below if it supports this extension.
//
-// Video encoders: the framework uses OMX_SetConfig to specify color aspects
-// of the coded video before the component transitions to idle state, as well
-// as before an input frame with a different color aspect is sent:
-// 1. The component should maintain an internal color aspect state, initialized
-// to Unspecified values.
-// 2. Upon OMX_SetConfig, it SHOULD update its internal state for the aspects that are not
-// Unspecified in the config param.
-// 3. If an aspect value cannot be encoded into the bitstream (including the Other value), that
-// aspect should be reset to the Unspecified value (in the internal state).
-// 4. OMX_GetConfig SHOULD return the current internal state.
-// 5. If changing the color aspects after the first input frame is not supported, and the config
-// params would actually cause a change, OMX_SetConfig should fail with the internal state
-// unchanged.
-// 6. If changing a portion of the aspects after the first input frame is supported, OMX_SetConfig
-// should succeed with the portion of the internal state updated.
+// bDataSpaceChanged and bRequestingDataSpace is assumed to be OMX_FALSE unless noted otherwise.
//
-// Video decoders: the framework uses OMX_SetConfig to specify color aspects
-// of the coded video parsed from the container before the component transitions
-// to idle state.
-// 1. The component should maintiain an internal color aspect state, initialized to Unspecified
-// values.
-// 2. Upon OMX_SetConfig, it SHOULD update its internal state for the aspects that are not
-// Unspecified in the config param, regardless of whether such aspects could be supplied by the
-// component bitstream. (E.g. it should blindly support all enumeration values, even unknown
-// ones, and the Other value).
-// 3. OMX_GetConfig SHOULD return the current internal state.
-// 4. When the component processes color aspect information in the bitstream with a non-Unspecified
-// value, it should update its internal state with that information just before the frame
-// with the new information is outputted, and the component SHALL signal an
-// OMX_EventPortSettingsChanged event with data2 set to the extension index (or
-// OMX_IndexConfigCommonOutputCrop, as it is handled identically).
-// 4a. Component shall not signal a separate event purely for color aspect change, if it occurs
-// together with a port definition (e.g. size) or crop change.
-// 5. If the aspects a component encounters in the bitstream cannot be represented with the below
-// enumeration values, it should set those aspects to Other. Restricted values in the bitstream
-// should be treated as defined by the relevant bitstream specifications/standards, or as
-// Unspecified, if not defined.
+// VIDEO ENCODERS: the framework uses OMX_SetConfig to specify color aspects of the coded video.
+// This may happen:
+// a) before the component transitions to idle state
+// b) before the input frame is sent via OMX_EmptyThisBuffer in executing state
+// c) during execution, just before an input frame with a different color aspect information
+// is sent.
+//
+// The framework also uses OMX_GetConfig to
+// d) verify the color aspects that will be written to the stream
+// e) (optional) verify the color aspects that should be reported to the container for a
+// given dataspace/pixelformat received
+//
+// 1. Encoders SHOULD maintain an internal color aspect state, initialized to Unspecified values.
+// This represents the values that will be written into the bitstream.
+// 2. Upon OMX_SetConfig, they SHOULD update their internal state to the aspects received
+// (including Unspecified values). For specific aspect values that are not supported by the
+// codec standard, encoders SHOULD substitute Unspecified values; or they MAY use a suitable
+// alternative (e.g. to suggest the use of BT.709 EOTF instead of SMPTE 240M.)
+// 3. OMX_GetConfig SHALL return the internal state (values that will be written).
+// 4. OMX_SetConfig SHALL always succeed before receiving the first frame. It MAY fail afterwards,
+// but only if the configured values would change AND the component does not support updating the
+// color information to those values mid-stream. If component supports updating a portion of
+// the color information, those values should be updated in the internal state, and OMX_SetConfig
+// SHALL succeed. Otherwise, the internal state SHALL remain intact and OMX_SetConfig SHALL fail
+// with OMX_ErrorUnsupportedSettings.
+// 5. When the framework receives an input frame with an unexpected dataspace, it will query
+// encoders for the color aspects that should be reported to the container using OMX_GetConfig
+// with bDataSpaceChanged set to OMX_TRUE, and nPixelFormat/nDataSpace containing the new
+// format/dataspace values. This allows vendors to use extended dataspace during capture and
+// composition (e.g. screenrecord) - while performing color-space conversion inside the encoder -
+// and encode and report a different color-space information in the bitstream/container.
+// sColorAspects contains the requested color aspects by the client for reference, which may
+// include aspects not supported by the encoding. This is used together with guidance for
+// dataspace selection; see 6. below.
+//
+// VIDEO DECODERS: the framework uses OMX_SetConfig to specify the default color aspects to use
+// for the video.
+// This may happen:
+// f) before the component transitions to idle state
+// g) during execution, when the resolution or the default color aspects change.
+//
+// The framework also uses OMX_GetConfig to
+// h) get the final color aspects reported by the coded bitstream after taking the default values
+// into account.
+//
+// 1. Decoders should maintain two color aspect states - the default state as reported by the
+// framework, and the coded state as reported by the bitstream - as each state can change
+// independently from the other.
+// 2. Upon OMX_SetConfig, it SHALL update its default state regardless of whether such aspects
+// could be supplied by the component bitstream. (E.g. it should blindly support all enumeration
+// values, even unknown ones, and the Other value). This SHALL always succeed.
+// 3. Upon OMX_GetConfig, the component SHALL return the final color aspects by replacing
+// Unspecified coded values with the default values. This SHALL always succeed.
+// 4. Whenever the component processes color aspect information in the bitstream even with an
+// Unspecified value, it SHOULD update its internal coded state with that information just before
+// the frame with the new information would be outputted, and the component SHALL signal an
+// OMX_EventPortSettingsChanged event with data2 set to the extension index.
+// NOTE: Component SHOULD NOT signal a separate event purely for color aspect change, if it occurs
+// together with a port definition (e.g. size) or crop change.
+// 5. If the aspects a component encounters in the bitstream cannot be represented with enumeration
+// values as defined below, the component SHALL set those aspects to Other. Restricted values in
+// the bitstream SHALL be treated as defined by the relevant bitstream specifications/standards,
+// or as Unspecified, if not defined.
+//
+// BOTH DECODERS AND ENCODERS: the framework uses OMX_GetConfig during idle and executing state to
+// i) (optional) get guidance for the dataspace to set for given color aspects, by setting
+// bRequestingDataSpace to OMX_TRUE. The component SHALL return OMX_ErrorUnsupportedSettings
+// IF it does not support this request.
+//
+// 6. This is an information request that can happen at any time, independent of the normal
+// configuration process. This allows vendors to use extended dataspace during capture, playback
+// and composition - while performing color-space conversion inside the component. Component
+// SHALL set the desired dataspace into nDataSpace. Otherwise, it SHALL return
+// OMX_ErrorUnsupportedSettings to let the framework choose a nearby standard dataspace.
+//
+// 6.a. For encoders, this query happens before the first frame is received using surface encoding.
+// This allows the encoder to use a specific dataspace for the color aspects (e.g. because the
+// device supports additional dataspaces, or because it wants to perform color-space extension
+// to facilitate a more optimal rendering/capture pipeline.).
+//
+// 6.b. For decoders, this query happens before the first frame, and every time the color aspects
+// change, while using surface buffers. This allows the decoder to use a specific dataspace for
+// the color aspects (e.g. because the device supports additional dataspaces, or because it wants
+// to perform color-space extension by inline color-space conversion to facilitate a more optimal
+// rendering pipeline.).
//
struct DescribeColorAspectsParams {
- OMX_U32 nSize; // IN
- OMX_VERSIONTYPE nVersion; // IN
- OMX_U32 nPortIndex; // IN
- ColorAspects sAspects; // IN/OUT
+ OMX_U32 nSize; // IN
+ OMX_VERSIONTYPE nVersion; // IN
+ OMX_U32 nPortIndex; // IN
+ OMX_BOOL bRequestingDataSpace; // IN
+ OMX_BOOL bDataSpaceChanged; // IN
+ OMX_U32 nPixelFormat; // IN
+ OMX_U32 nDataSpace; // OUT
+ ColorAspects sAspects; // IN/OUT
};
} // namespace android
diff --git a/include/media/openmax/OMX_AsString.h b/include/media/openmax/OMX_AsString.h
index aacffad..5fb8ca8 100644
--- a/include/media/openmax/OMX_AsString.h
+++ b/include/media/openmax/OMX_AsString.h
@@ -292,6 +292,7 @@
// case OMX_EventDynamicResourcesAvailable: return "DynamicResourcesAvailable";
// case OMX_EventPortFormatDetected: return "PortFormatDetected";
case OMX_EventOutputRendered: return "OutputRendered";
+ case OMX_EventDataSpaceChanged: return "DataSpaceChanged";
default: return def;
}
}
diff --git a/include/media/openmax/OMX_Core.h b/include/media/openmax/OMX_Core.h
index f746a69..99a7622 100644
--- a/include/media/openmax/OMX_Core.h
+++ b/include/media/openmax/OMX_Core.h
@@ -524,6 +524,21 @@
* frame.
*/
OMX_EventOutputRendered = 0x7F000001,
+
+ /** For framework internal use only: event sent by OMXNodeInstance when it receives a graphic
+ * input buffer with a new dataspace for encoding. |arg1| will contain the dataspace. |arg2|
+ * will contain the ColorAspects requested by the component (or framework defaults) using
+ * the following bitfield layout:
+ *
+ * +----------+-------------+----------------+------------+
+ * | Range | Primaries | MatrixCoeffs | Transfer |
+ * +----------+-------------+----------------+------------+
+ * bits: 31....24 23.......16 15...........8 7........0
+ *
+ * TODO: We would really need to tie this to an output buffer, but OMX does not provide a
+ * fool-proof way to do that for video encoders.
+ */
+ OMX_EventDataSpaceChanged,
OMX_EventMax = 0x7FFFFFFF
} OMX_EVENTTYPE;
diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index a22b81b..2187e5e 100644
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -74,26 +74,12 @@
} else {
mConsumer->setConsumerName(mName);
}
-
- mMessageThread = new MessageThread(this);
- mMessageThread->run();
}
ConsumerBase::~ConsumerBase() {
CB_LOGV("~ConsumerBase");
-
- mMessageThread->requestExit();
- {
- Mutex::Autolock lock(mMessageQueueLock);
- mMessageQueue.emplace(std::piecewise_construct,
- std::forward_as_tuple(EXIT),
- std::forward_as_tuple());
- mMessageAvailable.signal();
- }
-
- mMessageThread->join();
-
Mutex::Autolock lock(mMutex);
+
// Verify that abandon() has been called before we get here. This should
// be done by ConsumerBase::onLastStrongRef(), but it's possible for a
// derived class to override that method and not call
@@ -114,13 +100,6 @@
}
void ConsumerBase::onFrameAvailable(const BufferItem& item) {
- Mutex::Autolock lock(mMessageQueueLock);
- mMessageQueue.emplace(std::piecewise_construct,
- std::forward_as_tuple(ON_FRAME_AVAILABLE),
- std::forward_as_tuple(item));
- mMessageAvailable.signal();
-}
-void ConsumerBase::onFrameAvailableHandler(const BufferItem& item) {
CB_LOGV("onFrameAvailable");
sp<FrameAvailableListener> listener;
@@ -136,14 +115,6 @@
}
void ConsumerBase::onFrameReplaced(const BufferItem &item) {
- Mutex::Autolock lock(mMessageQueueLock);
- mMessageQueue.emplace(std::piecewise_construct,
- std::forward_as_tuple(ON_FRAME_REPLACED),
- std::forward_as_tuple(item));
- mMessageAvailable.signal();
-}
-
-void ConsumerBase::onFrameReplacedHandler(const BufferItem &item) {
CB_LOGV("onFrameReplaced");
sp<FrameAvailableListener> listener;
@@ -159,14 +130,6 @@
}
void ConsumerBase::onBuffersReleased() {
- Mutex::Autolock lock(mMessageQueueLock);
- mMessageQueue.emplace(std::piecewise_construct,
- std::forward_as_tuple(ON_BUFFERS_RELEASED),
- std::forward_as_tuple());
- mMessageAvailable.signal();
-}
-
-void ConsumerBase::onBuffersReleasedHandler() {
Mutex::Autolock lock(mMutex);
CB_LOGV("onBuffersReleased");
@@ -186,45 +149,6 @@
}
void ConsumerBase::onSidebandStreamChanged() {
- Mutex::Autolock lock(mMessageQueueLock);
- mMessageQueue.emplace(std::piecewise_construct,
- std::forward_as_tuple(ON_SIDEBAND_STREAM_CHANGED),
- std::forward_as_tuple());
- mMessageAvailable.signal();
-}
-
-void ConsumerBase::onSidebandStreamChangedHandler() {
-}
-
-bool ConsumerBase::MessageThread::threadLoop() {
- Mutex::Autolock lock(mConsumerBase->mMessageQueueLock);
-
- if (mConsumerBase->mMessageQueue.empty()) {
- mConsumerBase->mMessageAvailable.wait(mConsumerBase->mMessageQueueLock);
- }
-
- while (!mConsumerBase->mMessageQueue.empty()) {
- auto nextMessage = mConsumerBase->mMessageQueue.front();
-
- switch (nextMessage.first) {
- case ON_FRAME_AVAILABLE:
- mConsumerBase->onFrameAvailableHandler(nextMessage.second);
- break;
- case ON_FRAME_REPLACED:
- mConsumerBase->onFrameReplacedHandler(nextMessage.second);
- break;
- case ON_BUFFERS_RELEASED:
- mConsumerBase->onBuffersReleasedHandler();
- break;
- case ON_SIDEBAND_STREAM_CHANGED:
- mConsumerBase->onSidebandStreamChangedHandler();
- break;
- case EXIT:
- break;
- }
- mConsumerBase->mMessageQueue.pop();
- }
- return true;
}
void ConsumerBase::abandon() {
@@ -238,7 +162,7 @@
}
void ConsumerBase::abandonLocked() {
- CB_LOGV("abandonLocked");
+ CB_LOGV("abandonLocked");
for (int i =0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) {
freeBufferLocked(i);
}
diff --git a/libs/gui/tests/SurfaceTextureClient_test.cpp b/libs/gui/tests/SurfaceTextureClient_test.cpp
index 2356f54..a1578f6 100644
--- a/libs/gui/tests/SurfaceTextureClient_test.cpp
+++ b/libs/gui/tests/SurfaceTextureClient_test.cpp
@@ -564,7 +564,7 @@
ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0]));
ASSERT_EQ(OK, mANW->queueBuffer(mANW.get(), buf[0], -1));
- thread->run();
+ thread->run("MyThread");
ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[1]));
ASSERT_EQ(OK, mANW->queueBuffer(mANW.get(), buf[1], -1));
//ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[2]));
diff --git a/libs/gui/tests/SurfaceTextureGLThreadToGL.h b/libs/gui/tests/SurfaceTextureGLThreadToGL.h
index 14e42ac..2ce20eb 100644
--- a/libs/gui/tests/SurfaceTextureGLThreadToGL.h
+++ b/libs/gui/tests/SurfaceTextureGLThreadToGL.h
@@ -171,7 +171,7 @@
mProducerThread = producerThread;
producerThread->setEglObjects(mEglDisplay, mProducerEglSurface,
mProducerEglContext);
- producerThread->run();
+ producerThread->run("ProducerThread");
}
sp<ProducerThread> mProducerThread;
diff --git a/libs/gui/tests/SurfaceTextureGLToGL_test.cpp b/libs/gui/tests/SurfaceTextureGLToGL_test.cpp
index b8a7a90..c28b4d1 100644
--- a/libs/gui/tests/SurfaceTextureGLToGL_test.cpp
+++ b/libs/gui/tests/SurfaceTextureGLToGL_test.cpp
@@ -192,10 +192,6 @@
ASSERT_EQ(EGL_SUCCESS, eglGetError());
mProducerEglSurface = EGL_NO_SURFACE;
- // sleep for 10ms to allow any asynchronous operations to complete before
- // checking the reference counts
- usleep(10000);
-
// This test should have the only reference to buffer 0.
EXPECT_EQ(1, buffers[0]->getStrongCount());
diff --git a/libs/gui/tests/SurfaceTextureGL_test.cpp b/libs/gui/tests/SurfaceTextureGL_test.cpp
index 1a904b5..dddcf92 100644
--- a/libs/gui/tests/SurfaceTextureGL_test.cpp
+++ b/libs/gui/tests/SurfaceTextureGL_test.cpp
@@ -295,7 +295,7 @@
};
sp<Thread> pt(new ProducerThread(mANW, testPixels));
- pt->run();
+ pt->run("ProducerThread");
glViewport(0, 0, texWidth, texHeight);
@@ -484,7 +484,7 @@
sp<Thread> pt(new ProducerThread(mANW));
- pt->run();
+ pt->run("ProducerThread");
// eat a frame so GLConsumer will own an at least one slot
dw->waitForFrame();
@@ -681,7 +681,7 @@
};
sp<Thread> pt(new ProducerThread(mANW));
- pt->run();
+ pt->run("ProducerThread");
mFW->waitForFrame();
mFW->waitForFrame();
diff --git a/opengl/libs/EGL/egl_cache.cpp b/opengl/libs/EGL/egl_cache.cpp
index f368d75..8c135c8 100644
--- a/opengl/libs/EGL/egl_cache.cpp
+++ b/opengl/libs/EGL/egl_cache.cpp
@@ -165,7 +165,7 @@
// running, so there's no need to keep a ref around.
sp<Thread> deferredSaveThread(new DeferredSaveThread());
mSavePending = true;
- deferredSaveThread->run();
+ deferredSaveThread->run("DeferredSaveThread");
}
}
}