Skip to content

Commit

Permalink
audio_processor: Find DLC vars from board.ini
Browse files Browse the repository at this point in the history
FIXED=b:356757238
TEST=tast run dut14 audio.CrasEffects.beamforming_\*

Change-Id: Id2e11e31fdb4ab0f20de7e7dd0397a6f1f4e2014
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/6040422
Commit-Queue: Li-Yu Yu <aaronyu@google.com>
Tested-by: Li-Yu Yu <aaronyu@google.com>
Reviewed-by: Hung-Hsien Chen <hunghsienchen@chromium.org>
  • Loading branch information
afq984 authored and Chromeos LUCI committed Nov 26, 2024
1 parent be2a078 commit 37b5ee5
Show file tree
Hide file tree
Showing 10 changed files with 242 additions and 41 deletions.
15 changes: 12 additions & 3 deletions audio_processor/proto/cdcfg.proto
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,21 @@ message Plugin {
message DlcPlugin {
// The ID of the DLC.
// Also known as the `--id` option of `dlcservice_util`.
string dlc_id = 1;
oneof dlc_id_oneof {
string dlc_id = 1;
string dlc_id_var = 4;
}
// The path within the DLC root.
string path = 2;
oneof path_oneof {
string path = 2;
string path_var = 5;
}
// The name of the processor_create function.
// See also plugin_processor.h.
string constructor = 3;
oneof constructor_oneof {
string constructor = 3;
string constructor_var = 6;
}
}

// Wrap the inner audio processor so that it always receives
Expand Down
3 changes: 2 additions & 1 deletion audio_processor/src/bin/offline-pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ fn run(command: Command) {
constructor: command.plugin_name,
},
PluginOrPipeline::Pipeline(path) => {
parse(&NaiveResolverContext::default(), &path).unwrap_or_else(|err| panic!("{err:#}"))
parse(&NaiveResolverContext::default(), &Default::default(), &path)
.unwrap_or_else(|err| panic!("{err:#}"))
}
};
let pipeline_decl = vec![
Expand Down
202 changes: 180 additions & 22 deletions audio_processor/src/cdcfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

use std::cell::RefCell;
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;
Expand All @@ -11,6 +12,9 @@ use anyhow::bail;
use anyhow::Context;

use crate::config::Processor;
use crate::proto::cdcfg::dlc_plugin::Constructor_oneof;
use crate::proto::cdcfg::dlc_plugin::Dlc_id_oneof;
use crate::proto::cdcfg::dlc_plugin::Path_oneof;

pub trait ResolverContext {
fn get_wav_dump_root(&self) -> Option<&Path>;
Expand Down Expand Up @@ -62,8 +66,53 @@ impl ResolverContext for DlcIdCollector {
}
}

enum ConstOrVar<'a> {
Const(&'a str),
Var(&'a str),
}

impl<'a> From<&'a Dlc_id_oneof> for ConstOrVar<'a> {
fn from(value: &'a Dlc_id_oneof) -> Self {
match value {
Dlc_id_oneof::DlcId(s) => ConstOrVar::Const(s),
Dlc_id_oneof::DlcIdVar(s) => ConstOrVar::Var(s),
}
}
}

impl<'a> From<&'a Path_oneof> for ConstOrVar<'a> {
fn from(value: &'a Path_oneof) -> Self {
match value {
Path_oneof::Path(s) => ConstOrVar::Const(s),
Path_oneof::PathVar(s) => ConstOrVar::Var(s),
}
}
}

impl<'a> From<&'a Constructor_oneof> for ConstOrVar<'a> {
fn from(value: &'a Constructor_oneof) -> Self {
match value {
Constructor_oneof::Constructor(s) => ConstOrVar::Const(s),
Constructor_oneof::ConstructorVar(s) => ConstOrVar::Var(s),
}
}
}

fn get_const_or_var_str<'a>(
vars: &'a HashMap<String, String>,
const_or_var: ConstOrVar<'a>,
) -> anyhow::Result<&'a str> {
match const_or_var {
ConstOrVar::Const(s) => Ok(s),
ConstOrVar::Var(s) => Ok(vars
.get(s)
.with_context(|| format!("var {s:?} not found"))?),
}
}

