Merge branch 'librespot-org:dev' into arg_parse_fixup

Author: JasonLG1979

contrib/Dockerfile

@@ -9,8 +9,10 @@
 #
 # If only one architecture is desired, cargo can be invoked directly with the appropriate options :
 # $ docker run -v /tmp/librespot-build:/build librespot-cross cargo build --release --no-default-features --features "alsa-backend"
-# $ docker run -v /tmp/librespot-build:/build librespot-cross cargo build --release --target arm-unknown-linux-gnueabihf --no-default-features --features "alsa-backend"
-# $ docker run -v /tmp/librespot-build:/build librespot-cross cargo build --release --target arm-unknown-linux-gnueabi --no-default-features --features "alsa-backend"
+# $ docker run -v /tmp/librespot-build:/build librespot-cross cargo build --release --target arm-unknown-linux-gnueabihf --no-default-features --features alsa-backend
+# $ docker run -v /tmp/librespot-build:/build librespot-cross cargo build --release --target arm-unknown-linux-gnueabi --no-default-features --features alsa-backend
+# $ docker run -v /tmp/librespot-build:/build librespot-cross cargo build --release --target aarch64-unknown-linux-gnu --no-default-features --features alsa-backend
+
 # $ docker run -v /tmp/librespot-build:/build librespot-cross contrib/docker-build-pi-armv6hf.sh
 
 FROM debian:stretch

playback/src/audio_backend/alsa.rs

@@ -4,16 +4,18 @@ use crate::convert::Converter;
 use crate::decoder::AudioPacket;
 use crate::{NUM_CHANNELS, SAMPLE_RATE};
 use alsa::device_name::HintIter;
-use alsa::pcm::{Access, Format, HwParams, PCM};
+use alsa::pcm::{Access, Format, Frames, HwParams, PCM};
 use alsa::{Direction, ValueOr};
 use std::cmp::min;
 use std::process::exit;
-use std::time::Duration;
 use thiserror::Error;
 
-// 0.5 sec buffer.
-const PERIOD_TIME: Duration = Duration::from_millis(100);
-const BUFFER_TIME: Duration = Duration::from_millis(500);
+const MAX_BUFFER: Frames = (SAMPLE_RATE / 2) as Frames;
+const MIN_BUFFER: Frames = (SAMPLE_RATE / 10) as Frames;
+const ZERO_FRAMES: Frames = 0;
+
+const MAX_PERIOD_DIVISOR: Frames = 4;
+const MIN_PERIOD_DIVISOR: Frames = 10;
 
 #[derive(Debug, Error)]
 enum AlsaError {
@@ -195,28 +197,187 @@ fn open_device(dev_name: &str, format: AudioFormat) -> SinkResult<(PCM, usize)>
                 e,
             })?;
 
