Track: Return a copy of TrackMetadata/TrackRecord

src/library/dao/trackdao.cpp

@@ -745,14 +745,11 @@ TrackId TrackDAO::addTracksAddTrack(const TrackPointer& pTrack, bool unremove) {
         }
 
         // Time stamps are stored with timezone UTC in the database
-        mixxx::TrackRecord trackRecord;
-        pTrack->readTrackRecord(&trackRecord);
-        const mixxx::BeatsPointer pBeats = pTrack->getBeats();
         const auto trackDateAdded = QDateTime::currentDateTimeUtc();
         if (!insertTrackLibrary(
                     m_pQueryLibraryInsert.get(),
-                    trackRecord,
-                    pBeats,
+                    pTrack->getRecord(),
+                    pTrack->getBeats(),
                     trackLocationId,
                     fileAccess.info(),
                     trackDateAdded)) {
@@ -1475,6 +1472,12 @@ TrackPointer TrackDAO::getTrackById(TrackId trackId) const {
         // before, so just check and try for every track that has been
         // freshly loaded from the database.
         SoundSourceProxy(pTrack).updateTrackFromSource();
+        if (kLogger.debugEnabled() && pTrack->isDirty()) {
+            kLogger.debug()
+                    << "Updated track metadata from file tags:"
+                    << pTrack->getFileInfo().lastModified()
+                    << pTrack->getMetadata();
+        }
     }
 
     // Validate and refresh cover image hash values if needed.
@@ -1619,10 +1622,10 @@ bool TrackDAO::updateTrack(Track* pTrack) const {
 
     query.bindValue(":track_id", trackId.toVariant());
 
-    mixxx::TrackRecord trackRecord;
-    pTrack->readTrackRecord(&trackRecord);
-    const mixxx::BeatsPointer pBeats = pTrack->getBeats();
-    bindTrackLibraryValues(&query, trackRecord, pBeats);
+    bindTrackLibraryValues(
+            &query,
+            pTrack->getRecord(),
+            pTrack->getBeats());
 
     VERIFY_OR_DEBUG_ASSERT(query.exec()) {
         LOG_FAILED_QUERY(query);

src/library/dlgtagfetcher.cpp

@@ -11,8 +11,7 @@ namespace {
 
 QStringList trackColumnValues(
         const Track& track) {
-    mixxx::TrackMetadata trackMetadata;
-    track.readTrackMetadata(&trackMetadata);
+    const mixxx::TrackMetadata trackMetadata = track.getMetadata();
     const QString trackNumberAndTotal = TrackNumbers::joinAsString(
             trackMetadata.getTrackInfo().getTrackNumber(),
             trackMetadata.getTrackInfo().getTrackTotal());
@@ -166,8 +165,7 @@ void DlgTagFetcher::apply() {
     DEBUG_ASSERT(resultIndex < m_data.m_results.size());
     const mixxx::musicbrainz::TrackRelease& trackRelease =
             m_data.m_results[resultIndex];
-    mixxx::TrackMetadata trackMetadata;
-    m_track->readTrackMetadata(&trackMetadata);
+    mixxx::TrackMetadata trackMetadata = m_track->getMetadata();
     if (!trackRelease.artist.isEmpty()) {
         trackMetadata.refTrackInfo().setArtist(
                 trackRelease.artist);

src/sources/soundsourceproxy.cpp

@@ -537,9 +537,9 @@ bool SoundSourceProxy::updateTrackFromSource(
     // values if the corresponding file tags are missing. Depending
     // on the file type some kind of tags might even not be supported
     // at all and this information would get lost entirely otherwise!
-    mixxx::TrackMetadata trackMetadata;
     bool metadataSynchronized = false;
-    m_pTrack->readTrackMetadata(&trackMetadata, &metadataSynchronized);
+    mixxx::TrackMetadata trackMetadata =
+            m_pTrack->getMetadata(&metadataSynchronized);
     // If the file tags have already been parsed at least once, the
     // existing track metadata should not be updated implicitly, i.e.
     // if the user did not explicitly choose to (re-)import metadata
@@ -587,7 +587,7 @@ bool SoundSourceProxy::updateTrackFromSource(
                 << "from file"
                 << getUrl().toString();
         // make sure that the trackMetadata was not messed up due to the failure
-        m_pTrack->readTrackMetadata(&trackMetadata, &metadataSynchronized);
+        trackMetadata = m_pTrack->getMetadata(&metadataSynchronized);
     }
 
     // Partial import

src/test/trackupdate_test.cpp

@@ -1,10 +1,9 @@
 #include <QtDebug>
 
-#include "test/mixxxtest.h"
-
-#include "track/track.h"
 #include "library/coverart.h"
 #include "sources/soundsourceproxy.h"
+#include "test/mixxxtest.h"
+#include "track/track.h"
 
 namespace {
 
@@ -55,17 +54,15 @@ TEST_F(TrackUpdateTest, parseModifiedCleanOnce) {
     auto pTrack = newTestTrackParsedModified();
     pTrack->markClean();
 
-    mixxx::TrackMetadata trackMetadataBefore;
-    pTrack->readTrackMetadata(&trackMetadataBefore);
-    auto coverInfoBefore = pTrack->getCoverInfo();
+    const auto trackMetadataBefore = pTrack->getMetadata();
+    const auto coverInfoBefore = pTrack->getCoverInfo();
 
     // Re-update from source should have no effect
     ASSERT_FALSE(SoundSourceProxy(pTrack).updateTrackFromSource(
             SoundSourceProxy::UpdateTrackFromSourceMode::Once));
 
-    mixxx::TrackMetadata trackMetadataAfter;
-    pTrack->readTrackMetadata(&trackMetadataAfter);
-    auto coverInfoAfter = pTrack->getCoverInfo();
+    const auto trackMetadataAfter = pTrack->getMetadata();
+    const auto coverInfoAfter = pTrack->getCoverInfo();
 
     // Verify that the track has not been modified
     ASSERT_TRUE(pTrack->isMetadataSynchronized());
@@ -78,16 +75,14 @@ TEST_F(TrackUpdateTest, parseModifiedCleanAgainSkipCover) {
     auto pTrack = newTestTrackParsedModified();
     pTrack->markClean();
 
-    mixxx::TrackMetadata trackMetadataBefore;
-    pTrack->readTrackMetadata(&trackMetadataBefore);
-    auto coverInfoBefore = pTrack->getCoverInfo();
+    const auto trackMetadataBefore = pTrack->getMetadata();
+    const auto coverInfoBefore = pTrack->getCoverInfo();
 
     EXPECT_TRUE(SoundSourceProxy(pTrack).updateTrackFromSource(
             SoundSourceProxy::UpdateTrackFromSourceMode::Again));
 
-    mixxx::TrackMetadata trackMetadataAfter;
-    pTrack->readTrackMetadata(&trackMetadataAfter);
-    auto coverInfoAfter = pTrack->getCoverInfo();
+    const auto trackMetadataAfter = pTrack->getMetadata();
+    const auto coverInfoAfter = pTrack->getCoverInfo();
 
     // Updated
     EXPECT_TRUE(pTrack->isMetadataSynchronized());
@@ -104,16 +99,14 @@ TEST_F(TrackUpdateTest, parseModifiedCleanAgainUpdateCover) {
     pTrack->setCoverInfo(coverInfo);
     pTrack->markClean();
 
-    mixxx::TrackMetadata trackMetadataBefore;
-    pTrack->readTrackMetadata(&trackMetadataBefore);
-    auto coverInfoBefore = pTrack->getCoverInfo();
+    const auto trackMetadataBefore = pTrack->getMetadata();
+    const auto coverInfoBefore = pTrack->getCoverInfo();
 
     EXPECT_TRUE(SoundSourceProxy(pTrack).updateTrackFromSource(
             SoundSourceProxy::UpdateTrackFromSourceMode::Again));
 
-    mixxx::TrackMetadata trackMetadataAfter;
-    pTrack->readTrackMetadata(&trackMetadataAfter);
-    auto coverInfoAfter = pTrack->getCoverInfo();
+    const auto trackMetadataAfter = pTrack->getMetadata();
+    const auto coverInfoAfter = pTrack->getCoverInfo();
 
     // Updated
     EXPECT_TRUE(pTrack->isMetadataSynchronized());
@@ -125,16 +118,14 @@ TEST_F(TrackUpdateTest, parseModifiedCleanAgainUpdateCover) {
 TEST_F(TrackUpdateTest, parseModifiedDirtyAgain) {
     auto pTrack = newTestTrackParsedModified();
 
-    mixxx::TrackMetadata trackMetadataBefore;
-    pTrack->readTrackMetadata(&trackMetadataBefore);
-    auto coverInfoBefore = pTrack->getCoverInfo();
+    const auto trackMetadataBefore = pTrack->getMetadata();
+    const auto coverInfoBefore = pTrack->getCoverInfo();
 
     EXPECT_TRUE(SoundSourceProxy(pTrack).updateTrackFromSource(
             SoundSourceProxy::UpdateTrackFromSourceMode::Again));
 
-    mixxx::TrackMetadata trackMetadataAfter;
-    pTrack->readTrackMetadata(&trackMetadataAfter);
-    auto coverInfoAfter = pTrack->getCoverInfo();
+    const auto trackMetadataAfter = pTrack->getMetadata();
+    const auto coverInfoAfter = pTrack->getCoverInfo();
 
     // Updated
     EXPECT_TRUE(pTrack->isMetadataSynchronized());

src/track/track.cpp

@@ -239,26 +239,22 @@ bool Track::mergeImportedMetadata(
     return true;
 }
 
-void Track::readTrackMetadata(
-        mixxx::TrackMetadata* pTrackMetadata,
+mixxx::TrackMetadata Track::getMetadata(
         bool* pMetadataSynchronized) const {
-    DEBUG_ASSERT(pTrackMetadata);
-    QMutexLocker lock(&m_qMutex);
-    *pTrackMetadata = m_record.getMetadata();
+    const QMutexLocker locked(&m_qMutex);
     if (pMetadataSynchronized) {
         *pMetadataSynchronized = m_record.getMetadataSynchronized();
     }
+    return m_record.getMetadata();
 }
 
-void Track::readTrackRecord(
-        mixxx::TrackRecord* pTrackRecord,
+mixxx::TrackRecord Track::getRecord(
         bool* pDirty) const {
-    DEBUG_ASSERT(pTrackRecord);
-    QMutexLocker lock(&m_qMutex);
-    *pTrackRecord = m_record;
+    const QMutexLocker locked(&m_qMutex);
     if (pDirty) {
         *pDirty = m_bDirty;
     }
+    return m_record;
 }
 
 mixxx::ReplayGain Track::getReplayGain() const {

src/track/track.h

@@ -338,11 +338,10 @@ class Track : public QObject {
     bool mergeImportedMetadata(
             const mixxx::TrackMetadata& importedMetadata);
 
-    void readTrackMetadata(
-            mixxx::TrackMetadata* pTrackMetadata,
+    mixxx::TrackMetadata getMetadata(
             bool* pMetadataSynchronized = nullptr) const;
-    void readTrackRecord(
-            mixxx::TrackRecord* pTrackRecord,
+
+    mixxx::TrackRecord getRecord(
             bool* pDirty = nullptr) const;
 
     // Mark the track dirty if it isn't already.