fn resolve(
context: &dyn ResolverContext,
vars: &HashMap<String, String>,
proto: &crate::proto::cdcfg::Processor,
) -> anyhow::Result<crate::config::Processor> {
let Some(processor) = &proto.processor_oneof else {
Expand All @@ -83,13 +132,35 @@ fn resolve(
},
DlcPlugin(dlc_plugin) => Processor::Plugin {
path: context
.get_dlc_root_path(&dlc_plugin.dlc_id)
.get_dlc_root_path(get_const_or_var_str(
vars,
dlc_plugin
.dlc_id_oneof
.as_ref()
.context("missing dlc_id")?
.into(),
)?)
.context("context.dlc_root_path")?
.join(&dlc_plugin.path),
constructor: dlc_plugin.constructor.clone(),
.join(get_const_or_var_str(
vars,
dlc_plugin
.path_oneof
.as_ref()
.context("missing path")?
.into(),
)?),
constructor: get_const_or_var_str(
vars,
dlc_plugin
.constructor_oneof
.as_ref()
.context("missing constructor")?
.into(),
)?
.to_string(),
},
WrapChunk(wrap_chunk) => Processor::WrapChunk {
inner: Box::new(resolve(context, &wrap_chunk.inner).context("wrap_chunk inner")?),
inner: Box::new(resolve(context, vars, &wrap_chunk.inner).context("wrap_chunk inner")?),
inner_block_size: wrap_chunk
.inner_block_size
.try_into()
Expand All @@ -107,7 +178,8 @@ fn resolve(
.iter()
.enumerate()
.map(|(i, processor)| -> anyhow::Result<Processor> {
resolve(context, processor).with_context(|| format!("pipeline processor {i}"))
resolve(context, vars, processor)
.with_context(|| format!("pipeline processor {i}"))
})
.collect::<Result<Vec<_>, _>>()?,
},
Expand All @@ -132,7 +204,7 @@ fn resolve(
frame_rate: positive_or_none(check_format.frame_rate),
},
Peer(peer) => Processor::Peer {
processor: Box::new(resolve(context, &peer.processor).context("peer.processor")?),
processor: Box::new(resolve(context, vars, &peer.processor).context("peer.processor")?),
},
})
}
Expand All @@ -145,42 +217,70 @@ fn positive_or_none(val: i32) -> Option<usize> {
}
}

