Code review / cleanup

Author: cbjeukendrup

ninja_build.sh

@@ -47,7 +47,6 @@ MUSESCORE_NO_RPATH=${MUSESCORE_NO_RPATH:-"OFF"}
 MUSESCORE_MODULE_UPDATE=${MUSESCORE_MODULE_UPDATEE:-"ON"}
 MUSESCORE_BUILD_VST_MODULE=${MUSESCORE_BUILD_VST_MODULE:-"OFF"}
 MUSESCORE_BUILD_IMPEXP_VIDEOEXPORT_MODULE=${MUSESCORE_BUILD_IMPEXP_VIDEOEXPORT_MODULE:-"OFF"}
-MUSESCORE_BUILD_IMPEXP_LYRICS_MODULE==${MUSESCORE_BUILD_IMPEXP_LYRICS_MODULE:-"OFF"}
 MUSESCORE_BUILD_WEBSOCKET=${MUSESCORE_BUILD_WEBSOCKET:-"OFF"}
 MUSESCORE_BUILD_PIPEWIRE_AUDIO_DRIVER=${MUSESCORE_BUILD_PIPEWIRE_AUDIO_DRIVER:-"OFF"}
 MUSESCORE_COMPILE_USE_UNITY=${MUSESCORE_COMPILE_USE_UNITY:-"ON"}
