From aaef07e819192b0c204150e8d65eff741fabe1e3 Mon Sep 17 00:00:00 2001 From: sniperrifle2004 Date: Sun, 14 Jun 2020 05:16:59 +0200 Subject: [PATCH 1/6] Introduce an appropriate period for the desired buffer --- playback/src/audio_backend/alsa.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/playback/src/audio_backend/alsa.rs b/playback/src/audio_backend/alsa.rs index 8bdcb9d..c2e0663 100644 --- a/playback/src/audio_backend/alsa.rs +++ b/playback/src/audio_backend/alsa.rs @@ -41,7 +41,8 @@ fn open_device(dev_name: &str) -> Result<(PCM), Box> { hwp.set_format(Format::s16())?; hwp.set_rate(44100, ValueOr::Nearest)?; hwp.set_channels(2)?; - hwp.set_buffer_size_near(22050)?; // ~ 0.5s latency + hwp.set_period_size_near(5512, ValueOr::Nearest)?; // Period of roughly 125ms + hwp.set_buffer_size_near(22048)?; // ~ 0.5s latency pcm.hw_params(&hwp)?; let swp = pcm.sw_params_current()?; From 64081a12bb22d1f6a211a7ddde4176dea3af0579 Mon Sep 17 00:00:00 2001 From: sniperrifle2004 Date: Sun, 14 Jun 2020 05:20:14 +0200 Subject: [PATCH 2/6] Introduce a buffer for a full period Writing to the pcm more often than necessary is just a waste of resources and depending on the pcm it can have quite an impact on performance. The pcm expects full periods anyway. --- playback/src/audio_backend/alsa.rs | 60 ++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/playback/src/audio_backend/alsa.rs b/playback/src/audio_backend/alsa.rs index c2e0663..e92ad18 100644 --- a/playback/src/audio_backend/alsa.rs +++ b/playback/src/audio_backend/alsa.rs @@ -1,12 +1,21 @@ use super::{Open, Sink}; use alsa::device_name::HintIter; -use alsa::pcm::{Access, Format, HwParams, PCM}; +use alsa::pcm::{Access, Format, Frames, HwParams, PCM}; use alsa::{Direction, Error, ValueOr}; +use std::cmp::min; use std::ffi::CString; use std::io; use std::process::exit; -pub struct AlsaSink(Option, String); +const PERIOD_SIZE: usize = 5512; // Period of roughly 125ms +const BUFFERED_PERIODS: usize = 4; // ~ 0.5s latency + +pub struct AlsaSink { + pcm: Option, + device: String, + buffer: [i16; PERIOD_SIZE as usize], + buffered_data: usize, +} fn list_outputs() { for t in &["pcm", "ctl", "hwdep"] { @@ -41,8 +50,8 @@ fn open_device(dev_name: &str) -> Result<(PCM), Box> { hwp.set_format(Format::s16())?; hwp.set_rate(44100, ValueOr::Nearest)?; hwp.set_channels(2)?; - hwp.set_period_size_near(5512, ValueOr::Nearest)?; // Period of roughly 125ms - hwp.set_buffer_size_near(22048)?; // ~ 0.5s latency + hwp.set_period_size_near(PERIOD_SIZE as Frames, ValueOr::Nearest)?; + hwp.set_buffer_size_near((PERIOD_SIZE * BUFFERED_PERIODS) as Frames)?; pcm.hw_params(&hwp)?; let swp = pcm.sw_params_current()?; @@ -68,16 +77,21 @@ impl Open for AlsaSink { } .to_string(); - AlsaSink(None, name) + AlsaSink { + pcm: None, + device: name, + buffer: [0; PERIOD_SIZE], + buffered_data: 0, + } } } impl Sink for AlsaSink { fn start(&mut self) -> io::Result<()> { - if self.0.is_none() { - let pcm = open_device(&self.1); + if self.pcm.is_none() { + let pcm = open_device(&self.device); match pcm { - Ok(p) => self.0 = Some(p), + Ok(p) => self.pcm = Some(p), Err(e) => { error!("Alsa error PCM open {}", e); return Err(io::Error::new( @@ -93,20 +107,34 @@ impl Sink for AlsaSink { fn stop(&mut self) -> io::Result<()> { { - let pcm = self.0.as_ref().unwrap(); + let pcm = self.pcm.as_ref().unwrap(); pcm.drain().unwrap(); } - self.0 = None; + self.pcm = None; Ok(()) } fn write(&mut self, data: &[i16]) -> io::Result<()> { - let pcm = self.0.as_mut().unwrap(); - let io = pcm.io_i16().unwrap(); - - match io.writei(&data) { - Ok(_) => (), - Err(err) => pcm.try_recover(err, false).unwrap(), + let mut processed_data = 0; + while processed_data < data.len() { + let data_to_buffer = min( + PERIOD_SIZE - self.buffered_data, + data.len() - processed_data, + ); + let buffer_slice = + &mut self.buffer[self.buffered_data..self.buffered_data + data_to_buffer]; + buffer_slice.copy_from_slice(&data[processed_data..processed_data + data_to_buffer]); + self.buffered_data += data_to_buffer; + processed_data += data_to_buffer; + if self.buffered_data == PERIOD_SIZE { + self.buffered_data = 0; + let pcm = self.pcm.as_mut().unwrap(); + let io = pcm.io_i16().unwrap(); + match io.writei(&self.buffer) { + Ok(_) => (), + Err(err) => pcm.try_recover(err, false).unwrap(), + } + } } Ok(()) From cbe3c98fa122c0c3214f83e7c8e5044823f31e02 Mon Sep 17 00:00:00 2001 From: sniperrifle2004 Date: Sun, 14 Jun 2020 05:52:16 +0200 Subject: [PATCH 3/6] Clear buffer when the sink is stopped --- playback/src/audio_backend/alsa.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/playback/src/audio_backend/alsa.rs b/playback/src/audio_backend/alsa.rs index e92ad18..086bea3 100644 --- a/playback/src/audio_backend/alsa.rs +++ b/playback/src/audio_backend/alsa.rs @@ -111,6 +111,7 @@ impl Sink for AlsaSink { pcm.drain().unwrap(); } self.pcm = None; + self.buffered_data = 0; Ok(()) } From a68dfa0287790fa5583f41fcf301e25024f4e4f1 Mon Sep 17 00:00:00 2001 From: sniperrifle2004 Date: Sun, 14 Jun 2020 07:17:46 +0200 Subject: [PATCH 4/6] On stop write any chunk(s) left in the period buffer That should prevent a possible sudden stop --- playback/src/audio_backend/alsa.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/playback/src/audio_backend/alsa.rs b/playback/src/audio_backend/alsa.rs index 086bea3..01e9fec 100644 --- a/playback/src/audio_backend/alsa.rs +++ b/playback/src/audio_backend/alsa.rs @@ -107,7 +107,14 @@ impl Sink for AlsaSink { fn stop(&mut self) -> io::Result<()> { { - let pcm = self.pcm.as_ref().unwrap(); + let pcm = self.pcm.as_mut().unwrap(); + // Write any leftover data in the period buffer + // before draining the actual buffer + let io = pcm.io_i16().unwrap(); + match io.writei(&self.buffer[..self.buffered_data]) { + Ok(_) => (), + Err(err) => pcm.try_recover(err, false).unwrap(), + } pcm.drain().unwrap(); } self.pcm = None; From 82e54dfaba32cfc5d024781b1eb72716abc2549b Mon Sep 17 00:00:00 2001 From: sniperrifle2004 Date: Wed, 17 Jun 2020 03:34:46 +0200 Subject: [PATCH 5/6] Rewrite buffer around the actual period size This prevents over or underestimating of the period. While it is unlikely, with comparitively small period sizes overestimating can cause buffer underruns and underestimating causes more writes than necessary. It also properly accounts for the number of channels, which I had overlooked. --- playback/src/audio_backend/alsa.rs | 40 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/playback/src/audio_backend/alsa.rs b/playback/src/audio_backend/alsa.rs index 01e9fec..156123c 100644 --- a/playback/src/audio_backend/alsa.rs +++ b/playback/src/audio_backend/alsa.rs @@ -7,14 +7,13 @@ use std::ffi::CString; use std::io; use std::process::exit; -const PERIOD_SIZE: usize = 5512; // Period of roughly 125ms -const BUFFERED_PERIODS: usize = 4; // ~ 0.5s latency +const PREFERED_PERIOD_SIZE: Frames = 5512; // Period of roughly 125ms +const BUFFERED_PERIODS: Frames = 4; pub struct AlsaSink { pcm: Option, device: String, - buffer: [i16; PERIOD_SIZE as usize], - buffered_data: usize, + buffer: Vec, } fn list_outputs() { @@ -34,8 +33,9 @@ fn list_outputs() { } } -fn open_device(dev_name: &str) -> Result<(PCM), Box> { +fn open_device(dev_name: &str) -> Result<(PCM, Frames), Box> { let pcm = PCM::new(dev_name, Direction::Playback, false)?; + let period_size = PREFERED_PERIOD_SIZE; // http://www.linuxjournal.com/article/6735?page=0,1#N0x19ab2890.0x19ba78d8 // latency = period_size * periods / (rate * bytes_per_frame) // For 16 Bit stereo data, one frame has a length of four bytes. @@ -50,8 +50,8 @@ fn open_device(dev_name: &str) -> Result<(PCM), Box> { hwp.set_format(Format::s16())?; hwp.set_rate(44100, ValueOr::Nearest)?; hwp.set_channels(2)?; - hwp.set_period_size_near(PERIOD_SIZE as Frames, ValueOr::Nearest)?; - hwp.set_buffer_size_near((PERIOD_SIZE * BUFFERED_PERIODS) as Frames)?; + let period_size = hwp.set_period_size_near(period_size, ValueOr::Greater)?; + hwp.set_buffer_size_near(period_size * BUFFERED_PERIODS)?; pcm.hw_params(&hwp)?; let swp = pcm.sw_params_current()?; @@ -59,7 +59,7 @@ fn open_device(dev_name: &str) -> Result<(PCM), Box> { pcm.sw_params(&swp)?; } - Ok(pcm) + Ok((pcm, period_size)) } impl Open for AlsaSink { @@ -80,8 +80,7 @@ impl Open for AlsaSink { AlsaSink { pcm: None, device: name, - buffer: [0; PERIOD_SIZE], - buffered_data: 0, + buffer: vec![], } } } @@ -91,7 +90,11 @@ impl Sink for AlsaSink { if self.pcm.is_none() { let pcm = open_device(&self.device); match pcm { - Ok(p) => self.pcm = Some(p), + Ok((p, period_size)) => { + self.pcm = Some(p); + // Create a buffer for all samples for a full period + self.buffer = Vec::with_capacity((period_size * 2) as usize); + } Err(e) => { error!("Alsa error PCM open {}", e); return Err(io::Error::new( @@ -111,14 +114,13 @@ impl Sink for AlsaSink { // Write any leftover data in the period buffer // before draining the actual buffer let io = pcm.io_i16().unwrap(); - match io.writei(&self.buffer[..self.buffered_data]) { + match io.writei(&self.buffer[..]) { Ok(_) => (), Err(err) => pcm.try_recover(err, false).unwrap(), } pcm.drain().unwrap(); } self.pcm = None; - self.buffered_data = 0; Ok(()) } @@ -126,22 +128,20 @@ impl Sink for AlsaSink { let mut processed_data = 0; while processed_data < data.len() { let data_to_buffer = min( - PERIOD_SIZE - self.buffered_data, + self.buffer.capacity() - self.buffer.len(), data.len() - processed_data, ); - let buffer_slice = - &mut self.buffer[self.buffered_data..self.buffered_data + data_to_buffer]; - buffer_slice.copy_from_slice(&data[processed_data..processed_data + data_to_buffer]); - self.buffered_data += data_to_buffer; + self.buffer + .extend_from_slice(&data[processed_data..processed_data + data_to_buffer]); processed_data += data_to_buffer; - if self.buffered_data == PERIOD_SIZE { - self.buffered_data = 0; + if self.buffer.len() == self.buffer.capacity() { let pcm = self.pcm.as_mut().unwrap(); let io = pcm.io_i16().unwrap(); match io.writei(&self.buffer) { Ok(_) => (), Err(err) => pcm.try_recover(err, false).unwrap(), } + self.buffer.clear(); } } From 1e5d98b8fdd23e3e739b5fdb2ff59d45e12a0409 Mon Sep 17 00:00:00 2001 From: sniperrifle2004 Date: Wed, 17 Jun 2020 03:53:20 +0200 Subject: [PATCH 6/6] Actually store the period_size --- playback/src/audio_backend/alsa.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/playback/src/audio_backend/alsa.rs b/playback/src/audio_backend/alsa.rs index 156123c..ae76f05 100644 --- a/playback/src/audio_backend/alsa.rs +++ b/playback/src/audio_backend/alsa.rs @@ -35,7 +35,7 @@ fn list_outputs() { fn open_device(dev_name: &str) -> Result<(PCM, Frames), Box> { let pcm = PCM::new(dev_name, Direction::Playback, false)?; - let period_size = PREFERED_PERIOD_SIZE; + let mut period_size = PREFERED_PERIOD_SIZE; // http://www.linuxjournal.com/article/6735?page=0,1#N0x19ab2890.0x19ba78d8 // latency = period_size * periods / (rate * bytes_per_frame) // For 16 Bit stereo data, one frame has a length of four bytes. @@ -50,7 +50,7 @@ fn open_device(dev_name: &str) -> Result<(PCM, Frames), Box> { hwp.set_format(Format::s16())?; hwp.set_rate(44100, ValueOr::Nearest)?; hwp.set_channels(2)?; - let period_size = hwp.set_period_size_near(period_size, ValueOr::Greater)?; + period_size = hwp.set_period_size_near(period_size, ValueOr::Greater)?; hwp.set_buffer_size_near(period_size * BUFFERED_PERIODS)?; pcm.hw_params(&hwp)?;