Fix mixer script

badaix · Apr 2, 2024 · e57dc08 · e57dc08
1 parent 9253b00
commit e57dc08
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 50 deletions.
diff --git a/client/controller.cpp b/client/controller.cpp
@@ -230,6 +230,7 @@ void Controller::getNextMessage()
             player_->setVolumeCallback(
                 [this](const Player::Volume& volume)
                 {
+                // Cache the last volume and check if it really changed in the player's volume callback
                 static Player::Volume last_volume{-1, true};
                 if (volume != last_volume)
                 {

diff --git a/client/player/alsa_player.cpp b/client/player/alsa_player.cpp
@@ -659,6 +659,7 @@ void AlsaPlayer::worker()
             LOG(INFO, LOG_TAG) << "Failed to get chunk\n";
             while (active_ && !stream_->waitForChunk(100ms))
             {
+                // Log "Waiting for chunk" only every second second
                 static utils::logging::TimeConditional cond(2s);
                 LOG(DEBUG, LOG_TAG) << cond << "Waiting for chunk\n";
                 if ((handle_ != nullptr) && (chronos::getTickCount() - lastChunkTick > 5000))

diff --git a/client/player/player.cpp b/client/player/player.cpp
@@ -240,49 +240,39 @@ void Player::setVolume(const Volume& volume)
     else if (settings_.mixer.mode == ClientSettings::Mixer::Mode::script)
     {
 #ifdef SUPPORTS_VOLUME_SCRIPT
-        static std::optional<Volume> pending_volume_setting;
-        static bool script_running = false;
-        if (script_running)
+        static std::optional<Volume> pending_volume_change;
+        static boost::process::child mixer_script_process;
+        if (mixer_script_process.running())
         {
-            pending_volume_setting = volume;
-            LOG(DEBUG, LOG_TAG) << "Volume script still running, deferring this volume setting\n";
+            pending_volume_change = volume;
+            LOG(DEBUG, LOG_TAG) << "Volume mixer script still running, deferring this volume change\n";
         }
         else
         {
-            static std::optional<Volume> pending_volume_setting;
-            static boost::process::child c;
-            if (c.running())
+            try
             {
-                pending_volume_setting = volume;
-                LOG(DEBUG, LOG_TAG) << "Volume script still running, deferring this volume setting\n";
+                namespace bp = boost::process;
+                mixer_script_process = bp::child(bp::exe = settings_.mixer.parameter,
+                                                 bp::args = {"--volume", cpt::to_string(volume.volume), "--mute", volume.mute ? "true" : "false"},
+                                                 bp::on_exit(
+                                                     [&](int ret_val, std::error_code ec)
+                                                     {
+                    std::unique_lock<std::mutex> lock(mutex_);
+                    LOG(DEBUG, LOG_TAG) << "Error code: " << ec.message() << ", i: " << ret_val << "\n";
+                    if (pending_volume_change.has_value())
+                    {
+                        Volume v = pending_volume_change.value();
+                        pending_volume_change = std::nullopt;
+                        lock.unlock();
+                        setVolume(v);
+                    }
+                                                 }),
+                                                 io_context_);
             }
-            else
+            catch (const std::exception& e)
             {
-                try
-                {
-                    namespace bp = boost::process;
-                    c = bp::child(bp::exe = settings_.mixer.parameter,
-                                  bp::args = {"--volume", cpt::to_string(volume.volume), "--mute", volume.mute ? "true" : "false"},
-                                  bp::on_exit(
-                                      [&](int ret_val, std::error_code ec)
-                                      {
-                        std::unique_lock<std::mutex> lock(mutex_);
-                        LOG(DEBUG, LOG_TAG) << "Error code: " << ec.message() << ", i: " << ret_val << "\n";
-                        if (pending_volume_setting.has_value())
-                        {
-                            Volume v = pending_volume_setting.value();
-                            pending_volume_setting = std::nullopt;
-                            lock.unlock();
-                            setVolume(v);
-                        }
-                                  }),
-                                  io_context_);
-                }
-                catch (const std::exception& e)
-                {
-                    script_running = false;
-                    LOG(ERROR, LOG_TAG) << "Failed to run script '" + settings_.mixer.parameter + "', error: " << e.what() << "\n";
-                }
+                LOG(ERROR, LOG_TAG) << "Failed to run script '" + settings_.mixer.parameter + "', error: " << e.what() << "\n";
+                pending_volume_change = std::nullopt;
             }
         }
 #else

diff --git a/client/stream.cpp b/client/stream.cpp
@@ -1,6 +1,6 @@
 /***
     This file is part of snapcast
-    Copyright (C) 2014-2021  Johannes Pohl
+    Copyright (C) 2014-2024  Johannes Pohl
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -20,12 +20,18 @@
 #define NOMINMAX
 #endif // NOMINMAX
 
+// prototype/interface header file
 #include "stream.hpp"
+
+// local headers
 #include "common/aixlog.hpp"
 #include "common/snap_exception.hpp"
 #include "common/str_compat.hpp"
-#include "common/utils/logging.hpp"
 #include "time_provider.hpp"
+
+// 3rd party headers
+
+// standard headers
 #include <cmath>
 #include <cstring>
 #include <iostream>
@@ -41,7 +47,7 @@ static constexpr auto kCorrectionBegin = 100us;
 
 Stream::Stream(const SampleFormat& in_format, const SampleFormat& out_format)
     : in_format_(in_format), median_(0), shortMedian_(0), lastUpdate_(0), playedFrames_(0), correctAfterXFrames_(0), bufferMs_(cs::msec(500)), frame_delta_(0),
-      hard_sync_(true)
+      hard_sync_(true), time_cond_(1s)
 {
     buffer_.setSize(500);
     shortBuffer_.setSize(100);
@@ -459,8 +465,7 @@ bool Stream::getPlayerChunkOrSilence(void* outputBuffer, const chronos::usec& ou
     bool result = getPlayerChunk(outputBuffer, outputBufferDacTime, frames);
     if (!result)
     {
-        static utils::logging::TimeConditional cond(1s);
-        LOG(DEBUG, LOG_TAG) << cond << "Failed to get chunk, returning silence\n";
+        LOG(DEBUG, LOG_TAG) << time_cond_ << "Failed to get chunk, returning silence\n";
         getSilentPlayerChunk(outputBuffer, frames);
     }
     return result;

diff --git a/client/stream.hpp b/client/stream.hpp
@@ -1,6 +1,6 @@
 /***
     This file is part of snapcast
-    Copyright (C) 2014-2023  Johannes Pohl
+    Copyright (C) 2014-2024  Johannes Pohl
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -25,6 +25,7 @@
 #include "common/queue.h"
 #include "common/resampler.hpp"
 #include "common/sample_format.hpp"
+#include "common/utils/logging.hpp"
 #include "double_buffer.hpp"
 
 // 3rd party headers
@@ -134,6 +135,9 @@ class Stream
     mutable std::mutex mutex_;
 
     bool hard_sync_;
+
+    /// Log "failed to get chunk" only once per second
+    utils::logging::TimeConditional time_cond_;
 };
 
 

diff --git a/server/control_session_http.cpp b/server/control_session_http.cpp
@@ -1,6 +1,6 @@
 /***
     This file is part of snapcast
-    Copyright (C) 2014-2023  Johannes Pohl
+    Copyright (C) 2014-2024  Johannes Pohl
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -251,7 +251,7 @@ void ControlSessionHttp::handle_request(http::request<Body, http::basic_fields<A
     if (target.empty() || target[0] != '/' || target.find("..") != beast::string_view::npos)
         return send(bad_request("Illegal request-target"));
 
-    static string image_cache_target = "/__image_cache?name=";
+    static const string image_cache_target = "/__image_cache?name=";
     auto pos = target.find(image_cache_target);
     if (pos != std::string::npos)
     {

diff --git a/server/streamreader/airplay_stream.cpp b/server/streamreader/airplay_stream.cpp
@@ -23,9 +23,7 @@
 #include "base64.h"
 #include "common/aixlog.hpp"
 #include "common/snap_exception.hpp"
-#include "common/utils.hpp"
 #include "common/utils/file_utils.hpp"
-#include "common/utils/string_utils.hpp"
 
 using namespace std;
 
@@ -248,16 +246,15 @@ void AirplayStream::pipeReadLine()
     boost::asio::async_read_until(*pipe_fd_, streambuf_pipe_, delimiter,
                                   [this, delimiter](const std::error_code& ec, std::size_t bytes_transferred)
                                   {
-        static AixLog::Severity logseverity = AixLog::Severity::info;
         if (ec)
         {
             if ((ec.value() == boost::asio::error::eof) || (ec.value() == boost::asio::error::bad_descriptor))
             {
                 // For some reason, EOF is returned until the first metadata is written to the pipe.
                 // If shairport-sync has not finished setting up the pipe, bad file descriptor is returned.
                 static constexpr auto retry_ms = 2500ms;
-                LOG(logseverity, LOG_TAG) << "Waiting for metadata, retrying in " << retry_ms.count() << "ms\n";
-                logseverity = AixLog::Severity::debug;
+                LOG(read_logseverity_, LOG_TAG) << "Waiting for metadata, retrying in " << retry_ms.count() << "ms\n";
+                read_logseverity_ = AixLog::Severity::debug;
                 wait(pipe_open_timer_, retry_ms, [this] { pipeReadLine(); });
             }
             else
@@ -266,7 +263,7 @@ void AirplayStream::pipeReadLine()
             }
             return;
         }
-        logseverity = AixLog::Severity::info;
+        read_logseverity_ = AixLog::Severity::info;
 
         // Extract up to the first delimiter.
         std::string line{buffers_begin(streambuf_pipe_.data()), buffers_begin(streambuf_pipe_.data()) + bytes_transferred - delimiter.length()};

diff --git a/server/streamreader/airplay_stream.hpp b/server/streamreader/airplay_stream.hpp
@@ -101,6 +101,9 @@ class AirplayStream : public ProcessStream
     static void XMLCALL element_end(void* userdata, const char* element_name);
     static void XMLCALL data(void* userdata, const char* content, int length);
 #endif
+
+private:
+    AixLog::Severity read_logseverity_{AixLog::Severity::info};
 };
 
 } // namespace streamreader