Merge pull request #4873 from uklotzde/reset-missing-tag-metadata

Optionally reset metadata on reimport if file tags are missing

src/library/browse/browsethread.cpp

@@ -162,10 +162,13 @@ void BrowseThread::populateModel() {
                 thisPath.token());
         {
             mixxx::TrackMetadata trackMetadata;
+            // Both resetMissingTagMetadata = false/true have the same effect
+            constexpr auto resetMissingTagMetadata = false;
             SoundSourceProxy::importTrackMetadataAndCoverImageFromFile(
                     fileAccess,
                     &trackMetadata,
-                    nullptr);
+                    nullptr,
+                    resetMissingTagMetadata);
 
             item = new QStandardItem(fileAccess.info().fileName());
             item->setToolTip(item->text());

src/library/coverartutils.cpp

@@ -46,10 +46,13 @@ QString CoverArtUtils::supportedCoverArtExtensionsRegex() {
 QImage CoverArtUtils::extractEmbeddedCover(
         mixxx::FileAccess trackFileAccess) {
     QImage image;
+    // Both resetMissingTagMetadata = false/true have the same effect
+    constexpr auto resetMissingTagMetadata = false;
     SoundSourceProxy::importTrackMetadataAndCoverImageFromFile(
             std::move(trackFileAccess),
             nullptr,
-            &image);
+            &image,
+            resetMissingTagMetadata);
     return image;
 }
 

src/library/dlgtrackinfo.cpp

@@ -9,6 +9,7 @@
 #include "library/coverartcache.h"
 #include "library/coverartutils.h"
 #include "library/dlgtagfetcher.h"
+#include "library/library_prefs.h"
 #include "library/trackmodel.h"
 #include "moc_dlgtrackinfo.cpp"
 #include "preferences/colorpalettesettings.h"
@@ -36,10 +37,12 @@ const mixxx::Duration kMaxInterval = mixxx::Duration::fromMillis(
 } // namespace
 
 DlgTrackInfo::DlgTrackInfo(
+        UserSettingsPointer pUserSettings,
         const TrackModel* trackModel)
         // No parent because otherwise it inherits the style parent's
         // style which can make it unreadable. Bug #673411
         : QDialog(nullptr),
+          m_pUserSettings(std::move(pUserSettings)),
           m_pTrackModel(trackModel),
           m_tapFilter(this, kFilterLength, kMaxInterval),
           m_pWCoverArtLabel(make_parented<WCoverArtLabel>(this)),
@@ -663,10 +666,12 @@ void DlgTrackInfo::slotImportMetadataFromFile() {
     mixxx::TrackRecord trackRecord = m_pLoadedTrack->getRecord();
     mixxx::TrackMetadata trackMetadata = trackRecord.getMetadata();
     QImage coverImage;
+    const auto resetMissingTagMetadata = m_pUserSettings->getValue<bool>(
+            mixxx::library::prefs::kResetMissingTagMetadataOnImportConfigKey);
     const auto [importResult, sourceSynchronizedAt] =
             SoundSourceProxy(m_pLoadedTrack)
                     .importTrackMetadataAndCoverImage(
-                            &trackMetadata, &coverImage);
+                            &trackMetadata, &coverImage, resetMissingTagMetadata);
     if (importResult != mixxx::MetadataSource::ImportResult::Succeeded) {
         return;
     }

src/library/dlgtrackinfo.h

@@ -6,6 +6,7 @@
 
 #include "library/coverart.h"
 #include "library/ui_dlgtrackinfo.h"
+#include "preferences/usersettings.h"
 #include "track/beats.h"
 #include "track/keys.h"
 #include "track/track_decl.h"