-        hwp.set_buffer_time_near(BUFFER_TIME.as_micros() as u32, ValueOr::Nearest)
-            .map_err(AlsaError::HwParams)?;
+        // Clone the hwp while it's in
+        // a good working state so that
+        // in the event of an error setting
+        // the buffer and period sizes
+        // we can use the good working clone
+        // instead of the hwp that's in an
+        // error state.
+        let hwp_clone = hwp.clone();
+
+        // At a sampling rate of 44100:
+        // The largest buffer is 22050 Frames (500ms) with 5512 Frame periods (125ms).
+        // The smallest buffer is 4410 Frames (100ms) with 441 Frame periods (10ms).
+        // Actual values may vary.
+        //
+        // Larger buffer and period sizes are preferred as extremely small values
+        // will cause high CPU useage.
+        //
+        // If no buffer or period size is in those ranges or an error happens
+        // trying to set the buffer or period size use the device's defaults
+        // which may not be ideal but are *hopefully* serviceable.
+
+        let buffer_size = {
+            let max = match hwp.get_buffer_size_max() {
+                Err(e) => {
+                    trace!("Error getting the device's max Buffer size: {}", e);
+                    ZERO_FRAMES
+                }
+                Ok(s) => s,
+            };
 
-        hwp.set_period_time_near(PERIOD_TIME.as_micros() as u32, ValueOr::Nearest)
-            .map_err(AlsaError::HwParams)?;
+            let min = match hwp.get_buffer_size_min() {
+                Err(e) => {
+                    trace!("Error getting the device's min Buffer size: {}", e);
+                    ZERO_FRAMES
+                }
+                Ok(s) => s,
+            };
+
+            let buffer_size = if min < max {
+                match (MIN_BUFFER..=MAX_BUFFER)
+                    .rev()
+                    .find(|f| (min..=max).contains(f))
+                {
+                    Some(size) => {
+                        trace!("Desired Frames per Buffer: {:?}", size);
+
+                        match hwp.set_buffer_size_near(size) {
+                            Err(e) => {
+                                trace!("Error setting the device's Buffer size: {}", e);
+                                ZERO_FRAMES
+                            }
+                            Ok(s) => s,
+                        }
+                    }
+                    None => {
+                        trace!("No Desired Buffer size in range reported by the device.");
+                        ZERO_FRAMES
+                    }
+                }
+            } else {
+                trace!("The device's min reported Buffer size was greater than or equal to it's max reported Buffer size.");
+                ZERO_FRAMES
+            };
+
+            if buffer_size == ZERO_FRAMES {
+                trace!(
+                    "Desired Buffer Frame range: {:?} - {:?}",
+                    MIN_BUFFER,
+                    MAX_BUFFER
+                );
+
+                trace!(
+                    "Actual Buffer Frame range as reported by the device: {:?} - {:?}",
+                    min,
+                    max
+                );
+            }
 
-        pcm.hw_params(&hwp).map_err(AlsaError::Pcm)?;
+            buffer_size
+        };
+
+        let period_size = {
+            if buffer_size == ZERO_FRAMES {
+                ZERO_FRAMES
+            } else {
+                let max = match hwp.get_period_size_max() {
+                    Err(e) => {
+                        trace!("Error getting the device's max Period size: {}", e);
+                        ZERO_FRAMES
+                    }
+                    Ok(s) => s,
+                };
 
-        let swp = pcm.sw_params_current().map_err(AlsaError::Pcm)?;
+                let min = match hwp.get_period_size_min() {
+                    Err(e) => {
+                        trace!("Error getting the device's min Period size: {}", e);
+                        ZERO_FRAMES
+                    }
+                    Ok(s) => s,
+                };
+
+                let max_period = buffer_size / MAX_PERIOD_DIVISOR;
+                let min_period = buffer_size / MIN_PERIOD_DIVISOR;
+
+                let period_size = if min < max && min_period < max_period {
+                    match (min_period..=max_period)
+                        .rev()
+                        .find(|f| (min..=max).contains(f))
+                    {
+                        Some(size) => {
+                            trace!("Desired Frames per Period: {:?}", size);
+
+                            match hwp.set_period_size_near(size, ValueOr::Nearest) {
+                                Err(e) => {
+                                    trace!("Error setting the device's Period size: {}", e);
+                                    ZERO_FRAMES
+                                }
+                                Ok(s) => s,
+                            }
+                        }
+                        None => {
+                            trace!("No Desired Period size in range reported by the device.");
+                            ZERO_FRAMES
+                        }
+                    }
+                } else {
+                    trace!("The device's min reported Period size was greater than or equal to it's max reported Period size,");
+                    trace!("or the desired min Period size was greater than or equal to the desired max Period size.");
+                    ZERO_FRAMES
+                };
+
+                if period_size == ZERO_FRAMES {
+                    trace!("Buffer size: {:?}", buffer_size);
+
+                    trace!(
+                        "Desired Period Frame range: {:?} (Buffer size / {:?}) - {:?} (Buffer size / {:?})",
+                        min_period,
+                        MIN_PERIOD_DIVISOR,
+                        max_period,
+                        MAX_PERIOD_DIVISOR,
+                    );
+
+                    trace!(
+                        "Actual Period Frame range as reported by the device: {:?} - {:?}",
+                        min,
+                        max
+                    );
+                }
+
+                period_size
+            }
+        };
+
+        if buffer_size == ZERO_FRAMES || period_size == ZERO_FRAMES {
+            trace!(
+                "Failed to set Buffer and/or Period size, falling back to the device's defaults."
+            );
+
+            trace!("You may experience higher than normal CPU usage and/or audio issues.");
+
+            pcm.hw_params(&hwp_clone).map_err(AlsaError::Pcm)?;
+        } else {
+            pcm.hw_params(&hwp).map_err(AlsaError::Pcm)?;
+        }
+
+        let hwp = pcm.hw_params_current().map_err(AlsaError::Pcm)?;
 
         // Don't assume we got what we wanted. Ask to make sure.
         let frames_per_period = hwp.get_period_size().map_err(AlsaError::HwParams)?;
 
         let frames_per_buffer = hwp.get_buffer_size().map_err(AlsaError::HwParams)?;
 
