Merge branch 'develop' for release v3.49.0

epoupon · Feb 5, 2024 · f3652dd · f3652dd
2 parents 7a3a0dd + 7d53aba
commit f3652dd
Show file tree

Hide file tree

Showing 54 changed files with 806 additions and 617 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -4,7 +4,7 @@ project(lms)
 
 set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_SOURCE_DIR}/cmake/modules/)
 
-set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD 20)
 set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
 if (UNIX)
 	set(CMAKE_SHARED_LINKER_FLAGS "-Wl,--no-undefined")
@@ -70,6 +70,13 @@ elseif (IMAGE_LIBRARY STREQUAL STB AND NOT STB_FOUND)
 endif ()
 message(STATUS "IMAGE_LIBRARY set to ${IMAGE_LIBRARY}")
 
+# Benchmark
+option(BUILD_BENCHMARKS "Build benchmarks" OFF)
+if (BUILD_BENCHMARKS)
+	find_package(benchmark REQUIRED)
+	message(STATUS "Building benchmarks")
+endif()
+
 add_subdirectory(src)
 
 install(DIRECTORY approot DESTINATION share/lms)

diff --git a/Dockerfile-build-alpine b/Dockerfile-build-alpine
@@ -15,6 +15,7 @@ ARG	LMS_BUILD_PACKAGES=" \
 	gcc \
 	g++ \
 	musl-dev \
+	benchmark-dev \
 	boost-dev \
 	ffmpeg-dev \
 	libarchive-dev \
@@ -35,7 +36,7 @@ ARG LMS_BUILD_TYPE="Release"
 RUN \
 	DIR=/tmp/lms/build && mkdir -p ${DIR} && cd ${DIR}  && \
 		xx-info is-cross && export BUILD_TESTS=OFF || export BUILD_TESTS=ON && \
-		PKG_CONFIG_PATH=/$(xx-info)/usr/lib/pkgconfig cmake /tmp/lms/ -DCMAKE_INCLUDE_PATH=${PREFIX}/include -DCMAKE_BUILD_TYPE=${LMS_BUILD_TYPE} $(xx-clang --print-cmake-defines) -DCMAKE_PREFIX_PATH=/$(xx-info)/usr/lib/cmake -DBUILD_TESTING=${BUILD_TESTS} && \
+		PKG_CONFIG_PATH=/$(xx-info)/usr/lib/pkgconfig cmake /tmp/lms/ -DCMAKE_INCLUDE_PATH=${PREFIX}/include -DCMAKE_BUILD_TYPE=${LMS_BUILD_TYPE} $(xx-clang --print-cmake-defines) -DCMAKE_PREFIX_PATH=/$(xx-info)/usr/lib/cmake -DBUILD_TESTING=${BUILD_TESTS} -DBUILD_BENCHMARKS=ON && \
 		VERBOSE=1 make -j$(nproc) && \
 		xx-verify src/lms/lms && \
 		(xx-info is-cross || make test)
diff --git a/Dockerfile-build-arch b/Dockerfile-build-arch
@@ -1,6 +1,7 @@
 FROM archlinux:latest
 
 ARG BUILD_PACKAGES="\
+	benchmark \
 	clang \
 	cmake \
 	boost \
@@ -22,6 +23,6 @@ COPY . /tmp/lms/
 ARG LMS_BUILD_TYPE="Release"
 RUN \
 	DIR=/tmp/lms/build && mkdir -p ${DIR} && cd ${DIR}  && \
-		cmake /tmp/lms/ -DCMAKE_BUILD_TYPE=${LMS_BUILD_TYPE} -DCMAKE_INSTALL_PREFIX=/usr && \
+		cmake /tmp/lms/ -DCMAKE_BUILD_TYPE=${LMS_BUILD_TYPE} -DCMAKE_INSTALL_PREFIX=/usr -DBUILD_BENCHMARKS=ON && \
 		VERBOSE=1 make -j$(nproc) && \
 		make test
diff --git a/cmake/modules/FindFilesystem.cmake b/cmake/modules/FindFilesystem.cmake
@@ -110,9 +110,6 @@ cmake_push_check_state()
 
 set(CMAKE_REQUIRED_QUIET ${Filesystem_FIND_QUIETLY})
 
