From 1ca4b31b8def9d1961b2cc5622c9dfdfc0fa11d0 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 17 Dec 2022 13:58:56 +0100 Subject: [PATCH 01/16] Some changes to please codeQL --- .../scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp | 7 +++---- .../scrobbling/impl/listenbrainz/ListensSynchronizer.cpp | 7 +++---- src/lms/ui/explore/ReleaseView.cpp | 1 - 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp b/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp index 1e3e654d..e9265122 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp @@ -19,6 +19,7 @@ #include "ListenBrainzScrobbler.hpp" +#include #include #include #include @@ -234,8 +235,7 @@ namespace Scrobbling::ListenBrainz auto itContext {_userContexts.find(userId)}; if (itContext == std::cend(_userContexts)) { - [[maybe_unused]] auto [itNewContext, inserted] {_userContexts.emplace(userId, userId)}; - itContext = itNewContext; + std::tie(itContext, std::ignore) = _userContexts.emplace(userId, userId); } return itContext->second; @@ -246,8 +246,7 @@ namespace Scrobbling::ListenBrainz { return std::any_of(std::cbegin(_userContexts), std::cend(_userContexts), [](const auto& contextEntry) { - [[maybe_unused]] const auto& [userId, context] {contextEntry}; - return context.syncing; + return contextEntry.second.syncing; }); } diff --git a/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp b/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp index 2542ede7..f1a79df0 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp @@ -19,6 +19,7 @@ #include "ListenBrainzScrobbler.hpp" +#include #include #include #include @@ -354,8 +355,7 @@ namespace Scrobbling::ListenBrainz auto itContext {_userContexts.find(userId)}; if (itContext == std::cend(_userContexts)) { - [[maybe_unused]] auto [itNewContext, inserted] {_userContexts.emplace(userId, userId)}; - itContext = itNewContext; + std::tie(itContext, std::ignore) = _userContexts.emplace(userId, userId); } return itContext->second; @@ -366,8 +366,7 @@ namespace Scrobbling::ListenBrainz { return std::any_of(std::cbegin(_userContexts), std::cend(_userContexts), [](const auto& contextEntry) { - [[maybe_unused]] const auto& [userId, context] {contextEntry}; - return context.syncing; + return contextEntry.second.syncing; }); } diff --git a/src/lms/ui/explore/ReleaseView.cpp b/src/lms/ui/explore/ReleaseView.cpp index 3c1d4176..1ea63ccd 100644 --- a/src/lms/ui/explore/ReleaseView.cpp +++ b/src/lms/ui/explore/ReleaseView.cpp @@ -118,7 +118,6 @@ showReleaseInfoModal(Database::ReleaseId releaseId) artistTable->addWidget(std::move(artistsEntry)); } - // TODO: save in DB and mean all this for (TrackId trackId : Track::find(LmsApp->getDbSession(), Track::FindParameters {}.setRelease(releaseId).setRange(Range {0, 1})).results) { From fe424615aedcd02d8ccb95989501c64f546f4084 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 17 Dec 2022 14:11:31 +0100 Subject: [PATCH 02/16] Fixed display for roleless performers --- src/lms/ui/explore/ReleaseView.cpp | 9 ++++++++- src/lms/ui/explore/TrackListHelpers.cpp | 7 +++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/lms/ui/explore/ReleaseView.cpp b/src/lms/ui/explore/ReleaseView.cpp index 1ea63ccd..1cd3561f 100644 --- a/src/lms/ui/explore/ReleaseView.cpp +++ b/src/lms/ui/explore/ReleaseView.cpp @@ -77,7 +77,7 @@ showReleaseInfoModal(Database::ReleaseId releaseId) if (artistIds.results.empty()) return; - Wt::WString typeStr {Wt::WString::trn(type, artistIds.results.size())};; + Wt::WString typeStr {Wt::WString::trn(type, artistIds.results.size())}; for (ArtistId artistId : artistIds.results) artistMap[typeStr].insert(artistId); }; @@ -109,6 +109,13 @@ showReleaseInfoModal(Database::ReleaseId releaseId) addArtists(TrackArtistLinkType::Producer, "Lms.Explore.Artists.linktype-producer"); addPerformerArtists(); + if (auto itRolelessPerformers {artistMap.find("")}; itRolelessPerformers != std::cend(artistMap)) + { + Wt::WString performersStr {Wt::WString::trn("Lms.Explore.Artists.linktype-performer", itRolelessPerformers->second.size())}; + artistMap[performersStr] = std::move(itRolelessPerformers->second); + artistMap.erase(itRolelessPerformers); + } + for (const auto& [role, artistIds] : artistMap) { std::unique_ptr artistContainer {Utils::createArtistContainer(std::vector (std::cbegin(artistIds), std::cend(artistIds)))}; diff --git a/src/lms/ui/explore/TrackListHelpers.cpp b/src/lms/ui/explore/TrackListHelpers.cpp index d7f082fc..6da0f2f4 100644 --- a/src/lms/ui/explore/TrackListHelpers.cpp +++ b/src/lms/ui/explore/TrackListHelpers.cpp @@ -103,6 +103,13 @@ namespace UserInterface::TrackListHelpers addArtists(TrackArtistLinkType::Producer, "Lms.Explore.Artists.linktype-producer"); addPerformerArtists(); + if (auto itRolelessPerformers {artistMap.find("")}; itRolelessPerformers != std::cend(artistMap)) + { + Wt::WString performersStr {Wt::WString::trn("Lms.Explore.Artists.linktype-performer", itRolelessPerformers->second.size())}; + artistMap[performersStr] = std::move(itRolelessPerformers->second); + artistMap.erase(itRolelessPerformers); + } + for (const auto& [role, artistIds] : artistMap) { std::unique_ptr artistContainer {Utils::createArtistContainer(std::vector (std::cbegin(artistIds), std::cend(artistIds)))}; From 6a87c261037f1b6d44766ca0d59a1a231a7591ae Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 17 Dec 2022 14:12:17 +0100 Subject: [PATCH 03/16] Removed useless badge --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c0fabf8a..82a56945 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # LMS - Lightweight Music Server -[![Last Release](https://img.shields.io/github/v/release/epoupon/lms?logo=github&label=latest)](https://github.com/epoupon/lms/releases) [![Build](https://img.shields.io/github/workflow/status/epoupon/lms/Build?logo=github)](https://github.com/epoupon/lms/actions) +[![Last Release](https://img.shields.io/github/v/release/epoupon/lms?logo=github&label=latest)](https://github.com/epoupon/lms/releases) _LMS_ is a self-hosted music streaming software: access your music collection from anywhere using a web interface! From ce28aa953322cbf08b469550eef195094c253269 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 17 Dec 2022 15:33:16 +0100 Subject: [PATCH 04/16] Better handle tags when no role is set --- src/libs/metadata/impl/TagLibParser.cpp | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libs/metadata/impl/TagLibParser.cpp b/src/libs/metadata/impl/TagLibParser.cpp index d2e21215..1227c11d 100644 --- a/src/libs/metadata/impl/TagLibParser.cpp +++ b/src/libs/metadata/impl/TagLibParser.cpp @@ -140,6 +140,7 @@ getPerformerArtists(const TagLib::PropertyMap& properties, PerformerContainer performers; // picard stores like this: (see https://picard-docs.musicbrainz.org/en/appendices/tag_mapping.html#performer) + // We may hit both styles for the same track // PERFORMER: artist (role) if (const std::vector artistNames {getPropertyValuesFirstMatchAs(properties, artistTagNames)}; !artistNames.empty()) { @@ -151,23 +152,20 @@ getPerformerArtists(const TagLib::PropertyMap& properties, } } // PERFORMER:role (MP3) - else + for (const auto& [key, values] : properties) { - for (const auto& [key, values] : properties) + if (key.startsWith("PERFORMER:")) { - if (key.startsWith("PERFORMER")) + std::string performerStr {key.to8Bit(true)}; + std::string role; + if (const std::size_t rolePos {performerStr.find(':')}; rolePos != std::string::npos) { - std::string performerStr {key.to8Bit(true)}; - std::string role; - if (const std::size_t rolePos {performerStr.find(':')}; rolePos != std::string::npos) - { - role = StringUtils::stringToLower(performerStr.substr(rolePos + 1, performerStr.size() - rolePos + 1)); - StringUtils::capitalize(role); - } - - for (const auto& value : values) - performers[role].push_back(Artist {value.to8Bit(true)}); + role = StringUtils::stringToLower(performerStr.substr(rolePos + 1, performerStr.size() - rolePos + 1)); + StringUtils::capitalize(role); } + + for (const auto& value : values) + performers[role].push_back(Artist {value.to8Bit(true)}); } } From 040973209e699cff73f1255eca6fa721ffa95107 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 17 Dec 2022 19:08:35 +0100 Subject: [PATCH 05/16] Subsonic API: handle tracks and releases in getSimilarSongs endpoint. fixes #245 --- src/libs/subsonic/impl/SubsonicResource.cpp | 114 ++++++++++++++------ 1 file changed, 84 insertions(+), 30 deletions(-) diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index 82d1efa9..27dd9211 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -1169,49 +1169,103 @@ handleGetArtistsRequest(RequestContext& context) } static -Response -handleGetSimilarSongsRequestCommon(RequestContext& context, bool id3) +std::vector +findSimilarSongs(RequestContext& context, ArtistId artistId, std::size_t count) { - // Mandatory params - const ArtistId artistId {getMandatoryParameterAs(context.parameters, "id")}; + // API says: "Returns a random collection of songs from the given artist and similar artists" + const std::size_t similarArtistCount {count / 5}; + std::vector artistIds {Service::get()->getSimilarArtists(artistId, {TrackArtistLinkType::Artist, TrackArtistLinkType::ReleaseArtist}, similarArtistCount)}; + artistIds.push_back(artistId); - // Optional params - std::size_t count {getParameterAs(context.parameters, "count").value_or(50)}; - - const auto similarArtistIds {Service::get()->getSimilarArtists(artistId, {TrackArtistLinkType::Artist, TrackArtistLinkType::ReleaseArtist}, 5)}; + const std::size_t meanTrackCountPerArtist {(count / artistIds.size()) + 1}; auto transaction {context.dbSession.createSharedTransaction()}; - const Artist::pointer artist {Artist::find(context.dbSession, artistId)}; - if (!artist) - throw RequestedDataNotFoundError {}; + std::vector tracks; + tracks.reserve(count); - const User::pointer user {User::find(context.dbSession, context.userId)}; - if (!user) - throw UserNotAuthorizedError {}; - - // "Returns a random collection of songs from the given artist and similar artists" - const auto trackResults {Track::find(context.dbSession, Track::FindParameters {} - .setArtist(artist->getId()) - .setRange({0, count / 2}) - .setSortMethod(TrackSortMethod::Random))}; - - std::vector tracks {trackResults.results}; - - for (const ArtistId similarArtistId : similarArtistIds) + for (const ArtistId id : artistIds) { - const auto similarArtistTracks {Track::find(context.dbSession, Track::FindParameters {} - .setArtist(similarArtistId) - .setRange({0, (count / 2) / 5}) - .setSortMethod(TrackSortMethod::Random))}; + Track::FindParameters params; + params.setArtist(id); + params.setRange({0, meanTrackCountPerArtist}); + params.setSortMethod(TrackSortMethod::Random); + const auto artistTracks {Track::find(context.dbSession, params)}; tracks.insert(std::end(tracks), - std::begin(similarArtistTracks.results), - std::end(similarArtistTracks.results)); + std::begin(artistTracks.results), + std::end(artistTracks.results)); } + return tracks; +} + +static +std::vector +findSimilarSongs(RequestContext& context, ReleaseId releaseId, std::size_t count) +{ + // API says: "Returns a random collection of songs from the given artist and similar artists" + // so let's extend this for release + const std::size_t similarReleaseCount {count / 5}; + std::vector releaseIds {Service::get()->getSimilarReleases(releaseId, similarReleaseCount)}; + releaseIds.push_back(releaseId); + + const std::size_t meanTrackCountPerRelease {(count / releaseIds.size()) + 1}; + + auto transaction {context.dbSession.createSharedTransaction()}; + + std::vector tracks; + tracks.reserve(count); + + for (const ReleaseId id : releaseIds) + { + Track::FindParameters params; + params.setRelease(id); + params.setRange({0, meanTrackCountPerRelease}); + params.setSortMethod(TrackSortMethod::Random); + + const auto releaseTracks {Track::find(context.dbSession, params)}; + tracks.insert(std::end(tracks), + std::begin(releaseTracks.results), + std::end(releaseTracks.results)); + } + + return tracks; +} + +static +std::vector +findSimilarSongs(RequestContext&, TrackId trackId, std::size_t count) +{ + return Service::get()->findSimilarTracks({trackId}, count); +} + +static +Response +handleGetSimilarSongsRequestCommon(RequestContext& context, bool id3) +{ + // Optional params + std::size_t count {getParameterAs(context.parameters, "count").value_or(50)}; + + std::vector tracks; + + if (const auto artistId {getParameterAs(context.parameters, "id")}) + tracks = findSimilarSongs(context, *artistId, count); + else if (const auto releaseId {getParameterAs(context.parameters, "id")}) + tracks = findSimilarSongs(context, *releaseId, count); + else if (const auto trackId {getParameterAs(context.parameters, "id")}) + tracks = findSimilarSongs(context, *trackId, count); + else + throw BadParameterGenericError {"id"}; + Random::shuffleContainer(tracks); + auto transaction {context.dbSession.createSharedTransaction()}; + + User::pointer user {User::find(context.dbSession, context.userId)}; + if (!user) + throw UserNotAuthorizedError {}; + Response response {Response::createOkResponse(context.serverProtocolVersion)}; Response::Node& similarSongsNode {response.createNode(id3 ? "similarSongs2" : "similarSongs")}; for (const TrackId trackId : tracks) From c6fe63f3b2bd2263544b6b441018b47568551cef Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 30 Dec 2022 21:01:17 +0100 Subject: [PATCH 06/16] Simplified interface + fixed segfault when eof is reached. ref #287 --- src/libs/av/impl/TranscodeResourceHandler.cpp | 4 +- src/libs/av/impl/Transcoder.cpp | 13 ---- src/libs/av/impl/Transcoder.hpp | 3 - src/libs/utils/impl/ChildProcess.cpp | 71 ++++++++----------- src/libs/utils/impl/ChildProcess.hpp | 4 +- .../utils/include/utils/IChildProcess.hpp | 4 +- 6 files changed, 33 insertions(+), 66 deletions(-) diff --git a/src/libs/av/impl/TranscodeResourceHandler.cpp b/src/libs/av/impl/TranscodeResourceHandler.cpp index 0f2687d3..5e85b72e 100644 --- a/src/libs/av/impl/TranscodeResourceHandler.cpp +++ b/src/libs/av/impl/TranscodeResourceHandler.cpp @@ -46,6 +46,8 @@ namespace Av { if (_estimatedContentLength) LMS_LOG(TRANSCODE, DEBUG) << "Estimated content length = " << *_estimatedContentLength; + else + LMS_LOG(TRANSCODE, DEBUG) << "Not using estimated content length"; } Wt::Http::ResponseContinuation* @@ -58,8 +60,8 @@ namespace Av if (_bytesReadyCount > 0) { response.out().write(reinterpret_cast(&_buffer[0]), _bytesReadyCount); - _bytesReadyCount = 0; _totalServedByteCount += _bytesReadyCount; + _bytesReadyCount = 0; } if (!_transcoder.finished()) diff --git a/src/libs/av/impl/Transcoder.cpp b/src/libs/av/impl/Transcoder.cpp index 4b828d5a..5519021d 100644 --- a/src/libs/av/impl/Transcoder.cpp +++ b/src/libs/av/impl/Transcoder.cpp @@ -178,19 +178,6 @@ Transcoder::start() } } -void -Transcoder::asyncWaitForData(WaitCallback cb) -{ - assert(_childProcess); - - LOG(DEBUG) << "Want to wait for data"; - - _childProcess->asyncWaitForData([cb = std::move(cb)] - { - cb(); - }); -} - void Transcoder::asyncRead(std::byte* buffer, std::size_t bufferSize, ReadCallback readCallback) { diff --git a/src/libs/av/impl/Transcoder.hpp b/src/libs/av/impl/Transcoder.hpp index a4de2976..0f6cf4c7 100644 --- a/src/libs/av/impl/Transcoder.hpp +++ b/src/libs/av/impl/Transcoder.hpp @@ -40,9 +40,6 @@ namespace Av Transcoder(Transcoder&&) = delete; Transcoder& operator=(Transcoder&&) = delete; - using WaitCallback = std::function; - void asyncWaitForData(WaitCallback cb); - // non blocking calls using ReadCallback = std::function; void asyncRead(std::byte* buffer, std::size_t bufferSize, ReadCallback); diff --git a/src/libs/utils/impl/ChildProcess.cpp b/src/libs/utils/impl/ChildProcess.cpp index 90b44881..627bbcd9 100644 --- a/src/libs/utils/impl/ChildProcess.cpp +++ b/src/libs/utils/impl/ChildProcess.cpp @@ -44,7 +44,7 @@ namespace { public: SystemException(int err, const std::string& errMsg) - : ChildProcessException {errMsg + ": " + strerror(err)} + : ChildProcessException {errMsg + ": " + ::strerror(err)} {} SystemException(boost::system::error_code ec, const std::string& errMsg) @@ -117,42 +117,36 @@ ChildProcess::ChildProcess(boost::asio::io_context& ioContext, const std::filesy ChildProcess::~ChildProcess() { - if (!_waited) + LMS_LOG(CHILDPROCESS, DEBUG) << "Closing child process..."; { - LMS_LOG(CHILDPROCESS, DEBUG) << "Closing child process..."; - { - boost::system::error_code closeError; - _childStdout.close(closeError); - if (closeError) - LMS_LOG(CHILDPROCESS, ERROR) << "Closed failed: " << closeError.message(); - } - kill(); - wait(true); + boost::system::error_code closeError; + _childStdout.close(closeError); + if (closeError) + LMS_LOG(CHILDPROCESS, ERROR) << "Closed failed: " << closeError.message(); } -} -void -ChildProcess::drain() -{ - char buf[128]; + if (!_finished) + kill(); - while (boost::asio::read(_childStdout, boost::asio::buffer(buf)) > 0) - LMS_LOG(CHILDPROCESS, DEBUG) << "drained some bytes" << std::endl; + wait(true); } void ChildProcess::kill() { + // process may already have finished LMS_LOG(CHILDPROCESS, DEBUG) << "Killing child process..."; - ::kill(_childPID, SIGKILL); + if (::kill(_childPID, SIGKILL) == -1) + LMS_LOG(CHILDPROCESS, DEBUG) << "Kill failed: " << ::strerror(errno); } bool ChildProcess::wait(bool block) { - int wstatus {}; + assert(!_waited); - pid_t pid {waitpid(_childPID, &wstatus, block ? 0 : WNOHANG)}; + int wstatus {}; + const pid_t pid {waitpid(_childPID, &wstatus, block ? 0 : WNOHANG)}; if (pid == -1) throw SystemException {errno, "waitpid failed!"}; @@ -160,18 +154,22 @@ ChildProcess::wait(bool block) return false; if (WIFEXITED(wstatus)) + { _exitCode = WEXITSTATUS(wstatus); + LMS_LOG(CHILDPROCESS, DEBUG) << "Exit code = " << *_exitCode; + } _waited = true; return true; } - void ChildProcess::asyncRead(std::byte* data, std::size_t bufferSize, ReadCallback callback) { assert(!finished()); + LMS_LOG(CHILDPROCESS, DEBUG) << "Async read, bufferSize = " << bufferSize; + boost::asio::async_read(_childStdout, boost::asio::buffer(data, bufferSize), [this, callback {std::move(callback)}](const boost::system::error_code& error, std::size_t bytesTransferred) { @@ -180,33 +178,20 @@ ChildProcess::asyncRead(std::byte* data, std::size_t bufferSize, ReadCallback ca ReadResult readResult {ReadResult::Success}; if (error) { - _finished = true; - - if (error == boost::asio::error::eof) - readResult = ReadResult::EndOfFile; - else + if (error != boost::asio::error::eof) + { + // forbidden to read any captured param here as the ChildProcess instance may already have been killed return; + } + + readResult = ReadResult::EndOfFile; + _finished = true; } callback(readResult, bytesTransferred); }); } -void -ChildProcess::asyncWaitForData(WaitCallback cb) -{ - LMS_LOG(CHILDPROCESS, DEBUG) << "Async wait requested"; - assert(!finished()); - - _childStdout.async_wait(boost::asio::posix::stream_descriptor::wait_read, - [cb {std::move(cb)}](const boost::system::error_code& ec) - { - LMS_LOG(CHILDPROCESS, DEBUG) << "Wait CB, error = " << ec.message(); - if (!ec) - cb(); - }); -} - std::size_t ChildProcess::readSome(std::byte* data, std::size_t bufferSize) { @@ -220,7 +205,7 @@ ChildProcess::readSome(std::byte* data, std::size_t bufferSize) } bool -ChildProcess::finished() +ChildProcess::finished() const { return _finished; } diff --git a/src/libs/utils/impl/ChildProcess.hpp b/src/libs/utils/impl/ChildProcess.hpp index 46d7bcb0..8b5752dc 100644 --- a/src/libs/utils/impl/ChildProcess.hpp +++ b/src/libs/utils/impl/ChildProcess.hpp @@ -40,12 +40,10 @@ class ChildProcess : public IChildProcess private: void asyncRead(std::byte* data, std::size_t bufferSize, ReadCallback callback) override; - void asyncWaitForData(WaitCallback cb) override; std::size_t readSome(std::byte* data, std::size_t bufferSize) override; - bool finished() override; + bool finished() const override; void kill(); - void drain(); bool wait(bool block); // return true if waited using FileDescriptor = boost::asio::posix::stream_descriptor; diff --git a/src/libs/utils/include/utils/IChildProcess.hpp b/src/libs/utils/include/utils/IChildProcess.hpp index 7de5731a..72768892 100644 --- a/src/libs/utils/include/utils/IChildProcess.hpp +++ b/src/libs/utils/include/utils/IChildProcess.hpp @@ -48,9 +48,7 @@ class IChildProcess using ReadCallback = std::function; virtual void asyncRead(std::byte* data, std::size_t bufferSize, ReadCallback callback) = 0; - using WaitCallback = std::function; - virtual void asyncWaitForData(WaitCallback cb) = 0; virtual std::size_t readSome(std::byte* data, std::size_t bufferSize) = 0; - virtual bool finished() = 0; + virtual bool finished() const = 0; }; From f72f1b72042b435101c83028670eed9b6219c2b0 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 31 Dec 2022 15:26:04 +0100 Subject: [PATCH 07/16] Do not notify added tracks in play queue (misleading, not that useful info) but add a track counter in the play queue button instead. fixes #290 --- approot/mediaplayer.xml | 6 +++++- approot/messages.xml | 4 ---- approot/messages_fr.xml | 4 ---- approot/messages_it.xml | 4 ---- src/lms/ui/LmsApplication.cpp | 7 +++++-- src/lms/ui/MediaPlayer.cpp | 15 +++++++++------ src/lms/ui/MediaPlayer.hpp | 11 +++++++---- src/lms/ui/PlayQueue.cpp | 32 ++++++++++++-------------------- src/lms/ui/PlayQueue.hpp | 4 ++++ 9 files changed, 42 insertions(+), 45 deletions(-) diff --git a/approot/mediaplayer.xml b/approot/mediaplayer.xml index 3494fa2b..f37292f4 100644 --- a/approot/mediaplayer.xml +++ b/approot/mediaplayer.xml @@ -44,6 +44,10 @@ - + + +
{1}
+ + diff --git a/approot/messages.xml b/approot/messages.xml index 241caf7a..1bbe9621 100644 --- a/approot/messages.xml +++ b/approot/messages.xml @@ -209,10 +209,6 @@ Clear Create new playlist - - Added {1} track - Added {1} tracks - Play Queue Play Queue full! Radio mode diff --git a/approot/messages_fr.xml b/approot/messages_fr.xml index 0439a9e5..48d8c295 100644 --- a/approot/messages_fr.xml +++ b/approot/messages_fr.xml @@ -209,10 +209,6 @@ Effacer Créer une nouvelle liste de lecture - - {1} piste ajoutée - {1} pistes ajoutées - Liste de lecture Liste de lecture pleine! Mode radio diff --git a/approot/messages_it.xml b/approot/messages_it.xml index 57d57209..3ed491c8 100644 --- a/approot/messages_it.xml +++ b/approot/messages_it.xml @@ -209,10 +209,6 @@ Cancella - - Aggiunta {1} traccia - Aggiunte {1} tracce - Coda di riproduzione Coda di riproduzione piena! Modalità radio diff --git a/src/lms/ui/LmsApplication.cpp b/src/lms/ui/LmsApplication.cpp index 0e09f8ea..5437ef94 100644 --- a/src/lms/ui/LmsApplication.cpp +++ b/src/lms/ui/LmsApplication.cpp @@ -224,9 +224,7 @@ LmsApplication::init() enableUpdates(true); if (_authenticatedUser) - { onUserLoggedIn(); - } else if (Service<::Auth::IPasswordService>::exists()) processPasswordAuth(); } @@ -507,6 +505,11 @@ LmsApplication::createHome() { _mediaPlayer->stop(); }); + _playQueue->trackCountChanged.connect([this] (std::size_t trackCount) + { + _mediaPlayer->onPlayQueueUpdated(trackCount); + }); + _mediaPlayer->onPlayQueueUpdated(_playQueue->getCount()); const bool isAdmin {getUserType() == Database::UserType::ADMIN}; if (isAdmin) diff --git a/src/lms/ui/MediaPlayer.cpp b/src/lms/ui/MediaPlayer.cpp index 4bd40a2b..a5a2b21b 100644 --- a/src/lms/ui/MediaPlayer.cpp +++ b/src/lms/ui/MediaPlayer.cpp @@ -210,12 +210,9 @@ MediaPlayer::MediaPlayer() _title = bindNew("title"); _artist = bindNew("artist"); _release = bindNew("release"); - - { - Wt::WPushButton* playQueueBtn {bindNew("playqueue-btn", Wt::WString::tr("Lms.MediaPlayer.template.playqueue-btn"), Wt::TextFormat::XHTML)}; - playQueueBtn->setLink(Wt::WLink {Wt::LinkType::InternalPath, "/playqueue"}); - playQueueBtn->setToolTip(tr("Lms.PlayQueue.playqueue")); - } + _playQueue = bindNew("playqueue-btn", Wt::WString::tr("Lms.MediaPlayer.template.playqueue-btn").arg(0), Wt::TextFormat::XHTML); + _playQueue->setLink(Wt::WLink {Wt::LinkType::InternalPath, "/playqueue"}); + _playQueue->setToolTip(tr("Lms.PlayQueue.playqueue")); _settingsLoaded.connect([this](const std::string& settings) { @@ -330,5 +327,11 @@ MediaPlayer::setSettings(const Settings& settings) } } +void +MediaPlayer::onPlayQueueUpdated(std::size_t trackCount) +{ + _playQueue->setText(Wt::WString::tr("Lms.MediaPlayer.template.playqueue-btn").arg(trackCount)); +} + } // namespace UserInterface diff --git a/src/lms/ui/MediaPlayer.hpp b/src/lms/ui/MediaPlayer.hpp index 40307c4b..512e0270 100644 --- a/src/lms/ui/MediaPlayer.hpp +++ b/src/lms/ui/MediaPlayer.hpp @@ -101,7 +101,9 @@ class MediaPlayer : public Wt::WTemplate void stop(); std::optional getSettings() const { return _settings; } - void setSettings(const Settings& settings); + void setSettings(const Settings& settings); + + void onPlayQueueUpdated(std::size_t trackCount); // Signals Wt::JSignal<> playPrevious; @@ -123,9 +125,10 @@ class MediaPlayer : public Wt::WTemplate Wt::JSignal _settingsLoaded; - Wt::WText* _title {}; - Wt::WAnchor* _release {}; - Wt::WAnchor* _artist {}; + Wt::WText* _title {}; + Wt::WAnchor* _release {}; + Wt::WAnchor* _artist {}; + Wt::WPushButton* _playQueue {}; }; } // namespace UserInterface diff --git a/src/lms/ui/PlayQueue.cpp b/src/lms/ui/PlayQueue.cpp index 6face3a5..b128b563 100644 --- a/src/lms/ui/PlayQueue.cpp +++ b/src/lms/ui/PlayQueue.cpp @@ -343,6 +343,13 @@ PlayQueue::playNext() loadTrack(*_trackPos + 1, true); } +std::size_t +PlayQueue::getCount() +{ + auto transaction {LmsApp->getDbSession().createSharedTransaction()}; + return getQueue()->getCount(); +} + void PlayQueue::initTrackLists() { @@ -373,9 +380,10 @@ PlayQueue::updateInfo() auto transaction {LmsApp->getDbSession().createSharedTransaction()}; const Database::TrackList::pointer queue {getQueue()}; - const auto trackCount {queue->getCount()}; + const std::size_t trackCount {queue->getCount()}; _nbTracks->setText(Wt::WString::trn("Lms.track-count", trackCount).arg(trackCount)); _duration->setText(Utils::durationToString(queue->getDuration())); + trackCountChanged.emit(trackCount); } void @@ -435,40 +443,24 @@ PlayQueue::playShuffled(const std::vector& trackIds) clearTracks(); std::vector shuffledTrackIds {trackIds}; Random::shuffleContainer(shuffledTrackIds); - const std::size_t nbAddedTracks {enqueueTracks(shuffledTrackIds)}; + enqueueTracks(shuffledTrackIds); loadTrack(0, true); - - notifyAddedTracks(nbAddedTracks); } void PlayQueue::playOrAddLast(const std::vector& trackIds) { - const std::size_t nbAddedTracks {enqueueTracks(trackIds)}; + enqueueTracks(trackIds); if (!_trackPos) loadTrack(0, true); - - notifyAddedTracks(nbAddedTracks); } void PlayQueue::playAtIndex(const std::vector& trackIds, std::size_t index) { clearTracks(); - const std::size_t nbAddedTracks {enqueueTracks(trackIds)}; + enqueueTracks(trackIds); loadTrack(index, true); - - notifyAddedTracks(nbAddedTracks); -} - -void -PlayQueue::notifyAddedTracks(std::size_t nbAddedTracks) const -{ - if (nbAddedTracks > 0) - LmsApp->notifyMsg(Notification::Type::Info, Wt::WString::tr("Lms.PlayQueue.playqueue"), Wt::WString::trn("Lms.PlayQueue.nb-tracks-added", nbAddedTracks).arg(nbAddedTracks), std::chrono::seconds {2}); - - if (isFull()) - LmsApp->notifyMsg(Notification::Type::Warning, Wt::WString::tr("Lms.PlayQueue.playqueue"), Wt::WString::tr("Lms.PlayQueue.playqueue-full"), std::chrono::seconds {2}); } void diff --git a/src/lms/ui/PlayQueue.hpp b/src/lms/ui/PlayQueue.hpp index bef41f68..0599eb9e 100644 --- a/src/lms/ui/PlayQueue.hpp +++ b/src/lms/ui/PlayQueue.hpp @@ -66,7 +66,11 @@ class PlayQueue : public Template // Signal emitted when track is unselected (has to be stopped) Wt::Signal<> trackUnselected; + // Signal emitted when track count changed + Wt::Signal trackCountChanged; + constexpr std::size_t getCapacity() const { return _capacity; } + std::size_t getCount(); private: void initTrackLists(); From f247022e35296319e013da058b8b86a6ad57ad0d Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 1 Jan 2023 11:43:09 +0100 Subject: [PATCH 08/16] Removed deprecated lgtm workflow --- .lgtm.yml | 34 ---------------------------------- 1 file changed, 34 deletions(-) delete mode 100644 .lgtm.yml diff --git a/.lgtm.yml b/.lgtm.yml deleted file mode 100644 index 11ce21a0..00000000 --- a/.lgtm.yml +++ /dev/null @@ -1,34 +0,0 @@ -path_classifiers: - test: - - src/test - -extraction: - cpp: - prepare: - packages: - - build-essential - - cmake - - libboost-all-dev - - libconfig++-dev - - libavcodec-dev - - libavutil-dev - - libavformat-dev - - libstb-dev - - libtag1-dev - - libpam0g-dev - after_prepare: - - export WT_VERSION=4.7.2 - - export WT_INSTALL_PREFIX=${LGTM_WORKSPACE}/wt-${WT_VERSION} - - pushd ${LGTM_WORKSPACE} - - git clone https://github.com/emweb/wt.git ${LGTM_WORKSPACE}/wt - - pushd ${LGTM_WORKSPACE}/wt - - git checkout ${WT_VERSION} - - cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=${WT_INSTALL_PREFIX} -DBUILD_EXAMPLES=OFF -DENABLE_LIBWTTEST=OFF -DCONNECTOR_FCGI=OFF - - make install - - popd - configure: - command: - - export WT_VERSION=4.7.2 - - export WT_INSTALL_PREFIX=${LGTM_WORKSPACE}/wt-${WT_VERSION} - - cmake -DCMAKE_PREFIX_PATH=${WT_INSTALL_PREFIX} -DCMAKE_BUILD_TYPE=Release - From 4414e1a8898f1eea97bcedc99c2eaab6de7c503a Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 1 Jan 2023 11:44:49 +0100 Subject: [PATCH 09/16] Speed up codeql using make -j --- .github/workflows/codeql.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index b67055d1..5358ff70 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -37,7 +37,7 @@ jobs: pushd wt git checkout ${WT_VERSION} cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=${WT_INSTALL_PREFIX} -DBUILD_EXAMPLES=OFF -DENABLE_LIBWTTEST=OFF -DCONNECTOR_FCGI=OFF - make + make -j$(nproc) sudo make install popd From 01604e18cb3d332861b525024dca58d58dcf238e Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 1 Jan 2023 20:15:45 +0100 Subject: [PATCH 10/16] Speed up codeql using make -j --- .github/workflows/codeql.yml | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 5358ff70..0d730c78 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -26,15 +26,15 @@ jobs: - name: Checkout uses: actions/checkout@v3 - - name: Install Packages (cpp) - if: ${{ matrix.language == 'cpp' }} + - if: matrix.language == 'cpp' + name: Install dependencies (cpp) run: | sudo apt-get update sudo apt-get install --yes build-essential cmake libboost-all-dev libconfig++-dev libavcodec-dev libavutil-dev libavformat-dev libstb-dev libtag1-dev libpam0g-dev libgtest-dev - export WT_VERSION=4.7.2 + export WT_VERSION=4.9.0 export WT_INSTALL_PREFIX=/usr - git clone https://github.com/emweb/wt.git wt - pushd wt + git clone https://github.com/emweb/wt.git /tmp/wt + pushd /tmp/wt git checkout ${WT_VERSION} cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=${WT_INSTALL_PREFIX} -DBUILD_EXAMPLES=OFF -DENABLE_LIBWTTEST=OFF -DCONNECTOR_FCGI=OFF make -j$(nproc) @@ -47,9 +47,18 @@ jobs: languages: ${{ matrix.language }} queries: +security-and-quality - - name: Autobuild + - if: matrix.language == 'javascript' + name: Autobuild uses: github/codeql-action/autobuild@v2 + - if: matrix.language == 'cpp' + name: Build + run: | + mkdir -p build + cd build + cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Release .. + make -j$(nproc) + - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v2 with: From 80143a95888f402821fb4f5fe0d3eeaa3b20f240 Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 2 Jan 2023 19:39:09 +0100 Subject: [PATCH 11/16] Fixed sefault when no track are classified. fixes #297 --- .../services/recommendation/impl/features/FeaturesEngine.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libs/services/recommendation/impl/features/FeaturesEngine.cpp b/src/libs/services/recommendation/impl/features/FeaturesEngine.cpp index fc44eb48..1cfb6811 100644 --- a/src/libs/services/recommendation/impl/features/FeaturesEngine.cpp +++ b/src/libs/services/recommendation/impl/features/FeaturesEngine.cpp @@ -194,7 +194,6 @@ FeaturesEngine::loadFromTraining(const TrainSettings& trainSettings, const Progr [this] { return _loadCancelled; }); LMS_LOG(RECOMMENDATION, DEBUG) << "Training network DONE"; - LMS_LOG(RECOMMENDATION, DEBUG) << "Classifying tracks..."; TrackPositions trackPositions; for (std::size_t i {}; i < samples.size(); ++i) @@ -351,7 +350,7 @@ FeaturesEngine::load(bool forceReload, const ProgressCallback& progressCallback) trainSettings.featureSettingsMap = getDefaultTrainFeatureSettings(); loadFromTraining(trainSettings, progressCallback); - if (!_loadCancelled) + if (!_loadCancelled && _network) toCache().write(); } From 467a53f23443236030435a6b9ac81aa5d7938478 Mon Sep 17 00:00:00 2001 From: emeric Date: Wed, 4 Jan 2023 22:32:57 +0100 Subject: [PATCH 12/16] Made player display duration in the same format as in albums. fixes #291 --- docroot/js/mediaplayer.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docroot/js/mediaplayer.js b/docroot/js/mediaplayer.js index 279554c2..ea174380 100644 --- a/docroot/js/mediaplayer.js +++ b/docroot/js/mediaplayer.js @@ -114,15 +114,15 @@ LMS.mediaplayer = function () { } let _durationToString = function (duration) { - let minutes = parseInt(duration / 60, 10); - let seconds = parseInt(duration, 10) % 60; - - let res = ""; - - res += minutes + ":"; - res += (seconds < 10 ? "0" + seconds : seconds); - - return res; + const seconds = parseInt(duration, 10); + const h = Math.floor(seconds / 3600); + const m = Math.floor((seconds % 3600) / 60); + const s = Math.round(seconds % 60); + return [ + h, + m > 9 ? m : (h ? '0' + m : m || '0'), + s > 9 ? s : '0' + s + ].filter(Boolean).join(':'); } let _playTrack = function() { From 25c36f0a3ea9eac74a53ef1f3f19a816328f4556 Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 6 Jan 2023 20:48:59 +0100 Subject: [PATCH 13/16] Get rid of jQuery for active menu selection --- src/lms/ui/LmsApplication.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/lms/ui/LmsApplication.cpp b/src/lms/ui/LmsApplication.cpp index 5437ef94..9aaf02af 100644 --- a/src/lms/ui/LmsApplication.cpp +++ b/src/lms/ui/LmsApplication.cpp @@ -337,7 +337,6 @@ handlePathChange(Wt::WStackedWidget& stack, bool isAdmin) LMS_LOG(UI, DEBUG) << "Internal path changed to '" << wApp->internalPath() << "'"; - LmsApp->doJavaScript(R"($('.navbar-nav a.active').removeClass('active'); $('.navbar-nav a[href="' + location.pathname + '"]').closest('a').addClass('active');)"); for (const auto& view : views) { if (wApp->internalPathMatches(view.path)) @@ -348,6 +347,8 @@ handlePathChange(Wt::WStackedWidget& stack, bool isAdmin) stack.setCurrentIndex(view.index); if (view.title) LmsApp->setTitle(*view.title); + + LmsApp->doJavaScript(LmsApp->javaScriptClass() + ".updateActiveNav('" + view.path +"')"); return; } } @@ -396,9 +397,21 @@ LmsApplication::createHome() _coverResource = std::make_shared(); declareJavaScriptFunction("onLoadCover", "function(id) { id.className += \" Lms-cover-loaded\"}"); + declareJavaScriptFunction("updateActiveNav", +R"(function(current) { + const menuItems = document.querySelectorAll('.nav-item a[href]:not([href=""])'); + for (const menuItem of menuItems) { + if (menuItem.getAttribute("href").indexOf(current) !== -1) { + menuItem.classList.add('active'); + } + else { + menuItem.classList.remove('active'); + } + + } +})"); Wt::WTemplate* main {root()->addWidget(std::make_unique(Wt::WString::tr("Lms.main.template")))}; - main->addFunction("tr", &Wt::WTemplate::Functions::tr); Template* navbar {main->bindNew