@@ -94,7 +93,6 @@ function do_build() {
         -DMUSESCORE_REVISION="${MUSESCORE_REVISION}" \
         -DMUE_RUN_LRELEASE="${MUSESCORE_RUN_LRELEASE}" \
         -DMUE_BUILD_IMPEXP_VIDEOEXPORT_MODULE="${MUSESCORE_BUILD_IMPEXP_VIDEOEXPORT_MODULE}" \
-        -DMUE_BUILD_IMPEXP_LYRICS_MODULE="${MUSESCORE_BUILD_IMPEXP_LYRICS_MODULE}" \
         -DMUSE_MODULE_UPDATE="${MUSESCORE_MODULE_UPDATE}" \
         -DMUE_DOWNLOAD_SOUNDFONT="${MUSESCORE_DOWNLOAD_SOUNDFONT}" \
         -DMUSE_ENABLE_UNIT_TESTS="${MUSESCORE_BUILD_UNIT_TESTS}" \
@@ -173,7 +171,6 @@ case $TARGET in
             -DMUSESCORE_REVISION="${MUSESCORE_REVISION}" \
             -DMUE_RUN_LRELEASE="${MUSESCORE_RUN_LRELEASE}" \
             -DMUE_BUILD_IMPEXP_VIDEOEXPORT_MODULE="${MUSESCORE_BUILD_IMPEXP_VIDEOEXPORT_MODULE}" \
-            -DMUE_BUILD_IMPEXP_LYRICS_MODULE="${MUSESCORE_BUILD_IMPEXP_LYRICS_MODULE}" \
             -DMUSE_MODULE_UPDATE="${MUSESCORE_MODULE_UPDATE}" \
             -DMUE_DOWNLOAD_SOUNDFONT="${MUSESCORE_DOWNLOAD_SOUNDFONT}" \
             -DMUSE_ENABLE_UNIT_TESTS="${MUSESCORE_BUILD_UNIT_TESTS}" \

src/importexport/lyricsexport/CMakeLists.txt

@@ -18,28 +18,20 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
-declare_module(iex_lyricsexport)
+muse_create_module(iex_lyricsexport)
 
-set(MODULE_SRC
-    ${CMAKE_CURRENT_LIST_DIR}/lyricsexportmodule.cpp
-    ${CMAKE_CURRENT_LIST_DIR}/lyricsexportmodule.h
-    ${CMAKE_CURRENT_LIST_DIR}/ilyricsexportconfiguration.h
+target_sources(iex_lyricsexport PRIVATE
+    lyricsexportmodule.cpp
+    lyricsexportmodule.h
+    ilyricsexportconfiguration.h
 
-    ${CMAKE_CURRENT_LIST_DIR}/internal/lyricsexportconfiguration.cpp
-    ${CMAKE_CURRENT_LIST_DIR}/internal/lyricsexportconfiguration.h
-    ${CMAKE_CURRENT_LIST_DIR}/internal/iex_lyricsexport.cpp
-    ${CMAKE_CURRENT_LIST_DIR}/internal/iex_lyricsexport.h
-    )
-
-set(MODULE_LINK
-    engraving
+    internal/lyricsexportconfiguration.cpp
+    internal/lyricsexportconfiguration.h
+    internal/lrcwriter.cpp
+    internal/lrcwriter.h
 )
 
-if (QT_SUPPORT)
-    list(APPEND MODULE_LINK Qt::Core5Compat)
-endif()
-
-setup_module()
+target_link_libraries(iex_lyricsexport PRIVATE engraving)
 
 if (MUE_BUILD_IMPORTEXPORT_TESTS)
     add_subdirectory(tests)

src/importexport/lyricsexport/ilyricsexportconfiguration.h

@@ -19,10 +19,10 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
+
 #pragma once
 
-#include <string>
-#include <optional>
+#include "async/channel.h"
 
 #include "modularity/imoduleinterface.h"
 

src/importexport/lyricsexport/internal/iex_lyricsexport.cpp renamed to src/importexport/lyricsexport/internal/lrcwriter.cpp

@@ -5,7 +5,7 @@
  * MuseScore Studio
  * Music Composition & Notation
  *
- * Copyright (C) 2021 MuseScore Limited
+ * Copyright (C) 2025 MuseScore Limited
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -17,142 +17,108 @@
  * 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, see <https://www.gnu.org/licenses/>.
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
  */
 
-#include "io/file.h"
+#include "lrcwriter.h"
 
 #include <QBuffer>
 
+#include "io/file.h"
+#include "types/ret.h"
+
+#include "engraving/dom/lyrics.h"
 #include "engraving/dom/masterscore.h"
 #include "engraving/dom/repeatlist.h"
-#include "engraving/dom/lyrics.h"
 
-#include "iex_lyricsexport.h"
+using namespace Qt::Literals;
 
-using namespace mu::engraving;
+using namespace muse;
 using namespace muse::io;
+using namespace mu::engraving;
+using namespace mu::project;
+using namespace mu::iex::lrcexport;
 
-namespace mu::iex::lrcexport {
-// Interface implementation
-std::vector<project::INotationWriter::UnitType> LRCWriter::supportedUnitTypes() const
+std::vector<INotationWriter::UnitType> LRCWriter::supportedUnitTypes() const
 {
     return { UnitType::PER_PART };
 }
 
-//
-// LRCWriter::supportsUnitType
-//
-
 bool LRCWriter::supportsUnitType(UnitType ut) const { return ut == UnitType::PER_PART; }
 
-//
-// LRCWriter::write
-//
-
 muse::Ret LRCWriter::write(notation::INotationPtr notation, muse::io::IODevice& device, const Options&)
 {
     Score* score = notation->elements()->msScore();
     bool enhancedLrc = configuration()->lrcUseEnhancedFormat();
 
-    return write(score, &device, enhancedLrc);
+    return doWrite(score, &device, enhancedLrc);
 }
 
-//
-// LRCWriter::exportLrc
-//
-
-bool LRCWriter::exportLrc(mu::engraving::Score* score, muse::io::IODevice* device, bool enhancedLrc)
-{
-    write(score, device, enhancedLrc);
-    return true;
-}
-
-//
-// LRCWriter::writeScore
-//
-
 bool LRCWriter::writeScore(mu::engraving::Score* score, const muse::io::path_t& path, bool enhancedLrc)
 {
     File f(path);
     if (!f.open(IODevice::WriteOnly)) {
         return false;
     }
 
-    bool res = exportLrc(score, &f, enhancedLrc) && !f.hasError();
+    bool res = doWrite(score, &f, enhancedLrc) && !f.hasError();
     f.close();
 
     return res;
 }
 
-//
-// LRCWriter::writeList
-//
-
 muse::Ret LRCWriter::writeList(const notation::INotationPtrList&, muse::io::IODevice&, const Options&)
 {
-    return muse::Ret(muse::Ret::Code::NotSupported);
+    return make_ret(Ret::Code::NotSupported);
 }
 
-//
-// LRCWriter::writeMetadata
-//
-
 void LRCWriter::writeMetadata(muse::io::IODevice* device, const engraving::Score* score) const
 {
     QString metadata;
 
     // Title
-    QString title = QString::fromStdString(score->metaTag(muse::String("workTitle")).toStdString());
+    const QString title = score->metaTag(u"workTitle").toQString();
     if (!title.isEmpty()) {
-        metadata += QString("[ti:%1]\n").arg(title);
+        metadata += u"[ti:%1]\n"_s.arg(title);
     }
 
     // Composer/Artist
-    QString artist = QString::fromStdString(score->metaTag(muse::String("composer")).toStdString());
+    const QString artist = score->metaTag(u"composer").toQString();
     if (!artist.isEmpty()) {
-        metadata += QString("[ar:%1]\n").arg(artist);
+        metadata += u"[ar:%1]\n"_s.arg(artist);
     }
 
     if (!metadata.isEmpty()) {
         device->write(metadata.toUtf8());
     }
 }
 
-//
-// LRCWriter::write
-//
-
-muse::Ret LRCWriter::write(mu::engraving::Score* score, muse::io::IODevice* device, bool enhancedLrc)
+muse::Ret LRCWriter::doWrite(mu::engraving::Score* score, muse::io::IODevice* device, bool enhancedLrc)
 {
     writeMetadata(device, score);
 
     const auto lyrics = collectLyrics(score);
 
     // Write lyrics
-    for (auto it = lyrics.constBegin(); it != lyrics.constEnd(); ++it) {
+    for (const auto& [timestamp, text] : lyrics) {
         if (enhancedLrc) {
             // As there should only be words we replace spaces by "-"
-            QString lyricsText = it.value();
+            QString lyricsText = text;
             lyricsText.replace(QRegularExpression("\\s"), "-");
-            lyricsText.replace(QChar(0x00A0), QChar('-'));
+            lyricsText.replace(u'\u00A0', u'-');
 
-            device->write(QString("[%1] <%1> %2\n").arg(formatTimestamp(it.key()), lyricsText).toUtf8());
+            device->write(QString("[%1] <%1> %2\n").arg(formatTimestamp(timestamp), lyricsText).toUtf8());
         } else {
-            device->write(QString("[%1]%2\n").arg(formatTimestamp(it.key()), it.value()).toUtf8());
+            device->write(QString("[%1]%2\n").arg(formatTimestamp(timestamp), text).toUtf8());
         }
     }
 
-    return muse::Ret(muse::Ret::Code::Ok);
+    return make_ok();
 }
 
-//
-// LRCWriter::collectLyrics
-//
-
-QMap<qreal, QString> LRCWriter::collectLyrics(const mu::engraving::Score* score)
+std::map<double, QString> LRCWriter::collectLyrics(const mu::engraving::Score* score)
 {
-    QMap<qreal, QString> lyrics;
+    std::map<double, QString> lyrics;
     const RepeatList& repeats = score->repeatList();
 
     staff_idx_t lyricsStaff;
@@ -169,25 +135,25 @@ QMap<qreal, QString> LRCWriter::collectLyrics(const mu::engraving::Score* score)
                 continue;
             }
 
-            for (Segment* seg = toMeasure(mb)->first(); seg; seg = seg->next()) {
+            for (const Segment* seg = toMeasure(mb)->first(); seg; seg = seg->next()) {
                 if (!seg->isChordRestType()) {
                     continue;
                 }
 
-                for (EngravingItem* e : seg->elist()) {
+                for (const EngravingItem* e : seg->elist()) {
                     if (!e || !e->isChordRest()) {
                         continue;
                     }
 
-                    for (Lyrics* l : toChordRest(e)->lyrics()) {
+                    for (const Lyrics* l : toChordRest(e)->lyrics()) {
                         // if (l->text().empty())
                         if (l->plainText().isEmpty()) {
                             continue;
                         }
 
                         if ((lyricsStaff == e->staffIdx()) && (lyricsVoice == e->voice()) && (lyricNumber == l->subtype())) {
-                            const qreal time = score->utick2utime(l->tick().ticks() + tickOffset) * 1000;
-                            lyrics.insert(time, l->plainText());
+                            const double time = score->utick2utime(l->tick().ticks() + tickOffset) * 1000;
+                            lyrics.insert_or_assign(time, l->plainText());
                         }
                     }
                 }
@@ -197,23 +163,15 @@ QMap<qreal, QString> LRCWriter::collectLyrics(const mu::engraving::Score* score)
     return lyrics;
 }
 
-//
-// LRCWriter::formatTimestamp
-//
-
-QString LRCWriter::formatTimestamp(qreal ms) const
+QString LRCWriter::formatTimestamp(double ms) const
 {
     const int totalSec = static_cast<int>(ms / 1000);
-    return QString("%1:%2.%3")
+    return u"%1:%2.%3"_s
            .arg(totalSec / 60, 2, 10, QLatin1Char('0'))
            .arg(totalSec % 60, 2, 10, QLatin1Char('0'))
            .arg(static_cast<int>(ms) % 1000 / 10, 2, 10, QLatin1Char('0'));
 }
 
-//
-// LRCWriter::findStaffVoiceAndLyricToExport
-//
-
 void LRCWriter::findStaffVoiceAndLyricToExport(const mu::engraving::Score* score, mu::engraving::staff_idx_t& staff,
                                                mu::engraving::voice_idx_t& voice, int& lyricNumber)
 {
@@ -225,46 +183,45 @@ void LRCWriter::findStaffVoiceAndLyricToExport(const mu::engraving::Score* score
     const RepeatList& repeats = score->repeatList();
 
     for (const RepeatSegment* rs : repeats) {
-        const int tickOffset = rs->utick - rs->tick;
-
         for (const MeasureBase* mb = rs->firstMeasure(); mb; mb = mb->next()) {
             if (!mb->isMeasure()) {
                 continue;
             }
 
-            for (Segment* seg = toMeasure(mb)->first(); seg; seg = seg->next()) {
+            for (const Segment* seg = toMeasure(mb)->first(); seg; seg = seg->next()) {
                 if (!seg->isChordRestType()) {
                     continue;
                 }
 
-                for (EngravingItem* e : seg->elist()) {
+                for (const EngravingItem* e : seg->elist()) {
                     if (!e || !e->isChordRest()) {
                         continue;
                     }
 
-                    for (Lyrics* l : toChordRest(e)->lyrics()) {
+                    for (const Lyrics* l : toChordRest(e)->lyrics()) {
                         // if (l->text().empty())
                         if (l->plainText().isEmpty()) {
                             continue;
-                        } else {
-                            if (!lyricsFound) {
-                                lyricsFound = true;
-                                staff = e->staffIdx();
+                        }
+
+                        if (!lyricsFound) {
+                            lyricsFound = true;
+                            staff = e->staffIdx();
+                            voice = e->voice();
+                            lyricNumber = l->subtype();
+                            continue;
+                        }
+
+                        if (staff > e->staffIdx()) {
+                            staff = e->staffIdx();
+                            voice = e->voice();
+                            lyricNumber = l->subtype();
+                        } else if (staff == e->staffIdx()) {
+                            if (voice > e->voice()) {
                                 voice = e->voice();
                                 lyricNumber = l->subtype();
-                            } else {
-                                if (staff > e->staffIdx()) {
-                                    staff = e->staffIdx();
-                                    voice = e->voice();
-                                    lyricNumber = l->subtype();
-                                } else if (staff == e->staffIdx()) {
-                                    if (voice > e->voice()) {
-                                        voice = e->voice();
-                                        lyricNumber = l->subtype();
-                                    } else if (voice == e->voice()) {
-                                        lyricNumber = min(lyricNumber, l->subtype());
-                                    }
-                                }
+                            } else if (voice == e->voice()) {
+                                lyricNumber = std::min(lyricNumber, l->subtype());
                             }
                         }
                     }
@@ -273,4 +230,3 @@ void LRCWriter::findStaffVoiceAndLyricToExport(const mu::engraving::Score* score
         }
     }
 }
-} // namespace mu::iex::lrcexport