+        let swp = pcm.sw_params_current().map_err(AlsaError::Pcm)?;
+
         swp.set_start_threshold(frames_per_buffer - frames_per_period)
             .map_err(AlsaError::SwParams)?;
 
         pcm.sw_params(&swp).map_err(AlsaError::Pcm)?;
 
-        trace!("Frames per Buffer: {:?}", frames_per_buffer);
-        trace!("Frames per Period: {:?}", frames_per_period);
+        trace!("Actual Frames per Buffer: {:?}", frames_per_buffer);
+        trace!("Actual Frames per Period: {:?}", frames_per_period);
 
         // Let ALSA do the math for us.
         pcm.frames_to_bytes(frames_per_period) as usize

playback/src/player.rs

@@ -737,7 +737,14 @@ impl PlayerTrackLoader {
             }
         };
 
-        assert!(audio.duration >= 0);
+        if audio.duration < 0 {
+            error!(
+                "Track duration for <{}> cannot be {}",
+                spotify_id.to_uri(),
+                audio.duration
+            );
+            return None;
+        }
         let duration_ms = audio.duration as u32;
 
         // (Most) podcasts seem to support only 96 bit Vorbis, so fall back to it
@@ -943,9 +950,12 @@ impl Future for PlayerInternal {
                             exit(1);
                         }
                     }
-                    Poll::Ready(Err(_)) => {
-                        warn!("Unable to load <{:?}>\nSkipping to next track", track_id);
-                        assert!(self.state.is_loading());
+                    Poll::Ready(Err(e)) => {
+                        warn!(
+                            "Skipping to next track, unable to load track <{:?}>: {:?}",
+                            track_id, e
+                        );
+                        debug_assert!(self.state.is_loading());
                         self.send_event(PlayerEvent::EndOfTrack {
                             track_id,
                             play_request_id,
@@ -1045,8 +1055,11 @@ impl Future for PlayerInternal {
                                             }
                                         }
                                         Err(e) => {
-                                            error!("PlayerInternal poll: {}", e);
-                                            exit(1);
+                                            warn!("Skipping to next track, unable to decode samples for track <{:?}>: {:?}", track_id, e);
+                                            self.send_event(PlayerEvent::EndOfTrack {
+                                                track_id,
+                                                play_request_id,
+                                            })
                                         }
                                     }
                                 }
@@ -1058,8 +1071,11 @@ impl Future for PlayerInternal {
                             self.handle_packet(packet, normalisation_factor);
                         }
                         Err(e) => {
-                            error!("PlayerInternal poll: {}", e);
-                            exit(1);
+                            warn!("Skipping to next track, unable to get next packet for track <{:?}>: {:?}", track_id, e);
+                            self.send_event(PlayerEvent::EndOfTrack {
+                                track_id,
+                                play_request_id,
+                            })
                         }
                     }
                 } else {