@@ -27,6 +28,7 @@ class DlgTrackInfo : public QDialog, public Ui::DlgTrackInfo {
   public:
     // TODO: Remove dependency on TrackModel
     explicit DlgTrackInfo(
+            UserSettingsPointer pUserSettings,
             const TrackModel* trackModel = nullptr);
     ~DlgTrackInfo() override = default;
 
@@ -100,6 +102,8 @@ class DlgTrackInfo : public QDialog, public Ui::DlgTrackInfo {
     void updateTrackMetadataFields();
     void updateSpinBpmFromBeats();
 
+    const UserSettingsPointer m_pUserSettings;
+
     const TrackModel* const m_pTrackModel;
 
     TrackPointer m_pLoadedTrack;

src/library/library_prefs.cpp

@@ -54,6 +54,11 @@ const ConfigKey mixxx::library::prefs::kSyncTrackMetadataConfigKey =
                 mixxx::library::prefs::kConfigGroup,
                 QStringLiteral("SyncTrackMetadataExport")};
 
+const ConfigKey mixxx::library::prefs::kResetMissingTagMetadataOnImportConfigKey =
+        ConfigKey{
+                mixxx::library::prefs::kConfigGroup,
+                QStringLiteral("ResetMissingTagMetadataOnImport")};
+
 // The naming is unchanged for backward compatibility
 const ConfigKey mixxx::library::prefs::kSyncSeratoMetadataConfigKey =
         ConfigKey{

src/library/library_prefs.h

@@ -34,7 +34,7 @@ const bool kEditMetadataSelectedClickDefault = false;
 
 extern const ConfigKey kSyncTrackMetadataConfigKey;
 
-extern const ConfigKey kSyncSeratoMetadataConfigKey;
+extern const ConfigKey kResetMissingTagMetadataOnImportConfigKey;
 
 extern const ConfigKey kSyncSeratoMetadataConfigKey;
 

src/sources/metadatasource.h

@@ -31,14 +31,20 @@ class MetadataSource {
         Unavailable,
     };
 
-    // Read both track metadata and cover art at once, because this
-    // is the most common use case. Both pointers are output parameters
-    // and might be passed a nullptr if their result is not needed.
-    // If no metadata is available for a track then the source should
-    // return Unavailable as this default implementation does.
+    /// Read both track metadata and cover art at once, because this
+    /// is the most common use case. Both pointers are output parameters
+    /// and might be passed a nullptr if their result is not needed.
+    /// If no metadata is available for a track then the source should
+    /// return Unavailable as this default implementation does.
+    /// The flag resetMissingTagMetadata controls if existing metadata
+    /// should be discarded if the corresponding file tags are missing.
     virtual std::pair<ImportResult, QDateTime> importTrackMetadataAndCoverImage(
-            TrackMetadata* /*pTrackMetadata*/,
-            QImage* /*pCoverImage*/) const {
+            TrackMetadata* pTrackMetadata,
+            QImage* pCoverImage,
+            bool resetMissingTagMetadata) const {
+        Q_UNUSED(pTrackMetadata)
+        Q_UNUSED(pCoverImage)
+        Q_UNUSED(resetMissingTagMetadata)
         return std::make_pair(ImportResult::Unavailable, QDateTime());
     }
 

src/sources/metadatasourcetaglib.cpp

@@ -105,7 +105,8 @@ MetadataSourceTagLib::afterExport(ExportResult exportResult) const {
 std::pair<MetadataSource::ImportResult, QDateTime>
 MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         TrackMetadata* pTrackMetadata,
-        QImage* pCoverImage) const {
+        QImage* pCoverImage,
+        bool resetMissingTagMetadata) const {
     VERIFY_OR_DEBUG_ASSERT(pTrackMetadata || pCoverImage) {
         kLogger.warning()
                 << "Nothing to import"
@@ -136,13 +137,14 @@ MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         if (taglib::hasID3v2Tag(file)) {
             const TagLib::ID3v2::Tag* pTag = file.ID3v2Tag();
             DEBUG_ASSERT(pTag);
-            taglib::id3v2::importTrackMetadataFromTag(pTrackMetadata, *pTag);
+            taglib::id3v2::importTrackMetadataFromTag(
+                    pTrackMetadata, *pTag, resetMissingTagMetadata);
             taglib::id3v2::importCoverImageFromTag(pCoverImage, *pTag);
             return afterImport(ImportResult::Succeeded);
         } else if (taglib::hasAPETag(file)) {
             const TagLib::APE::Tag* pTag = file.APETag();
             DEBUG_ASSERT(pTag);
-            taglib::ape::importTrackMetadataFromTag(pTrackMetadata, *pTag);
+            taglib::ape::importTrackMetadataFromTag(pTrackMetadata, *pTag, resetMissingTagMetadata);
             taglib::ape::importCoverImageFromTag(pCoverImage, *pTag);
             return afterImport(ImportResult::Succeeded);
         } else if (taglib::hasID3v1Tag(file)) {
@@ -166,7 +168,7 @@ MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         if (taglib::hasMP4Tag(file)) {
             const TagLib::MP4::Tag* pTag = file.tag();
             DEBUG_ASSERT(pTag);
-            taglib::mp4::importTrackMetadataFromTag(pTrackMetadata, *pTag);
+            taglib::mp4::importTrackMetadataFromTag(pTrackMetadata, *pTag, resetMissingTagMetadata);
             taglib::mp4::importCoverImageFromTag(pCoverImage, *pTag);
             return afterImport(ImportResult::Succeeded);
         }
@@ -183,13 +185,17 @@ MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         if (taglib::hasXiphComment(file)) {
             TagLib::Ogg::XiphComment* pTag = file.xiphComment();
             DEBUG_ASSERT(pTag);
-            taglib::xiph::importTrackMetadataFromTag(pTrackMetadata, *pTag, taglib::FileType::FLAC);
+            taglib::xiph::importTrackMetadataFromTag(pTrackMetadata,
+                    *pTag,
+                    taglib::FileType::FLAC,
+                    resetMissingTagMetadata);
             coverImageImported = taglib::xiph::importCoverImageFromTag(pCoverImage, *pTag);
             importSucceeded = true;
         } else if (taglib::hasID3v2Tag(file)) {
             const TagLib::ID3v2::Tag* pTag = file.ID3v2Tag();
             DEBUG_ASSERT(pTag);
-            taglib::id3v2::importTrackMetadataFromTag(pTrackMetadata, *pTag);
+            taglib::id3v2::importTrackMetadataFromTag(
+                    pTrackMetadata, *pTag, resetMissingTagMetadata);
             coverImageImported = taglib::id3v2::importCoverImageFromTag(pCoverImage, *pTag);
             importSucceeded = true;
         }
@@ -216,7 +222,10 @@ MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         }
         TagLib::Ogg::XiphComment* pTag = file.tag();
         if (pTag) {
-            taglib::xiph::importTrackMetadataFromTag(pTrackMetadata, *pTag, taglib::FileType::OGG);
+            taglib::xiph::importTrackMetadataFromTag(pTrackMetadata,
+                    *pTag,
+                    taglib::FileType::OGG,
+                    resetMissingTagMetadata);
             taglib::xiph::importCoverImageFromTag(pCoverImage, *pTag);
             return afterImport(ImportResult::Succeeded);
         }
@@ -229,7 +238,10 @@ MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         }
         TagLib::Ogg::XiphComment* pTag = file.tag();
         if (pTag) {
-            taglib::xiph::importTrackMetadataFromTag(pTrackMetadata, *pTag, taglib::FileType::OPUS);
+            taglib::xiph::importTrackMetadataFromTag(pTrackMetadata,
+                    *pTag,
+                    taglib::FileType::OPUS,
+                    resetMissingTagMetadata);
             taglib::xiph::importCoverImageFromTag(pCoverImage, *pTag);
             return afterImport(ImportResult::Succeeded);
         }
@@ -243,7 +255,7 @@ MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         if (taglib::hasAPETag(file)) {
             const TagLib::APE::Tag* pTag = file.APETag();
             DEBUG_ASSERT(pTag);
-            taglib::ape::importTrackMetadataFromTag(pTrackMetadata, *pTag);
+            taglib::ape::importTrackMetadataFromTag(pTrackMetadata, *pTag, resetMissingTagMetadata);
             taglib::ape::importCoverImageFromTag(pCoverImage, *pTag);
             return afterImport(ImportResult::Succeeded);
         }
@@ -257,7 +269,8 @@ MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         if (taglib::hasID3v2Tag(file)) {
             const TagLib::ID3v2::Tag* pTag = file.ID3v2Tag();
             DEBUG_ASSERT(pTag);
-            taglib::id3v2::importTrackMetadataFromTag(pTrackMetadata, *pTag);
+            taglib::id3v2::importTrackMetadataFromTag(
+                    pTrackMetadata, *pTag, resetMissingTagMetadata);
             taglib::id3v2::importCoverImageFromTag(pCoverImage, *pTag);
             return afterImport(ImportResult::Succeeded);
         } else if (file.hasInfoTag()) {
@@ -276,7 +289,8 @@ MetadataSourceTagLib::importTrackMetadataAndCoverImage(
         if (taglib::hasID3v2Tag(file)) {
             const TagLib::ID3v2::Tag* pTag = file.tag();
             DEBUG_ASSERT(pTag);
-            taglib::id3v2::importTrackMetadataFromTag(pTrackMetadata, *pTag);
+            taglib::id3v2::importTrackMetadataFromTag(
+                    pTrackMetadata, *pTag, resetMissingTagMetadata);
             taglib::id3v2::importCoverImageFromTag(pCoverImage, *pTag);
             return afterImport(ImportResult::Succeeded);
         } else if (file.importTrackMetadataFromTextChunks(pTrackMetadata)) {

src/sources/metadatasourcetaglib.h

@@ -23,7 +23,8 @@ class MetadataSourceTagLib : public MetadataSource {
 
     std::pair<ImportResult, QDateTime> importTrackMetadataAndCoverImage(
             TrackMetadata* pTrackMetadata,
-            QImage* pCoverArt) const override;
+            QImage* pCoverArt,
+            bool resetMissingTagMetadata) const override;
 
     std::pair<ExportResult, QDateTime> exportTrackMetadata(
             const TrackMetadata& trackMetadata) const override;

src/sources/soundsourcemodplug.cpp

@@ -82,7 +82,8 @@ SoundSourceModPlug::~SoundSourceModPlug() {
 std::pair<MetadataSource::ImportResult, QDateTime>
 SoundSourceModPlug::importTrackMetadataAndCoverImage(
         TrackMetadata* pTrackMetadata,
-        QImage* pCoverArt) const {
+        QImage* pCoverArt,
+        bool resetMissingTagMetadata) const {
     if (pTrackMetadata != nullptr) {
         QFile modFile(getLocalFileName());
         modFile.open(QIODevice::ReadOnly);
@@ -111,7 +112,8 @@ SoundSourceModPlug::importTrackMetadataAndCoverImage(
 
     // The modplug library currently does not support reading cover-art from
     // modplug files -- kain88 (Oct 2014)
-    return MetadataSourceTagLib::importTrackMetadataAndCoverImage(nullptr, pCoverArt);
+    return MetadataSourceTagLib::importTrackMetadataAndCoverImage(
+            nullptr, pCoverArt, resetMissingTagMetadata);
 }
 
 SoundSource::OpenResult SoundSourceModPlug::tryOpen(

src/sources/soundsourcemodplug.h

@@ -28,7 +28,8 @@ class SoundSourceModPlug final : public SoundSource {
 
     std::pair<ImportResult, QDateTime> importTrackMetadataAndCoverImage(
             TrackMetadata* pTrackMetadata,
-            QImage* pCoverArt) const override;
+            QImage* pCoverArt,
+            bool resetMissingTagMetadata) const override;
 
     void close() override;
 

src/sources/soundsourceproxy.cpp

@@ -545,7 +545,8 @@ std::pair<mixxx::MetadataSource::ImportResult, QDateTime>
 SoundSourceProxy::importTrackMetadataAndCoverImageFromFile(
         mixxx::FileAccess trackFileAccess,
         mixxx::TrackMetadata* pTrackMetadata,
-        QImage* pCoverImage) {
+        QImage* pCoverImage,
+        bool resetMissingTagMetadata) {
     if (!trackFileAccess.info().checkFileExists()) {
         // Silently ignore missing files to avoid spaming the log:
         // https://bugs.launchpad.net/mixxx/+bug/1875237
@@ -567,21 +568,24 @@ SoundSourceProxy::importTrackMetadataAndCoverImageFromFile(
     }
     return SoundSourceProxy(pTrack).importTrackMetadataAndCoverImage(
             pTrackMetadata,
-            pCoverImage);
+            pCoverImage,
+            resetMissingTagMetadata);
 }
 
 std::pair<mixxx::MetadataSource::ImportResult, QDateTime>
 SoundSourceProxy::importTrackMetadataAndCoverImage(
         mixxx::TrackMetadata* pTrackMetadata,
-        QImage* pCoverImage) const {
+        QImage* pCoverImage,
+        bool resetMissingTagMetadata) const {
     if (!m_pSoundSource) {
         // The file doesn't seem to be readable or the file format
         // is not supported.
         return importTrackMetadataAndCoverImageUnavailable();
     }
     return m_pSoundSource->importTrackMetadataAndCoverImage(
             pTrackMetadata,
-            pCoverImage);
+            pCoverImage,
+            resetMissingTagMetadata);
 }
 
 namespace {
@@ -688,7 +692,8 @@ SoundSourceProxy::UpdateTrackFromSourceResult SoundSourceProxy::updateTrackFromS
     auto [metadataImportResult, sourceSynchronizedAt] =
             importTrackMetadataAndCoverImage(
                     &trackMetadata,
-                    pCoverImg);
+                    pCoverImg,
+                    syncParams.resetMissingTagMetadataOnImport);
     VERIFY_OR_DEBUG_ASSERT(!sourceSynchronizedAt.isValid() ||
             sourceSynchronizedAt.timeSpec() == Qt::UTC) {
         qWarning() << "Converting source synchronization time to UTC:" << sourceSynchronizedAt;

src/sources/soundsourceproxy.h

@@ -102,7 +102,8 @@ class SoundSourceProxy {
     importTrackMetadataAndCoverImageFromFile(
             mixxx::FileAccess trackFileAccess,
             mixxx::TrackMetadata* pTrackMetadata,
-            QImage* pCoverImage);
+            QImage* pCoverImage,
+            bool resetMissingTagMetadata);
 
     /// Import both track metadata and/or the cover image of the
     /// captured track object from the corresponding file.
@@ -117,7 +118,8 @@ class SoundSourceProxy {
     std::pair<mixxx::MetadataSource::ImportResult, QDateTime>
     importTrackMetadataAndCoverImage(
             mixxx::TrackMetadata* pTrackMetadata,
-            QImage* pCoverImage) const;
+            QImage* pCoverImage,
+            bool resetMissingTagMetadata) const;
 
     /// Controls which (metadata/coverart) and how tags are (re-)imported from
     /// audio files when creating a SoundSourceProxy.

src/test/coverartcache_test.cpp

@@ -13,10 +13,13 @@ class CoverArtCacheTest : public LibraryTest, public CoverArtCache {
   protected:
     void loadCoverFromMetadata(const QString& trackLocation) {
         QImage img;
+        // Both resetMissingTagMetadata = false/true have the same effect
+        constexpr auto resetMissingTagMetadata = false;
         SoundSourceProxy::importTrackMetadataAndCoverImageFromFile(
                 mixxx::FileAccess(mixxx::FileInfo(trackLocation)),
                 nullptr,
-                &img);
+                &img,
+                resetMissingTagMetadata);
         ASSERT_FALSE(img.isNull());
 
         CoverInfo info;

src/test/metadatatest.cpp

@@ -46,7 +46,10 @@ class MetadataTest : public testing::Test {
         pFrame.release();
 
         mixxx::TrackMetadata trackMetadata;
-        mixxx::taglib::id3v2::importTrackMetadataFromTag(&trackMetadata, tag);
+        // Both resetMissingTagMetadata = false/true have the same effect
+        constexpr auto resetMissingTagMetadata = false;
+        mixxx::taglib::id3v2::importTrackMetadataFromTag(
+                &trackMetadata, tag, resetMissingTagMetadata);
 
         EXPECT_DOUBLE_EQ(expectedValue, trackMetadata.getTrackInfo().getBpm().value());
     }
@@ -128,7 +131,10 @@ TEST_F(MetadataTest, ID3v2Year) {
                 mixxx::taglib::id3v2::exportTrackMetadataIntoTag(&tag, trackMetadata);
             }
             mixxx::TrackMetadata trackMetadata;
-            mixxx::taglib::id3v2::importTrackMetadataFromTag(&trackMetadata, tag);
+            // Both resetMissingTagMetadata = false/true have the same effect
+            constexpr auto resetMissingTagMetadata = false;
+            mixxx::taglib::id3v2::importTrackMetadataFromTag(
+                    &trackMetadata, tag, resetMissingTagMetadata);
             if (4 > majorVersion) {
                 // ID3v2.3.0: parsed + formatted
                 const QString actualYear(trackMetadata.getTrackInfo().getYear());