From 50a58f93728b30e128644ab1d7d6b088538eebe3 Mon Sep 17 00:00:00 2001 From: Dennis Kempin Date: Wed, 23 Jun 2021 11:34:31 -0700 Subject: [PATCH] Integrate audio_streams into crosvm, add stub libcras implementation The `# ignored by ebuild` tag will remove the path to libcras_stub and allows crosvm to be built with the actual libcras implementation. This allows all other platforms to build without depending on `third_party/adhd/cras/client/libcras`, which is a prerequisite for externalizing crosvm. An empty libcras_stub crate is provided to keep cargo happy in external builds. To build with cargo against libcras, the setup_cros_cargo.sh script can be used. BUG=b:191511078 TEST=Tests in crosvm and cros_sdk both pass: $ ./test_all $ cros_run_unit_tests --package=crosvm Cq-Depend: chromium:2993483 Change-Id: I86aad23a86c78e580c1724fb311f870b25d6b09e Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2988154 Tested-by: kokoro Commit-Queue: Dennis Kempin Reviewed-by: Chih-Yang Hsia --- Cargo.lock | 17 ++--------------- Cargo.toml | 9 +++++---- audio_streams/.gitignore | 2 -- audio_streams/Android.bp | 7 ------- audio_streams/Cargo.toml | 6 +++--- bin/clippy | 25 +++++++++++++++++++++++-- devices/Cargo.toml | 3 ++- devices/src/pci/ac97.rs | 8 ++++++++ devices/src/pci/pci_device.rs | 4 ++-- libcras_stub/Cargo.toml | 8 ++++++++ libcras_stub/README.md | 12 ++++++++++++ libcras_stub/src/libcras.rs | 1 + run_tests | 2 +- setup_cros_cargo.sh | 14 ++++++++++++++ src/main.rs | 7 ++++--- 15 files changed, 85 insertions(+), 40 deletions(-) delete mode 100644 audio_streams/.gitignore delete mode 100644 audio_streams/Android.bp create mode 100644 libcras_stub/Cargo.toml create mode 100644 libcras_stub/README.md create mode 100644 libcras_stub/src/libcras.rs create mode 100755 setup_cros_cargo.sh diff --git a/Cargo.lock b/Cargo.lock index 34c82c4a26..7ee68c2af5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -86,6 +86,8 @@ dependencies = [ name = "audio_streams" version = "0.1.0" dependencies = [ + "async-trait", + "cros_async", "sync", "sys_util", ] @@ -166,14 +168,6 @@ dependencies = [ "bitflags", ] -[[package]] -name = "cras-sys" -version = "0.1.0" -dependencies = [ - "audio_streams", - "data_model", -] - [[package]] name = "crc32fast" version = "1.2.1" @@ -621,13 +615,6 @@ checksum = "18794a8ad5b29321f790b55d93dfba91e125cb1a9edbd4f8e3150acc771c1a5e" [[package]] name = "libcras" version = "0.1.0" -dependencies = [ - "audio_streams", - "cras-sys", - "data_model", - "libc", - "sys_util", -] [[package]] name = "libcrosvm_control" diff --git a/Cargo.toml b/Cargo.toml index f5b9a0146d..dd48ac8327 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,11 +43,13 @@ exclude = [ "sys_util", "tempfile", "vm_memory", + "audio_streams", ] [features] audio = ["devices/audio"] -chromeos = ["base/chromeos"] +audio_cras = ["devices/audio_cras"] +chromeos = ["base/chromeos", "audio_cras"] composite-disk = ["protos/composite-disk", "protobuf", "disk/composite-disk"] default = ["audio", "gpu", "usb"] default-no-sandbox = [] @@ -113,12 +115,11 @@ base = "*" [patch.crates-io] assertions = { path = "assertions" } -audio_streams = { path = "../../third_party/adhd/audio_streams" } # ignored by ebuild +audio_streams = { path = "audio_streams" } base = { path = "base" } -cras-sys = { path = "../../third_party/adhd/cras/client/cras-sys" } # ignored by ebuild cros_fuzz = { path = "../../platform2/cros-fuzz" } # ignored by ebuild data_model = { path = "data_model" } -libcras = { path = "../../third_party/adhd/cras/client/libcras" } # ignored by ebuild +libcras = { path = "libcras_stub" } # ignored by ebuild minijail = { path = "../../aosp/external/minijail/rust/minijail" } # ignored by ebuild p9 = { path = "../../platform2/vm_tools/p9" } # ignored by ebuild sync = { path = "sync" } diff --git a/audio_streams/.gitignore b/audio_streams/.gitignore deleted file mode 100644 index fa8d85ac52..0000000000 --- a/audio_streams/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -Cargo.lock -target diff --git a/audio_streams/Android.bp b/audio_streams/Android.bp deleted file mode 100644 index 47b5be6ba3..0000000000 --- a/audio_streams/Android.bp +++ /dev/null @@ -1,7 +0,0 @@ -rust_library_rlib { - name: "libaudio_streams", - host_supported: true, - crate_name: "audio_streams", - edition: "2018", - srcs: ["src/audio_streams.rs"], -} diff --git a/audio_streams/Cargo.toml b/audio_streams/Cargo.toml index 084411c082..49160a8197 100644 --- a/audio_streams/Cargo.toml +++ b/audio_streams/Cargo.toml @@ -9,6 +9,6 @@ path = "src/audio_streams.rs" [dependencies] async-trait = "0.1.36" -cros_async = { path = "../../../platform/crosvm/cros_async" } # provided by ebuild -sync = { path = "../../../platform/crosvm/sync" } # provided by ebuild -sys_util = { path = "../../../platform/crosvm/sys_util" } # provided by ebuild +cros_async = { path = "../cros_async" } # provided by ebuild +sync = { path = "../sync" } # provided by ebuild +sys_util = { path = "../sys_util" } # provided by ebuild diff --git a/bin/clippy b/bin/clippy index 6d8568d771..d04e4dfd57 100755 --- a/bin/clippy +++ b/bin/clippy @@ -65,6 +65,27 @@ SUPPRESS=( new-ret-no-self ) +FEATURES=( + default + direct + audio + gpu + plugin + power-monitor-powerd + tpm + usb + video-decoder + video-encoder + wl-dmabuf + x + virgl_renderer_next + composite-disk + virgl_renderer + gfxstream + gdb +) +printf -v FEATURES_LIST '%s,' "${FEATURES[@]}" + # Needed or else clippy won't re-run on code that has already compiled. if [[ "${USE_CACHE}" == false ]]; then cargo clean @@ -76,5 +97,5 @@ RUST_SYSROOT=$(rustc --print sysroot) RUSTFLAGS="${RUSTFLAGS:-}" export RUSTFLAGS="$RUSTFLAGS --sysroot=$RUST_SYSROOT" -cargo clippy --all-features --all-targets -- ${SUPPRESS[@]/#/-Aclippy::} \ - "${CLIPPY_ARGS[@]}" -D warnings +cargo clippy --features ${FEATURES_LIST} --all-targets -- \ + ${SUPPRESS[@]/#/-Aclippy::} "${CLIPPY_ARGS[@]}" -D warnings diff --git a/devices/Cargo.toml b/devices/Cargo.toml index 3891dc79d4..bbf2884f88 100644 --- a/devices/Cargo.toml +++ b/devices/Cargo.toml @@ -6,6 +6,7 @@ edition = "2018" [features] audio = [] +audio_cras = ["libcras"] direct = [] gpu = ["gpu_display","rutabaga_gfx"] tpm = ["protos/trunks", "tpm2"] @@ -32,7 +33,7 @@ rutabaga_gfx = { path = "../rutabaga_gfx", optional = true } hypervisor = { path = "../hypervisor" } kvm_sys = { path = "../kvm_sys" } libc = "*" -libcras = "*" +libcras = { version = "*", optional = true } libvda = { path = "../libvda", optional = true } linux_input_sys = { path = "../linux_input_sys" } minijail = "*" diff --git a/devices/src/pci/ac97.rs b/devices/src/pci/ac97.rs index 3b396ff0f6..eaf6330061 100644 --- a/devices/src/pci/ac97.rs +++ b/devices/src/pci/ac97.rs @@ -10,6 +10,7 @@ use std::str::FromStr; use audio_streams::shm_streams::{NullShmStreamSource, ShmStreamSource}; use base::{error, Event, RawDescriptor}; +#[cfg(feature = "audio_cras")] use libcras::{CrasClient, CrasClientType, CrasSocketType, CrasSysError}; use resources::{Alloc, MmioType, SystemAllocator}; use vm_memory::GuestMemory; @@ -39,6 +40,7 @@ const PCI_DEVICE_ID_INTEL_82801AA_5: u16 = 0x2415; #[derive(Debug, Clone)] pub enum Ac97Backend { NULL, + #[cfg(feature = "audio_cras")] CRAS, VIOS, } @@ -71,6 +73,7 @@ impl FromStr for Ac97Backend { type Err = Ac97Error; fn from_str(s: &str) -> std::result::Result { match s { + #[cfg(feature = "audio_cras")] "cras" => Ok(Ac97Backend::CRAS), "vios" => Ok(Ac97Backend::VIOS), "null" => Ok(Ac97Backend::NULL), @@ -85,6 +88,7 @@ pub struct Ac97Parameters { pub backend: Ac97Backend, pub capture: bool, pub vios_server_path: Option, + #[cfg(feature = "audio_cras")] client_type: Option, } @@ -92,6 +96,7 @@ impl Ac97Parameters { /// Set CRAS client type by given client type string. /// /// `client_type` - The client type string. + #[cfg(feature = "audio_cras")] pub fn set_client_type(&mut self, client_type: &str) -> std::result::Result<(), CrasSysError> { self.client_type = Some(client_type.parse()?); Ok(()) @@ -145,6 +150,7 @@ impl Ac97Dev { /// to create `Ac97Dev` with the given back-end, it'll fallback to the null audio device. pub fn try_new(mem: GuestMemory, param: Ac97Parameters) -> Result { match param.backend { + #[cfg(feature = "audio_cras")] Ac97Backend::CRAS => Self::create_cras_audio_device(param, mem.clone()).or_else(|e| { error!( "Ac97Dev: create_cras_audio_device: {}. Fallback to null audio device", @@ -160,12 +166,14 @@ impl Ac97Dev { /// Return the minijail policy file path for the current Ac97Dev. pub fn minijail_policy(&self) -> &'static str { match self.backend { + #[cfg(feature = "audio_cras")] Ac97Backend::CRAS => "cras_audio_device", Ac97Backend::VIOS => "vios_audio_device", Ac97Backend::NULL => "null_audio_device", } } + #[cfg(feature = "audio_cras")] fn create_cras_audio_device(params: Ac97Parameters, mem: GuestMemory) -> Result { let mut server = Box::new( CrasClient::with_type(CrasSocketType::Unified) diff --git a/devices/src/pci/pci_device.rs b/devices/src/pci/pci_device.rs index a316fcff0f..c8f9b5a912 100644 --- a/devices/src/pci/pci_device.rs +++ b/devices/src/pci/pci_device.rs @@ -27,7 +27,7 @@ pub enum Error { /// Registering an IO BAR failed. IoRegistrationFailed(u64, pci_configuration::Error), /// Create cras client failed. - #[cfg(feature = "audio")] + #[cfg(all(feature = "audio", feature = "audio_cras"))] CreateCrasClientFailed(libcras::Error), /// Create VioS client failed. #[cfg(feature = "audio")] @@ -45,7 +45,7 @@ impl Display for Error { match self { CapabilitiesSetup(e) => write!(f, "failed to add capability {}", e), - #[cfg(feature = "audio")] + #[cfg(all(feature = "audio", feature = "audio_cras"))] CreateCrasClientFailed(e) => write!(f, "failed to create CRAS Client: {}", e), #[cfg(feature = "audio")] CreateViosClientFailed(e) => write!(f, "failed to create VioS Client: {}", e), diff --git a/libcras_stub/Cargo.toml b/libcras_stub/Cargo.toml new file mode 100644 index 0000000000..7d40cf2636 --- /dev/null +++ b/libcras_stub/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "libcras" +version = "0.1.0" +authors = ["The Chromium OS Authors"] +edition = "2018" + +[lib] +path = "src/libcras.rs" diff --git a/libcras_stub/README.md b/libcras_stub/README.md new file mode 100644 index 0000000000..bdd5e74247 --- /dev/null +++ b/libcras_stub/README.md @@ -0,0 +1,12 @@ +# Stub crate for libcras + +libcras is used by ChromeOS to play audio through the cras server. + +In ChromeOS builds, the `audio_cras` cargo feature is enabled and this crate is +replaced with the actual [libcras] implementation. + +On other platforms, the feature flag will remain disabled and this crate is used +to satisfy cargo dependencies on libcras. + +[libcras]: + https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/adhd/cras/client/libcras/ diff --git a/libcras_stub/src/libcras.rs b/libcras_stub/src/libcras.rs new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/libcras_stub/src/libcras.rs @@ -0,0 +1 @@ + diff --git a/run_tests b/run_tests index 16338d5368..d874cbb9c7 100755 --- a/run_tests +++ b/run_tests @@ -72,7 +72,7 @@ CRATE_REQUIREMENTS: Dict[str, List[Requirements]] = { # Just like for crates, lists requirements for each cargo feature flag. FEATURE_REQUIREMENTS: Dict[str, List[Requirements]] = { - "chromeos": [], + "chromeos": [Requirements.DISABLED], "audio": [], "gpu": [Requirements.CROS_BUILD], "plugin": [Requirements.PRIVILEGED, Requirements.X86_64], diff --git a/setup_cros_cargo.sh b/setup_cros_cargo.sh new file mode 100755 index 0000000000..aa5a65a456 --- /dev/null +++ b/setup_cros_cargo.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# Copyright 2021 The Chromium OS Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file +# +# To build crosvm using cargo against libraries and crates provided by ChromeOS +# use this script to update path references in Cargo.toml. +# +# TODO(b/194323235): Add documentation for ChromeOS developer workflows. + +sed -i 's|path = "libcras_stub"|path = "../../third_party/adhd/cras/client/libcras"|g' \ + Cargo.toml + +echo "Modified Cargo.toml with new paths. Please do not commit those." diff --git a/src/main.rs b/src/main.rs index 3f4ae0cc4d..9423645841 100644 --- a/src/main.rs +++ b/src/main.rs @@ -533,6 +533,7 @@ fn parse_ac97_options(s: &str) -> argument::Result { argument::Error::Syntax(format!("invalid capture option: {}", e)) })?; } + #[cfg(feature = "audio_cras")] "client_type" => { ac97_params .set_client_type(v) @@ -2800,7 +2801,7 @@ mod tests { ); } - #[cfg(feature = "audio")] + #[cfg(feature = "audio_cras")] #[test] fn parse_ac97_vaild() { parse_ac97_options("backend=cras").expect("parse should have succeded"); @@ -2812,13 +2813,13 @@ mod tests { parse_ac97_options("backend=null").expect("parse should have succeded"); } - #[cfg(feature = "audio")] + #[cfg(feature = "audio_cras")] #[test] fn parse_ac97_capture_vaild() { parse_ac97_options("backend=cras,capture=true").expect("parse should have succeded"); } - #[cfg(feature = "audio")] + #[cfg(feature = "audio_cras")] #[test] fn parse_ac97_client_type() { parse_ac97_options("backend=cras,capture=true,client_type=crosvm")