From b979245d6c9491d5cd6bd12b250ccd6dc7a13609 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 5 Aug 2021 17:59:51 +0200 Subject: [PATCH 01/17] decoder/Bridge: call UpdateStreamTag() only if there is no pending seek If UpdateStreamTag() gets called while an initial seek is pending, the result will never be submitted to a MusicChunk. By avoiding the UpdateStreamTag() call in that case (by moving UpdateStreamTag() to after the PrepareInitialSeek() check), the song_tag is preserved until UpdateStreamTag() is called again from SubmitData(). This fixes missing tags in the "httpd" output. Closes https://github.com/MusicPlayerDaemon/MPD/issues/1137 --- NEWS | 1 + src/decoder/Bridge.cxx | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index a56c573e9..ab2ddd9ca 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,7 @@ ver 0.22.10 (not yet released) * tags - fix crash caused by bug in TagBuilder and a few potential reference leaks * output + - httpd: fix missing tag after seeking into a new song - oss: fix channel order of multi-channel files ver 0.22.9 (2021/06/23) diff --git a/src/decoder/Bridge.cxx b/src/decoder/Bridge.cxx index 79ef2c4f7..30a56d53d 100644 --- a/src/decoder/Bridge.cxx +++ b/src/decoder/Bridge.cxx @@ -581,10 +581,6 @@ DecoderBridge::SubmitTag(InputStream *is, Tag &&tag) noexcept decoder_tag = std::make_unique(std::move(tag)); - /* check for a new stream tag */ - - UpdateStreamTag(is); - /* check if we're seeking */ if (PrepareInitialSeek()) @@ -593,6 +589,10 @@ DecoderBridge::SubmitTag(InputStream *is, Tag &&tag) noexcept function here */ return DecoderCommand::SEEK; + /* check for a new stream tag */ + + UpdateStreamTag(is); + /* send tag to music pipe */ if (stream_tag != nullptr) From ef2fc4e6f618fad0823eb68943e132ec565aeafc Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 5 Aug 2021 19:04:55 +0200 Subject: [PATCH 02/17] db/simple/Directory: remove obsolete API doc --- src/db/plugins/simple/Directory.hxx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/db/plugins/simple/Directory.hxx b/src/db/plugins/simple/Directory.hxx index 670860de6..6b8899440 100644 --- a/src/db/plugins/simple/Directory.hxx +++ b/src/db/plugins/simple/Directory.hxx @@ -206,7 +206,6 @@ public: * Looks up a directory by its relative URI. * * @param uri the relative URI - * @return the Directory, or nullptr if none was found */ gcc_pure LookupResult LookupDirectory(std::string_view uri) noexcept; From 1761fb14af6c810735fa456cd19274681a89a1c9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 5 Aug 2021 20:06:15 +0200 Subject: [PATCH 03/17] fs/Traits: add PathTraitsUTF8::IsAbsoluteOrHasScheme() --- src/fs/Traits.cxx | 7 +++++++ src/fs/Traits.hxx | 7 +++++++ src/playlist/PlaylistSong.cxx | 4 ++-- src/song/DetachedSong.cxx | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/fs/Traits.cxx b/src/fs/Traits.cxx index 35d7fa1fd..32f65105a 100644 --- a/src/fs/Traits.cxx +++ b/src/fs/Traits.cxx @@ -19,6 +19,7 @@ #include "Traits.hxx" #include "util/StringCompare.hxx" +#include "util/UriExtract.hxx" #include @@ -220,6 +221,12 @@ PathTraitsUTF8::Build(string_view a, string_view b) noexcept return BuildPathImpl(a, b); } +bool +PathTraitsUTF8::IsAbsoluteOrHasScheme(const_pointer p) noexcept +{ + return IsAbsolute(p) || uri_has_scheme(p); +} + PathTraitsUTF8::const_pointer PathTraitsUTF8::GetBase(const_pointer p) noexcept { diff --git a/src/fs/Traits.hxx b/src/fs/Traits.hxx index a5495a29f..f88cf33ac 100644 --- a/src/fs/Traits.hxx +++ b/src/fs/Traits.hxx @@ -274,6 +274,13 @@ struct PathTraitsUTF8 { return IsSeparator(*p); } + /** + * Is this any kind of absolute URI? (Unlike IsAbsolute(), + * this includes URIs/URLs with a scheme) + */ + [[gnu::pure]] [[gnu::nonnull]] + static bool IsAbsoluteOrHasScheme(const_pointer p) noexcept; + gcc_pure gcc_nonnull_all static bool IsSpecialFilename(const_pointer name) noexcept { return (name[0] == '.' && name[1] == 0) || diff --git a/src/playlist/PlaylistSong.cxx b/src/playlist/PlaylistSong.cxx index e03d580db..135e66393 100644 --- a/src/playlist/PlaylistSong.cxx +++ b/src/playlist/PlaylistSong.cxx @@ -92,8 +92,8 @@ playlist_check_translate_song(DetachedSong &song, std::string_view base_uri, } #endif - if (base_uri.data() != nullptr && !uri_has_scheme(uri) && - !PathTraitsUTF8::IsAbsolute(uri)) + if (base_uri.data() != nullptr && + !PathTraitsUTF8::IsAbsoluteOrHasScheme(uri)) song.SetURI(PathTraitsUTF8::Build(base_uri, uri)); return playlist_check_load_song(song, loader); diff --git a/src/song/DetachedSong.cxx b/src/song/DetachedSong.cxx index 5ba67d097..e94f5bc72 100644 --- a/src/song/DetachedSong.cxx +++ b/src/song/DetachedSong.cxx @@ -60,7 +60,7 @@ DetachedSong::IsInDatabase() const noexcept GetRealURI() is never relative */ const char *_uri = GetURI(); - return !uri_has_scheme(_uri) && !PathTraitsUTF8::IsAbsolute(_uri); + return !PathTraitsUTF8::IsAbsoluteOrHasScheme(_uri); } SignedSongTime From 8e0d39ae944263339773c6804846751701092fc6 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 5 Aug 2021 20:10:51 +0200 Subject: [PATCH 04/17] db/update/Playlist: prepend "../" only for relative URIs Prepending "../" to absolute URIs would break them. --- NEWS | 1 + src/db/update/Playlist.cxx | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index ab2ddd9ca..28a9acb36 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ ver 0.22.10 (not yet released) - support "albumart" for virtual tracks in CUE sheets * database - simple: fix crash bug + - simple: fix absolute paths in CUE "as_directory" entries * input - curl: fix crash bug after stream with Icy metadata was closed by peer - tidal: remove defunct unmaintained plugin diff --git a/src/db/update/Playlist.cxx b/src/db/update/Playlist.cxx index e3ad54ae0..927818ee0 100644 --- a/src/db/update/Playlist.cxx +++ b/src/db/update/Playlist.cxx @@ -30,6 +30,7 @@ #include "playlist/SongEnumerator.hxx" #include "storage/FileInfo.hxx" #include "storage/StorageInterface.hxx" +#include "fs/Traits.hxx" #include "util/StringFormat.hxx" #include "Log.hxx" @@ -70,7 +71,14 @@ UpdateWalk::UpdatePlaylistFile(Directory &parent, std::string_view name, auto db_song = std::make_unique(std::move(*song), *directory); - db_song->target = "../" + db_song->filename; + db_song->target = + PathTraitsUTF8::IsAbsoluteOrHasScheme(db_song->filename.c_str()) + ? db_song->filename + /* prepend "../" to relative paths to + go from the virtual directory + (DEVICE_PLAYLIST) to the containing + directory */ + : "../" + db_song->filename; db_song->filename = StringFormat<64>("track%04u", ++track); From 1985786ed25b82eddfdabe104c8698967dbb4cca Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 5 Aug 2021 18:57:11 +0200 Subject: [PATCH 05/17] db/simple: prune CUE entries from database for non-existent songs Closes https://github.com/MusicPlayerDaemon/MPD/issues/1019 --- NEWS | 1 + src/db/plugins/simple/Directory.cxx | 17 +++++++++++++++++ src/db/plugins/simple/Directory.hxx | 9 ++++++++- src/db/update/Walk.cxx | 27 +++++++++++++++++++++++++++ src/db/update/Walk.hxx | 6 ++++++ 5 files changed, 59 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 28a9acb36..3d6f0a112 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ ver 0.22.10 (not yet released) * database - simple: fix crash bug - simple: fix absolute paths in CUE "as_directory" entries + - simple: prune CUE entries from database for non-existent songs * input - curl: fix crash bug after stream with Icy metadata was closed by peer - tidal: remove defunct unmaintained plugin diff --git a/src/db/plugins/simple/Directory.cxx b/src/db/plugins/simple/Directory.cxx index 4c7bb2873..b03994730 100644 --- a/src/db/plugins/simple/Directory.cxx +++ b/src/db/plugins/simple/Directory.cxx @@ -109,6 +109,23 @@ Directory::FindChild(std::string_view name) const noexcept return nullptr; } +bool +Directory::TargetExists(std::string_view _target) const noexcept +{ + StringView target{_target}; + + if (target.SkipPrefix("../")) { + if (parent == nullptr) + return false; + + return parent->TargetExists(target); + } + + /* sorry for the const_cast ... */ + const auto lr = const_cast(this)->LookupDirectory(target); + return lr.directory->FindSong(lr.rest) != nullptr; +} + void Directory::PruneEmpty() noexcept { diff --git a/src/db/plugins/simple/Directory.hxx b/src/db/plugins/simple/Directory.hxx index 6b8899440..87e8351e7 100644 --- a/src/db/plugins/simple/Directory.hxx +++ b/src/db/plugins/simple/Directory.hxx @@ -118,13 +118,17 @@ public: return new Directory(std::string(), nullptr); } + bool IsPlaylist() const noexcept { + return device == DEVICE_PLAYLIST; + } + /** * Is this really a regular file which is being treated like a * directory? */ bool IsReallyAFile() const noexcept { return device == DEVICE_INARCHIVE || - device == DEVICE_PLAYLIST || + IsPlaylist() || device == DEVICE_CONTAINER; } @@ -210,6 +214,9 @@ public: gcc_pure LookupResult LookupDirectory(std::string_view uri) noexcept; + [[gnu::pure]] + bool TargetExists(std::string_view target) const noexcept; + gcc_pure bool IsEmpty() const noexcept { return children.empty() && diff --git a/src/db/update/Walk.cxx b/src/db/update/Walk.cxx index b1fb675e6..c5fb8230d 100644 --- a/src/db/update/Walk.cxx +++ b/src/db/update/Walk.cxx @@ -133,6 +133,28 @@ UpdateWalk::PurgeDeletedFromDirectory(Directory &directory) noexcept } } +void +UpdateWalk::PurgeDanglingFromPlaylists(Directory &directory) noexcept +{ + /* recurse */ + for (Directory &child : directory.children) + PurgeDanglingFromPlaylists(child); + + if (!directory.IsPlaylist()) + /* this check is only for virtual directories + representing a playlist file */ + return; + + directory.ForEachSongSafe([&](Song &song){ + if (!song.target.empty() && + !PathTraitsUTF8::IsAbsoluteOrHasScheme(song.target.c_str()) && + !directory.TargetExists(song.target)) { + editor.DeleteSong(directory, &song); + modified = true; + } + }); +} + #ifndef _WIN32 static bool update_directory_stat(Storage &storage, Directory &directory) noexcept @@ -530,5 +552,10 @@ UpdateWalk::Walk(Directory &root, const char *path, bool discard) noexcept UpdateDirectory(root, exclude_list, info); } + { + const ScopeDatabaseLock protect; + PurgeDanglingFromPlaylists(root); + } + return modified; } diff --git a/src/db/update/Walk.hxx b/src/db/update/Walk.hxx index 433d1a3a1..984230ce9 100644 --- a/src/db/update/Walk.hxx +++ b/src/db/update/Walk.hxx @@ -85,6 +85,12 @@ private: void PurgeDeletedFromDirectory(Directory &directory) noexcept; + /** + * Remove all virtual songs inside playlists whose "target" + * field points to a non-existing song file. + */ + void PurgeDanglingFromPlaylists(Directory &directory) noexcept; + void UpdateSongFile2(Directory &directory, const char *name, const char *suffix, const StorageFileInfo &info) noexcept; From 5d73eda115d8044101b172b546abb0d296ed6a29 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 17:19:41 +0200 Subject: [PATCH 06/17] doc/plugins.rst: move filter graph URL to ffmpeg.org --- doc/plugins.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/plugins.rst b/doc/plugins.rst index bac76f106..eb88435ad 100644 --- a/doc/plugins.rst +++ b/doc/plugins.rst @@ -1169,7 +1169,7 @@ This plugin requires building with ``libavfilter`` (FFmpeg). * - **graph "..."** - Specifies the ``libavfilter`` graph; read the `FFmpeg documentation - `_ + `_ for details From dde77ec6bd63414a07d280052e400cedce5197ed Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 17:17:39 +0200 Subject: [PATCH 07/17] python/build/libs.py: update CURL to 7.78.0 --- python/build/libs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/build/libs.py b/python/build/libs.py index d3e50ab09..a62a0bb67 100644 --- a/python/build/libs.py +++ b/python/build/libs.py @@ -385,8 +385,8 @@ openssl = OpenSSLProject( ) curl = AutotoolsProject( - 'https://curl.se/download/curl-7.76.1.tar.xz', - '64bb5288c39f0840c07d077e30d9052e1cbb9fa6c2dc52523824cc859e679145', + 'https://curl.se/download/curl-7.78.0.tar.xz', + 'be42766d5664a739c3974ee3dfbbcbe978a4ccb1fe628bb1d9b59ac79e445fb5', 'lib/libcurl.a', [ '--disable-shared', '--enable-static', From 0f56ddb80554e014714b9d0385a0bf175521a61d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 17:20:58 +0200 Subject: [PATCH 08/17] python/build/libs.py: update OpenSSL to 3.0.0-beta2 --- python/build/libs.py | 4 ++-- python/build/project.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/python/build/libs.py b/python/build/libs.py index a62a0bb67..16ff0cff7 100644 --- a/python/build/libs.py +++ b/python/build/libs.py @@ -379,8 +379,8 @@ ffmpeg = FfmpegProject( ) openssl = OpenSSLProject( - 'https://www.openssl.org/source/openssl-3.0.0-alpha16.tar.gz', - '08ce8244b59d75f40f91170dfcb012bf25309cdcb1fef9502e39d694f883d1d1', + 'https://www.openssl.org/source/openssl-3.0.0-beta2.tar.gz', + 'e76ab22879201b12f014393ee4becec7f264d8f6955b1036839128002868df71', 'include/openssl/ossl_typ.h', ) diff --git a/python/build/project.py b/python/build/project.py index 374ccdb14..e0868b27b 100644 --- a/python/build/project.py +++ b/python/build/project.py @@ -20,7 +20,7 @@ class Project: self.base = base if name is None or version is None: - m = re.match(r'^([-\w]+)-(\d[\d.]*[a-z]?[\d.]*(?:-alpha\d+)?)(\+.*)?$', self.base) + m = re.match(r'^([-\w]+)-(\d[\d.]*[a-z]?[\d.]*(?:-(?:alpha|beta)\d+)?)$', self.base) if name is None: name = m.group(1) if version is None: version = m.group(2) From 694debd4cc7f13e70da78ee0b6eb85745620e099 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 20 Apr 2021 05:37:49 +0200 Subject: [PATCH 09/17] build/openssl: pass RANLIB=... to "make install" The "install_dev" target runs ranlib during installation, and this can break the Android build. --- python/build/openssl.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python/build/openssl.py b/python/build/openssl.py index a7350e6ac..ecf88d4e2 100644 --- a/python/build/openssl.py +++ b/python/build/openssl.py @@ -17,6 +17,12 @@ class OpenSSLProject(MakeProject): 'build_libs', ] + def get_make_install_args(self, toolchain): + # OpenSSL's Makefile runs "ranlib" during installation + return MakeProject.get_make_install_args(self, toolchain) + [ + 'RANLIB=' + toolchain.ranlib, + ] + def build(self, toolchain): src = self.unpack(toolchain, out_of_tree=False) From b0e95388554eeef78e3e3c4c6eb401b297b7603b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 17:30:45 +0200 Subject: [PATCH 10/17] build/openssl: pass --cross-compile-prefix to ./Configure --- android/build.py | 3 ++- python/build/openssl.py | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/android/build.py b/android/build.py index eb6600111..664bcccdd 100755 --- a/android/build.py +++ b/android/build.py @@ -91,8 +91,9 @@ class AndroidNdkToolchain: self.arch = arch self.install_prefix = install_prefix + self.toolchain_arch = abi_info['toolchain_arch'] - toolchain_path = os.path.join(ndk_path, 'toolchains', abi_info['toolchain_arch'] + '-' + gcc_version, 'prebuilt', build_arch) + toolchain_path = os.path.join(ndk_path, 'toolchains', self.toolchain_arch + '-' + gcc_version, 'prebuilt', build_arch) llvm_path = os.path.join(ndk_path, 'toolchains', 'llvm', 'prebuilt', build_arch) llvm_triple = abi_info['llvm_triple'] + android_api_level diff --git a/python/build/openssl.py b/python/build/openssl.py index ecf88d4e2..605a04c74 100644 --- a/python/build/openssl.py +++ b/python/build/openssl.py @@ -48,6 +48,7 @@ class OpenSSLProject(MakeProject): } openssl_arch = openssl_archs[toolchain.arch] + cross_compile_prefix = toolchain.toolchain_arch + '-' subprocess.check_call(['./Configure', 'no-shared', @@ -56,6 +57,7 @@ class OpenSSLProject(MakeProject): 'no-tests', 'no-asm', # "asm" causes build failures on Windows openssl_arch, + '--cross-compile-prefix=' + cross_compile_prefix, '--prefix=' + toolchain.install_prefix], cwd=src, env=toolchain.env) MakeProject.build(self, toolchain, src) From ad6e303047397a9ef6cf9790e309f1cf438f21c8 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 17:53:24 +0200 Subject: [PATCH 11/17] mixer/alsa: move code to GetNormalizedVolume() --- src/mixer/plugins/AlsaMixerPlugin.cxx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/mixer/plugins/AlsaMixerPlugin.cxx b/src/mixer/plugins/AlsaMixerPlugin.cxx index 9d264993a..23b641cca 100644 --- a/src/mixer/plugins/AlsaMixerPlugin.cxx +++ b/src/mixer/plugins/AlsaMixerPlugin.cxx @@ -95,6 +95,13 @@ public: void Close() noexcept override; int GetVolume() override; void SetVolume(unsigned volume) override; + +private: + [[gnu::pure]] + double GetNormalizedVolume() const noexcept { + return get_normalized_playback_volume(elem, + SND_MIXER_SCHN_FRONT_LEFT); + } }; static constexpr Domain alsa_mixer_domain("alsa_mixer"); @@ -273,7 +280,7 @@ AlsaMixer::GetVolume() throw FormatRuntimeError("snd_mixer_handle_events() failed: %s", snd_strerror(err)); - return lround(100 * get_normalized_playback_volume(elem, SND_MIXER_SCHN_FRONT_LEFT)); + return lround(100 * GetNormalizedVolume()); } void @@ -281,7 +288,7 @@ AlsaMixer::SetVolume(unsigned volume) { assert(handle != nullptr); - double cur = get_normalized_playback_volume(elem, SND_MIXER_SCHN_FRONT_LEFT); + double cur = GetNormalizedVolume(); int delta = volume - lround(100.*cur); int err = set_normalized_playback_volume(elem, cur + 0.01*delta, delta); if (err < 0) From 5f5b5f63af7018095513ff84ac6d08418fdb42c9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 17:55:23 +0200 Subject: [PATCH 12/17] mixer/alsa: move code to NormalizedToPercent() --- src/mixer/plugins/AlsaMixerPlugin.cxx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mixer/plugins/AlsaMixerPlugin.cxx b/src/mixer/plugins/AlsaMixerPlugin.cxx index 23b641cca..fd4c6817d 100644 --- a/src/mixer/plugins/AlsaMixerPlugin.cxx +++ b/src/mixer/plugins/AlsaMixerPlugin.cxx @@ -97,6 +97,11 @@ public: void SetVolume(unsigned volume) override; private: + [[gnu::const]] + static unsigned NormalizedToPercent(double normalized) noexcept { + return lround(100 * normalized); + } + [[gnu::pure]] double GetNormalizedVolume() const noexcept { return get_normalized_playback_volume(elem, @@ -280,7 +285,7 @@ AlsaMixer::GetVolume() throw FormatRuntimeError("snd_mixer_handle_events() failed: %s", snd_strerror(err)); - return lround(100 * GetNormalizedVolume()); + return NormalizedToPercent(GetNormalizedVolume()); } void @@ -289,7 +294,7 @@ AlsaMixer::SetVolume(unsigned volume) assert(handle != nullptr); double cur = GetNormalizedVolume(); - int delta = volume - lround(100.*cur); + int delta = volume - NormalizedToPercent(cur); int err = set_normalized_playback_volume(elem, cur + 0.01*delta, delta); if (err < 0) throw FormatRuntimeError("failed to set ALSA volume: %s", From e8f328d8adc91568e5c0c683e640b4e70bdeb563 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 17:56:29 +0200 Subject: [PATCH 13/17] mixer/alsa: move code to GetPercentVolume() --- src/mixer/plugins/AlsaMixerPlugin.cxx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/mixer/plugins/AlsaMixerPlugin.cxx b/src/mixer/plugins/AlsaMixerPlugin.cxx index fd4c6817d..844b56105 100644 --- a/src/mixer/plugins/AlsaMixerPlugin.cxx +++ b/src/mixer/plugins/AlsaMixerPlugin.cxx @@ -107,6 +107,11 @@ private: return get_normalized_playback_volume(elem, SND_MIXER_SCHN_FRONT_LEFT); } + + [[gnu::pure]] + unsigned GetPercentVolume() const noexcept { + return NormalizedToPercent(GetNormalizedVolume()); + } }; static constexpr Domain alsa_mixer_domain("alsa_mixer"); @@ -285,7 +290,7 @@ AlsaMixer::GetVolume() throw FormatRuntimeError("snd_mixer_handle_events() failed: %s", snd_strerror(err)); - return NormalizedToPercent(GetNormalizedVolume()); + return GetPercentVolume(); } void From 3b6d4e66735a0ec9bfe205a528df1a617e78b575 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 18:00:04 +0200 Subject: [PATCH 14/17] mixer/alsa: move alsa_mixer_elem_callback() into the AlsaMixer class --- src/mixer/plugins/AlsaMixerPlugin.cxx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/mixer/plugins/AlsaMixerPlugin.cxx b/src/mixer/plugins/AlsaMixerPlugin.cxx index 844b56105..17f38a14b 100644 --- a/src/mixer/plugins/AlsaMixerPlugin.cxx +++ b/src/mixer/plugins/AlsaMixerPlugin.cxx @@ -112,6 +112,10 @@ private: unsigned GetPercentVolume() const noexcept { return NormalizedToPercent(GetNormalizedVolume()); } + + static int ElemCallback(snd_mixer_elem_t *elem, + unsigned mask) noexcept; + }; static constexpr Domain alsa_mixer_domain("alsa_mixer"); @@ -155,8 +159,8 @@ AlsaMixerMonitor::DispatchSockets() noexcept * */ -static int -alsa_mixer_elem_callback(snd_mixer_elem_t *elem, unsigned mask) +int +AlsaMixer::ElemCallback(snd_mixer_elem_t *elem, unsigned mask) noexcept { AlsaMixer &mixer = *(AlsaMixer *) snd_mixer_elem_get_callback_private(elem); @@ -244,7 +248,7 @@ AlsaMixer::Setup() throw FormatRuntimeError("no such mixer control: %s", control); snd_mixer_elem_set_callback_private(elem, this); - snd_mixer_elem_set_callback(elem, alsa_mixer_elem_callback); + snd_mixer_elem_set_callback(elem, ElemCallback); monitor = new AlsaMixerMonitor(event_loop, handle); } From 351b39e0c5b70380074464522264e5106bf476da Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 17:59:26 +0200 Subject: [PATCH 15/17] mixer/alsa: skip the snd_mixer_handle_events() call in alsa_mixer_elem_callback() snd_mixer_handle_events() has already been called by DispatchSockets(). This way, we can also skip the exception handler. --- src/mixer/plugins/AlsaMixerPlugin.cxx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/mixer/plugins/AlsaMixerPlugin.cxx b/src/mixer/plugins/AlsaMixerPlugin.cxx index 17f38a14b..27243cfb4 100644 --- a/src/mixer/plugins/AlsaMixerPlugin.cxx +++ b/src/mixer/plugins/AlsaMixerPlugin.cxx @@ -166,11 +166,8 @@ AlsaMixer::ElemCallback(snd_mixer_elem_t *elem, unsigned mask) noexcept snd_mixer_elem_get_callback_private(elem); if (mask & SND_CTL_EVENT_MASK_VALUE) { - try { - int volume = mixer.GetVolume(); - mixer.listener.OnMixerVolumeChanged(mixer, volume); - } catch (...) { - } + int volume = mixer.GetPercentVolume(); + mixer.listener.OnMixerVolumeChanged(mixer, volume); } return 0; From 04eb911a5110a2a61ea7fcb5995e77bdcc21bbc4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 18:15:52 +0200 Subject: [PATCH 16/17] mixer/alsa: use cached values to work around rounding errors This replaces 967af603270246f1c97e11b8bad6a0eb65c81318 with a more effective workaround. Closes https://github.com/MusicPlayerDaemon/MPD/issues/822 --- NEWS | 2 ++ src/mixer/plugins/AlsaMixerPlugin.cxx | 45 ++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 3d6f0a112..f0a8434c2 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,8 @@ ver 0.22.10 (not yet released) * output - httpd: fix missing tag after seeking into a new song - oss: fix channel order of multi-channel files +* mixer + - alsa: fix yet more rounding errors ver 0.22.9 (2021/06/23) * database diff --git a/src/mixer/plugins/AlsaMixerPlugin.cxx b/src/mixer/plugins/AlsaMixerPlugin.cxx index 27243cfb4..8328e11a4 100644 --- a/src/mixer/plugins/AlsaMixerPlugin.cxx +++ b/src/mixer/plugins/AlsaMixerPlugin.cxx @@ -80,6 +80,24 @@ class AlsaMixer final : public Mixer { AlsaMixerMonitor *monitor; + /** + * These fields are our workaround for rounding errors when + * the resolution of a mixer knob isn't fine enough to + * represent all 101 possible values (0..100). + * + * "desired_volume" is the percent value passed to + * SetVolume(), and "resulting_volume" is the volume which was + * actually set, and would be returned by the next + * GetPercentVolume() call. + * + * When GetVolume() is called, we compare the + * "resulting_volume" with the value returned by + * GetPercentVolume(), and if it's the same, we're still on + * the same value that was previously set (but may have been + * rounded down or up). + */ + int desired_volume, resulting_volume; + public: AlsaMixer(EventLoop &_event_loop, MixerListener &_listener) :Mixer(alsa_mixer_plugin, _listener), @@ -167,6 +185,17 @@ AlsaMixer::ElemCallback(snd_mixer_elem_t *elem, unsigned mask) noexcept if (mask & SND_CTL_EVENT_MASK_VALUE) { int volume = mixer.GetPercentVolume(); + + if (mixer.resulting_volume >= 0 && + volume == mixer.resulting_volume) + /* still the same volume (this might be a + callback caused by SetVolume()) - switch to + desired_volume */ + volume = mixer.desired_volume; + else + /* flush */ + mixer.desired_volume = mixer.resulting_volume = -1; + mixer.listener.OnMixerVolumeChanged(mixer, volume); } @@ -253,6 +282,8 @@ AlsaMixer::Setup() void AlsaMixer::Open() { + desired_volume = resulting_volume = -1; + int err; err = snd_mixer_open(&handle, 0); @@ -291,7 +322,12 @@ AlsaMixer::GetVolume() throw FormatRuntimeError("snd_mixer_handle_events() failed: %s", snd_strerror(err)); - return GetPercentVolume(); + int volume = GetPercentVolume(); + if (resulting_volume >= 0 && volume == resulting_volume) + /* we're still on the value passed to SetVolume() */ + volume = desired_volume; + + return volume; } void @@ -299,12 +335,13 @@ AlsaMixer::SetVolume(unsigned volume) { assert(handle != nullptr); - double cur = GetNormalizedVolume(); - int delta = volume - NormalizedToPercent(cur); - int err = set_normalized_playback_volume(elem, cur + 0.01*delta, delta); + int err = set_normalized_playback_volume(elem, 0.01*volume, 1); if (err < 0) throw FormatRuntimeError("failed to set ALSA volume: %s", snd_strerror(err)); + + desired_volume = volume; + resulting_volume = GetPercentVolume(); } const MixerPlugin alsa_mixer_plugin = { From 64c39af556e7c15dcbbad27da897d0bf94b391dc Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 6 Aug 2021 18:16:59 +0200 Subject: [PATCH 17/17] release v0.22.10 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index f0a8434c2..ab1194dee 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.22.10 (not yet released) +ver 0.22.10 (2021/08/06) * protocol - support "albumart" for virtual tracks in CUE sheets * database