diff --git a/NEWS b/NEWS index 1a9840bdf..23f2c41ff 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ ver 0.23.9 (not yet released) - ffmpeg: support FFmpeg 5.1 * output - pipewire: set app icon +* fix bogus volume levels with multiple partitions * improve iconv detection * macOS: fix macOS 10 build problem (0.23.8 regression) diff --git a/meson.build b/meson.build index 51d6139a4..22f8c466c 100644 --- a/meson.build +++ b/meson.build @@ -352,7 +352,7 @@ sources = [ 'src/TagStream.cxx', 'src/TagAny.cxx', 'src/TimePrint.cxx', - 'src/mixer/Volume.cxx', + 'src/mixer/Memento.cxx', 'src/PlaylistFile.cxx', ] diff --git a/src/Partition.cxx b/src/Partition.cxx index cf3396652..e5d02feab 100644 --- a/src/Partition.cxx +++ b/src/Partition.cxx @@ -23,7 +23,6 @@ #include "Log.hxx" #include "lib/fmt/ExceptionFormatter.hxx" #include "song/DetachedSong.hxx" -#include "mixer/Volume.hxx" #include "IdleFlags.hxx" #include "client/Listener.hxx" #include "client/Client.hxx" @@ -206,7 +205,7 @@ Partition::OnBorderPause() noexcept void Partition::OnMixerVolumeChanged(Mixer &, int) noexcept { - InvalidateHardwareVolume(); + mixer_memento.InvalidateHardwareVolume(); /* notify clients */ EmitIdle(IDLE_MIXER); diff --git a/src/Partition.hxx b/src/Partition.hxx index b90a01101..a4136836d 100644 --- a/src/Partition.hxx +++ b/src/Partition.hxx @@ -25,6 +25,7 @@ #include "queue/Listener.hxx" #include "output/MultipleOutputs.hxx" #include "mixer/Listener.hxx" +#include "mixer/Memento.hxx" #include "player/Control.hxx" #include "player/Listener.hxx" #include "protocol/RangeArg.hxx" @@ -76,6 +77,8 @@ struct Partition final : QueueListener, PlayerListener, MixerListener { MultipleOutputs outputs; + MixerMemento mixer_memento; + PlayerControl pc; ReplayGainMode replay_gain_mode = ReplayGainMode::OFF; diff --git a/src/StateFile.cxx b/src/StateFile.cxx index 46490b88d..5ad1e4d60 100644 --- a/src/StateFile.cxx +++ b/src/StateFile.cxx @@ -27,7 +27,6 @@ #include "storage/StorageState.hxx" #include "Partition.hxx" #include "Instance.hxx" -#include "mixer/Volume.hxx" #include "SongLoader.hxx" #include "util/Domain.hxx" #include "Log.hxx" @@ -47,7 +46,7 @@ StateFile::StateFile(StateFileConfig &&_config, void StateFile::RememberVersions() noexcept { - prev_volume_version = sw_volume_state_get_hash(); + prev_volume_version = partition.mixer_memento.GetSoftwareVolumeStateHash(); prev_output_version = audio_output_state_get_version(); prev_playlist_version = playlist_state_get_hash(partition.playlist, partition.pc); @@ -59,7 +58,7 @@ StateFile::RememberVersions() noexcept bool StateFile::IsModified() const noexcept { - return prev_volume_version != sw_volume_state_get_hash() || + return prev_volume_version != partition.mixer_memento.GetSoftwareVolumeStateHash() || prev_output_version != audio_output_state_get_version() || prev_playlist_version != playlist_state_get_hash(partition.playlist, partition.pc) @@ -72,7 +71,7 @@ StateFile::IsModified() const noexcept inline void StateFile::Write(BufferedOutputStream &os) { - save_sw_volume_state(os); + partition.mixer_memento.SaveSoftwareVolumeState(os); audio_output_state_save(os, partition.outputs); #ifdef ENABLE_DATABASE @@ -125,7 +124,7 @@ try { const char *line; while ((line = file.ReadLine()) != nullptr) { - success = read_sw_volume_state(line, partition.outputs) || + success = partition.mixer_memento.LoadSoftwareVolumeState(line, partition.outputs) || audio_output_state_read(line, partition.outputs) || playlist_state_restore(config, line, file, song_loader, partition.playlist, diff --git a/src/command/OtherCommands.cxx b/src/command/OtherCommands.cxx index 43a53f7c8..217bcaaec 100644 --- a/src/command/OtherCommands.cxx +++ b/src/command/OtherCommands.cxx @@ -33,7 +33,6 @@ #include "TimePrint.hxx" #include "decoder/DecoderPrint.hxx" #include "ls.hxx" -#include "mixer/Volume.hxx" #include "time/ChronoUtil.hxx" #include "util/UriUtil.hxx" #include "util/StringAPI.hxx" @@ -325,7 +324,7 @@ handle_getvol(Client &client, Request, Response &r) { auto &partition = client.GetPartition(); - const auto volume = volume_level_get(partition.outputs); + const auto volume = partition.mixer_memento.GetVolume(partition.outputs); if (volume >= 0) r.Fmt(FMT_STRING("volume: {}\n"), volume); @@ -337,7 +336,8 @@ handle_setvol(Client &client, Request args, Response &) { unsigned level = args.ParseUnsigned(0, 100); - volume_level_change(client.GetPartition().outputs, level); + auto &partition = client.GetPartition(); + partition.mixer_memento.SetVolume(partition.outputs, level); return CommandResult::OK; } @@ -346,9 +346,11 @@ handle_volume(Client &client, Request args, Response &r) { int relative = args.ParseInt(0, -100, 100); - auto &outputs = client.GetPartition().outputs; + auto &partition = client.GetPartition(); + auto &outputs = partition.outputs; + auto &mixer_memento = partition.mixer_memento; - const int old_volume = volume_level_get(outputs); + const int old_volume = mixer_memento.GetVolume(outputs); if (old_volume < 0) { r.Error(ACK_ERROR_SYSTEM, "No mixer"); return CommandResult::ERROR; @@ -361,7 +363,7 @@ handle_volume(Client &client, Request args, Response &r) new_volume = 100; if (new_volume != old_volume) - volume_level_change(outputs, new_volume); + mixer_memento.SetVolume(outputs, new_volume); return CommandResult::OK; } diff --git a/src/command/OutputCommands.cxx b/src/command/OutputCommands.cxx index cb8557f79..46456d03c 100644 --- a/src/command/OutputCommands.cxx +++ b/src/command/OutputCommands.cxx @@ -33,7 +33,11 @@ handle_enableoutput(Client &client, Request args, Response &r) assert(args.size == 1); unsigned device = args.ParseUnsigned(0); - if (!audio_output_enable_index(client.GetPartition().outputs, device)) { + auto &partition = client.GetPartition(); + + if (!audio_output_enable_index(partition.outputs, + partition.mixer_memento, + device)) { r.Error(ACK_ERROR_NO_EXIST, "No such audio output"); return CommandResult::ERROR; } @@ -47,7 +51,11 @@ handle_disableoutput(Client &client, Request args, Response &r) assert(args.size == 1); unsigned device = args.ParseUnsigned(0); - if (!audio_output_disable_index(client.GetPartition().outputs, device)) { + auto &partition = client.GetPartition(); + + if (!audio_output_disable_index(partition.outputs, + partition.mixer_memento, + device)) { r.Error(ACK_ERROR_NO_EXIST, "No such audio output"); return CommandResult::ERROR; } @@ -61,7 +69,11 @@ handle_toggleoutput(Client &client, Request args, Response &r) assert(args.size == 1); unsigned device = args.ParseUnsigned(0); - if (!audio_output_toggle_index(client.GetPartition().outputs, device)) { + auto &partition = client.GetPartition(); + + if (!audio_output_toggle_index(partition.outputs, + partition.mixer_memento, + device)) { r.Error(ACK_ERROR_NO_EXIST, "No such audio output"); return CommandResult::ERROR; } diff --git a/src/command/PlayerCommands.cxx b/src/command/PlayerCommands.cxx index ca84468b7..d72949af6 100644 --- a/src/command/PlayerCommands.cxx +++ b/src/command/PlayerCommands.cxx @@ -25,7 +25,6 @@ #include "SingleMode.hxx" #include "client/Client.hxx" #include "client/Response.hxx" -#include "mixer/Volume.hxx" #include "Partition.hxx" #include "Instance.hxx" #include "IdleFlags.hxx" @@ -131,7 +130,7 @@ handle_status(Client &client, [[maybe_unused]] Request args, Response &r) const auto &playlist = partition.playlist; - const auto volume = volume_level_get(partition.outputs); + const auto volume = partition.mixer_memento.GetVolume(partition.outputs); if (volume >= 0) r.Fmt(FMT_STRING("volume: {}\n"), volume); diff --git a/src/mixer/Volume.cxx b/src/mixer/Memento.cxx similarity index 66% rename from src/mixer/Volume.cxx rename to src/mixer/Memento.cxx index f86f4cbb3..c2ec128ab 100644 --- a/src/mixer/Volume.cxx +++ b/src/mixer/Memento.cxx @@ -17,11 +17,10 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "Volume.hxx" +#include "Memento.hxx" #include "output/MultipleOutputs.hxx" #include "Idle.hxx" #include "util/StringCompare.hxx" -#include "system/PeriodClock.hxx" #include "io/BufferedOutputStream.hxx" #include @@ -30,22 +29,8 @@ #define SW_VOLUME_STATE "sw_volume: " -static unsigned volume_software_set = 100; - -/** the cached hardware mixer value; invalid if negative */ -static int last_hardware_volume = -1; -/** the age of #last_hardware_volume */ -static PeriodClock hardware_volume_clock; - -void -InvalidateHardwareVolume() noexcept -{ - /* flush the hardware volume cache */ - last_hardware_volume = -1; -} - int -volume_level_get(const MultipleOutputs &outputs) noexcept +MixerMemento::GetVolume(const MultipleOutputs &outputs) noexcept { if (last_hardware_volume >= 0 && !hardware_volume_clock.CheckUpdate(std::chrono::seconds(1))) @@ -56,8 +41,8 @@ volume_level_get(const MultipleOutputs &outputs) noexcept return last_hardware_volume; } -static bool -software_volume_change(MultipleOutputs &outputs, unsigned volume) +inline bool +MixerMemento::SetSoftwareVolume(MultipleOutputs &outputs, unsigned volume) { assert(volume <= 100); @@ -67,8 +52,8 @@ software_volume_change(MultipleOutputs &outputs, unsigned volume) return true; } -static void -hardware_volume_change(MultipleOutputs &outputs, unsigned volume) +inline void +MixerMemento::SetHardwareVolume(MultipleOutputs &outputs, unsigned volume) { /* reset the cache */ last_hardware_volume = -1; @@ -77,7 +62,7 @@ hardware_volume_change(MultipleOutputs &outputs, unsigned volume) } void -volume_level_change(MultipleOutputs &outputs, unsigned volume) +MixerMemento::SetVolume(MultipleOutputs &outputs, unsigned volume) { assert(volume <= 100); @@ -85,11 +70,11 @@ volume_level_change(MultipleOutputs &outputs, unsigned volume) idle_add(IDLE_MIXER); - hardware_volume_change(outputs, volume); + SetHardwareVolume(outputs, volume); } bool -read_sw_volume_state(const char *line, MultipleOutputs &outputs) +MixerMemento::LoadSoftwareVolumeState(const char *line, MultipleOutputs &outputs) { char *end = nullptr; long int sv; @@ -100,19 +85,13 @@ read_sw_volume_state(const char *line, MultipleOutputs &outputs) sv = strtol(line, &end, 10); if (*end == 0 && sv >= 0 && sv <= 100) - software_volume_change(outputs, sv); + SetSoftwareVolume(outputs, sv); return true; } void -save_sw_volume_state(BufferedOutputStream &os) +MixerMemento::SaveSoftwareVolumeState(BufferedOutputStream &os) const { os.Format(SW_VOLUME_STATE "%u\n", volume_software_set); } - -unsigned -sw_volume_state_get_hash() noexcept -{ - return volume_software_set; -} diff --git a/src/mixer/Memento.hxx b/src/mixer/Memento.hxx new file mode 100644 index 000000000..32d23fdc9 --- /dev/null +++ b/src/mixer/Memento.hxx @@ -0,0 +1,73 @@ +/* + * Copyright 2003-2021 The Music Player Daemon Project + * http://www.musicpd.org + * + * 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 + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include "system/PeriodClock.hxx" + +class MultipleOutputs; +class BufferedOutputStream; + +/** + * Cache for hardware/software volume levels. + */ +class MixerMemento { + unsigned volume_software_set = 100; + + /** the cached hardware mixer value; invalid if negative */ + int last_hardware_volume = -1; + + /** the age of #last_hardware_volume */ + PeriodClock hardware_volume_clock; + +public: + /** + * Flush the hardware volume cache. + */ + void InvalidateHardwareVolume() noexcept { + last_hardware_volume = -1; + } + + [[gnu::pure]] + int GetVolume(const MultipleOutputs &outputs) noexcept; + + /** + * Throws on error. + */ + void SetVolume(MultipleOutputs &outputs, unsigned volume); + + bool LoadSoftwareVolumeState(const char *line, MultipleOutputs &outputs); + + void SaveSoftwareVolumeState(BufferedOutputStream &os) const; + + /** + * Generates a hash number for the current state of the software + * volume control. This is used by timer_save_state_file() to + * determine whether the state has changed and the state file should + * be saved. + */ + [[gnu::pure]] + unsigned GetSoftwareVolumeStateHash() const noexcept { + return volume_software_set; + } + +private: + bool SetSoftwareVolume(MultipleOutputs &outputs, unsigned volume); + void SetHardwareVolume(MultipleOutputs &outputs, unsigned volume); +}; diff --git a/src/mixer/Volume.hxx b/src/mixer/Volume.hxx deleted file mode 100644 index b245fe7fe..000000000 --- a/src/mixer/Volume.hxx +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright 2003-2021 The Music Player Daemon Project - * http://www.musicpd.org - * - * 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 - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - -#ifndef MPD_VOLUME_HXX -#define MPD_VOLUME_HXX - -class MultipleOutputs; -class BufferedOutputStream; - -void -InvalidateHardwareVolume() noexcept; - -[[gnu::pure]] -int -volume_level_get(const MultipleOutputs &outputs) noexcept; - -/** - * Throws on error. - */ -void -volume_level_change(MultipleOutputs &outputs, unsigned volume); - -bool -read_sw_volume_state(const char *line, MultipleOutputs &outputs); - -void -save_sw_volume_state(BufferedOutputStream &os); - -/** - * Generates a hash number for the current state of the software - * volume control. This is used by timer_save_state_file() to - * determine whether the state has changed and the state file should - * be saved. - */ -[[gnu::pure]] -unsigned -sw_volume_state_get_hash() noexcept; - -#endif diff --git a/src/output/OutputCommand.cxx b/src/output/OutputCommand.cxx index 5772072cb..88c51df5d 100644 --- a/src/output/OutputCommand.cxx +++ b/src/output/OutputCommand.cxx @@ -28,13 +28,15 @@ #include "MultipleOutputs.hxx" #include "Client.hxx" #include "mixer/MixerControl.hxx" -#include "mixer/Volume.hxx" +#include "mixer/Memento.hxx" #include "Idle.hxx" extern unsigned audio_output_state_version; bool -audio_output_enable_index(MultipleOutputs &outputs, unsigned idx) +audio_output_enable_index(MultipleOutputs &outputs, + MixerMemento &mixer_memento, + unsigned idx) { if (idx >= outputs.Size()) return false; @@ -46,7 +48,7 @@ audio_output_enable_index(MultipleOutputs &outputs, unsigned idx) idle_add(IDLE_OUTPUT); if (ao.GetMixer() != nullptr) { - InvalidateHardwareVolume(); + mixer_memento.InvalidateHardwareVolume(); idle_add(IDLE_MIXER); } @@ -58,7 +60,9 @@ audio_output_enable_index(MultipleOutputs &outputs, unsigned idx) } bool -audio_output_disable_index(MultipleOutputs &outputs, unsigned idx) +audio_output_disable_index(MultipleOutputs &outputs, + MixerMemento &mixer_memento, + unsigned idx) { if (idx >= outputs.Size()) return false; @@ -72,7 +76,7 @@ audio_output_disable_index(MultipleOutputs &outputs, unsigned idx) auto *mixer = ao.GetMixer(); if (mixer != nullptr) { mixer_close(mixer); - InvalidateHardwareVolume(); + mixer_memento.InvalidateHardwareVolume(); idle_add(IDLE_MIXER); } @@ -84,7 +88,9 @@ audio_output_disable_index(MultipleOutputs &outputs, unsigned idx) } bool -audio_output_toggle_index(MultipleOutputs &outputs, unsigned idx) +audio_output_toggle_index(MultipleOutputs &outputs, + MixerMemento &mixer_memento, + unsigned idx) { if (idx >= outputs.Size()) return false; @@ -97,7 +103,7 @@ audio_output_toggle_index(MultipleOutputs &outputs, unsigned idx) auto *mixer = ao.GetMixer(); if (mixer != nullptr) { mixer_close(mixer); - InvalidateHardwareVolume(); + mixer_memento.InvalidateHardwareVolume(); idle_add(IDLE_MIXER); } } diff --git a/src/output/OutputCommand.hxx b/src/output/OutputCommand.hxx index 19c18215f..1b586f34f 100644 --- a/src/output/OutputCommand.hxx +++ b/src/output/OutputCommand.hxx @@ -28,26 +28,33 @@ #define MPD_OUTPUT_COMMAND_HXX class MultipleOutputs; +class MixerMemento; /** * Enables an audio output. Returns false if the specified output * does not exist. */ bool -audio_output_enable_index(MultipleOutputs &outputs, unsigned idx); +audio_output_enable_index(MultipleOutputs &outputs, + MixerMemento &mixer_memento, + unsigned idx); /** * Disables an audio output. Returns false if the specified output * does not exist. */ bool -audio_output_disable_index(MultipleOutputs &outputs, unsigned idx); +audio_output_disable_index(MultipleOutputs &outputs, + MixerMemento &mixer_memento, + unsigned idx); /** * Toggles an audio output. Returns false if the specified output * does not exist. */ bool -audio_output_toggle_index(MultipleOutputs &outputs, unsigned idx); +audio_output_toggle_index(MultipleOutputs &outputs, + MixerMemento &mixer_memento, + unsigned idx); #endif