From fdd900b1733cc5dd18e4c811a3a846bc9263683f Mon Sep 17 00:00:00 2001 From: eivajon Date: Fri, 9 Aug 2024 16:02:22 +0200 Subject: [PATCH 1/2] Migrate from argparse to clap. This commits removes the argparse dependency in favour of CLAP. CLAP allows us to parse environment variables in the same structure as we do with other arguments. --- gstscream/Cargo.lock | 125 +++++++++++++++++++++++++++++++++-- gstscream/Cargo.toml | 2 +- gstscream/src/receiver.rs | 28 ++++++-- gstscream/src/sender.rs | 74 ++++++++++++++------- gstscream/src/sender_util.rs | 43 +++++++++--- 5 files changed, 223 insertions(+), 49 deletions(-) diff --git a/gstscream/Cargo.lock b/gstscream/Cargo.lock index 19be058..09f12db 100644 --- a/gstscream/Cargo.lock +++ b/gstscream/Cargo.lock @@ -39,10 +39,53 @@ dependencies = [ ] [[package]] -name = "argparse" -version = "0.2.2" +name = "anstream" +version = "0.6.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "64e15c1ab1f89faffbf04a634d5e1962e9074f2741eef6d97f3c4e322426d526" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "is_terminal_polyfill", + "utf8parse", +] + +[[package]] +name = "anstyle" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bec1de6f59aedf83baf9ff929c98f2ad654b97c9510f4e70cf6f661d49fd5b1" + +[[package]] +name = "anstyle-parse" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb47de1e80c2b463c735db5b217a0ddc39d612e7ac9e2e96a5aed1f57616c1cb" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d36fc52c7f6c869915e99412912f22093507da8d9e942ceaf66fe4b7c14422a" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "anstyle-wincon" +version = "3.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f8ebf5827e4ac4fd5946560e6a99776ea73b596d80898f357007317a7141e47" +checksum = "5bf74e1b6e971609db8ca7a9ce79fd5768ab6ae46441c572e46cf596f59e57f8" +dependencies = [ + "anstyle", + "windows-sys", +] [[package]] name = "array-init" @@ -128,6 +171,52 @@ dependencies = [ "windows-targets", ] +[[package]] +name = "clap" +version = "4.5.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c937d4061031a6d0c8da4b9a4f98a172fc2976dfb1c19213a9cf7d0d3c837e36" +dependencies = [ + "clap_builder", + "clap_derive", +] + +[[package]] +name = "clap_builder" +version = "4.5.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85379ba512b21a328adf887e85f7742d12e96eb31f3ef077df4ffc26b506ffed" +dependencies = [ + "anstream", + "anstyle", + "clap_lex", + "strsim", +] + +[[package]] +name = "clap_derive" +version = "4.5.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "501d359d5f3dcaf6ecdeee48833ae73ec6e42723a1e52419c79abf9507eec0a0" +dependencies = [ + "heck 0.5.0", + "proc-macro2", + "quote", + "syn 2.0.48", +] + +[[package]] +name = "clap_lex" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1462739cb27611015575c0c11df5df7601141071f07518d56fcc1be504cbec97" + +[[package]] +name = "colorchoice" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3fd119d74b830634cea2a0f58bbd0d54540518a14397557951e79340abc28c0" + [[package]] name = "core-foundation-sys" version = "0.8.6" @@ -269,7 +358,7 @@ name = "glib-macros" version = "0.20.0" source = "git+https://github.com/gtk-rs/gtk-rs-core?branch=master#03c5a850037c582a0a5fca07f8e1cd1cf924c4fc" dependencies = [ - "heck", + "heck 0.4.1", "proc-macro-crate", "proc-macro2", "quote", @@ -420,9 +509,9 @@ dependencies = [ name = "gstscream" version = "0.6.0" dependencies = [ - "argparse", "array-init", "chrono", + "clap", "failure", "gst-plugin-version-helper", "gstreamer", @@ -467,6 +556,12 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" +[[package]] +name = "heck" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" + [[package]] name = "iana-time-zone" version = "0.1.60" @@ -500,6 +595,12 @@ dependencies = [ "hashbrown 0.14.3", ] +[[package]] +name = "is_terminal_polyfill" +version = "1.70.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7943c866cc5cd64cbc25b2e01621d07fa8eb2a1a23160ee81ce38704e97b8ecf" + [[package]] name = "itertools" version = "0.12.1" @@ -711,6 +812,12 @@ version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6ecd384b10a64542d77071bd64bd7b231f4ed5940fba55e98c3de13824cf3d7" +[[package]] +name = "strsim" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" + [[package]] name = "syn" version = "1.0.109" @@ -752,7 +859,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a2d580ff6a20c55dfb86be5f9c238f67835d0e81cbdea8bf5680e0897320331" dependencies = [ "cfg-expr", - "heck", + "heck 0.4.1", "pkg-config", "toml", "version-compare", @@ -841,6 +948,12 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" +[[package]] +name = "utf8parse" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" + [[package]] name = "version-compare" version = "0.1.1" diff --git a/gstscream/Cargo.toml b/gstscream/Cargo.toml index 514b01c..b231a74 100644 --- a/gstscream/Cargo.toml +++ b/gstscream/Cargo.toml @@ -22,7 +22,7 @@ libc = "0.2.68" failure = "0.1" gtypes = "0.2.0" chrono = "0.4" -argparse = "0.2.2" +clap = { version = "4.5.11", features=["env","derive"] } [lib] name = "gstscream" diff --git a/gstscream/src/receiver.rs b/gstscream/src/receiver.rs index 40bb3ac..30435cb 100644 --- a/gstscream/src/receiver.rs +++ b/gstscream/src/receiver.rs @@ -1,8 +1,9 @@ #![allow(clippy::uninlined_format_args)] extern crate failure; -use failure::Error; +use std::fmt::Display; -use std::env; +use clap::Parser; +use failure::Error; extern crate gtypes; @@ -12,16 +13,26 @@ use gst::prelude::*; extern crate chrono; +#[derive(clap::Parser)] +#[command(version, about, long_about = None)] +struct Arguments { + #[arg(env)] + /// The gstreamer pipeline to use for the receiver application. + recvpipeline: String, +} + fn main() { + let args = Arguments::parse(); + println!("{args}"); gst::init().expect("Failed to initialize"); let main_loop = glib::MainLoop::new(None, false); - start(&main_loop).expect("Failed to start"); + start(&main_loop, args).expect("Failed to start"); } -pub fn start(main_loop: &glib::MainLoop) -> Result<(), Error> { - let pls: String = env::var("RECVPIPELINE").unwrap(); +fn start(main_loop: &glib::MainLoop, args: Arguments) -> Result<(), Error> { + let pls: String = args.recvpipeline; println!("RECVPIPELINE={}", pls); let pipeline = gst::parse::launch(&pls).unwrap(); @@ -42,7 +53,6 @@ pub fn start(main_loop: &glib::MainLoop) -> Result<(), Error> { .add_watch(move |_, msg| { use gst::MessageView; - // println!("bus {:?}", msg.view()); let main_loop = &main_loop_clone; match msg.view() { MessageView::Eos(..) => { @@ -71,3 +81,9 @@ pub fn start(main_loop: &glib::MainLoop) -> Result<(), Error> { Ok(()) } + +impl Display for Arguments { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Configuration:\n\t- Pipeline {}", self.recvpipeline) + } +} diff --git a/gstscream/src/sender.rs b/gstscream/src/sender.rs index bbf242b..efa8a73 100644 --- a/gstscream/src/sender.rs +++ b/gstscream/src/sender.rs @@ -1,9 +1,9 @@ #![allow(clippy::uninlined_format_args)] +use clap::Parser; use failure::Error; -use std::env; +use std::fmt::Display; use std::process::exit; -extern crate argparse; extern crate failure; #[macro_use] extern crate lazy_static; @@ -11,36 +11,45 @@ extern crate lazy_static; use gst::glib; use gst::prelude::*; -use argparse::{ArgumentParser, StoreOption, StoreTrue}; - mod sender_util; +#[derive(clap::Parser)] +#[command(version, about, long_about = None)] +struct Arguments { + #[arg(short, long, default_value_t = false)] + /// Produces verbose logs. + verbose: bool, + + #[arg(short, long)] + /// Rate multiplication factor. + ratemultiply: Option, + + #[arg(env)] + /// The gstreamer pipeline to use for the sender application. + sendpipeline: String, + + #[arg(env, default_value_t = 1000)] + /// The logging interval for sender statistics. + sender_stats_timer: u32, + + #[arg(env, default_value = "sender_scream_stats.csv")] + /// The logging interval for sender statistics. + sender_stats_file_name: std::path::PathBuf, +} + fn main() { + let args = Arguments::parse(); + println!("{}", args); + gst::init().expect("Failed to initialize gst_init"); let main_loop = glib::MainLoop::new(None, false); - start(&main_loop).expect("Failed to start"); + start(&main_loop, args).expect("Failed to start"); } -pub fn start(main_loop: &glib::MainLoop) -> Result<(), Error> { - let mut ratemultiply_opt: Option = None; - let mut verbose = false; - { - // this block limits scope of borrows by ap.refer() method - let mut ap = ArgumentParser::new(); - ap.set_description("Sender"); - ap.refer(&mut verbose) - .add_option(&["-v", "--verbose"], StoreTrue, "Be verbose"); - ap.refer(&mut ratemultiply_opt).add_option( - &["-r", "--ratemultiply"], - StoreOption, - "Set ratemultiply", - ); - - ap.parse_args_or_exit(); - } +fn start(main_loop: &glib::MainLoop, args: Arguments) -> Result<(), Error> { + let pls = args.sendpipeline; - let pls = env::var("SENDPIPELINE").unwrap(); println!("Pipeline: {}", pls); let n_encoder0 = pls.matches("name=encoder0").count(); let n_encoders = pls.matches("name=encoder").count(); @@ -68,13 +77,15 @@ pub fn start(main_loop: &glib::MainLoop) -> Result<(), Error> { &pipeline_clone, n, &Some("screamtx".to_string() + &n_string), + args.sender_stats_timer, + args.sender_stats_file_name.clone(), ); sender_util::run_time_bitrate_set( &pipeline_clone, - verbose, + args.verbose, &Some("screamtx".to_string() + &n_string), &Some("encoder".to_string() + &n_string), - ratemultiply_opt, + args.ratemultiply, ); } @@ -110,3 +121,16 @@ pub fn start(main_loop: &glib::MainLoop) -> Result<(), Error> { println!("Done"); Ok(()) } +impl Display for Arguments { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Configuration:\n\t- Verbose: {},\n\t- Ratemultiply: {:?},\n\t- Pipeline: {},\n\t- Logging interval: {},\n\t- sender_stats_file: {}", + self.verbose, + self.ratemultiply, + self.sendpipeline, + self.sender_stats_timer, + self.sender_stats_file_name.display() + ) + } +} diff --git a/gstscream/src/sender_util.rs b/gstscream/src/sender_util.rs index efa31cf..33777a5 100644 --- a/gstscream/src/sender_util.rs +++ b/gstscream/src/sender_util.rs @@ -1,6 +1,5 @@ #![allow(clippy::uninlined_format_args)] use std::convert::TryInto; -use std::env; use std::fs::File; use std::io::Write; use std::sync::{Arc, Mutex}; @@ -18,26 +17,48 @@ struct RateInfo { count: u32, } -pub fn stats(bin: &gst::Pipeline, n: usize, screamtx_name_opt: &Option) { - let sender_stats_timer: u32 = env::var("SENDER_STATS_TIMER") - .ok() - .and_then(|s| s.parse().ok()) - .unwrap_or(1000); - println!("SENDER_STATS_TIMER={}", sender_stats_timer); +pub fn stats( + bin: &gst::Pipeline, + n: usize, + screamtx_name_opt: &Option, + sender_stats_timer: u32, + mut sender_stats_file_name: std::path::PathBuf, +) { if sender_stats_timer == 0 { return; } + if screamtx_name_opt.is_none() { println!("no scream name"); return; } + + println!("SENDER_STATS_TIMER={}", sender_stats_timer); + let pipeline_clock = bin.pipeline_clock(); let repl_string = "_".to_owned() + &n.to_string() + ".csv"; - let sender_stats_file_name: String = env::var("SENDER_STATS_FILE_NAME") - .unwrap_or_else(|_| "sender_scream_stats.csv".into()) - .replace(".csv", &repl_string); - println!("SENDER_STATS_FILE_NAME={}", sender_stats_file_name); + let file_name = match sender_stats_file_name.file_name() { + Some(file) => file, + None => { + println!("Invalid SENDER_STATS_FILE_NAME."); + return; + } + }; + let file_name = match file_name.to_str() { + Some(file_name) => file_name, + None => { + println!("SENDER_STATS_FILE_NAME must be a utf8 encoded string"); + return; + } + } + .to_string(); + + sender_stats_file_name.set_file_name(file_name.replace(".csv", &repl_string)); + println!( + "SENDER_STATS_FILE_NAME={}", + sender_stats_file_name.display() + ); let mut out: File = File::create(&sender_stats_file_name).unwrap(); let scream_name = screamtx_name_opt.as_ref().unwrap(); From 1f6fa2e01d3cd0cd46139438bfc8303fc834a7e1 Mon Sep 17 00:00:00 2001 From: eivajon Date: Fri, 9 Aug 2024 15:53:49 +0200 Subject: [PATCH 2/2] Remove private structure that was never constructed and use clamp --- gstscream/src/screamrx/ScreamRx.rs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/gstscream/src/screamrx/ScreamRx.rs b/gstscream/src/screamrx/ScreamRx.rs index 90961dd..3605960 100644 --- a/gstscream/src/screamrx/ScreamRx.rs +++ b/gstscream/src/screamrx/ScreamRx.rs @@ -106,23 +106,6 @@ impl Default for ScreamRx { } } -use std::io::Write; -struct WriteToGstInfo { - cat: gst::DebugCategory, -} -impl Write for WriteToGstInfo { - fn write(&mut self, buf: &[u8]) -> Result { - let str_buf = String::from_utf8_lossy(buf); - gst::error!(self.cat, "{}", str_buf); - let len = str_buf.len(); - Ok(len) - } - fn flush(&mut self) -> Result<(), std::io::Error> { - // nothing to do - Ok(()) - } -} - impl ScreamRx { pub fn ScreamReceiverPluginInit(&mut self, rtcp_srcpad: Option>>) { gst::info!(CAT, "Init"); @@ -189,7 +172,7 @@ impl ScreamRx { * to the range [2ms,100ms] */ let mut rate: f64 = 0.02 * self.averageReceivedRate / (100.0 * 8.0); // RTCP overhead - rate = f64::min(500.0, f64::max(10.0, rate)); + rate = rate.clamp(10., 500.); /* * More than one stream ?, increase the feedback rate as * we currently don't bundle feedback packets