Merge pull request #728 from uklotzde/WriteAudioTags

Write audio tags back into files

build/depends.py

@@ -942,6 +942,7 @@ def sources(self, build):
                    "library/dlgmissing.cpp",
                    "library/dlgtagfetcher.cpp",
                    "library/dlgtrackinfo.cpp",
+                   "library/dlgtrackmetadataexport.cpp",
 
                    "library/browse/browsetablemodel.cpp",
                    "library/browse/browsethread.cpp",
@@ -1081,12 +1082,14 @@ def sources(self, build):
                    "track/playcounter.cpp",
                    "track/replaygain.cpp",
                    "track/track.cpp",
+                   "track/trackcache.cpp",
                    "track/trackmetadata.cpp",
                    "track/trackmetadatataglib.cpp",
                    "track/tracknumbers.cpp",
                    "track/albuminfo.cpp",
                    "track/trackinfo.cpp",
                    "track/trackrecord.cpp",
+                   "track/trackref.cpp",
 
                    "mixer/auxiliary.cpp",
                    "mixer/baseplayer.cpp",

plugins/soundsourcem4a/SConscript

@@ -22,6 +22,8 @@ m4a_sources = [
     "util/sample.cpp",
     "util/logger.cpp",
     "util/indexrange.cpp",
+    "track/albuminfo.cpp",
+    "track/trackinfo.cpp",
     "track/trackmetadata.cpp",
     "track/trackmetadatataglib.cpp",
     "track/tracknumbers.cpp",

plugins/soundsourcemediafoundation/SConscript

@@ -30,6 +30,8 @@ if int(build.flags['mediafoundation']):
          'util/sample.cpp',
          'util/logger.cpp',
          'util/indexrange.cpp',
+         'track/albuminfo.cpp',
+         'track/trackinfo.cpp',
          'track/trackmetadata.cpp',
          'track/trackmetadatataglib.cpp',
          'track/tracknumbers.cpp',

plugins/soundsourcewv/SConscript

@@ -22,6 +22,8 @@ wv_sources = [
     "util/sample.cpp",
     "util/logger.cpp",
     "util/indexrange.cpp",
+    "track/albuminfo.cpp",
+    "track/trackinfo.cpp",
     "track/trackmetadata.cpp",
     "track/trackmetadatataglib.cpp",
     "track/tracknumbers.cpp",

src/library/basetrackcache.cpp

@@ -10,6 +10,7 @@
 #include "library/searchqueryparser.h"
 #include "library/queryutil.h"
 #include "track/keyutils.h"
+#include "track/trackcache.h"
 #include "util/performancetimer.h"
 
 namespace {
@@ -144,12 +145,16 @@ void BaseTrackCache::setSearchColumns(const QStringList& columns) {
 }
 
 TrackPointer BaseTrackCache::lookupCachedTrack(TrackId trackId) const {
-    // Only get the track from the TrackDAO if it's in the cache and marked as
-    // dirty.
-    if (m_bIsCaching && m_dirtyTracks.contains(trackId)) {
-        return m_trackDAO.getTrack(trackId, true);
+    TrackCacheLocker cacheLocker(
+            TrackCache::instance().lookupById(trackId));
+    auto pTrack = cacheLocker.getTrack();
+    if (pTrack && pTrack->isDirty()) {
+        m_dirtyTracks.insert(trackId);
+        return pTrack;
+    } else {
+        m_dirtyTracks.remove(trackId);
+        return TrackPointer();
     }
-    return TrackPointer();
 }
 
 bool BaseTrackCache::updateIndexWithTrackpointer(TrackPointer pTrack) {

src/library/basetrackcache.h

@@ -126,7 +126,11 @@ class BaseTrackCache : public QObject {
 
     QVector<TrackId> m_trackOrder;
 
-    QSet<TrackId> m_dirtyTracks;
+    // This set is updated by signals from the Track object. It might contain
+    // false positives, i.e. track ids of tracks that are neither cached nor
+    // dirty. Each invocation of lookupCachedTrack() will take care of
+    // updating this set by inserting and removing entries as required.
+    mutable QSet<TrackId> m_dirtyTracks;
 
     bool m_bIndexBuilt;
     bool m_bIsCaching;

src/library/browse/browsetablemodel.cpp

@@ -11,7 +11,6 @@
 #include "mixer/playerinfo.h"
 #include "control/controlobject.h"
 #include "library/dao/trackdao.h"
-#include "sources/metadatasourcetaglib.h"
 #include "util/dnd.h"
 
 BrowseTableModel::BrowseTableModel(QObject* parent,
@@ -109,6 +108,14 @@ TrackPointer BrowseTableModel::getTrack(const QModelIndex& index) const {
             + "\n" +track_location);
         return TrackPointer();
     }
+    // NOTE(uklotzde, 2015-12-08): Accessing tracks from the browse view
+    // will implicitly add them to the library. Is this really what we
+    // want here??
+    // NOTE(rryan, 2015-12-27): This was intentional at the time since
+    // some people use Browse instead of the library and we want to let
+    // them edit the tracks in a way that persists across sessions
+    // and we didn't want to edit the files on disk by default
+    // unless the user opts in to that.
     return m_pTrackCollection->getTrackDAO()
             .getOrAddTrack(track_location, true, NULL);
 }
@@ -233,108 +240,93 @@ Qt::ItemFlags BrowseTableModel::flags(const QModelIndex &index) const {
     QString track_location = getTrackLocation(index);
     int column = index.column();
 
-    if (isTrackInUse(track_location) ||
-            column == COLUMN_FILENAME ||
-            column == COLUMN_BITRATE ||
-            column == COLUMN_DURATION ||
-            column == COLUMN_TYPE ||
-            column == COLUMN_FILE_MODIFIED_TIME ||
-            column == COLUMN_FILE_CREATION_TIME ||
-            column == COLUMN_REPLAYGAIN) {
+    switch (column) {
+    case COLUMN_FILENAME:
+    case COLUMN_BITRATE:
+    case COLUMN_DURATION:
+    case COLUMN_TYPE:
+    case COLUMN_FILE_MODIFIED_TIME:
+    case COLUMN_FILE_CREATION_TIME:
+    case COLUMN_REPLAYGAIN:
+        // read-only
         return defaultFlags;
-    } else {
+    default:
+        // editable
         return defaultFlags | Qt::ItemIsEditable;
     }
 }
 
-bool BrowseTableModel::isTrackInUse(const QString& track_location) const {
-    if (PlayerInfo::instance().isFileLoaded(track_location)) {
-        return true;
-    }
-
-    if (m_pRecordingManager->getRecordingLocation() == track_location) {
-        return true;
-    }
-
-    return false;
-}
-
-bool BrowseTableModel::setData(const QModelIndex &index, const QVariant &value,
-                               int role) {
+bool BrowseTableModel::setData(
+        const QModelIndex& index,
+        const QVariant& value,
+        int role) {
     Q_UNUSED(role);
 
-    if (!index.isValid()) {
+    QStandardItem* item = itemFromIndex(index);
+    DEBUG_ASSERT(nullptr != item);
+
+    TrackPointer pTrack(getTrack(index));
+    if (!pTrack) {
+        qWarning() << "BrowseTableModel::setData():"
+                << "Failed to resolve track"
+                << getTrackLocation(index);
+        // restore previous item content
+        item->setText(index.data().toString());
+        item->setToolTip(item->text());
         return false;
     }
-    qDebug() << "BrowseTableModel::setData(" << index.data() << ")";
-    int row = index.row();
-    int col = index.column();
-
-    mixxx::TrackMetadata trackMetadata;
-
-    // set tagger information
-    trackMetadata.refAlbumInfo().setTitle(this->index(row, COLUMN_ALBUM).data().toString());
-    trackMetadata.refAlbumInfo().setArtist(this->index(row, COLUMN_ALBUMARTIST).data().toString());
-
-    trackMetadata.refTrackInfo().setArtist(this->index(row, COLUMN_ARTIST).data().toString());
-    trackMetadata.refTrackInfo().setTitle(this->index(row, COLUMN_TITLE).data().toString());
-    trackMetadata.refTrackInfo().setKey(this->index(row, COLUMN_KEY).data().toString());
-    trackMetadata.refTrackInfo().setBpm(mixxx::Bpm(this->index(row, COLUMN_BPM).data().toDouble()));
-    trackMetadata.refTrackInfo().setComment(this->index(row, COLUMN_COMMENT).data().toString());
-    trackMetadata.refTrackInfo().setTrackNumber(this->index(row, COLUMN_TRACK_NUMBER).data().toString());
-    trackMetadata.refTrackInfo().setYear(this->index(row, COLUMN_YEAR).data().toString());
-    trackMetadata.refTrackInfo().setGenre(this->index(row, COLUMN_GENRE).data().toString());
-    trackMetadata.refTrackInfo().setComposer(this->index(row, COLUMN_COMPOSER).data().toString());
-    trackMetadata.refTrackInfo().setGrouping(this->index(row, COLUMN_GROUPING).data().toString());
 
     // check if one the item were edited
-    if (col == COLUMN_ARTIST) {
-        trackMetadata.refTrackInfo().setArtist(value.toString());
-    } else if (col == COLUMN_TITLE) {
-        trackMetadata.refTrackInfo().setTitle(value.toString());
-    } else if (col == COLUMN_ALBUM) {
-        trackMetadata.refAlbumInfo().setTitle(value.toString());
-    } else if (col == COLUMN_BPM) {
-        trackMetadata.refTrackInfo().setBpm(mixxx::Bpm(value.toDouble()));
-    } else if (col == COLUMN_KEY) {
-        trackMetadata.refTrackInfo().setKey(value.toString());
-    } else if (col == COLUMN_TRACK_NUMBER) {
-        trackMetadata.refTrackInfo().setTrackNumber(value.toString());
-    } else if (col == COLUMN_COMMENT) {
-        trackMetadata.refTrackInfo().setComment(value.toString());
-    } else if (col == COLUMN_GENRE) {
-        trackMetadata.refTrackInfo().setGenre(value.toString());
-    } else if (col == COLUMN_COMPOSER) {
-        trackMetadata.refTrackInfo().setComposer(value.toString());
-    } else if (col == COLUMN_YEAR) {
-        trackMetadata.refTrackInfo().setYear(value.toString());
-    } else if (col == COLUMN_ALBUMARTIST) {
-        trackMetadata.refAlbumInfo().setArtist(value.toString());
-    } else if (col == COLUMN_GROUPING) {
-        trackMetadata.refTrackInfo().setGrouping(value.toString());
-    } else {
-        qWarning() << "BrowseTableModel::setData(): no tagger column";
-        return false;
-    }
-
-    QStandardItem* item = itemFromIndex(index);
-    QString track_location(getTrackLocation(index));
-    if (mixxx::MetadataSource::ExportResult::Succeeded ==
-            mixxx::MetadataSourceTagLib(track_location).exportTrackMetadata(trackMetadata).first) {
-        // Modify underlying interalPointer object
-        item->setText(value.toString());
-        item->setToolTip(item->text());
-        return true;
-    } else {
-        // reset to old value in error
+    int col = index.column();
+    switch (col) {
+    case COLUMN_ARTIST:
+        pTrack->setArtist(value.toString());
+        break;
+    case COLUMN_TITLE:
+        pTrack->setTitle(value.toString());
+        break;
+    case COLUMN_ALBUM:
+        pTrack->setAlbum(value.toString());
+        break;
+    case COLUMN_BPM:
+        pTrack->setBpm(value.toDouble());
+        break;
+    case COLUMN_KEY:
+        pTrack->setKeyText(value.toString());
+        break;
+    case COLUMN_TRACK_NUMBER:
+        pTrack->setTrackNumber(value.toString());
+        break;
+    case COLUMN_COMMENT:
+        pTrack->setComment(value.toString());
+        break;
+    case COLUMN_GENRE:
+        pTrack->setGenre(value.toString());
+        break;
+    case COLUMN_COMPOSER:
+        pTrack->setComposer(value.toString());
+        break;
+    case COLUMN_YEAR:
+        pTrack->setYear(value.toString());
+        break;
+    case COLUMN_ALBUMARTIST:
+        pTrack->setAlbumArtist(value.toString());
+        break;
+    case COLUMN_GROUPING:
+        pTrack->setGrouping(value.toString());
+        break;
+    default:
+        qWarning() << "BrowseTableModel::setData():"
+            << "No tagger column";
+        // restore previous item context
         item->setText(index.data().toString());
         item->setToolTip(item->text());
-        QMessageBox::critical(
-            0, tr("Mixxx Library"),
-            tr("Could not update file metadata.")
-            + "\n" +track_location);
         return false;
     }
+
+    item->setText(value.toString());
+    item->setToolTip(item->text());
+    return true;
 }
 
 void BrowseTableModel::trackLoaded(QString group, TrackPointer pTrack) {
@@ -361,10 +353,7 @@ void BrowseTableModel::trackLoaded(QString group, TrackPointer pTrack) {
 }
 
 bool BrowseTableModel::isColumnSortable(int column) {
-    if (COLUMN_PREVIEW == column) {
-        return false;
-    }
-    return true;
+    return COLUMN_PREVIEW != column;
 }
 
 QAbstractItemDelegate* BrowseTableModel::delegateForColumn(const int i, QObject* pParent) {

src/library/browse/browsetablemodel.h

@@ -37,6 +37,10 @@ const int COLUMN_REPLAYGAIN = 20;
 // The BrowseTable models displays tracks
 // of given directory on the HDD.
 // Usage: Recording and Browse feature.
+//
+// TODO(XXX): Editing track metadata outside of the table view
+// (e.g. in the property dialog) does not update the table view!
+// Editing single fields in the table view works as expected.
 class BrowseTableModel : public QStandardItemModel, public virtual TrackModel {
     Q_OBJECT
 
@@ -73,7 +77,7 @@ class BrowseTableModel : public QStandardItemModel, public virtual TrackModel {
 
   private:
     void addSearchColumn(int index);
-    bool isTrackInUse(const QString& file) const;
+
     QList<int> m_searchColumns;
     MDir m_current_directory;
     TrackCollection* m_pTrackCollection;