fn resolve_str(context: &dyn ResolverContext, s: &str) -> anyhow::Result<Processor> {
fn resolve_str(
context: &dyn ResolverContext,
vars: &HashMap<String, String>,
s: &str,
) -> anyhow::Result<Processor> {
let cfg: crate::proto::cdcfg::Processor =
protobuf::text_format::parse_from_str(s).context("protobuf text_format error")?;
resolve(context, &cfg)
resolve(context, vars, &cfg)
}

pub fn parse(context: &dyn ResolverContext, path: &Path) -> anyhow::Result<Processor> {
pub fn parse(
context: &dyn ResolverContext,
vars: &HashMap<String, String>,
path: &Path,
) -> anyhow::Result<Processor> {
let s = std::fs::read_to_string(path)
.with_context(|| format!("read_to_string {}", path.display()))?;
resolve_str(context, &s)
resolve_str(context, vars, &s)
}

/// Get all DLC IDs specified in a given processing config.
pub fn get_required_dlcs(path: &Path) -> anyhow::Result<HashSet<String>> {
pub fn get_required_dlcs(
vars: &HashMap<String, String>,
path: &Path,
) -> anyhow::Result<HashSet<String>> {
let resolver = DlcIdCollector::default();
parse(&resolver, path)?;
parse(&resolver, vars, path)?;
Ok(resolver.dlcs.take())
}

#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;

use super::resolve_str;
use super::NaiveResolverContext;
use super::ResolverContext;
use crate::cdcfg::DlcIdCollector;
use crate::config::Processor;

fn assert_resolve_error(
context: &dyn ResolverContext,
vars: &HashMap<String, String>,
s: &str,
err_substring: &str,
) {
let err = resolve_str(context, vars, s).unwrap_err();
assert!(
err.to_string().contains(err_substring),
"{:?} does not contain {err_substring}",
err.to_string()
);
}

#[test]
fn invalid_text_format() {
let context: NaiveResolverContext = Default::default();
let err = resolve_str(&context, "abcd").unwrap_err();
assert!(
err.to_string().contains("protobuf text_format error"),
"{err}"
assert_resolve_error(
&context,
&HashMap::new(),
"abcd",
"protobuf text_format error",
);
}

Expand All @@ -189,6 +289,7 @@ mod tests {
let context: NaiveResolverContext = Default::default();
let processor = resolve_str(
&context,
&HashMap::new(),
r#"pipeline {
processors {
wrap_chunk {
Expand Down Expand Up @@ -262,6 +363,7 @@ mod tests {
let context: NaiveResolverContext = Default::default();
let err = resolve_str(
&context,
&HashMap::new(),
r#"pipeline {
processors {
shuffle_channels {
Expand Down Expand Up @@ -290,14 +392,14 @@ mod tests {
let spec = r#"maybe_wav_dump { filename: "a.wav" }"#;

let context: NaiveResolverContext = Default::default();
let processor = resolve_str(&context, spec).unwrap();
let processor = resolve_str(&context, &HashMap::new(), spec).unwrap();
assert_eq!(processor, Processor::Nothing);

let context = NaiveResolverContext {
wav_dump_root: Some(PathBuf::from("/tmp")),
..Default::default()
};
let processor = resolve_str(&context, spec).unwrap();
let processor = resolve_str(&context, &HashMap::new(), spec).unwrap();
assert_eq!(
processor,
Processor::WavSink {
Expand All @@ -318,7 +420,7 @@ pipeline {
dlc_plugin { dlc_id: "second-dlc" path: "libsecond.so" constructor: "plugin_processor_create2" }
}
}"#;
let processor = resolve_str(&context, spec).unwrap();
let processor = resolve_str(&context, &HashMap::new(), spec).unwrap();
assert_eq!(
processor,
Processor::Pipeline {
Expand All @@ -340,26 +442,82 @@ pipeline {
);

let context = DlcIdCollector::default();
resolve_str(&context, spec).unwrap();
resolve_str(&context, &HashMap::new(), spec).unwrap();
assert_eq!(
context.dlcs.borrow().clone(),
HashSet::from(["nc-ap-dlc".to_string(), "second-dlc".to_string()])
);
}

#[test]
fn dlc_missing_fields() {
let context: NaiveResolverContext = Default::default();
assert_resolve_error(
&context,
&HashMap::new(),
r#"dlc_plugin { path: "libdenoiser.so" constructor: "plugin_processor_create" }"#,
"missing dlc_id",
);
assert_resolve_error(
&context,
&HashMap::new(),
r#"dlc_plugin { dlc_id: "nc-ap-dlc" constructor: "plugin_processor_create" }"#,
"missing path",
);
assert_resolve_error(
&context,
&HashMap::new(),
r#"dlc_plugin { dlc_id: "nc-ap-dlc" path: "libdenoiser.so" }"#,
"missing constructor",
);
}

#[test]
fn dlc_var() {
let context: NaiveResolverContext = Default::default();
let vars = HashMap::from_iter(
[("a", "aaa"), ("b", "bbb"), ("c", "ccc")]
.iter()
.map(|(k, v)| (k.to_string(), v.to_string())),
);
let spec = r#"dlc_plugin { dlc_id_var: "a" path_var: "b" constructor_var: "c" }"#;
let processor = resolve_str(&context, &vars, spec).unwrap();
assert_eq!(
processor,
Processor::Plugin {
path: PathBuf::from("/run/imageloader/aaa/package/root/bbb"),
constructor: "ccc".into(),
}
);
}

#[test]
fn dlc_var_missing() {
let context: NaiveResolverContext = Default::default();
assert_resolve_error(
&context,
&HashMap::new(),
r#"dlc_plugin { dlc_id_var: "xyz" path: "libdenoiser.so" constructor: "plugin_processor_create" }"#,
r#"var "xyz" not found"#,
);
}

#[test]
fn duplicate_channel_0() {
let spec = "maybe_duplicate_channel_0 {}";

let context: NaiveResolverContext = Default::default();
assert_eq!(resolve_str(&context, spec).unwrap(), Processor::Nothing);
assert_eq!(
resolve_str(&context, &HashMap::new(), spec).unwrap(),
Processor::Nothing
);

let context = NaiveResolverContext {
duplicate_channel_0: Some(2),
..Default::default()
};
assert_eq!(
resolve_str(&context, spec).unwrap(),
resolve_str(&context, &HashMap::new(), spec).unwrap(),
Processor::ShuffleChannels {
channel_indexes: vec![0; 2]
}
Expand Down
2 changes: 1 addition & 1 deletion audio_processor/src/processors/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ mod tests {
#[test]
fn measurement_display_empty() {
let m = Measurement::default();
format!("{}", m); // should not panic, e.g. division by zero
drop(format!("{}", m)); // should not panic, e.g. division by zero
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions cras-config/for_all_boards/processor/beamforming.txtpb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ pipeline {
peer {
processor {
dlc_plugin {
dlc_id: "intelligo-beamforming-dlc"
path: "libigo_processor.so"
dlc_id_var: "beamforming_dlc_id"
path_var: "beamforming_dlc_path"
constructor: "plugin_processor_create"
}
}
Expand Down
Loading

0 comments on commit 37b5ee5

Please sign in to comment.