diff options
36 files changed, 612 insertions, 384 deletions
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index 20b960d218..e9a135c14d 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -1729,7 +1729,8 @@ binder::Status InstalldNativeService::dexopt(const std::string& apkPath, int32_t const std::unique_ptr<std::string>& packageName, const std::string& instructionSet, int32_t dexoptNeeded, const std::unique_ptr<std::string>& outputPath, int32_t dexFlags, const std::string& compilerFilter, const std::unique_ptr<std::string>& uuid, - const std::unique_ptr<std::string>& sharedLibraries) { + const std::unique_ptr<std::string>& sharedLibraries, + const std::unique_ptr<std::string>& seInfo) { ENFORCE_UID(AID_SYSTEM); CHECK_ARGUMENT_UUID(uuid); if (packageName && *packageName != "*") { @@ -1744,9 +1745,9 @@ binder::Status InstalldNativeService::dexopt(const std::string& apkPath, int32_t const char* compiler_filter = compilerFilter.c_str(); const char* volume_uuid = uuid ? uuid->c_str() : nullptr; const char* shared_libraries = sharedLibraries ? sharedLibraries->c_str() : nullptr; - + const char* se_info = seInfo ? seInfo->c_str() : nullptr; int res = android::installd::dexopt(apk_path, uid, pkgname, instruction_set, dexoptNeeded, - oat_dir, dexFlags, compiler_filter, volume_uuid, shared_libraries); + oat_dir, dexFlags, compiler_filter, volume_uuid, shared_libraries, se_info); return res ? error(res, "Failed to dexopt") : ok(); } diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h index f5b7142a12..fe8aa14703 100644 --- a/cmds/installd/InstalldNativeService.h +++ b/cmds/installd/InstalldNativeService.h @@ -82,7 +82,8 @@ public: const std::unique_ptr<std::string>& packageName, const std::string& instructionSet, int32_t dexoptNeeded, const std::unique_ptr<std::string>& outputPath, int32_t dexFlags, const std::string& compilerFilter, const std::unique_ptr<std::string>& uuid, - const std::unique_ptr<std::string>& sharedLibraries); + const std::unique_ptr<std::string>& sharedLibraries, + const std::unique_ptr<std::string>& seInfo); binder::Status rmdex(const std::string& codePath, const std::string& instructionSet); diff --git a/cmds/installd/binder/android/os/IInstalld.aidl b/cmds/installd/binder/android/os/IInstalld.aidl index 03ff96e866..e738b810d8 100644 --- a/cmds/installd/binder/android/os/IInstalld.aidl +++ b/cmds/installd/binder/android/os/IInstalld.aidl @@ -50,7 +50,8 @@ interface IInstalld { @utf8InCpp String instructionSet, int dexoptNeeded, @nullable @utf8InCpp String outputPath, int dexFlags, @utf8InCpp String compilerFilter, @nullable @utf8InCpp String uuid, - @nullable @utf8InCpp String sharedLibraries); + @nullable @utf8InCpp String sharedLibraries, + @nullable @utf8InCpp String seInfo); void rmdex(@utf8InCpp String codePath, @utf8InCpp String instructionSet); diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index f7e8d130a0..63afdcd111 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -36,6 +36,7 @@ #include <cutils/sched_policy.h> #include <log/log.h> // TODO: Move everything to base/logging. #include <private/android_filesystem_config.h> +#include <selinux/android.h> #include <system/thread_defs.h> #include "dexopt.h" @@ -1302,17 +1303,9 @@ static bool prepare_secondary_dex_oat_dir(const std::string& dex_path, int uid, } std::string dex_dir = dex_path.substr(0, dirIndex); - // Assign the gid to the cache gid so that the oat file storage - // is counted towards the app cache. - int32_t cache_gid = multiuser_get_cache_gid( - multiuser_get_user_id(uid), multiuser_get_app_id(uid)); - // If UID doesn't have a specific cache GID, use UID value - if (cache_gid == -1) { - cache_gid = uid; - } - // Create oat file output directory. - if (prepare_app_cache_dir(dex_dir, "oat", 02711, uid, cache_gid) != 0) { + mode_t oat_dir_mode = S_IRWXU | S_IRWXG | S_IXOTH; + if (prepare_app_cache_dir(dex_dir, "oat", oat_dir_mode, uid, uid) != 0) { LOG(ERROR) << "Could not prepare oat dir for secondary dex: " << dex_path; return false; } @@ -1322,7 +1315,7 @@ static bool prepare_secondary_dex_oat_dir(const std::string& dex_path, int uid, oat_dir_out->assign(oat_dir); // Create oat/isa output directory. - if (prepare_app_cache_dir(*oat_dir_out, instruction_set, 02711, uid, cache_gid) != 0) { + if (prepare_app_cache_dir(*oat_dir_out, instruction_set, oat_dir_mode, uid, uid) != 0) { LOG(ERROR) << "Could not prepare oat/isa dir for secondary dex: " << dex_path; return false; } @@ -1366,12 +1359,15 @@ static bool process_dexoptanalyzer_result(const std::string& dex_path, int resul // Processes the dex_path as a secondary dex files and return true if the path dex file should // be compiled. Returns false for errors (logged) or true if the secondary dex path was process // successfully. -// When returning true, dexopt_needed_out is assigned a valid OatFileAsssitant::DexOptNeeded -// code and oat_dir_out is assigned the oat dir path where the oat file should be stored. +// When returning true, the output parameters will be: +// - is_public_out: whether or not the oat file should not be made public +// - dexopt_needed_out: valid OatFileAsssitant::DexOptNeeded +// - oat_dir_out: the oat dir path where the oat file should be stored +// - dex_path_out: the real path of the dex file static bool process_secondary_dex_dexopt(const char* original_dex_path, const char* pkgname, int dexopt_flags, const char* volume_uuid, int uid, const char* instruction_set, - const char* compiler_filter, int* dexopt_needed_out, std::string* oat_dir_out, - std::string* dex_path_out) { + const char* compiler_filter, bool* is_public_out, int* dexopt_needed_out, + std::string* oat_dir_out, std::string* dex_path_out) { int storage_flag; if ((dexopt_flags & DEXOPT_STORAGE_CE) != 0) { @@ -1407,7 +1403,8 @@ static bool process_secondary_dex_dexopt(const char* original_dex_path, const ch } // Check if the path exist. If not, there's nothing to do. - if (access(dex_path.c_str(), F_OK) != 0) { + struct stat dex_path_stat; + if (stat(dex_path.c_str(), &dex_path_stat) != 0) { if (errno == ENOENT) { // Secondary dex files might be deleted any time by the app. // Nothing to do if that's the case @@ -1418,6 +1415,11 @@ static bool process_secondary_dex_dexopt(const char* original_dex_path, const ch } } + // Check if we should make the oat file public. + // Note that if the dex file is not public the compiled code cannot be made public. + *is_public_out = ((dexopt_flags & DEXOPT_PUBLIC) != 0) && + ((dex_path_stat.st_mode & S_IROTH) != 0); + // Prepare the oat directories. if (!prepare_secondary_dex_oat_dir(dex_path, uid, instruction_set, oat_dir_out)) { return false; @@ -1458,14 +1460,14 @@ static bool process_secondary_dex_dexopt(const char* original_dex_path, const ch int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* instruction_set, int dexopt_needed, const char* oat_dir, int dexopt_flags, const char* compiler_filter, - const char* volume_uuid, const char* shared_libraries) { + const char* volume_uuid, const char* shared_libraries, const char* se_info) { CHECK(pkgname != nullptr); CHECK(pkgname[0] != 0); if ((dexopt_flags & ~DEXOPT_MASK) != 0) { LOG_FATAL("dexopt flags contains unknown fields\n"); } - bool is_public = ((dexopt_flags & DEXOPT_PUBLIC) != 0); + bool is_public = (dexopt_flags & DEXOPT_PUBLIC) != 0; bool vm_safe_mode = (dexopt_flags & DEXOPT_SAFEMODE) != 0; bool debuggable = (dexopt_flags & DEXOPT_DEBUGGABLE) != 0; bool boot_complete = (dexopt_flags & DEXOPT_BOOTCOMPLETE) != 0; @@ -1477,7 +1479,8 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins std::string dex_real_path; if (is_secondary_dex) { if (process_secondary_dex_dexopt(dex_path, pkgname, dexopt_flags, volume_uuid, uid, - instruction_set, compiler_filter, &dexopt_needed, &oat_dir_str, &dex_real_path)) { + instruction_set, compiler_filter, &is_public, &dexopt_needed, &oat_dir_str, + &dex_real_path)) { oat_dir = oat_dir_str.c_str(); dex_path = dex_real_path.c_str(); if (dexopt_needed == NO_DEXOPT_NEEDED) { @@ -1516,6 +1519,19 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins return -1; } + // Ensure that the oat dir and the compiler artifacts of secondary dex files have the correct + // selinux context (we generate them on the fly during the dexopt invocation and they don't + // fully inherit their parent context). + // Note that for primary apk the oat files are created before, in a separate installd + // call which also does the restorecon. TODO(calin): unify the paths. + if (is_secondary_dex) { + if (selinux_android_restorecon_pkgdir(oat_dir, se_info, uid, + SELINUX_ANDROID_RESTORECON_RECURSE)) { + LOG(ERROR) << "Failed to restorecon " << oat_dir; + return -1; + } + } + // Create a swap file if necessary. unique_fd swap_fd = maybe_open_dexopt_swap_file(out_oat_path); @@ -1857,8 +1873,9 @@ int dexopt(const char* const params[DEXOPT_PARAM_COUNT]) { atoi(params[6]), // dexopt_flags params[7], // compiler_filter parse_null(params[8]), // volume_uuid - parse_null(params[9])); // shared_libraries - static_assert(DEXOPT_PARAM_COUNT == 10U, "Unexpected dexopt param count"); + parse_null(params[9]), // shared_libraries + parse_null(params[10])); // se_info + static_assert(DEXOPT_PARAM_COUNT == 11U, "Unexpected dexopt param count"); } } // namespace installd diff --git a/cmds/installd/dexopt.h b/cmds/installd/dexopt.h index dbf3faeacd..88144b7bc3 100644 --- a/cmds/installd/dexopt.h +++ b/cmds/installd/dexopt.h @@ -60,10 +60,10 @@ bool reconcile_secondary_dex_file(const std::string& dex_path, int dexopt(const char *apk_path, uid_t uid, const char *pkgName, const char *instruction_set, int dexopt_needed, const char* oat_dir, int dexopt_flags, const char* compiler_filter, - const char* volume_uuid, const char* shared_libraries); + const char* volume_uuid, const char* shared_libraries, const char* se_info); -static constexpr size_t DEXOPT_PARAM_COUNT = 10U; -static_assert(DEXOPT_PARAM_COUNT == 10U, "Unexpected dexopt param size"); +static constexpr size_t DEXOPT_PARAM_COUNT = 11U; +static_assert(DEXOPT_PARAM_COUNT == 11U, "Unexpected dexopt param size"); // Helper for the above, converting arguments. int dexopt(const char* const params[DEXOPT_PARAM_COUNT]); diff --git a/cmds/service/service.cpp b/cmds/service/service.cpp index 428b87cd51..09921e45d3 100644 --- a/cmds/service/service.cpp +++ b/cmds/service/service.cpp @@ -68,18 +68,12 @@ static String8 good_old_string(const String16& src) int main(int argc, char* const argv[]) { - sp<IServiceManager> sm = defaultServiceManager(); - fflush(stdout); - if (sm == NULL) { - aerr << "service: Unable to get default service manager!" << endl; - return 20; - } - bool wantsUsage = false; + bool wantsVendorServices = false; int result = 0; while (1) { - int ic = getopt(argc, argv, "h?"); + int ic = getopt(argc, argv, "vh?"); if (ic < 0) break; @@ -88,6 +82,9 @@ int main(int argc, char* const argv[]) case '?': wantsUsage = true; break; + case 'v': + wantsVendorServices = true; + break; default: aerr << "service: Unknown option -" << ic << endl; wantsUsage = true; @@ -95,6 +92,16 @@ int main(int argc, char* const argv[]) break; } } + + if (wantsVendorServices) { + ProcessState::initWithDriver("/dev/vndbinder"); + } + sp<IServiceManager> sm = defaultServiceManager(); + fflush(stdout); + if (sm == NULL) { + aerr << "service: Unable to get default service manager!" << endl; + return 20; + } if (optind >= argc) { wantsUsage = true; diff --git a/cmds/servicemanager/service_manager.c b/cmds/servicemanager/service_manager.c index 45bb1d05e8..1f56a47757 100644 --- a/cmds/servicemanager/service_manager.c +++ b/cmds/servicemanager/service_manager.c @@ -65,7 +65,11 @@ static struct selabel_handle* sehandle; static bool check_mac_perms(pid_t spid, uid_t uid, const char *tctx, const char *perm, const char *name) { char *sctx = NULL; +#ifdef VENDORSERVICEMANAGER + const char *class = "vndservice_manager"; +#else const char *class = "service_manager"; +#endif bool allowed; struct audit_data ad; diff --git a/libs/binder/tests/schd-dbg.cpp b/libs/binder/tests/schd-dbg.cpp index 27320712f0..fe9e05a4f1 100644 --- a/libs/binder/tests/schd-dbg.cpp +++ b/libs/binder/tests/schd-dbg.cpp @@ -15,6 +15,7 @@ #include <pthread.h> #include <sys/wait.h> #include <unistd.h> +#include <fstream> using namespace std; using namespace android; @@ -41,6 +42,8 @@ vector<sp<IBinder> > workers; #define DUMP_PRICISION 3 +string trace_path = "/sys/kernel/debug/tracing"; + // the default value int no_process = 2; int iterations = 100; @@ -48,6 +51,23 @@ int payload_size = 16; int no_inherent = 0; int no_sync = 0; int verbose = 0; +int trace; + +bool traceIsOn() { + fstream file; + file.open(trace_path + "/tracing_on", ios::in); + char on; + file >> on; + file.close(); + return on == '1'; +} + +void traceStop() { + ofstream file; + file.open(trace_path + "/tracing_on", ios::out | ios::trunc); + file << '0' << endl; + file.close(); +} // the deadline latency that we are interested in uint64_t deadline_us = 2500; @@ -197,13 +217,29 @@ struct Results { uint64_t m_transactions = 0; uint64_t m_total_time = 0; uint64_t m_miss = 0; - + bool tracing; + Results(bool _tracing) : tracing(_tracing) { + } + inline bool miss_deadline(uint64_t nano) { + return nano > deadline_us * 1000; + } void add_time(uint64_t nano) { m_best = min(nano, m_best); m_worst = max(nano, m_worst); m_transactions += 1; m_total_time += nano; - if (nano > deadline_us * 1000) m_miss++; + if (miss_deadline(nano)) m_miss++; + if (miss_deadline(nano) && tracing) { + // There might be multiple process pair running the test concurrently + // each may execute following statements and only the first one actually + // stop the trace and any traceStop() afterthen has no effect. + traceStop(); + cout << endl; + cout << "deadline triggered: halt & stop trace" << endl; + cout << "log:" + trace_path + "/trace" << endl; + cout << endl; + exit(1); + } } void dump() { double best = (double)m_best / 1.0E6; @@ -212,8 +248,9 @@ struct Results { // FIXME: libjson? cout << std::setprecision(DUMP_PRICISION) << "{ \"avg\":" << setw(5) << left << average << ", \"wst\":" << setw(5) << left << worst - << ", \"bst\":" << setw(5) << left << best << ", \"miss\":" << m_miss - << "}"; + << ", \"bst\":" << setw(5) << left << best << ", \"miss\":" << setw(5) + << left << m_miss << ", \"meetR\":" << setw(3) << left + << (1.0 - (double)m_miss / m_transactions) << "}"; } }; @@ -272,7 +309,7 @@ static void thread_transaction(Results* results_fifo) { void worker_fx(int num, int no_process, int iterations, int payload_size, Pipe p) { int dummy; - Results results_other, results_fifo; + Results results_other(false), results_fifo(trace); // Create BinderWorkerService and for go. ProcessState::self()->startThreadPool(); @@ -389,8 +426,28 @@ int main(int argc, char** argv) { } if (string(argv[i]) == "-v") { verbose = 1; - i++; } + // The -trace argument is used like that: + // + // First start trace with atrace command as usual + // >atrace --async_start sched freq + // + // then use schd-dbg with -trace arguments + //./schd-dbg -trace -deadline_us 2500 + // + // This makes schd-dbg to stop trace once it detects a transaction + // duration over the deadline. By writing '0' to + // /sys/kernel/debug/tracing and halt the process. The tracelog is + // then available on /sys/kernel/debug/trace + if (string(argv[i]) == "-trace") { + trace = 1; + } + } + if (trace && !traceIsOn()) { + cout << "trace is not running" << endl; + cout << "check " << trace_path + "/tracing_on" << endl; + cout << "use atrace --async_start first" << endl; + exit(-1); } vector<Pipe> pipes; thread_dump("main"); diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h index aacc385fa0..dfeed50f3d 100644 --- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h +++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_client.h @@ -94,9 +94,12 @@ class BufferHubBuffer : public pdx::Client { } IonBuffer* buffer() { return &slices_[0]; } + const IonBuffer* buffer() const { return &slices_[0]; } + // If index is greater than or equal to slice_count(), the result is // undefined. IonBuffer* slice(size_t index) { return &slices_[index]; } + const IonBuffer* slice(size_t index) const { return &slices_[index]; } int slice_count() const { return static_cast<int>(slices_.size()); } int id() const { return id_; } @@ -171,9 +174,8 @@ class BufferProducer : public pdx::ClientBase<BufferProducer, BufferHubBuffer> { int Post(const LocalHandle& ready_fence) { return Post(ready_fence, nullptr, 0); } - template < - typename Meta, - typename = typename std::enable_if<!std::is_void<Meta>::value>::type> + template <typename Meta, typename = typename std::enable_if< + !std::is_void<Meta>::value>::type> int Post(const LocalHandle& ready_fence, const Meta& meta) { return Post(ready_fence, &meta, sizeof(meta)); } diff --git a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h index ed11551974..7ed024fb8c 100644 --- a/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h +++ b/libs/vr/libbufferhub/include/private/dvr/bufferhub_rpc.h @@ -119,6 +119,14 @@ class FenceHandle { using LocalFence = FenceHandle<pdx::LocalHandle>; using BorrowedFence = FenceHandle<pdx::BorrowedHandle>; +struct QueueInfo { + size_t meta_size_bytes; + int id; + + private: + PDX_SERIALIZABLE_MEMBERS(QueueInfo, meta_size_bytes, id); +}; + // BufferHub Service RPC interface. Defines the endpoints, op codes, and method // type signatures supported by bufferhubd. struct BufferHubRPC { @@ -151,6 +159,7 @@ struct BufferHubRPC { kOpConsumerSetIgnore, kOpCreateProducerQueue, kOpCreateConsumerQueue, + kOpGetQueueInfo, kOpProducerQueueAllocateBuffers, kOpProducerQueueDetachBuffer, kOpConsumerQueueImportBuffers, @@ -192,18 +201,19 @@ struct BufferHubRPC { // Buffer Queue Methods. PDX_REMOTE_METHOD(CreateProducerQueue, kOpCreateProducerQueue, - int(size_t meta_size_bytes, int usage_set_mask, - int usage_clear_mask, int usage_deny_set_mask, - int usage_deny_clear_mask)); + QueueInfo(size_t meta_size_bytes, int usage_set_mask, + int usage_clear_mask, int usage_deny_set_mask, + int usage_deny_clear_mask)); PDX_REMOTE_METHOD(CreateConsumerQueue, kOpCreateConsumerQueue, - std::pair<LocalChannelHandle, size_t>(Void)); + LocalChannelHandle(Void)); + PDX_REMOTE_METHOD(GetQueueInfo, kOpGetQueueInfo, QueueInfo(Void)); PDX_REMOTE_METHOD(ProducerQueueAllocateBuffers, kOpProducerQueueAllocateBuffers, std::vector<std::pair<LocalChannelHandle, size_t>>( int width, int height, int format, int usage, size_t slice_count, size_t buffer_count)); PDX_REMOTE_METHOD(ProducerQueueDetachBuffer, kOpProducerQueueDetachBuffer, - int(size_t slot)); + void(size_t slot)); PDX_REMOTE_METHOD(ConsumerQueueImportBuffers, kOpConsumerQueueImportBuffers, std::vector<std::pair<LocalChannelHandle, size_t>>(Void)); }; diff --git a/libs/vr/libbufferhub/include/private/dvr/ion_buffer.h b/libs/vr/libbufferhub/include/private/dvr/ion_buffer.h index e449cbdb02..ffc42d6123 100644 --- a/libs/vr/libbufferhub/include/private/dvr/ion_buffer.h +++ b/libs/vr/libbufferhub/include/private/dvr/ion_buffer.h @@ -60,21 +60,20 @@ class IonBuffer { int LockYUV(int usage, int x, int y, int width, int height, struct android_ycbcr* yuv); int Unlock(); - buffer_handle_t handle() const { if (buffer_.get()) return buffer_->handle; - else return nullptr; } - int width() const { if (buffer_.get()) return buffer_->getWidth(); - else return 0; } - int height() const { if (buffer_.get()) return buffer_->getHeight(); - else return 0; } - int layer_count() const { if (buffer_.get()) return buffer_->getLayerCount(); - else return 0; } - int stride() const { if (buffer_.get()) return buffer_->getStride(); - else return 0; } + + const sp<GraphicBuffer>& buffer() const { return buffer_; } + buffer_handle_t handle() const { + return buffer_.get() ? buffer_->handle : nullptr; + } + int width() const { return buffer_.get() ? buffer_->getWidth() : 0; } + int height() const { return buffer_.get() ? buffer_->getHeight() : 0; } + int layer_count() const { + return buffer_.get() ? buffer_->getLayerCount() : 0; + } + int stride() const { return buffer_.get() ? buffer_->getStride() : 0; } int layer_stride() const { return 0; } - int format() const { if (buffer_.get()) return buffer_->getPixelFormat(); - else return 0; } - int usage() const { if (buffer_.get()) return buffer_->getUsage(); - else return 0; } + int format() const { return buffer_.get() ? buffer_->getPixelFormat() : 0; } + int usage() const { return buffer_.get() ? buffer_->getUsage() : 0; } private: sp<GraphicBuffer> buffer_; diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp index 031401a1b6..e491abcd52 100644 --- a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp +++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp @@ -11,34 +11,36 @@ #include <pdx/file_handle.h> #include <private/dvr/bufferhub_rpc.h> +using android::pdx::ErrorStatus; +using android::pdx::LocalChannelHandle; +using android::pdx::Status; + namespace android { namespace dvr { -BufferHubQueue::BufferHubQueue(LocalChannelHandle channel_handle, - size_t meta_size) +BufferHubQueue::BufferHubQueue(LocalChannelHandle channel_handle) : Client{pdx::default_transport::ClientChannel::Create( std::move(channel_handle))}, - meta_size_(meta_size), - meta_buffer_tmp_(meta_size ? new uint8_t[meta_size] : nullptr), + meta_size_(0), buffers_(BufferHubQueue::kMaxQueueCapacity), epollhup_pending_(BufferHubQueue::kMaxQueueCapacity, false), available_buffers_(BufferHubQueue::kMaxQueueCapacity), fences_(BufferHubQueue::kMaxQueueCapacity), - capacity_(0) { + capacity_(0), + id_(-1) { Initialize(); } -BufferHubQueue::BufferHubQueue(const std::string& endpoint_path, - size_t meta_size) +BufferHubQueue::BufferHubQueue(const std::string& endpoint_path) : Client{pdx::default_transport::ClientChannelFactory::Create( endpoint_path)}, - meta_size_(meta_size), - meta_buffer_tmp_(meta_size ? new uint8_t[meta_size] : nullptr), + meta_size_(0), buffers_(BufferHubQueue::kMaxQueueCapacity), epollhup_pending_(BufferHubQueue::kMaxQueueCapacity, false), available_buffers_(BufferHubQueue::kMaxQueueCapacity), fences_(BufferHubQueue::kMaxQueueCapacity), - capacity_(0) { + capacity_(0), + id_(-1) { Initialize(); } @@ -55,26 +57,47 @@ void BufferHubQueue::Initialize() { BufferHubQueue::kEpollQueueEventIndex)}}; ret = epoll_fd_.Control(EPOLL_CTL_ADD, event_fd(), &event); if (ret < 0) { - ALOGE("Failed to register ConsumerQueue into epoll event: %s", + ALOGE("BufferHubQueue::Initialize: Failed to add event fd to epoll set: %s", strerror(-ret)); } } +Status<void> BufferHubQueue::ImportQueue() { + auto status = InvokeRemoteMethod<BufferHubRPC::GetQueueInfo>(); + if (!status) { + ALOGE("BufferHubQueue::ImportQueue: Failed to import queue: %s", + status.GetErrorMessage().c_str()); + return ErrorStatus(status.error()); + } else { + SetupQueue(status.get().meta_size_bytes, status.get().id); + return {}; + } +} + +void BufferHubQueue::SetupQueue(size_t meta_size_bytes, int id) { + meta_size_ = meta_size_bytes; + id_ = id; + meta_buffer_tmp_.reset(meta_size_ > 0 ? new uint8_t[meta_size_] : nullptr); +} + std::unique_ptr<ConsumerQueue> BufferHubQueue::CreateConsumerQueue() { - Status<std::pair<LocalChannelHandle, size_t>> status = - InvokeRemoteMethod<BufferHubRPC::CreateConsumerQueue>(); + if (auto status = CreateConsumerQueueHandle()) + return std::unique_ptr<ConsumerQueue>(new ConsumerQueue(status.take())); + else + return nullptr; +} +Status<LocalChannelHandle> BufferHubQueue::CreateConsumerQueueHandle() { + auto status = InvokeRemoteMethod<BufferHubRPC::CreateConsumerQueue>(); if (!status) { - ALOGE("Cannot create ConsumerQueue: %s", status.GetErrorMessage().c_str()); - return nullptr; + ALOGE( + "BufferHubQueue::CreateConsumerQueue: Failed to create consumer queue: " + "%s", + status.GetErrorMessage().c_str()); + return ErrorStatus(status.error()); } - auto return_value = status.take(); - - ALOGD_IF(TRACE, "BufferHubQueue::CreateConsumerQueue: meta_size_bytes=%zu", - return_value.second); - return ConsumerQueue::Create(std::move(return_value.first), - return_value.second); + return status; } bool BufferHubQueue::WaitForBuffers(int timeout) { @@ -89,7 +112,8 @@ bool BufferHubQueue::WaitForBuffers(int timeout) { } if (ret < 0 && ret != -EINTR) { - ALOGE("Failed to wait for buffers: %s", strerror(-ret)); + ALOGE("BufferHubQueue::WaitForBuffers: Failed to wait for buffers: %s", + strerror(-ret)); return false; } @@ -108,7 +132,8 @@ bool BufferHubQueue::WaitForBuffers(int timeout) { } else if (is_queue_event_index(index)) { HandleQueueEvent(events[i]); } else { - ALOGW("Unknown event index: %" PRId64, index); + ALOGW("BufferHubQueue::WaitForBuffers: Unknown event index: %" PRId64, + index); } } } @@ -134,7 +159,8 @@ void BufferHubQueue::HandleBufferEvent(size_t slot, const epoll_event& event) { if (events & EPOLLIN) { int ret = OnBufferReady(buffer, &fences_[slot]); if (ret < 0) { - ALOGE("Failed to set buffer ready: %s", strerror(-ret)); + ALOGE("BufferHubQueue::HandleBufferEvent: Failed to set buffer ready: %s", + strerror(-ret)); return; } Enqueue(buffer, slot); @@ -144,8 +170,8 @@ void BufferHubQueue::HandleBufferEvent(size_t slot, const epoll_event& event) { // epoll FD is cleaned up when the replacement consumer client is imported, // we shouldn't detach again if |epollhub_pending_[slot]| is set. ALOGW( - "Receives EPOLLHUP at slot: %zu, buffer event fd: %d, EPOLLHUP " - "pending: %d", + "BufferHubQueue::HandleBufferEvent: Received EPOLLHUP at slot: %zu, " + "buffer event fd: %d, EPOLLHUP pending: %d", slot, buffer->event_fd(), int{epollhup_pending_[slot]}); if (epollhup_pending_[slot]) { epollhup_pending_[slot] = false; @@ -153,7 +179,10 @@ void BufferHubQueue::HandleBufferEvent(size_t slot, const epoll_event& event) { DetachBuffer(slot); } } else { - ALOGW("Unknown event, slot=%zu, epoll events=%d", slot, events); + ALOGW( + "BufferHubQueue::HandleBufferEvent: Unknown event, slot=%zu, epoll " + "events=%d", + slot, events); } } @@ -169,12 +198,13 @@ void BufferHubQueue::HandleQueueEvent(const epoll_event& event) { if (events & EPOLLIN) { // Note that after buffer imports, if |count()| still returns 0, epoll // wait will be tried again to acquire the newly imported buffer. - int ret = OnBufferAllocated(); - if (ret < 0) { - ALOGE("Failed to import buffer: %s", strerror(-ret)); + auto buffer_status = OnBufferAllocated(); + if (!buffer_status) { + ALOGE("BufferHubQueue::HandleQueueEvent: Failed to import buffer: %s", + buffer_status.GetErrorMessage().c_str()); } } else { - ALOGW("Unknown epoll events=%d", events); + ALOGW("BufferHubQueue::HandleQueueEvent: Unknown epoll events=%d", events); } } @@ -233,7 +263,7 @@ int BufferHubQueue::DetachBuffer(size_t slot) { void BufferHubQueue::Enqueue(std::shared_ptr<BufferHubBuffer> buf, size_t slot) { if (count() == capacity_) { - ALOGE("Buffer queue is full!"); + ALOGE("BufferHubQueue::Enqueue: Buffer queue is full!"); return; } @@ -274,7 +304,7 @@ std::shared_ptr<BufferHubBuffer> BufferHubQueue::Dequeue(int timeout, available_buffers_.PopFront(); if (!buf) { - ALOGE("Dequeue: Buffer to be dequeued is nullptr"); + ALOGE("BufferHubQueue::Dequeue: Buffer to be dequeued is nullptr"); return nullptr; } @@ -289,15 +319,22 @@ std::shared_ptr<BufferHubBuffer> BufferHubQueue::Dequeue(int timeout, ProducerQueue::ProducerQueue(size_t meta_size) : ProducerQueue(meta_size, 0, 0, 0, 0) {} -ProducerQueue::ProducerQueue(LocalChannelHandle handle, size_t meta_size) - : BASE(std::move(handle), meta_size) {} +ProducerQueue::ProducerQueue(LocalChannelHandle handle) + : BASE(std::move(handle)) { + auto status = ImportQueue(); + if (!status) { + ALOGE("ProducerQueue::ProducerQueue: Failed to import queue: %s", + status.GetErrorMessage().c_str()); + Close(-status.error()); + } +} ProducerQueue::ProducerQueue(size_t meta_size, int usage_set_mask, int usage_clear_mask, int usage_deny_set_mask, int usage_deny_clear_mask) - : BASE(BufferHubRPC::kClientPath, meta_size) { + : BASE(BufferHubRPC::kClientPath) { auto status = InvokeRemoteMethod<BufferHubRPC::CreateProducerQueue>( - meta_size_, usage_set_mask, usage_clear_mask, usage_deny_set_mask, + meta_size, usage_set_mask, usage_clear_mask, usage_deny_set_mask, usage_deny_clear_mask); if (!status) { ALOGE("ProducerQueue::ProducerQueue: Failed to create producer queue: %s", @@ -305,12 +342,14 @@ ProducerQueue::ProducerQueue(size_t meta_size, int usage_set_mask, Close(-status.error()); return; } + + SetupQueue(status.get().meta_size_bytes, status.get().id); } int ProducerQueue::AllocateBuffer(int width, int height, int format, int usage, size_t slice_count, size_t* out_slot) { if (out_slot == nullptr) { - ALOGE("Parameter out_slot cannot be null."); + ALOGE("ProducerQueue::AllocateBuffer: Parameter out_slot cannot be null."); return -EINVAL; } @@ -362,7 +401,7 @@ int ProducerQueue::AddBuffer(const std::shared_ptr<BufferProducer>& buf, } int ProducerQueue::DetachBuffer(size_t slot) { - Status<int> status = + auto status = InvokeRemoteMethod<BufferHubRPC::ProducerQueueDetachBuffer>(slot); if (!status) { ALOGE( @@ -378,7 +417,9 @@ int ProducerQueue::DetachBuffer(size_t slot) { std::shared_ptr<BufferProducer> ProducerQueue::Dequeue( int timeout, size_t* slot, LocalHandle* release_fence) { if (slot == nullptr || release_fence == nullptr) { - ALOGE("invalid parameter, slot=%p, release_fence=%p", slot, release_fence); + ALOGE( + "ProducerQueue::Dequeue: invalid parameter, slot=%p, release_fence=%p", + slot, release_fence); return nullptr; } @@ -392,21 +433,27 @@ int ProducerQueue::OnBufferReady(std::shared_ptr<BufferHubBuffer> buf, return buffer->Gain(release_fence); } -ConsumerQueue::ConsumerQueue(LocalChannelHandle handle, size_t meta_size) - : BASE(std::move(handle), meta_size) { - // TODO(b/34387835) Import consumer queue in case the ProducerQueue we are +ConsumerQueue::ConsumerQueue(LocalChannelHandle handle) + : BufferHubQueue(std::move(handle)) { + auto status = ImportQueue(); + if (!status) { + ALOGE("ConsumerQueue::ConsumerQueue: Failed to import queue: %s", + status.GetErrorMessage().c_str()); + Close(-status.error()); + } + + // TODO(b/34387835) Import buffers in case the ProducerQueue we are // based on was not empty. } -int ConsumerQueue::ImportBuffers() { - Status<std::vector<std::pair<LocalChannelHandle, size_t>>> status = - InvokeRemoteMethod<BufferHubRPC::ConsumerQueueImportBuffers>(); +Status<size_t> ConsumerQueue::ImportBuffers() { + auto status = InvokeRemoteMethod<BufferHubRPC::ConsumerQueueImportBuffers>(); if (!status) { ALOGE( "ConsumerQueue::ImportBuffers failed to import consumer buffer through " "BufferBub, error: %s", status.GetErrorMessage().c_str()); - return -status.error(); + return ErrorStatus(status.error()); } int last_error = 0; @@ -431,7 +478,10 @@ int ConsumerQueue::ImportBuffers() { } } - return imported_buffers > 0 ? imported_buffers : last_error; + if (imported_buffers > 0) + return {imported_buffers}; + else + return ErrorStatus(-last_error); } int ConsumerQueue::AddBuffer(const std::shared_ptr<BufferConsumer>& buf, @@ -445,15 +495,17 @@ std::shared_ptr<BufferConsumer> ConsumerQueue::Dequeue( LocalHandle* acquire_fence) { if (meta_size != meta_size_) { ALOGE( - "metadata size (%zu) for the dequeuing buffer does not match metadata " - "size (%zu) for the queue.", + "ConsumerQueue::Dequeue: Metadata size (%zu) for the dequeuing buffer " + "does not match metadata size (%zu) for the queue.", meta_size, meta_size_); return nullptr; } if (slot == nullptr || meta == nullptr || acquire_fence == nullptr) { - ALOGE("invalid parameter, slot=%p, meta=%p, acquire_fence=%p", slot, meta, - acquire_fence); + ALOGE( + "ConsumerQueue::Dequeue: Invalid parameter, slot=%p, meta=%p, " + "acquire_fence=%p", + slot, meta, acquire_fence); return nullptr; } @@ -467,15 +519,19 @@ int ConsumerQueue::OnBufferReady(std::shared_ptr<BufferHubBuffer> buf, return buffer->Acquire(acquire_fence, meta_buffer_tmp_.get(), meta_size_); } -int ConsumerQueue::OnBufferAllocated() { - const int ret = ImportBuffers(); - if (ret == 0) { - ALOGW("No new buffer can be imported on buffer allocated event."); - } else if (ret < 0) { - ALOGE("Failed to import buffers on buffer allocated event."); +Status<void> ConsumerQueue::OnBufferAllocated() { + auto status = ImportBuffers(); + if (!status) { + ALOGE("ConsumerQueue::OnBufferAllocated: Failed to import buffers: %s", + status.GetErrorMessage().c_str()); + return ErrorStatus(status.error()); + } else if (status.get() == 0) { + ALOGW("ConsumerQueue::OnBufferAllocated: No new buffers allocated!"); + return ErrorStatus(ENOBUFS); + } else { + ALOGD_IF(TRACE, "Imported %zu consumer buffers.", status.get()); + return {}; } - ALOGD_IF(TRACE, "Imported %d consumer buffers.", ret); - return ret; } } // namespace dvr diff --git a/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_client.h b/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_client.h index 37cd8c733d..2b70c5b337 100644 --- a/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_client.h +++ b/libs/vr/libbufferhubqueue/include/private/dvr/buffer_hub_queue_client.h @@ -4,6 +4,7 @@ #include <gui/BufferQueueDefs.h> #include <pdx/client.h> +#include <pdx/status.h> #include <private/dvr/buffer_hub_client.h> #include <private/dvr/epoll_file_descriptor.h> #include <private/dvr/ring_buffer.h> @@ -41,6 +42,9 @@ class BufferHubQueue : public pdx::Client { // Return the default buffer format of this buffer queue. int32_t default_format() const { return default_format_; } + // Create a new consumer in handle form for immediate transport over RPC. + Status<LocalChannelHandle> CreateConsumerQueueHandle(); + // Return the number of buffers avaiable for dequeue. size_t count() const { return available_buffers_.GetSize(); } @@ -51,7 +55,7 @@ class BufferHubQueue : public pdx::Client { size_t metadata_size() const { return meta_size_; } // Return whether the buffer queue is alrady full. - bool is_full() const { return available_buffers_.IsFull(); } + bool is_full() const { return available_buffers_.IsFull(); } explicit operator bool() const { return epoll_fd_.IsValid(); } @@ -83,9 +87,18 @@ class BufferHubQueue : public pdx::Client { // timeout. static constexpr int kNoTimeOut = -1; + int id() const { return id_; } + protected: - BufferHubQueue(LocalChannelHandle channel, size_t meta_size); - BufferHubQueue(const std::string& endpoint_path, size_t meta_size); + BufferHubQueue(LocalChannelHandle channel); + BufferHubQueue(const std::string& endpoint_path); + + // Imports the queue parameters by querying BufferHub for the parameters for + // this channel. + Status<void> ImportQueue(); + + // Sets up the queue with the given parameters. + void SetupQueue(size_t meta_size_bytes_, int id); // Called by ProducerQueue::AddBuffer and ConsumerQueue::AddBuffer only. to // register a buffer for epoll and internal bookkeeping. @@ -112,7 +125,7 @@ class BufferHubQueue : public pdx::Client { LocalHandle* fence) = 0; // Called when a buffer is allocated remotely. - virtual int OnBufferAllocated() = 0; + virtual Status<void> OnBufferAllocated() { return {}; } // Data members to handle arbitrary metadata passed through BufferHub. It is // fair to enforce that all buffers in the same queue share the same metadata @@ -235,6 +248,9 @@ class BufferHubQueue : public pdx::Client { // Epoll fd used to wait for BufferHub events. EpollFileDescriptor epoll_fd_; + // Global id for the queue that is consistent across processes. + int id_; + BufferHubQueue(const BufferHubQueue&) = delete; void operator=(BufferHubQueue&) = delete; }; @@ -267,9 +283,8 @@ class ProducerQueue : public pdx::ClientBase<ProducerQueue, BufferHubQueue> { } // Import a |ProducerQueue| from a channel handle. - template <typename Meta> static std::unique_ptr<ProducerQueue> Import(LocalChannelHandle handle) { - return BASE::Create(std::move(handle), sizeof(Meta)); + return BASE::Create(std::move(handle)); } // Get a buffer producer. Note that the method doesn't check whether the @@ -311,18 +326,15 @@ class ProducerQueue : public pdx::ClientBase<ProducerQueue, BufferHubQueue> { // static template methods inherited from ClientBase, which take the same // arguments as the constructors. explicit ProducerQueue(size_t meta_size); - ProducerQueue(LocalChannelHandle handle, size_t meta_size); + ProducerQueue(LocalChannelHandle handle); ProducerQueue(size_t meta_size, int usage_set_mask, int usage_clear_mask, int usage_deny_set_mask, int usage_deny_clear_mask); int OnBufferReady(std::shared_ptr<BufferHubBuffer> buf, LocalHandle* release_fence) override; - - // Producer buffer is always allocated from the client (i.e. local) side. - int OnBufferAllocated() override { return 0; } }; -class ConsumerQueue : public pdx::ClientBase<ConsumerQueue, BufferHubQueue> { +class ConsumerQueue : public BufferHubQueue { public: // Get a buffer consumer. Note that the method doesn't check whether the // buffer slot has a valid buffer that has been imported already. When no @@ -333,10 +345,14 @@ class ConsumerQueue : public pdx::ClientBase<ConsumerQueue, BufferHubQueue> { BufferHubQueue::GetBuffer(slot)); } + // Import a |ConsumerQueue| from a channel handle. + static std::unique_ptr<ConsumerQueue> Import(LocalChannelHandle handle) { + return std::unique_ptr<ConsumerQueue>(new ConsumerQueue(std::move(handle))); + } + // Import newly created buffers from the service side. - // Returns number of buffers successfully imported; or negative error code - // when buffer import fails. - int ImportBuffers(); + // Returns number of buffers successfully imported or an error. + Status<size_t> ImportBuffers(); // Dequeue a consumer buffer to read. The returned buffer in |Acquired|'ed // mode, and caller should call Releasse() once it's done writing to release @@ -353,10 +369,11 @@ class ConsumerQueue : public pdx::ClientBase<ConsumerQueue, BufferHubQueue> { std::shared_ptr<BufferConsumer> Dequeue(int timeout, size_t* slot, void* meta, size_t meta_size, LocalHandle* acquire_fence); + private: - friend BASE; + friend BufferHubQueue; - ConsumerQueue(LocalChannelHandle handle, size_t meta_size); + ConsumerQueue(LocalChannelHandle handle); // Add a consumer buffer to populate the queue. Once added, a consumer buffer // is NOT available to use until the producer side |Post| it. |WaitForBuffers| @@ -367,7 +384,7 @@ class ConsumerQueue : public pdx::ClientBase<ConsumerQueue, BufferHubQueue> { int OnBufferReady(std::shared_ptr<BufferHubBuffer> buf, LocalHandle* acquire_fence) override; - int OnBufferAllocated() override; + Status<void> OnBufferAllocated() override; }; } // namespace dvr diff --git a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp index 811543d697..171577d940 100644 --- a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp +++ b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp @@ -22,16 +22,20 @@ constexpr int kBufferSliceCount = 1; // number of slices in each buffer class BufferHubQueueTest : public ::testing::Test { public: template <typename Meta> - void CreateQueues(int usage_set_mask = 0, int usage_clear_mask = 0, + bool CreateQueues(int usage_set_mask = 0, int usage_clear_mask = 0, int usage_deny_set_mask = 0, int usage_deny_clear_mask = 0) { producer_queue_ = ProducerQueue::Create<Meta>(usage_set_mask, usage_clear_mask, usage_deny_set_mask, usage_deny_clear_mask); - ASSERT_NE(nullptr, producer_queue_); + if (!producer_queue_) + return false; consumer_queue_ = producer_queue_->CreateConsumerQueue(); - ASSERT_NE(nullptr, consumer_queue_); + if (!consumer_queue_) + return false; + + return true; } void AllocateBuffer() { @@ -51,7 +55,7 @@ class BufferHubQueueTest : public ::testing::Test { TEST_F(BufferHubQueueTest, TestDequeue) { const size_t nb_dequeue_times = 16; - CreateQueues<size_t>(); + ASSERT_TRUE(CreateQueues<size_t>()); // Allocate only one buffer. AllocateBuffer(); @@ -77,7 +81,7 @@ TEST_F(BufferHubQueueTest, TestProducerConsumer) { size_t slot; uint64_t seq; - CreateQueues<uint64_t>(); + ASSERT_TRUE(CreateQueues<uint64_t>()); for (size_t i = 0; i < nb_buffer; i++) { AllocateBuffer(); @@ -127,7 +131,7 @@ struct TestMetadata { }; TEST_F(BufferHubQueueTest, TestMetadata) { - CreateQueues<TestMetadata>(); + ASSERT_TRUE(CreateQueues<TestMetadata>()); AllocateBuffer(); std::vector<TestMetadata> ms = { @@ -149,7 +153,7 @@ TEST_F(BufferHubQueueTest, TestMetadata) { } TEST_F(BufferHubQueueTest, TestMetadataMismatch) { - CreateQueues<int64_t>(); + ASSERT_TRUE(CreateQueues<int64_t>()); AllocateBuffer(); int64_t mi = 3; @@ -166,7 +170,7 @@ TEST_F(BufferHubQueueTest, TestMetadataMismatch) { } TEST_F(BufferHubQueueTest, TestEnqueue) { - CreateQueues<int64_t>(); + ASSERT_TRUE(CreateQueues<int64_t>()); AllocateBuffer(); size_t slot; @@ -181,7 +185,7 @@ TEST_F(BufferHubQueueTest, TestEnqueue) { } TEST_F(BufferHubQueueTest, TestAllocateBuffer) { - CreateQueues<int64_t>(); + ASSERT_TRUE(CreateQueues<int64_t>()); size_t s1; AllocateBuffer(); @@ -227,7 +231,7 @@ TEST_F(BufferHubQueueTest, TestAllocateBuffer) { TEST_F(BufferHubQueueTest, TestUsageSetMask) { const int set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN; - CreateQueues<int64_t>(set_mask, 0, 0, 0); + ASSERT_TRUE(CreateQueues<int64_t>(set_mask, 0, 0, 0)); // When allocation, leave out |set_mask| from usage bits on purpose. size_t slot; @@ -243,7 +247,7 @@ TEST_F(BufferHubQueueTest, TestUsageSetMask) { TEST_F(BufferHubQueueTest, TestUsageClearMask) { const int clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN; - CreateQueues<int64_t>(0, clear_mask, 0, 0); + ASSERT_TRUE(CreateQueues<int64_t>(0, clear_mask, 0, 0)); // When allocation, add |clear_mask| into usage bits on purpose. size_t slot; @@ -259,7 +263,7 @@ TEST_F(BufferHubQueueTest, TestUsageClearMask) { TEST_F(BufferHubQueueTest, TestUsageDenySetMask) { const int deny_set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN; - CreateQueues<int64_t>(0, 0, deny_set_mask, 0); + ASSERT_TRUE(CreateQueues<int64_t>(0, 0, deny_set_mask, 0)); // Now that |deny_set_mask| is illegal, allocation without those bits should // be able to succeed. @@ -278,7 +282,7 @@ TEST_F(BufferHubQueueTest, TestUsageDenySetMask) { TEST_F(BufferHubQueueTest, TestUsageDenyClearMask) { const int deny_clear_mask = GRALLOC_USAGE_SW_WRITE_OFTEN; - CreateQueues<int64_t>(0, 0, 0, deny_clear_mask); + ASSERT_TRUE(CreateQueues<int64_t>(0, 0, 0, deny_clear_mask)); // Now that clearing |deny_clear_mask| is illegal (i.e. setting these bits are // mandatory), allocation with those bits should be able to succeed. diff --git a/libs/vr/libdisplay/display_client.cpp b/libs/vr/libdisplay/display_client.cpp index 9952e59480..6d39cdb8c5 100644 --- a/libs/vr/libdisplay/display_client.cpp +++ b/libs/vr/libdisplay/display_client.cpp @@ -160,8 +160,7 @@ std::shared_ptr<ProducerQueue> DisplaySurfaceClient::GetProducerQueue() { return nullptr; } - producer_queue_ = - ProducerQueue::Import<DisplaySurfaceMetadata>(status.take()); + producer_queue_ = ProducerQueue::Import(status.take()); } return producer_queue_; } diff --git a/libs/vr/libdisplay/video_mesh_surface_client.cpp b/libs/vr/libdisplay/video_mesh_surface_client.cpp index 04cc19484b..a2307e547a 100644 --- a/libs/vr/libdisplay/video_mesh_surface_client.cpp +++ b/libs/vr/libdisplay/video_mesh_surface_client.cpp @@ -30,8 +30,7 @@ std::shared_ptr<ProducerQueue> VideoMeshSurfaceClient::GetProducerQueue() { return nullptr; } - producer_queue_ = - ProducerQueue::Import<VideoMeshSurfaceBufferMetadata>(status.take()); + producer_queue_ = ProducerQueue::Import(status.take()); } return producer_queue_; } diff --git a/libs/vr/libpdx/private/pdx/rpc/serialization.h b/libs/vr/libpdx/private/pdx/rpc/serialization.h index 9a012ede7d..f12aef1329 100644 --- a/libs/vr/libpdx/private/pdx/rpc/serialization.h +++ b/libs/vr/libpdx/private/pdx/rpc/serialization.h @@ -247,7 +247,7 @@ inline EnableIfEnum<T, std::size_t> GetSerializedSize(T v) { inline std::size_t GetSerializedSize(const EmptyVariant&); template <typename... Types> inline std::size_t GetSerializedSize(const Variant<Types...>&); -template <typename T, typename Enabled> +template <typename T, typename Enabled = EnableIfHasSerializableMembers<T>> inline constexpr std::size_t GetSerializedSize(const T&); template <typename T> inline constexpr std::size_t GetSerializedSize(const PointerWrapper<T>&); @@ -293,7 +293,7 @@ inline std::size_t GetSerializedSize(const Variant<Types...>& variant) { } // Overload for structs/classes with SerializableMembers defined. -template <typename T, typename Enabled = EnableIfHasSerializableMembers<T>> +template <typename T, typename Enabled> inline constexpr std::size_t GetSerializedSize(const T& value) { return SerializableTraits<T>::GetSerializedSize(value); } @@ -836,7 +836,7 @@ inline EnableIfEnum<T> SerializeObject(const T& value, MessageWriter* writer, inline void SerializeObject(const EmptyVariant&, MessageWriter*, void*&); template <typename... Types> inline void SerializeObject(const Variant<Types...>&, MessageWriter*, void*&); -template <typename T, typename Enabled> +template <typename T, typename Enabled = EnableIfHasSerializableMembers<T>> inline void SerializeObject(const T&, MessageWriter*, void*&); template <typename T> inline void SerializeObject(const PointerWrapper<T>&, MessageWriter*, void*&); @@ -887,7 +887,7 @@ inline void SerializeObject(const Variant<Types...>& variant, } // Overload for serializable structure/class types. -template <typename T, typename Enabled = EnableIfHasSerializableMembers<T>> +template <typename T, typename Enabled> inline void SerializeObject(const T& value, MessageWriter* writer, void*& buffer) { SerializableTraits<T>::SerializeObject(value, writer, buffer); @@ -1379,7 +1379,7 @@ inline EnableIfEnum<T, ErrorType> DeserializeObject(T* value, } // Forward declarations for nested definitions. -template <typename T, typename Enabled> +template <typename T, typename Enabled = EnableIfHasSerializableMembers<T>> inline ErrorType DeserializeObject(T*, MessageReader*, const void*&, const void*&); template <typename T> @@ -1438,7 +1438,7 @@ inline ErrorType DeserializeObject(Variant<Types...>*, const void*&); // Deserializes a Serializable type. -template <typename T, typename Enable = EnableIfHasSerializableMembers<T>> +template <typename T, typename Enable> inline ErrorType DeserializeObject(T* value, MessageReader* reader, const void*& start, const void*& end) { return SerializableTraits<T>::DeserializeObject(value, reader, start, end); diff --git a/libs/vr/libpdx/private/pdx/rpc/variant.h b/libs/vr/libpdx/private/pdx/rpc/variant.h index 09789e5a46..cb44a51b8d 100644 --- a/libs/vr/libpdx/private/pdx/rpc/variant.h +++ b/libs/vr/libpdx/private/pdx/rpc/variant.h @@ -39,15 +39,11 @@ using EnableIfNotConstructible = template <typename... Types> struct HasType : std::false_type {}; template <typename T, typename U> -struct HasType<T, U> : std::is_same<T, U> {}; +struct HasType<T, U> : std::is_same<std::decay_t<T>, std::decay_t<U>> {}; template <typename T, typename First, typename... Rest> struct HasType<T, First, Rest...> - : std::integral_constant< - bool, std::is_same<T, First>::value || HasType<T, Rest...>::value> {}; - -template <typename T, typename... Types> -using HasTypeIgnoreRef = - HasType<typename std::remove_reference<T>::type, Types...>; + : std::integral_constant<bool, HasType<T, First>::value || + HasType<T, Rest...>::value> {}; // Defines set operations on a set of Types... template <typename... Types> @@ -59,8 +55,8 @@ struct Set { struct IsSubset<T> : HasType<T, Types...> {}; template <typename First, typename... Rest> struct IsSubset<First, Rest...> - : std::integral_constant< - bool, IsSubset<First>::value && IsSubset<Rest...>::value> {}; + : std::integral_constant<bool, IsSubset<First>::value && + IsSubset<Rest...>::value> {}; }; // Determines the number of elements of Types... that are constructible from @@ -80,18 +76,18 @@ struct ConstructibleCount<From, First, Rest...> // Enable if T is an element of Types... template <typename R, typename T, typename... Types> using EnableIfElement = - typename std::enable_if<HasTypeIgnoreRef<T, Types...>::value, R>::type; + typename std::enable_if<HasType<T, Types...>::value, R>::type; // Enable if T is not an element of Types... template <typename R, typename T, typename... Types> using EnableIfNotElement = - typename std::enable_if<!HasTypeIgnoreRef<T, Types...>::value, R>::type; + typename std::enable_if<!HasType<T, Types...>::value, R>::type; // Enable if T is convertible to an element of Types... T is considered // convertible IIF a single element of Types... is assignable from T and T is // not a direct element of Types... template <typename R, typename T, typename... Types> using EnableIfConvertible = - typename std::enable_if<!HasTypeIgnoreRef<T, Types...>::value && + typename std::enable_if<!HasType<T, Types...>::value && ConstructibleCount<T, Types...>::value == 1, R>::type; @@ -102,7 +98,7 @@ using EnableIfConvertible = // in conversion. template <typename R, typename T, typename... Types> using EnableIfAssignable = - typename std::enable_if<HasTypeIgnoreRef<T, Types...>::value || + typename std::enable_if<HasType<T, Types...>::value || ConstructibleCount<T, Types...>::value == 1, R>::type; @@ -362,15 +358,13 @@ class Variant { template <typename T> using TypeTag = detail::TypeTag<T>; template <typename T> - using TypeTagIgnoreRef = TypeTag<typename std::remove_reference<T>::type>; + using DecayedTypeTag = TypeTag<std::decay_t<T>>; template <std::size_t I> using TypeForIndex = detail::TypeForIndex<I, Types...>; template <std::size_t I> using TypeTagForIndex = detail::TypeTagForIndex<I, Types...>; template <typename T> using HasType = detail::HasType<T, Types...>; - template <typename T> - using HasTypeIgnoreRef = detail::HasTypeIgnoreRef<T, Types...>; template <typename R, typename T> using EnableIfElement = detail::EnableIfElement<R, T, Types...>; template <typename R, typename T> @@ -381,13 +375,12 @@ class Variant { struct Direct {}; struct Convert {}; template <typename T> - using SelectConstructor = - detail::Select<HasTypeIgnoreRef<T>::value, Direct, Convert>; + using SelectConstructor = detail::Select<HasType<T>::value, Direct, Convert>; // Constructs by type tag when T is an direct element of Types... template <typename T> explicit Variant(T&& value, Direct) - : value_(0, &index_, TypeTagIgnoreRef<T>{}, std::forward<T>(value)) {} + : value_(0, &index_, DecayedTypeTag<T>{}, std::forward<T>(value)) {} // Conversion constructor when T is not a direct element of Types... template <typename T> explicit Variant(T&& value, Convert) @@ -421,7 +414,7 @@ class Variant { // convertible to multiple elements of Types. template <typename T> EnableIfElement<Variant&, T> operator=(T&& value) { - Assign(TypeTagIgnoreRef<T>{}, std::forward<T>(value)); + Assign(DecayedTypeTag<T>{}, std::forward<T>(value)); return *this; } @@ -487,7 +480,7 @@ class Variant { template <typename T> constexpr std::int32_t index_of() const { static_assert(HasType<T>::value, "T is not an element type of Variant."); - return value_.template index(TypeTag<T>{}); + return value_.template index(DecayedTypeTag<T>{}); } // Returns the index of the active type. If the Variant is empty -1 is @@ -509,14 +502,14 @@ class Variant { template <typename T> T* get() { if (is<T>()) - return &value_.template get(TypeTag<T>{}); + return &value_.template get(DecayedTypeTag<T>{}); else return nullptr; } template <typename T> const T* get() const { if (is<T>()) - return &value_.template get(TypeTag<T>{}); + return &value_.template get(DecayedTypeTag<T>{}); else return nullptr; } @@ -537,7 +530,7 @@ class Variant { private: std::int32_t index_ = kEmptyIndex; - detail::Union<Types...> value_; + detail::Union<std::decay_t<Types>...> value_; // Constructs an element from the given arguments and sets the Variant to the // resulting type. diff --git a/libs/vr/libpdx/thread_local_buffer_tests.cpp b/libs/vr/libpdx/thread_local_buffer_tests.cpp index 1747d798a1..6cdaf1071a 100644 --- a/libs/vr/libpdx/thread_local_buffer_tests.cpp +++ b/libs/vr/libpdx/thread_local_buffer_tests.cpp @@ -89,13 +89,13 @@ TEST(ThreadLocalBufferTest, ThreadSlots) { EXPECT_NE(id1, id2); } -// TODO(b/36456321): Fix this and enable it again. // Tests that thread-local buffers are allocated at the first buffer request. -TEST(ThreadLocalBufferTest, DISABLED_InitialValue) { +TEST(ThreadLocalBufferTest, InitialValue) { struct TypeTagX; using SendSlotX = ThreadLocalSlot<TypeTagX, kSendBufferIndex>; auto value1 = ThreadLocalBufferTest::GetSlotValue<SendSlotX>(); + MessageBuffer<SendSlotX>::GetBuffer(); auto value2 = ThreadLocalBufferTest::GetSlotValue<SendSlotX>(); EXPECT_EQ(0U, value1); diff --git a/libs/vr/libpdx/variant_tests.cpp b/libs/vr/libpdx/variant_tests.cpp index c30c055a7f..325f33f3c5 100644 --- a/libs/vr/libpdx/variant_tests.cpp +++ b/libs/vr/libpdx/variant_tests.cpp @@ -368,6 +368,13 @@ TEST(Variant, Constructor) { } { + TestType<int> i(1); + Variant<int, bool, float> v(i.get()); + ASSERT_TRUE(v.is<int>()); + EXPECT_EQ(1, std::get<int>(v)); + } + + { TestType<bool> b(true); Variant<int, bool, float> v(b.take()); ASSERT_TRUE(v.is<bool>()); @@ -375,6 +382,13 @@ TEST(Variant, Constructor) { } { + TestType<bool> b(true); + Variant<int, bool, float> v(b.get()); + ASSERT_TRUE(v.is<bool>()); + EXPECT_EQ(true, std::get<bool>(v)); + } + + { Variant<const char*> c("test"); Variant<std::string> s(c); ASSERT_TRUE(s.is<std::string>()); @@ -1060,8 +1074,8 @@ TEST(Variant, HasType) { EXPECT_FALSE((detail::HasType<char, int, float, bool>::value)); EXPECT_FALSE(detail::HasType<>::value); - EXPECT_TRUE((detail::HasTypeIgnoreRef<int&, int, float, bool>::value)); - EXPECT_FALSE((detail::HasTypeIgnoreRef<char&, int, float, bool>::value)); + EXPECT_TRUE((detail::HasType<int&, int, float, bool>::value)); + EXPECT_FALSE((detail::HasType<char&, int, float, bool>::value)); } TEST(Variant, Set) { diff --git a/libs/vr/libpdx_uds/client_channel.cpp b/libs/vr/libpdx_uds/client_channel.cpp index 4cbdb94f72..924335f1d0 100644 --- a/libs/vr/libpdx_uds/client_channel.cpp +++ b/libs/vr/libpdx_uds/client_channel.cpp @@ -67,7 +67,7 @@ struct TransactionState { ResponseHeader<LocalHandle> response; }; -Status<void> ReadAndDiscardData(int socket_fd, size_t size) { +Status<void> ReadAndDiscardData(const BorrowedHandle& socket_fd, size_t size) { while (size > 0) { // If there is more data to read in the message than the buffers provided // by the caller, read and discard the extra data from the socket. @@ -83,9 +83,10 @@ Status<void> ReadAndDiscardData(int socket_fd, size_t size) { return ErrorStatus(EIO); } -Status<void> SendRequest(int socket_fd, TransactionState* transaction_state, - int opcode, const iovec* send_vector, - size_t send_count, size_t max_recv_len) { +Status<void> SendRequest(const BorrowedHandle& socket_fd, + TransactionState* transaction_state, int opcode, + const iovec* send_vector, size_t send_count, + size_t max_recv_len) { size_t send_len = CountVectorSize(send_vector, send_count); InitRequest(&transaction_state->request, opcode, send_len, max_recv_len, false); @@ -95,7 +96,8 @@ Status<void> SendRequest(int socket_fd, TransactionState* transaction_state, return status; } -Status<void> ReceiveResponse(int socket_fd, TransactionState* transaction_state, +Status<void> ReceiveResponse(const BorrowedHandle& socket_fd, + TransactionState* transaction_state, const iovec* receive_vector, size_t receive_count, size_t max_recv_len) { auto status = ReceiveData(socket_fd, &transaction_state->response); @@ -164,7 +166,7 @@ Status<void> ClientChannel::SendImpulse(int opcode, const void* buffer, InitRequest(&request, opcode, length, 0, true); memcpy(request.impulse_payload.data(), buffer, length); - return SendData(channel_handle_.value(), request); + return SendData(BorrowedHandle{channel_handle_.value()}, request); } Status<int> ClientChannel::SendAndReceive(void* transaction_state, int opcode, @@ -182,11 +184,11 @@ Status<int> ClientChannel::SendAndReceive(void* transaction_state, int opcode, auto* state = static_cast<TransactionState*>(transaction_state); size_t max_recv_len = CountVectorSize(receive_vector, receive_count); - auto status = SendRequest(channel_handle_.value(), state, opcode, send_vector, - send_count, max_recv_len); + auto status = SendRequest(BorrowedHandle{channel_handle_.value()}, state, + opcode, send_vector, send_count, max_recv_len); if (status) { - status = ReceiveResponse(channel_handle_.value(), state, receive_vector, - receive_count, max_recv_len); + status = ReceiveResponse(BorrowedHandle{channel_handle_.value()}, state, + receive_vector, receive_count, max_recv_len); } if (!result.PropagateError(status)) { const int return_code = state->response.ret_code; diff --git a/libs/vr/libpdx_uds/client_channel_factory.cpp b/libs/vr/libpdx_uds/client_channel_factory.cpp index f059453ce0..323236d513 100644 --- a/libs/vr/libpdx_uds/client_channel_factory.cpp +++ b/libs/vr/libpdx_uds/client_channel_factory.cpp @@ -111,11 +111,11 @@ Status<std::unique_ptr<pdx::ClientChannel>> ClientChannelFactory::Connect( remote.sun_path); RequestHeader<BorrowedHandle> request; InitRequest(&request, opcodes::CHANNEL_OPEN, 0, 0, false); - status = SendData(socket_fd.Get(), request); + status = SendData(socket_fd.Borrow(), request); if (!status) return ErrorStatus(status.error()); ResponseHeader<LocalHandle> response; - status = ReceiveData(socket_fd.Get(), &response); + status = ReceiveData(socket_fd.Borrow(), &response); if (!status) return ErrorStatus(status.error()); int ref = response.ret_code; diff --git a/libs/vr/libpdx_uds/ipc_helper.cpp b/libs/vr/libpdx_uds/ipc_helper.cpp index fe5c98631e..d604f62b82 100644 --- a/libs/vr/libpdx_uds/ipc_helper.cpp +++ b/libs/vr/libpdx_uds/ipc_helper.cpp @@ -26,18 +26,19 @@ struct MessagePreamble { uint32_t fd_count{0}; }; -Status<void> SendPayload::Send(int socket_fd) { +Status<void> SendPayload::Send(const BorrowedHandle& socket_fd) { return Send(socket_fd, nullptr); } -Status<void> SendPayload::Send(int socket_fd, const ucred* cred) { +Status<void> SendPayload::Send(const BorrowedHandle& socket_fd, + const ucred* cred) { MessagePreamble preamble; preamble.magic = kMagicPreamble; preamble.data_size = buffer_.size(); preamble.fd_count = file_handles_.size(); - ssize_t ret = - RETRY_EINTR(send(socket_fd, &preamble, sizeof(preamble), MSG_NOSIGNAL)); + ssize_t ret = RETRY_EINTR( + send(socket_fd.Get(), &preamble, sizeof(preamble), MSG_NOSIGNAL)); if (ret < 0) return ErrorStatus(errno); if (ret != sizeof(preamble)) @@ -71,7 +72,7 @@ Status<void> SendPayload::Send(int socket_fd, const ucred* cred) { } } - ret = RETRY_EINTR(sendmsg(socket_fd, &msg, MSG_NOSIGNAL)); + ret = RETRY_EINTR(sendmsg(socket_fd.Get(), &msg, MSG_NOSIGNAL)); if (ret < 0) return ErrorStatus(errno); if (static_cast<size_t>(ret) != buffer_.size()) @@ -125,14 +126,15 @@ Status<ChannelReference> SendPayload::PushChannelHandle( return ErrorStatus{EOPNOTSUPP}; } -Status<void> ReceivePayload::Receive(int socket_fd) { +Status<void> ReceivePayload::Receive(const BorrowedHandle& socket_fd) { return Receive(socket_fd, nullptr); } -Status<void> ReceivePayload::Receive(int socket_fd, ucred* cred) { +Status<void> ReceivePayload::Receive(const BorrowedHandle& socket_fd, + ucred* cred) { MessagePreamble preamble; - ssize_t ret = - RETRY_EINTR(recv(socket_fd, &preamble, sizeof(preamble), MSG_WAITALL)); + ssize_t ret = RETRY_EINTR( + recv(socket_fd.Get(), &preamble, sizeof(preamble), MSG_WAITALL)); if (ret < 0) return ErrorStatus(errno); else if (ret == 0) @@ -157,7 +159,7 @@ Status<void> ReceivePayload::Receive(int socket_fd, ucred* cred) { msg.msg_control = alloca(msg.msg_controllen); } - ret = RETRY_EINTR(recvmsg(socket_fd, &msg, MSG_WAITALL)); + ret = RETRY_EINTR(recvmsg(socket_fd.Get(), &msg, MSG_WAITALL)); if (ret < 0) return ErrorStatus(errno); else if (ret == 0) @@ -219,8 +221,10 @@ bool ReceivePayload::GetChannelHandle(ChannelReference /*ref*/, return false; } -Status<void> SendData(int socket_fd, const void* data, size_t size) { - ssize_t size_written = RETRY_EINTR(send(socket_fd, data, size, MSG_NOSIGNAL)); +Status<void> SendData(const BorrowedHandle& socket_fd, const void* data, + size_t size) { + ssize_t size_written = + RETRY_EINTR(send(socket_fd.Get(), data, size, MSG_NOSIGNAL)); if (size_written < 0) return ErrorStatus(errno); if (static_cast<size_t>(size_written) != size) @@ -228,11 +232,13 @@ Status<void> SendData(int socket_fd, const void* data, size_t size) { return {}; } -Status<void> SendDataVector(int socket_fd, const iovec* data, size_t count) { +Status<void> SendDataVector(const BorrowedHandle& socket_fd, const iovec* data, + size_t count) { msghdr msg = {}; msg.msg_iov = const_cast<iovec*>(data); msg.msg_iovlen = count; - ssize_t size_written = RETRY_EINTR(sendmsg(socket_fd, &msg, MSG_NOSIGNAL)); + ssize_t size_written = + RETRY_EINTR(sendmsg(socket_fd.Get(), &msg, MSG_NOSIGNAL)); if (size_written < 0) return ErrorStatus(errno); if (static_cast<size_t>(size_written) != CountVectorSize(data, count)) @@ -240,8 +246,10 @@ Status<void> SendDataVector(int socket_fd, const iovec* data, size_t count) { return {}; } -Status<void> ReceiveData(int socket_fd, void* data, size_t size) { - ssize_t size_read = RETRY_EINTR(recv(socket_fd, data, size, MSG_WAITALL)); +Status<void> ReceiveData(const BorrowedHandle& socket_fd, void* data, + size_t size) { + ssize_t size_read = + RETRY_EINTR(recv(socket_fd.Get(), data, size, MSG_WAITALL)); if (size_read < 0) return ErrorStatus(errno); else if (size_read == 0) @@ -251,11 +259,12 @@ Status<void> ReceiveData(int socket_fd, void* data, size_t size) { return {}; } -Status<void> ReceiveDataVector(int socket_fd, const iovec* data, size_t count) { +Status<void> ReceiveDataVector(const BorrowedHandle& socket_fd, + const iovec* data, size_t count) { msghdr msg = {}; msg.msg_iov = const_cast<iovec*>(data); msg.msg_iovlen = count; - ssize_t size_read = RETRY_EINTR(recvmsg(socket_fd, &msg, MSG_WAITALL)); + ssize_t size_read = RETRY_EINTR(recvmsg(socket_fd.Get(), &msg, MSG_WAITALL)); if (size_read < 0) return ErrorStatus(errno); else if (size_read == 0) diff --git a/libs/vr/libpdx_uds/private/uds/ipc_helper.h b/libs/vr/libpdx_uds/private/uds/ipc_helper.h index 80530bf2ff..82950a2395 100644 --- a/libs/vr/libpdx_uds/private/uds/ipc_helper.h +++ b/libs/vr/libpdx_uds/private/uds/ipc_helper.h @@ -25,8 +25,8 @@ namespace uds { class SendPayload : public MessageWriter, public OutputResourceMapper { public: - Status<void> Send(int socket_fd); - Status<void> Send(int socket_fd, const ucred* cred); + Status<void> Send(const BorrowedHandle& socket_fd); + Status<void> Send(const BorrowedHandle& socket_fd, const ucred* cred); // MessageWriter void* GetNextWriteBufferSection(size_t size) override; @@ -50,8 +50,8 @@ class SendPayload : public MessageWriter, public OutputResourceMapper { class ReceivePayload : public MessageReader, public InputResourceMapper { public: - Status<void> Receive(int socket_fd); - Status<void> Receive(int socket_fd, ucred* cred); + Status<void> Receive(const BorrowedHandle& socket_fd); + Status<void> Receive(const BorrowedHandle& socket_fd, ucred* cred); // MessageReader BufferSection GetNextReadBufferSection() override; @@ -111,25 +111,27 @@ class ResponseHeader { }; template <typename T> -inline Status<void> SendData(int socket_fd, const T& data) { +inline Status<void> SendData(const BorrowedHandle& socket_fd, const T& data) { SendPayload payload; rpc::Serialize(data, &payload); return payload.Send(socket_fd); } template <typename FileHandleType> -inline Status<void> SendData(int socket_fd, +inline Status<void> SendData(const BorrowedHandle& socket_fd, const RequestHeader<FileHandleType>& request) { SendPayload payload; rpc::Serialize(request, &payload); return payload.Send(socket_fd, &request.cred); } -Status<void> SendData(int socket_fd, const void* data, size_t size); -Status<void> SendDataVector(int socket_fd, const iovec* data, size_t count); +Status<void> SendData(const BorrowedHandle& socket_fd, const void* data, + size_t size); +Status<void> SendDataVector(const BorrowedHandle& socket_fd, const iovec* data, + size_t count); template <typename T> -inline Status<void> ReceiveData(int socket_fd, T* data) { +inline Status<void> ReceiveData(const BorrowedHandle& socket_fd, T* data) { ReceivePayload payload; Status<void> status = payload.Receive(socket_fd); if (status && rpc::Deserialize(data, &payload) != rpc::ErrorCode::NO_ERROR) @@ -138,7 +140,7 @@ inline Status<void> ReceiveData(int socket_fd, T* data) { } template <typename FileHandleType> -inline Status<void> ReceiveData(int socket_fd, +inline Status<void> ReceiveData(const BorrowedHandle& socket_fd, RequestHeader<FileHandleType>* request) { ReceivePayload payload; Status<void> status = payload.Receive(socket_fd, &request->cred); @@ -147,8 +149,10 @@ inline Status<void> ReceiveData(int socket_fd, return status; } -Status<void> ReceiveData(int socket_fd, void* data, size_t size); -Status<void> ReceiveDataVector(int socket_fd, const iovec* data, size_t count); +Status<void> ReceiveData(const BorrowedHandle& socket_fd, void* data, + size_t size); +Status<void> ReceiveDataVector(const BorrowedHandle& socket_fd, + const iovec* data, size_t count); size_t CountVectorSize(const iovec* data, size_t count); void InitRequest(android::pdx::uds::RequestHeader<BorrowedHandle>* request, diff --git a/libs/vr/libpdx_uds/private/uds/service_endpoint.h b/libs/vr/libpdx_uds/private/uds/service_endpoint.h index 2b24f62b84..f747abc065 100644 --- a/libs/vr/libpdx_uds/private/uds/service_endpoint.h +++ b/libs/vr/libpdx_uds/private/uds/service_endpoint.h @@ -117,18 +117,20 @@ class Endpoint : public pdx::Endpoint { return next_message_id_.fetch_add(1, std::memory_order_relaxed); } - void BuildCloseMessage(int channel_id, Message* message); + void BuildCloseMessage(int32_t channel_id, Message* message); Status<void> AcceptConnection(Message* message); - Status<void> ReceiveMessageForChannel(int channel_id, Message* message); + Status<void> ReceiveMessageForChannel(const BorrowedHandle& channel_fd, + Message* message); Status<void> OnNewChannel(LocalHandle channel_fd); - Status<ChannelData*> OnNewChannelLocked(LocalHandle channel_fd, - Channel* channel_state); - Status<void> CloseChannelLocked(int channel_id); - Status<void> ReenableEpollEvent(int fd); - Channel* GetChannelState(int channel_id); - int GetChannelSocketFd(int channel_id); - int GetChannelEventFd(int channel_id); + Status<std::pair<int32_t, ChannelData*>> OnNewChannelLocked( + LocalHandle channel_fd, Channel* channel_state); + Status<void> CloseChannelLocked(int32_t channel_id); + Status<void> ReenableEpollEvent(const BorrowedHandle& channel_fd); + Channel* GetChannelState(int32_t channel_id); + BorrowedHandle GetChannelSocketFd(int32_t channel_id); + BorrowedHandle GetChannelEventFd(int32_t channel_id); + int32_t GetChannelId(const BorrowedHandle& channel_fd); std::string endpoint_path_; bool is_blocking_; @@ -137,7 +139,9 @@ class Endpoint : public pdx::Endpoint { LocalHandle epoll_fd_; mutable std::mutex channel_mutex_; - std::map<int, ChannelData> channels_; + std::map<int32_t, ChannelData> channels_; + std::map<int, int32_t> channel_fd_to_id_; + int32_t last_channel_id_{0}; Service* service_{nullptr}; std::atomic<uint32_t> next_message_id_; diff --git a/libs/vr/libpdx_uds/service_endpoint.cpp b/libs/vr/libpdx_uds/service_endpoint.cpp index 6f32867dca..65fd59fa34 100644 --- a/libs/vr/libpdx_uds/service_endpoint.cpp +++ b/libs/vr/libpdx_uds/service_endpoint.cpp @@ -220,9 +220,11 @@ Status<void> Endpoint::AcceptConnection(Message* message) { return ErrorStatus(errno); } - auto status = ReceiveMessageForChannel(channel_fd.Get(), message); + // Borrow the channel handle before we pass (move) it into OnNewChannel(). + BorrowedHandle borrowed_channel_handle = channel_fd.Borrow(); + auto status = OnNewChannel(std::move(channel_fd)); if (status) - status = OnNewChannel(std::move(channel_fd)); + status = ReceiveMessageForChannel(borrowed_channel_handle, message); return status; } @@ -247,7 +249,7 @@ Status<void> Endpoint::OnNewChannel(LocalHandle channel_fd) { return status; } -Status<Endpoint::ChannelData*> Endpoint::OnNewChannelLocked( +Status<std::pair<int32_t, Endpoint::ChannelData*>> Endpoint::OnNewChannelLocked( LocalHandle channel_fd, Channel* channel_state) { epoll_event event; event.events = EPOLLIN | EPOLLRDHUP | EPOLLONESHOT; @@ -259,19 +261,28 @@ Status<Endpoint::ChannelData*> Endpoint::OnNewChannelLocked( return ErrorStatus(errno); } ChannelData channel_data; - const int channel_id = channel_fd.Get(); channel_data.event_set.AddDataFd(channel_fd); channel_data.data_fd = std::move(channel_fd); channel_data.channel_state = channel_state; - auto pair = channels_.emplace(channel_id, std::move(channel_data)); - return &pair.first->second; + for (;;) { + // Try new channel IDs until we find one which is not already in the map. + if (last_channel_id_++ == std::numeric_limits<int32_t>::max()) + last_channel_id_ = 1; + auto iter = channels_.lower_bound(last_channel_id_); + if (iter == channels_.end() || iter->first != last_channel_id_) { + channel_fd_to_id_.emplace(channel_data.data_fd.Get(), last_channel_id_); + iter = channels_.emplace_hint(iter, last_channel_id_, + std::move(channel_data)); + return std::make_pair(last_channel_id_, &iter->second); + } + } } -Status<void> Endpoint::ReenableEpollEvent(int fd) { +Status<void> Endpoint::ReenableEpollEvent(const BorrowedHandle& fd) { epoll_event event; event.events = EPOLLIN | EPOLLRDHUP | EPOLLONESHOT; - event.data.fd = fd; - if (epoll_ctl(epoll_fd_.Get(), EPOLL_CTL_MOD, fd, &event) < 0) { + event.data.fd = fd.Get(); + if (epoll_ctl(epoll_fd_.Get(), EPOLL_CTL_MOD, fd.Get(), &event) < 0) { ALOGE( "Endpoint::ReenableEpollEvent: Failed to re-enable channel to " "endpoint: %s\n", @@ -286,16 +297,17 @@ Status<void> Endpoint::CloseChannel(int channel_id) { return CloseChannelLocked(channel_id); } -Status<void> Endpoint::CloseChannelLocked(int channel_id) { +Status<void> Endpoint::CloseChannelLocked(int32_t channel_id) { ALOGD_IF(TRACE, "Endpoint::CloseChannelLocked: channel_id=%d", channel_id); - auto channel_data = channels_.find(channel_id); - if (channel_data == channels_.end()) + auto iter = channels_.find(channel_id); + if (iter == channels_.end()) return ErrorStatus{EINVAL}; + int channel_fd = iter->second.data_fd.Get(); Status<void> status; epoll_event dummy; // See BUGS in man 2 epoll_ctl. - if (epoll_ctl(epoll_fd_.Get(), EPOLL_CTL_DEL, channel_id, &dummy) < 0) { + if (epoll_ctl(epoll_fd_.Get(), EPOLL_CTL_DEL, channel_fd, &dummy) < 0) { status.SetError(errno); ALOGE( "Endpoint::CloseChannelLocked: Failed to remove channel from endpoint: " @@ -305,7 +317,8 @@ Status<void> Endpoint::CloseChannelLocked(int channel_id) { status.SetValue(); } - channels_.erase(channel_data); + channel_fd_to_id_.erase(channel_fd); + channels_.erase(iter); return status; } @@ -348,10 +361,10 @@ Status<RemoteChannelHandle> Endpoint::PushChannel(Message* message, } std::lock_guard<std::mutex> autolock(channel_mutex_); - *channel_id = local_socket.Get(); auto channel_data = OnNewChannelLocked(std::move(local_socket), channel); if (!channel_data) return channel_data.error_status(); + *channel_id = channel_data.get().first; // Flags are ignored for now. // TODO(xiaohuit): Implement those. @@ -359,7 +372,7 @@ Status<RemoteChannelHandle> Endpoint::PushChannel(Message* message, auto* state = static_cast<MessageState*>(message->GetState()); Status<ChannelReference> ref = state->PushChannelHandle( remote_socket.Borrow(), - channel_data.get()->event_set.event_fd().Borrow()); + channel_data.get().second->event_set.event_fd().Borrow()); if (!ref) return ref.error_status(); state->sockets_to_close.push_back(std::move(remote_socket)); @@ -373,32 +386,42 @@ Status<int> Endpoint::CheckChannel(const Message* /*message*/, return ErrorStatus(EFAULT); } -Channel* Endpoint::GetChannelState(int channel_id) { +Channel* Endpoint::GetChannelState(int32_t channel_id) { std::lock_guard<std::mutex> autolock(channel_mutex_); auto channel_data = channels_.find(channel_id); return (channel_data != channels_.end()) ? channel_data->second.channel_state : nullptr; } -int Endpoint::GetChannelSocketFd(int channel_id) { +BorrowedHandle Endpoint::GetChannelSocketFd(int32_t channel_id) { std::lock_guard<std::mutex> autolock(channel_mutex_); + BorrowedHandle handle; auto channel_data = channels_.find(channel_id); - return (channel_data != channels_.end()) ? channel_data->second.data_fd.Get() - : -1; + if (channel_data != channels_.end()) + handle = channel_data->second.data_fd.Borrow(); + return handle; } -int Endpoint::GetChannelEventFd(int channel_id) { +BorrowedHandle Endpoint::GetChannelEventFd(int32_t channel_id) { std::lock_guard<std::mutex> autolock(channel_mutex_); + BorrowedHandle handle; auto channel_data = channels_.find(channel_id); - return (channel_data != channels_.end()) - ? channel_data->second.event_set.event_fd().Get() - : -1; + if (channel_data != channels_.end()) + handle = channel_data->second.event_set.event_fd().Borrow(); + return handle; +} + +int32_t Endpoint::GetChannelId(const BorrowedHandle& channel_fd) { + std::lock_guard<std::mutex> autolock(channel_mutex_); + auto iter = channel_fd_to_id_.find(channel_fd.Get()); + return (iter != channel_fd_to_id_.end()) ? iter->second : -1; } -Status<void> Endpoint::ReceiveMessageForChannel(int channel_id, - Message* message) { +Status<void> Endpoint::ReceiveMessageForChannel( + const BorrowedHandle& channel_fd, Message* message) { RequestHeader<LocalHandle> request; - auto status = ReceiveData(channel_id, &request); + int32_t channel_id = GetChannelId(channel_fd); + auto status = ReceiveData(channel_fd.Borrow(), &request); if (!status) { if (status.error() == ESHUTDOWN) { BuildCloseMessage(channel_id, message); @@ -434,12 +457,12 @@ Status<void> Endpoint::ReceiveMessageForChannel(int channel_id, state->request = std::move(request); if (request.send_len > 0 && !request.is_impulse) { state->request_data.resize(request.send_len); - status = ReceiveData(channel_id, state->request_data.data(), + status = ReceiveData(channel_fd, state->request_data.data(), state->request_data.size()); } if (status && request.is_impulse) - status = ReenableEpollEvent(channel_id); + status = ReenableEpollEvent(channel_fd); if (!status) { if (status.error() == ESHUTDOWN) { @@ -454,7 +477,7 @@ Status<void> Endpoint::ReceiveMessageForChannel(int channel_id, return status; } -void Endpoint::BuildCloseMessage(int channel_id, Message* message) { +void Endpoint::BuildCloseMessage(int32_t channel_id, Message* message) { ALOGD_IF(TRACE, "Endpoint::BuildCloseMessage: channel_id=%d", channel_id); MessageInfo info; info.pid = -1; @@ -496,22 +519,22 @@ Status<void> Endpoint::MessageReceive(Message* message) { auto status = AcceptConnection(message); if (!status) return status; - return ReenableEpollEvent(socket_fd_.Get()); + return ReenableEpollEvent(socket_fd_.Borrow()); } - int channel_id = event.data.fd; + BorrowedHandle channel_fd{event.data.fd}; if (event.events & (EPOLLRDHUP | EPOLLHUP)) { - BuildCloseMessage(channel_id, message); + BuildCloseMessage(GetChannelId(channel_fd), message); return {}; } - return ReceiveMessageForChannel(channel_id, message); + return ReceiveMessageForChannel(channel_fd, message); } Status<void> Endpoint::MessageReply(Message* message, int return_code) { - const int channel_id = message->GetChannelId(); - const int channel_socket = GetChannelSocketFd(channel_id); - if (channel_socket < 0) + const int32_t channel_id = message->GetChannelId(); + auto channel_socket = GetChannelSocketFd(channel_id); + if (!channel_socket) return ErrorStatus{EBADF}; auto* state = static_cast<MessageState*>(message->GetState()); @@ -524,8 +547,7 @@ Status<void> Endpoint::MessageReply(Message* message, int return_code) { return CloseChannel(channel_id); } else { // Reply with the event fd. - auto push_status = state->PushFileHandle( - BorrowedHandle{GetChannelEventFd(channel_socket)}); + auto push_status = state->PushFileHandle(GetChannelEventFd(channel_id)); state->response_data.clear(); // Just in case... if (!push_status) return push_status.error_status(); diff --git a/libs/vr/libvrsensor/include/private/dvr/latency_model.h b/libs/vr/libvrsensor/include/private/dvr/latency_model.h index 1bb3c4fdd8..40b4638d69 100644 --- a/libs/vr/libvrsensor/include/private/dvr/latency_model.h +++ b/libs/vr/libvrsensor/include/private/dvr/latency_model.h @@ -6,23 +6,21 @@ namespace android { namespace dvr { -// This class holds a rolling average of the sensor latency. +// This class models the latency from sensors. It will look at the first +// window_size measurements and return their average after that. class LatencyModel { public: - LatencyModel(size_t window_size, double weight_mass_in_window); + LatencyModel(size_t window_size); ~LatencyModel() = default; void AddLatency(int64_t latency_ns); - int64_t CurrentLatencyEstimate() const { - return static_cast<int64_t>(rolling_average_); - } + int64_t CurrentLatencyEstimate() const { return latency_; } private: - // The rolling average of the latencies. - double rolling_average_ = 0; - - // The alpha parameter for an exponential moving average. - double alpha_; + size_t window_size_; + int64_t latency_sum_ = 0; + size_t num_summed_ = 0; + int64_t latency_ = 0; }; } // namespace dvr diff --git a/libs/vr/libvrsensor/latency_model.cpp b/libs/vr/libvrsensor/latency_model.cpp index 8233889383..d3a45210a7 100644 --- a/libs/vr/libvrsensor/latency_model.cpp +++ b/libs/vr/libvrsensor/latency_model.cpp @@ -5,28 +5,19 @@ namespace android { namespace dvr { -LatencyModel::LatencyModel(size_t window_size, double weight_mass_in_window) { - // Compute an alpha so the weight of the last window_size measurements is - // weight_mass_in_window of the total weights. - - // The weight in a series of k measurements: - // alpha + (1 + (1 - alpha) + (1 - alpha)^2 + ... (1 - alpha)^k-1) - // = alpha x (1 - (1 - alpha) ^ k) / alpha - // = 1 - (1 - alpha) ^ k - // weight_mass_in_window = 1 - (1 - alpha) ^ k / lim_k->inf (1 - alpha) ^ k - // weight_mass_in_window = 1 - (1 - alpha) ^ k / 1 - // 1 - weight_mass_in_window = (1 - alpha) ^ k - // log(1 - weight_mass_in_window) = k * log(1 - alpha) - // 10 ^ (log(1 - weight_mass_in_window) / k) = 1 - alpha - // alpha = 1 - 10 ^ (log(1 - weight_mass_in_window) / k) - // alpha = 1 - 10 ^ (log(1 - weight_mass_in_window) / window_size) - - alpha_ = 1 - std::pow(10.0, std::log10(1 - weight_mass_in_window) / - static_cast<double>(window_size)); -} +LatencyModel::LatencyModel(size_t window_size) : window_size_(window_size) {} void LatencyModel::AddLatency(int64_t latency_ns) { - rolling_average_ = latency_ns * alpha_ + rolling_average_ * (1 - alpha_); + // Not enough samples yet? + if (num_summed_ < window_size_) { + // Accumulate. + latency_sum_ += latency_ns; + + // Have enough samples for latency estimate? + if (++num_summed_ == window_size_) { + latency_ = latency_sum_ / window_size_; + } + } } } // namespace dvr diff --git a/services/vr/bufferhubd/buffer_hub.cpp b/services/vr/bufferhubd/buffer_hub.cpp index de4950ea81..2ce60e57ac 100644 --- a/services/vr/bufferhubd/buffer_hub.cpp +++ b/services/vr/bufferhubd/buffer_hub.cpp @@ -16,7 +16,9 @@ #include "producer_queue_channel.h" using android::pdx::Channel; +using android::pdx::ErrorStatus; using android::pdx::Message; +using android::pdx::Status; using android::pdx::rpc::DispatchRemoteMethod; using android::pdx::default_transport::Endpoint; @@ -28,9 +30,7 @@ BufferHubService::BufferHubService() BufferHubService::~BufferHubService() {} -bool BufferHubService::IsInitialized() const { - return BASE::IsInitialized(); -} +bool BufferHubService::IsInitialized() const { return BASE::IsInitialized(); } std::string BufferHubService::DumpState(size_t /*max_length*/) { std::ostringstream stream; @@ -374,7 +374,7 @@ int BufferHubService::OnGetPersistentBuffer(Message& message, } } -int BufferHubService::OnCreateProducerQueue( +Status<QueueInfo> BufferHubService::OnCreateProducerQueue( pdx::Message& message, size_t meta_size_bytes, int usage_set_mask, int usage_clear_mask, int usage_deny_set_mask, int usage_deny_clear_mask) { // Use the producer channel id as the global queue id. @@ -386,7 +386,7 @@ int BufferHubService::OnCreateProducerQueue( if (const auto channel = message.GetChannel<BufferHubChannel>()) { ALOGE("BufferHubService::OnCreateProducerQueue: already created: queue=%d", queue_id); - return -EALREADY; + return ErrorStatus(EALREADY); } int error; @@ -394,10 +394,10 @@ int BufferHubService::OnCreateProducerQueue( this, queue_id, meta_size_bytes, usage_set_mask, usage_clear_mask, usage_deny_set_mask, usage_deny_clear_mask, &error)) { message.SetChannel(producer_channel); - return 0; + return {{meta_size_bytes, queue_id}}; } else { ALOGE("BufferHubService::OnCreateBuffer: Failed to create producer!!"); - return error; + return ErrorStatus(-error); } } diff --git a/services/vr/bufferhubd/buffer_hub.h b/services/vr/bufferhubd/buffer_hub.h index 8a7dca843d..150e399cdd 100644 --- a/services/vr/bufferhubd/buffer_hub.h +++ b/services/vr/bufferhubd/buffer_hub.h @@ -7,6 +7,7 @@ #include <hardware/gralloc.h> #include <pdx/service.h> +#include <private/dvr/bufferhub_rpc.h> namespace android { namespace dvr { @@ -73,9 +74,9 @@ class BufferHubChannel : public pdx::Channel { slice_count(slice_count), name(name) {} - BufferInfo(int id, size_t consumer_count, size_t capacity, int usage_set_mask, - int usage_clear_mask, int usage_deny_set_mask, - int usage_deny_clear_mask) + BufferInfo(int id, size_t consumer_count, size_t capacity, + int usage_set_mask, int usage_clear_mask, + int usage_deny_set_mask, int usage_deny_clear_mask) : id(id), type(kProducerQueueType), consumer_count(consumer_count), @@ -168,9 +169,9 @@ class BufferHubService : public pdx::ServiceBase<BufferHubService> { int format, int usage, size_t meta_size_bytes, size_t slice_count); int OnGetPersistentBuffer(pdx::Message& message, const std::string& name); - int OnCreateProducerQueue(pdx::Message& message, size_t meta_size_bytes, - int usage_set_mask, int usage_clear_mask, - int usage_deny_set_mask, int usage_deny_clear_mask); + pdx::Status<QueueInfo> OnCreateProducerQueue( + pdx::Message& message, size_t meta_size_bytes, int usage_set_mask, + int usage_clear_mask, int usage_deny_set_mask, int usage_deny_clear_mask); BufferHubService(const BufferHubService&) = delete; void operator=(const BufferHubService&) = delete; diff --git a/services/vr/bufferhubd/consumer_queue_channel.cpp b/services/vr/bufferhubd/consumer_queue_channel.cpp index ae87acd1d8..cc16f1f483 100644 --- a/services/vr/bufferhubd/consumer_queue_channel.cpp +++ b/services/vr/bufferhubd/consumer_queue_channel.cpp @@ -40,6 +40,11 @@ bool ConsumerQueueChannel::HandleMessage(Message& message) { *producer, &ProducerQueueChannel::OnCreateConsumerQueue, message); return true; + case BufferHubRPC::GetQueueInfo::Opcode: + DispatchRemoteMethod<BufferHubRPC::GetQueueInfo>( + *producer, &ProducerQueueChannel::OnGetQueueInfo, message); + return true; + case BufferHubRPC::ConsumerQueueImportBuffers::Opcode: DispatchRemoteMethod<BufferHubRPC::ConsumerQueueImportBuffers>( *this, &ConsumerQueueChannel::OnConsumerQueueImportBuffers, message); diff --git a/services/vr/bufferhubd/producer_queue_channel.cpp b/services/vr/bufferhubd/producer_queue_channel.cpp index 7952642456..7741058fce 100644 --- a/services/vr/bufferhubd/producer_queue_channel.cpp +++ b/services/vr/bufferhubd/producer_queue_channel.cpp @@ -3,6 +3,9 @@ #include "consumer_queue_channel.h" #include "producer_channel.h" +using android::pdx::ErrorStatus; +using android::pdx::Message; +using android::pdx::Status; using android::pdx::RemoteChannelHandle; using android::pdx::rpc::DispatchRemoteMethod; @@ -58,6 +61,11 @@ bool ProducerQueueChannel::HandleMessage(Message& message) { *this, &ProducerQueueChannel::OnCreateConsumerQueue, message); return true; + case BufferHubRPC::GetQueueInfo::Opcode: + DispatchRemoteMethod<BufferHubRPC::GetQueueInfo>( + *this, &ProducerQueueChannel::OnGetQueueInfo, message); + return true; + case BufferHubRPC::ProducerQueueAllocateBuffers::Opcode: DispatchRemoteMethod<BufferHubRPC::ProducerQueueAllocateBuffers>( *this, &ProducerQueueChannel::OnProducerQueueAllocateBuffers, @@ -84,8 +92,8 @@ BufferHubChannel::BufferInfo ProducerQueueChannel::GetBufferInfo() const { usage_deny_clear_mask_); } -std::pair<RemoteChannelHandle, size_t> -ProducerQueueChannel::OnCreateConsumerQueue(Message& message) { +Status<RemoteChannelHandle> ProducerQueueChannel::OnCreateConsumerQueue( + Message& message) { ATRACE_NAME("ProducerQueueChannel::OnCreateConsumerQueue"); ALOGD_IF(TRACE, "ProducerQueueChannel::OnCreateConsumerQueue: channel_id=%d", channel_id()); @@ -97,7 +105,7 @@ ProducerQueueChannel::OnCreateConsumerQueue(Message& message) { "ProducerQueueChannel::OnCreateConsumerQueue: failed to push consumer " "channel: %s", status.GetErrorMessage().c_str()); - REPLY_ERROR_RETURN(message, status.error(), {}); + return ErrorStatus(ENOMEM); } const auto channel_status = service()->SetChannel( @@ -108,13 +116,17 @@ ProducerQueueChannel::OnCreateConsumerQueue(Message& message) { "ProducerQueueChannel::OnCreateConsumerQueue: failed to set new " "consumer channel: %s", channel_status.GetErrorMessage().c_str()); - REPLY_ERROR_RETURN(message, channel_status.error(), {}); + return ErrorStatus(ENOMEM); } - return std::make_pair(status.take(), meta_size_bytes_); + return {status.take()}; +} + +Status<QueueInfo> ProducerQueueChannel::OnGetQueueInfo(Message&) { + return {{meta_size_bytes_, buffer_id()}}; } -std::vector<std::pair<RemoteChannelHandle, size_t>> +Status<std::vector<std::pair<RemoteChannelHandle, size_t>>> ProducerQueueChannel::OnProducerQueueAllocateBuffers(Message& message, int width, int height, int format, int usage, @@ -135,7 +147,7 @@ ProducerQueueChannel::OnProducerQueueAllocateBuffers(Message& message, "not permitted. Violating usage_deny_set_mask, the following bits " "shall not be set: %d.", usage, usage_deny_set_mask_); - REPLY_ERROR_RETURN(message, EINVAL, buffer_handles); + return ErrorStatus(EINVAL); } if (~usage & usage_deny_clear_mask_) { @@ -144,7 +156,7 @@ ProducerQueueChannel::OnProducerQueueAllocateBuffers(Message& message, "not permitted. Violating usage_deny_clear_mask, the following bits " "must be set: %d.", usage, usage_deny_clear_mask_); - REPLY_ERROR_RETURN(message, EINVAL, buffer_handles); + return ErrorStatus(EINVAL); } // Force set mask and clear mask. Note that |usage_set_mask_| takes precedence @@ -152,24 +164,24 @@ ProducerQueueChannel::OnProducerQueueAllocateBuffers(Message& message, int effective_usage = (usage & ~usage_clear_mask_) | usage_set_mask_; for (size_t i = 0; i < buffer_count; i++) { - auto buffer_handle_slot = AllocateBuffer(message, width, height, format, - effective_usage, slice_count); - if (!buffer_handle_slot.first) { + auto status = AllocateBuffer(message, width, height, format, + effective_usage, slice_count); + if (!status) { ALOGE( - "ProducerQueueChannel::OnProducerQueueAllocateBuffers: failed to " + "ProducerQueueChannel::OnProducerQueueAllocateBuffers: Failed to " "allocate new buffer."); - REPLY_ERROR_RETURN(message, ENOMEM, buffer_handles); + return ErrorStatus(status.error()); } - buffer_handles.emplace_back(std::move(buffer_handle_slot.first), - buffer_handle_slot.second); + buffer_handles.push_back(status.take()); } - return buffer_handles; + return {std::move(buffer_handles)}; } -std::pair<RemoteChannelHandle, size_t> ProducerQueueChannel::AllocateBuffer( - Message& message, int width, int height, int format, int usage, - size_t slice_count) { +Status<std::pair<RemoteChannelHandle, size_t>> +ProducerQueueChannel::AllocateBuffer(Message& message, int width, int height, + int format, int usage, + size_t slice_count) { ATRACE_NAME("ProducerQueueChannel::AllocateBuffer"); ALOGD_IF(TRACE, "ProducerQueueChannel::AllocateBuffer: producer_channel_id=%d", @@ -177,7 +189,7 @@ std::pair<RemoteChannelHandle, size_t> ProducerQueueChannel::AllocateBuffer( if (capacity_ >= BufferHubRPC::kMaxQueueCapacity) { ALOGE("ProducerQueueChannel::AllocateBuffer: reaches kMaxQueueCapacity."); - return {}; + return ErrorStatus(E2BIG); } // Here we are creating a new BufferHubBuffer, initialize the producer @@ -189,7 +201,7 @@ std::pair<RemoteChannelHandle, size_t> ProducerQueueChannel::AllocateBuffer( if (!status) { ALOGE("ProducerQueueChannel::AllocateBuffer: failed to push channel: %s", status.GetErrorMessage().c_str()); - return {}; + return ErrorStatus(status.error()); } ALOGD_IF(TRACE, @@ -199,14 +211,14 @@ std::pair<RemoteChannelHandle, size_t> ProducerQueueChannel::AllocateBuffer( auto buffer_handle = status.take(); int error; - const auto producer_channel = ProducerChannel::Create( - service(), buffer_id, width, height, format, usage, - meta_size_bytes_, slice_count, &error); + const auto producer_channel = + ProducerChannel::Create(service(), buffer_id, width, height, format, + usage, meta_size_bytes_, slice_count, &error); if (!producer_channel) { ALOGE( "ProducerQueueChannel::AllocateBuffer: Failed to create " "BufferHubBuffer producer!!"); - return {}; + return ErrorStatus(ENOMEM); } ALOGD_IF( @@ -221,7 +233,7 @@ std::pair<RemoteChannelHandle, size_t> ProducerQueueChannel::AllocateBuffer( "ProducerQueueChannel::AllocateBuffer: failed to set producer channel " "for new BufferHubBuffer: %s", channel_status.GetErrorMessage().c_str()); - return {}; + return ErrorStatus(ENOMEM); } // Register the newly allocated buffer's channel_id into the first empty @@ -235,7 +247,7 @@ std::pair<RemoteChannelHandle, size_t> ProducerQueueChannel::AllocateBuffer( ALOGE( "ProducerQueueChannel::AllocateBuffer: Cannot find empty slot for new " "buffer allocation."); - return {}; + return ErrorStatus(E2BIG); } buffers_[slot] = producer_channel; @@ -250,29 +262,29 @@ std::pair<RemoteChannelHandle, size_t> ProducerQueueChannel::AllocateBuffer( consumer_channel->RegisterNewBuffer(producer_channel, slot); } - return {std::move(buffer_handle), slot}; + return {{std::move(buffer_handle), slot}}; } -int ProducerQueueChannel::OnProducerQueueDetachBuffer(Message& /*message*/, - size_t slot) { +Status<void> ProducerQueueChannel::OnProducerQueueDetachBuffer( + Message& /*message*/, size_t slot) { if (buffers_[slot].expired()) { ALOGE( "ProducerQueueChannel::OnProducerQueueDetachBuffer: trying to detach " "an invalid buffer producer at slot %zu", slot); - return -EINVAL; + return ErrorStatus(EINVAL); } if (capacity_ == 0) { ALOGE( "ProducerQueueChannel::OnProducerQueueDetachBuffer: trying to detach a " "buffer producer while the queue's capacity is already zero."); - return -EINVAL; + return ErrorStatus(EINVAL); } buffers_[slot].reset(); capacity_--; - return 0; + return {}; } void ProducerQueueChannel::AddConsumer(ConsumerQueueChannel* channel) { diff --git a/services/vr/bufferhubd/producer_queue_channel.h b/services/vr/bufferhubd/producer_queue_channel.h index 49611d409a..a12a37d6be 100644 --- a/services/vr/bufferhubd/producer_queue_channel.h +++ b/services/vr/bufferhubd/producer_queue_channel.h @@ -3,6 +3,7 @@ #include "buffer_hub.h" +#include <pdx/status.h> #include <private/dvr/bufferhub_rpc.h> namespace android { @@ -10,17 +11,14 @@ namespace dvr { class ProducerQueueChannel : public BufferHubChannel { public: - using Message = pdx::Message; - using RemoteChannelHandle = pdx::RemoteChannelHandle; - static std::shared_ptr<ProducerQueueChannel> Create( BufferHubService* service, int channel_id, size_t meta_size_bytes, int usage_set_mask, int usage_clear_mask, int usage_deny_set_mask, int usage_deny_clear_mask, int* error); ~ProducerQueueChannel() override; - bool HandleMessage(Message& message) override; - void HandleImpulse(Message& message) override; + bool HandleMessage(pdx::Message& message) override; + void HandleImpulse(pdx::Message& message) override; BufferInfo GetBufferInfo() const override; @@ -28,19 +26,22 @@ class ProducerQueueChannel : public BufferHubChannel { // producer queue. // Returns a handle for the service channel, as well as the size of the // metadata associated with the queue. - std::pair<RemoteChannelHandle, size_t> OnCreateConsumerQueue( - Message& message); + pdx::Status<pdx::RemoteChannelHandle> OnCreateConsumerQueue( + pdx::Message& message); + + pdx::Status<QueueInfo> OnGetQueueInfo(pdx::Message& message); // Allocate a new BufferHubProducer according to the input spec. Client may // handle this as if a new producer is created through kOpCreateBuffer. - std::vector<std::pair<RemoteChannelHandle, size_t>> - OnProducerQueueAllocateBuffers(Message& message, int width, int height, + pdx::Status<std::vector<std::pair<pdx::RemoteChannelHandle, size_t>>> + OnProducerQueueAllocateBuffers(pdx::Message& message, int width, int height, int format, int usage, size_t slice_count, size_t buffer_count); // Detach a BufferHubProducer indicated by |slot|. Note that the buffer must // be in Gain'ed state for the producer queue to detach. - int OnProducerQueueDetachBuffer(Message& message, size_t slot); + pdx::Status<void> OnProducerQueueDetachBuffer(pdx::Message& message, + size_t slot); void AddConsumer(ConsumerQueueChannel* channel); void RemoveConsumer(ConsumerQueueChannel* channel); @@ -56,10 +57,9 @@ class ProducerQueueChannel : public BufferHubChannel { // and our return type is a RemoteChannelHandle. // Returns the remote channdel handle and the slot number for the newly // allocated buffer. - std::pair<RemoteChannelHandle, size_t> AllocateBuffer(Message& message, - int width, int height, - int format, int usage, - size_t slice_count); + pdx::Status<std::pair<pdx::RemoteChannelHandle, size_t>> AllocateBuffer( + pdx::Message& message, int width, int height, int format, int usage, + size_t slice_count); // Size of the meta data associated with all the buffers allocated from the // queue. Now we assume the metadata size is immutable once the queue is diff --git a/services/vr/sensord/pose_service.cpp b/services/vr/sensord/pose_service.cpp index 7534732ec2..e3f8171169 100644 --- a/services/vr/sensord/pose_service.cpp +++ b/services/vr/sensord/pose_service.cpp @@ -65,8 +65,7 @@ static constexpr char kPoseRingBufferName[] = "PoseService:RingBuffer"; static constexpr int kDatasetIdLength = 36; static constexpr char kDatasetIdChars[] = "0123456789abcdef-"; -static constexpr int kLatencyWindowSize = 100; -static constexpr double kLatencyWindowMass = 0.5; +static constexpr int kLatencyWindowSize = 200; // These are the flags used by BufferProducer::CreatePersistentUncachedBlob, // plus PRIVATE_ADSP_HEAP to allow access from the DSP. @@ -115,7 +114,7 @@ PoseService::PoseService(SensorThread* sensor_thread) photon_timestamp_(0), // Will be updated by external service, but start with a non-zero value: display_period_ns_(16000000), - sensor_latency_(kLatencyWindowSize, kLatencyWindowMass) { + sensor_latency_(kLatencyWindowSize) { last_known_pose_ = { .orientation = {1.0f, 0.0f, 0.0f, 0.0f}, .translation = {0.0f, 0.0f, 0.0f, 0.0f}, diff --git a/services/vr/vr_window_manager/composer/impl/vr_hwc.cpp b/services/vr/vr_window_manager/composer/impl/vr_hwc.cpp index b5724ca714..9ba01f9d3a 100644 --- a/services/vr/vr_window_manager/composer/impl/vr_hwc.cpp +++ b/services/vr/vr_window_manager/composer/impl/vr_hwc.cpp @@ -211,19 +211,14 @@ Error HwcDisplay::GetFrame( queued_client_target = true; } else { if (!layer.info.buffer.get() || !layer.info.fence.get()) { - ALOGE("Layer requested without valid buffer"); - return Error::BAD_LAYER; + ALOGV("Layer requested without valid buffer"); + continue; } frame.push_back(layer.info); } } - if (frame.empty()) { - ALOGE("Requested frame with no layers"); - return Error::BAD_LAYER; - } - out_frames->swap(frame); return Error::NONE; } diff --git a/services/vr/vr_window_manager/display_view.cpp b/services/vr/vr_window_manager/display_view.cpp index 52984b70d3..88768a0a7a 100644 --- a/services/vr/vr_window_manager/display_view.cpp +++ b/services/vr/vr_window_manager/display_view.cpp @@ -219,6 +219,11 @@ base::unique_fd DisplayView::OnFrame(std::unique_ptr<HwcCallback::Frame> frame, visibility = ViewMode::Hidden; current_vr_app_ = app; } + } else if ((use_2dmode_ || !is_vr_active) && app != 0 && + visibility == ViewMode::Hidden) { + // This is the case for the VR app launching a 2D intent of itself on some + // display. + visibility = ViewMode::App; } else if (!current_vr_app_) { // The VR app is running. current_vr_app_ = app; |