-# All of our tests required C++17 or later
-set(CMAKE_CXX_STANDARD 17)
-
 # Normalize and check the component list we were given
 set(want_components ${Filesystem_FIND_COMPONENTS})
 if(Filesystem_FIND_COMPONENTS STREQUAL "")

diff --git a/src/libs/av/impl/TranscodingResourceHandler.cpp b/src/libs/av/impl/TranscodingResourceHandler.cpp
@@ -68,7 +68,7 @@ namespace Av::Transcoding
         {
             Wt::Http::ResponseContinuation* continuation{ response.createContinuation() };
             continuation->waitForMoreData();
-            _transcoder.asyncRead(_buffer.data(), _buffer.size(), [=](std::size_t nbBytesRead)
+            _transcoder.asyncRead(_buffer.data(), _buffer.size(), [this, continuation](std::size_t nbBytesRead)
                 {
                     LMS_LOG(TRANSCODING, DEBUG, "Have " << nbBytesRead << " more bytes to send back");
 

diff --git a/src/libs/database/impl/MediaLibrary.cpp b/src/libs/database/impl/MediaLibrary.cpp
@@ -43,7 +43,7 @@ namespace Database
     {
         session.checkReadTransaction();
 
-        return session.getDboSession().query<int>("SELECT COUNT(*) FROM cluster");
+        return session.getDboSession().query<int>("SELECT COUNT(*) FROM media_library");
     }
 
     MediaLibrary::pointer MediaLibrary::find(Session& session, MediaLibraryId id)

diff --git a/src/libs/database/impl/Migration.cpp b/src/libs/database/impl/Migration.cpp
@@ -182,8 +182,8 @@ CREATE TABLE IF NOT EXISTS "track_backup" (
     static void migrateFromV40(Session& session)
     {
         // add artist_display_name in Release and Track
-        session.getDboSession().execute("ALTER TABLE release ADD artist_display_name TEXT");
-        session.getDboSession().execute("ALTER TABLE track ADD artist_display_name TEXT");
+        session.getDboSession().execute("ALTER TABLE release ADD artist_display_name TEXT NOT NULL DEFAULT ''");
+        session.getDboSession().execute("ALTER TABLE track ADD artist_display_name TEXT NOT NULL DEFAULT ''");
 
         // Just increment the scan version of the settings to make the next scheduled scan rescan everything
         session.getDboSession().execute("UPDATE scan_settings SET scan_version = scan_version + 1");
@@ -237,7 +237,7 @@ CREATE TABLE IF NOT EXISTS "track_backup" (
     static void migrateFromV44(Session& session)
     {
         // add bitrate
-        session.getDboSession().execute("ALTER TABLE track ADD bitrate INTEGER");
+        session.getDboSession().execute("ALTER TABLE track ADD bitrate INTEGER NOT NULL DEFAULT 0");
 
         // Just increment the scan version of the settings to make the next scheduled scan rescan everything
         session.getDboSession().execute("UPDATE scan_settings SET scan_version = scan_version + 1");
@@ -365,7 +365,36 @@ CREATE TABLE IF NOT EXISTS "track_backup" (
 ))");
 
 // Migrate data, with the new media_library_id field set to 1
-        session.getDboSession().execute("INSERT INTO track_backup SELECT id, version, scan_version, track_number, disc_number, total_track, disc_subtitle, name, duration, bitrate, date, year, original_date, original_year, file_path, file_last_write, file_added, has_cover, mbid, recording_mbid, copyright, copyright_url, track_replay_gain, release_replay_gain, artist_display_name, release_id, 1 FROM track");
+        session.getDboSession().execute(R"(INSERT INTO track_backup 
+SELECT
+ id,
+ version,
+ scan_version,
+ track_number,
+ disc_number,
+ total_track,
+ disc_subtitle,
+ name,
+ duration,
+ COALESCE(bitrate, 0),
+ date,
+ year,
+ original_date,
+ original_year,
+ file_path,
+ file_last_write,
+ file_added,
+ has_cover,
+ mbid,
+ recording_mbid,
+ copyright,
+ copyright_url,
+ track_replay_gain,
+ release_replay_gain,
+ COALESCE(artist_display_name, ""),
+ release_id,
+ 1
+ FROM track)");
         session.getDboSession().execute("DROP TABLE track");
         session.getDboSession().execute("ALTER TABLE track_backup RENAME TO track");
     }

diff --git a/src/libs/database/impl/Track.cpp b/src/libs/database/impl/Track.cpp
@@ -45,8 +45,7 @@ namespace Database
         {
             session.checkReadTransaction();
 
-            std::string selectStatement{ params.distinct ? "SELECT DISTINCT" : "SELECT" };
-            auto query{ session.getDboSession().query<ResultType>(selectStatement + " " + std::string{ itemToSelect } + " FROM track t") };
+            auto query{ session.getDboSession().query<ResultType>("SELECT " + std::string{ itemToSelect } + " FROM track t") };
 
             assert(params.keywords.empty() || params.name.empty());
             for (std::string_view keyword : params.keywords)
@@ -118,6 +117,8 @@ namespace Database
                     }
                     query.where(oss.str());
                 }
+
+                query.groupBy("t.id");
             }
 
             assert(!(params.nonRelease && params.release.isValid()));

diff --git a/src/libs/database/include/database/MediaLibrary.hpp b/src/libs/database/include/database/MediaLibrary.hpp
@@ -17,6 +17,8 @@
  * along with LMS.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#pragma once
+
 #include <filesystem>
 #include <string>
 #include <string_view>
@@ -38,7 +40,7 @@ namespace Database
         MediaLibrary() = default;
 
         // find
-        std::size_t getCount(Session& session);
+        static std::size_t getCount(Session& session);
         static pointer find(Session& session, MediaLibraryId id);
         static pointer find(Session& session, std::string_view name);
         static pointer find(Session& session, const std::filesystem::path& path);

diff --git a/src/libs/database/include/database/Track.hpp b/src/libs/database/include/database/Track.hpp
@@ -79,7 +79,6 @@ namespace Database {
             TrackListId							trackList;		// matching this trackList
             std::optional<int>					trackNumber;	// matching this track number
             MediaLibraryId                      mediaLibrary;   // If set, tracks in this library
-            bool								distinct{ true };
 
             FindParameters& setClusters(const std::vector<ClusterId>& _clusters) { clusters = _clusters; return *this; }
             FindParameters& setKeywords(const std::vector<std::string_view>& _keywords) { keywords = _keywords; return *this; }
@@ -96,7 +95,6 @@ namespace Database {
             FindParameters& setTrackList(TrackListId _trackList) { trackList = _trackList; return *this; }
             FindParameters& setTrackNumber(int _trackNumber) { trackNumber = _trackNumber; return *this; }
             FindParameters& setMediaLibrary(MediaLibraryId  _mediaLibrary) { mediaLibrary = _mediaLibrary; return *this; }
-            FindParameters& setDistinct(bool _distinct) { distinct = _distinct; return *this; }
         };
 
         struct PathResult

diff --git a/src/libs/database/test/Artist.cpp b/src/libs/database/test/Artist.cpp
@@ -92,7 +92,7 @@ TEST_F(DatabaseFixture, Artist_singleTrack)
 
     {
         auto transaction{ session.createReadTransaction() };
-        EXPECT_TRUE(Artist::findOrphanIds(session).results.empty());
+        EXPECT_EQ(Artist::findOrphanIds(session).results.size(), 0);
     }
 
     {
@@ -108,7 +108,7 @@ TEST_F(DatabaseFixture, Artist_singleTrack)
         EXPECT_EQ(artistLink->getArtist()->getId(), artist.getId());
 
         ASSERT_EQ(track->getArtists({ TrackArtistLinkType::Artist }).size(), 1);
-        EXPECT_TRUE(track->getArtists({ TrackArtistLinkType::ReleaseArtist }).empty());
+        EXPECT_EQ(track->getArtists({ TrackArtistLinkType::ReleaseArtist }).size(), 0);
         EXPECT_EQ(track->getArtists({}).size(), 1);
     }
 
@@ -120,7 +120,7 @@ TEST_F(DatabaseFixture, Artist_singleTrack)
         EXPECT_EQ(artists.front(), artist.getId());
 
         ASSERT_EQ(track->getArtistIds({ TrackArtistLinkType::Artist }).size(), 1);
-        EXPECT_TRUE(track->getArtistIds({ TrackArtistLinkType::ReleaseArtist }).empty());
+        EXPECT_EQ(track->getArtistIds({ TrackArtistLinkType::ReleaseArtist }).size(), 0);
         EXPECT_EQ(track->getArtistIds({}).size(), 1);
     }
 
@@ -195,7 +195,7 @@ TEST_F(DatabaseFixture, Artist_singleTracktMultiRoles)
 
     {
         auto transaction{ session.createReadTransaction() };
-        EXPECT_TRUE(Artist::findOrphanIds(session, Range{}).results.empty());
+        EXPECT_EQ(Artist::findOrphanIds(session, Range{}).results.size(), 0);
     }
 
     {
@@ -260,7 +260,7 @@ TEST_F(DatabaseFixture, Artist_singleTrackMultiArtists)
 
     {
         auto transaction{ session.createReadTransaction() };
-        EXPECT_TRUE(Artist::findOrphanIds(session).results.empty());
+        EXPECT_EQ(Artist::findOrphanIds(session).results.size(), 0);
     }
 
     {
@@ -273,7 +273,7 @@ TEST_F(DatabaseFixture, Artist_singleTrackMultiArtists)
 
         EXPECT_EQ(track->getArtists({}).size(), 2);
         EXPECT_EQ(track->getArtists({ TrackArtistLinkType::Artist }).size(), 2);
-        EXPECT_TRUE(track->getArtists({ TrackArtistLinkType::ReleaseArtist }).empty());
+        EXPECT_EQ(track->getArtists({ TrackArtistLinkType::ReleaseArtist }).size(), 0);
         EXPECT_EQ(Artist::findIds(session, Artist::FindParameters{}).results.size(), 2);
         EXPECT_EQ(Artist::findIds(session, Artist::FindParameters{}.setSortMethod(ArtistSortMethod::Random)).results.size(), 2);
     }
@@ -317,7 +317,7 @@ TEST_F(DatabaseFixture, Artist_findByName)
     {
         auto transaction{ session.createReadTransaction() };
 
-        EXPECT_TRUE(Artist::findIds(session, Artist::FindParameters{}.setKeywords({ "N" })).results.empty());
+        EXPECT_EQ(Artist::findIds(session, Artist::FindParameters{}.setKeywords({ "N" })).results.size(), 0);
 
         const auto artistsByAAA{ Artist::findIds(session, Artist::FindParameters {}.setKeywords({"A"})) };
         ASSERT_EQ(artistsByAAA.results.size(), 1);
@@ -327,7 +327,7 @@ TEST_F(DatabaseFixture, Artist_findByName)
         ASSERT_EQ(artistsByZZZ.results.size(), 1);
         EXPECT_EQ(artistsByZZZ.results.front(), artist.getId());
 
-        EXPECT_TRUE(Artist::find(session, "NNN").empty());
+        EXPECT_EQ(Artist::find(session, "NNN").size(), 0);
         EXPECT_EQ(Artist::find(session, "AAA").size(), 1);
     }
 }
@@ -348,19 +348,19 @@ TEST_F(DatabaseFixture, Artist_findByNameEscaped)
             const auto artists{ Artist::find(session, R"(MyArtist%)") };
             ASSERT_TRUE(artists.size() == 1);
             EXPECT_EQ(artists.front()->getId(), artist1.getId());
-            EXPECT_TRUE(Artist::find(session, R"(MyArtistFoo)").empty());
+            EXPECT_EQ(Artist::find(session, R"(MyArtistFoo)").size(), 0);
         }
         {
             const auto artists{ Artist::find(session, R"(%MyArtist)") };
             ASSERT_TRUE(artists.size() == 1);
             EXPECT_EQ(artists.front()->getId(), artist2.getId());
-            EXPECT_TRUE(Artist::find(session, R"(FooMyArtist)").empty());
+            EXPECT_EQ(Artist::find(session, R"(FooMyArtist)").size(), 0);
         }
         {
             const auto artists{ Artist::find(session, R"(%_MyArtist)") };
             ASSERT_TRUE(artists.size() == 1);
             ASSERT_EQ(artists.front()->getId(), artist3.getId());
-            EXPECT_TRUE(Artist::find(session, R"(%CMyArtist)").empty());
+            EXPECT_EQ(Artist::find(session, R"(%CMyArtist)").size(), 0);
         }
     }