Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WiP] Track metadata import/export #2336

Closed
wants to merge 10 commits into from
2 changes: 1 addition & 1 deletion src/analyzer/trackanalysisscheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ bool TrackAnalysisScheduler::submitNextTrack(Worker* worker) {
DEBUG_ASSERT(nextTrackId.isValid());
if (nextTrackId.isValid()) {
TrackPointer nextTrack =
m_library->trackCollection().getTrackDAO().getTrack(nextTrackId);
m_library->trackCollection().getTrackById(nextTrackId);
if (nextTrack) {
if (m_pendingTrackIds.insert(nextTrackId).second) {
if (worker->submitNextTrack(std::move(nextTrack))) {
Expand Down
2 changes: 1 addition & 1 deletion src/library/autodj/autodjfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ void AutoDJFeature::slotAddRandomTrack() {
}

if (randomTrackId.isValid()) {
pRandomTrack = m_pTrackCollection->getTrackDAO().getTrack(randomTrackId);
pRandomTrack = m_pTrackCollection->getTrackById(randomTrackId);
VERIFY_OR_DEBUG_ASSERT(pRandomTrack) {
qWarning() << "Track does not exist:"
<< randomTrackId;
Expand Down
5 changes: 3 additions & 2 deletions src/library/banshee/bansheeplaylistmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,9 @@ TrackPointer BansheePlaylistModel::getTrack(const QModelIndex& index) const {
}

bool track_already_in_library = false;
TrackPointer pTrack = m_pTrackCollection->getTrackDAO()
.getOrAddTrack(location, true, &track_already_in_library);
TrackPointer pTrack = m_pTrackCollection->getOrAddTrack(
TrackRef::fromFileInfo(location),
&track_already_in_library);

// If this track was not in the Mixxx library it is now added and will be
// saved with the metadata from Banshee. If it was already in the library
Expand Down
5 changes: 3 additions & 2 deletions src/library/baseexternalplaylistmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ TrackPointer BaseExternalPlaylistModel::getTrack(const QModelIndex& index) const
}

bool track_already_in_library = false;
TrackPointer pTrack = m_pTrackCollection->getTrackDAO()
.getOrAddTrack(location, true, &track_already_in_library);
TrackPointer pTrack = m_pTrackCollection->getOrAddTrack(
TrackRef::fromFileInfo(location),
&track_already_in_library);

// If this track was not in the Mixxx library it is now added and will be
// saved with the metadata from iTunes. If it was already in the library
Expand Down
5 changes: 3 additions & 2 deletions src/library/baseexternaltrackmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ TrackPointer BaseExternalTrackModel::getTrack(const QModelIndex& index) const {
}

bool track_already_in_library = false;
TrackPointer pTrack = m_pTrackCollection->getTrackDAO()
.getOrAddTrack(location, true, &track_already_in_library);
TrackPointer pTrack = m_pTrackCollection->getOrAddTrack(
TrackRef::fromFileInfo(location),
&track_already_in_library);

if (pTrack) {
// If this track was not in the Mixxx library it is now added and will be
Expand Down
2 changes: 1 addition & 1 deletion src/library/baseplaylistfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <QFileInfo>
#include <QStandardPaths>

#include "library/dao/playlistdao.h"
#include "library/export/trackexportwizard.h"
#include "library/library.h"
#include "library/parser.h"
Expand All @@ -26,7 +27,6 @@ BasePlaylistFeature::BasePlaylistFeature(QObject* parent,
: LibraryFeature(pConfig, parent),
m_pTrackCollection(pTrackCollection),
m_playlistDao(pTrackCollection->getPlaylistDAO()),
m_trackDao(pTrackCollection->getTrackDAO()),
m_pPlaylistTableModel(NULL),
m_rootViewName(rootViewName) {
m_pCreatePlaylistAction = new QAction(tr("Create New Playlist"),this);
Expand Down
4 changes: 1 addition & 3 deletions src/library/baseplaylistfeature.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@
#include <QString>

#include "library/libraryfeature.h"
#include "library/dao/playlistdao.h"
#include "library/dao/trackdao.h"
#include "track/track.h"

class WLibrary;
class KeyboardEventFilter;
class PlaylistDAO;
class PlaylistTableModel;
class TrackCollection;
class TreeItem;
Expand Down Expand Up @@ -89,7 +88,6 @@ class BasePlaylistFeature : public LibraryFeature {

TrackCollection* m_pTrackCollection;
PlaylistDAO &m_playlistDao;
TrackDAO &m_trackDao;
PlaylistTableModel* m_pPlaylistTableModel;
QAction *m_pCreatePlaylistAction;
QAction *m_pDeletePlaylistAction;
Expand Down
4 changes: 2 additions & 2 deletions src/library/basesqltablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ bool BaseSqlTableModel::setData(

// TODO(rryan) ugly and only works because the mixxx library tables are the
// only ones that aren't read-only. This should be moved into BTC.
TrackPointer pTrack = m_pTrackCollection->getTrackDAO().getTrack(trackId);
TrackPointer pTrack = m_pTrackCollection->getTrackById(trackId);
if (!pTrack) {
return false;
}
Expand Down Expand Up @@ -919,7 +919,7 @@ TrackId BaseSqlTableModel::getTrackId(const QModelIndex& index) const {
}

TrackPointer BaseSqlTableModel::getTrack(const QModelIndex& index) const {
return m_pTrackCollection->getTrackDAO().getTrack(getTrackId(index));
return m_pTrackCollection->getTrackById(getTrackId(index));
}

QString BaseSqlTableModel::getTrackLocation(const QModelIndex& index) const {
Expand Down
2 changes: 1 addition & 1 deletion src/library/basesqltablemodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class BaseSqlTableModel : public QAbstractTableModel, public TrackModel {
// Functions that might be reimplemented/overridden in derived classes
///////////////////////////////////////////////////////////////////////////
// This class also has protected variables that should be used in children
// m_database, m_pTrackCollection, m_trackDAO
// m_database, m_pTrackCollection

// calls readWriteFlags() by default, reimplement this if the child calls
// should be readOnly
Expand Down
1 change: 0 additions & 1 deletion src/library/basetrackcache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ BaseTrackCache::BaseTrackCache(TrackCollection* pTrackCollection,
m_columnCache(columns),
m_bIndexBuilt(false),
m_bIsCaching(isCaching),
m_trackDAO(pTrackCollection->getTrackDAO()),
m_database(pTrackCollection->database()),
m_pQueryParser(new SearchQueryParser(pTrackCollection)) {
m_searchColumns << "artist"
Expand Down
1 change: 0 additions & 1 deletion src/library/basetrackcache.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ class BaseTrackCache : public QObject {
bool m_bIndexBuilt;
bool m_bIsCaching;
QHash<TrackId, QVector<QVariant> > m_trackInfo;
TrackDAO& m_trackDAO;
QSqlDatabase m_database;
SearchQueryParser* m_pQueryParser;
ControlProxy* m_pKeyNotationCP;
Expand Down
10 changes: 5 additions & 5 deletions src/library/browse/browsetablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,13 @@ void BrowseTableModel::setPath(const MDir& path) {
}

TrackPointer BrowseTableModel::getTrack(const QModelIndex& index) const {
QString track_location = getTrackLocation(index);
if (m_pRecordingManager->getRecordingLocation() == track_location) {
QString trackLocation = getTrackLocation(index);
if (m_pRecordingManager->getRecordingLocation() == trackLocation) {
QMessageBox::critical(
0, tr("Mixxx Library"),
tr("Could not load the following file because"
" it is in use by Mixxx or another application.")
+ "\n" +track_location);
+ "\n" + trackLocation);
return TrackPointer();
}
// NOTE(uklotzde, 2015-12-08): Accessing tracks from the browse view
Expand All @@ -170,8 +170,8 @@ TrackPointer BrowseTableModel::getTrack(const QModelIndex& index) const {
// 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);
return m_pTrackCollection->getOrAddTrack(
TrackRef::fromFileInfo(trackLocation));
}

QString BrowseTableModel::getTrackLocation(const QModelIndex& index) const {
Expand Down
4 changes: 2 additions & 2 deletions src/library/dao/autodjcratesdao.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -710,8 +710,8 @@ TrackId AutoDJCratesDAO::getRandomTrackIdFromAutoDj(int percentActive) {
// Signaled by the track DAO when a track's information is updated.
void AutoDJCratesDAO::slotTrackDirty(TrackId trackId) {
// Update our record of the number of times played, if that changed.
TrackPointer pTrack = m_pTrackCollection->getTrackDAO().getTrack(trackId);
if (pTrack == NULL) {
TrackPointer pTrack = m_pTrackCollection->getTrackById(trackId);
if (!pTrack) {
return;
}
const PlayCounter playCounter(pTrack->getPlayCounter());
Expand Down
89 changes: 28 additions & 61 deletions src/library/dao/trackdao.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1182,12 +1182,22 @@ struct ColumnPopulator {

#define ARRAYLENGTH(x) (sizeof(x) / sizeof(*x))

TrackPointer TrackDAO::getTrackFromDB(TrackId trackId) const {
TrackPointer TrackDAO::getTrackById(TrackId trackId) const {
if (!trackId.isValid()) {
return TrackPointer();
}

ScopedTimer t("TrackDAO::getTrackFromDB");
// The GlobalTrackCache is only locked while executing the following line.
auto pTrack = GlobalTrackCacheLocker().lookupTrackById(trackId);
if (pTrack) {
return pTrack;
}

// Accessing the database is a time consuming operation that should not
// be executed with a lock on the GlobalTrackCache. The GlobalTrackCache
// will be locked again after the query has been executed (see below)
// and potential race conditions will be resolved.
ScopedTimer t("TrackDAO::getTrackById");
QSqlQuery query(m_database);

ColumnPopulator columns[] = {
Expand Down Expand Up @@ -1281,7 +1291,7 @@ TrackPointer TrackDAO::getTrackFromDB(TrackId trackId) const {
const QString trackLocation(queryRecord.value(0).toString());

GlobalTrackCacheResolver cacheResolver(TrackFile(trackLocation), trackId);
TrackPointer pTrack = cacheResolver.getTrack();
pTrack = cacheResolver.getTrack();
VERIFY_OR_DEBUG_ASSERT(pTrack) {
// Just to be safe, but this should never happen!!
return pTrack;
Expand Down Expand Up @@ -1337,33 +1347,6 @@ TrackPointer TrackDAO::getTrackFromDB(TrackId trackId) const {
SoundSourceProxy(pTrack).updateTrackFromSource();
}

// Data migration: Reload track total from file tags if not initialized
// yet. The added column "tracktotal" has been initialized with the
// default value "//".
// See also: Schema revision 26 in schema.xml
if (pTrack->getTrackTotal() == "//") {
// Reload track total from file tags if the special track
// total migration value "//" indicates that the track total
// is missing and needs to be reloaded.
qDebug() << "Reloading value for 'tracktotal' once-only from file"
" to replace the default value introduced with a previous"
" schema upgrade";
mixxx::TrackMetadata trackMetadata;
if (SoundSourceProxy(pTrack).importTrackMetadata(&trackMetadata) == mixxx::MetadataSource::ImportResult::Succeeded) {
// Copy the track total from the temporary track object
pTrack->setTrackTotal(trackMetadata.getTrackInfo().getTrackTotal());
// Also set the track number if it is still empty due
// to insufficient parsing capabilities of Mixxx in
// previous versions.
if (!trackMetadata.getTrackInfo().getTrackNumber().isEmpty() && pTrack->getTrackNumber().isEmpty()) {
pTrack->setTrackNumber(trackMetadata.getTrackInfo().getTrackNumber());
}
} else {
qWarning() << "Failed to reload value for 'tracktotal' from file tags:"
<< trackLocation;
}
}

// Listen to dirty and changed signals
connect(pTrack.get(),
&Track::dirty,
Expand Down Expand Up @@ -1394,18 +1377,6 @@ TrackPointer TrackDAO::getTrackFromDB(TrackId trackId) const {
return pTrack;
}

TrackPointer TrackDAO::getTrack(TrackId trackId) const {
//qDebug() << "TrackDAO::getTrack" << QThread::currentThread() << m_database.connectionName();

// The GlobalTrackCache is only locked while executing the following line.
TrackPointer pTrack = GlobalTrackCacheLocker().lookupTrackById(trackId);
// Accessing the database is a time consuming operation that should
// not be executed with a lock on the GlobalTrackCache. The GlobalTrackCache will
// be locked again after the query has been executed and potential
// race conditions will be resolved in getTrackFromDB()
return pTrack ? pTrack : getTrackFromDB(trackId);
}

// Saves a track's info back to the database
bool TrackDAO::updateTrack(Track* pTrack) {
const TrackId trackId = pTrack->getId();
Expand Down Expand Up @@ -1747,11 +1718,11 @@ bool TrackDAO::detectMovedTracks(QList<QPair<TrackRef, TrackRef>>* pReplacedTrac
return true;
}

void TrackDAO::markTracksAsMixxxDeleted(const QString& dir) {
void TrackDAO::hideAllTracks(const QDir& rootDir) {
// Capture entries that start with the directory prefix dir.
// dir needs to end in a slash otherwise we might match other
// directories.
QString likeClause = SqlLikeWildcardEscaper::apply(dir + "/", kSqlLikeMatchAll) + kSqlLikeMatchAll;
QString likeClause = SqlLikeWildcardEscaper::apply(rootDir.absolutePath() + "/", kSqlLikeMatchAll) + kSqlLikeMatchAll;

QSqlQuery query(m_database);
query.prepare(QString("SELECT library.id FROM library INNER JOIN track_locations "
Expand All @@ -1760,7 +1731,7 @@ void TrackDAO::markTracksAsMixxxDeleted(const QString& dir) {
.arg(SqlStringFormatter::format(m_database, likeClause), kSqlLikeMatchAll));

if (!query.exec()) {
LOG_FAILED_QUERY(query) << "could not get tracks within directory:" << dir;
LOG_FAILED_QUERY(query) << "could not get tracks within directory:" << rootDir;
}

QStringList trackIds;
Expand Down Expand Up @@ -1971,18 +1942,17 @@ void TrackDAO::detectCoverArtForTracksWithoutCover(volatile const bool* pCancel,
}
}

TrackPointer TrackDAO::getOrAddTrack(const QString& trackLocation,
bool processCoverArt,
bool* pAlreadyInLibrary) {
TrackPointer TrackDAO::getOrAddTrackByLocation(
const QString& trackLocation,
bool* pAlreadyInLibrary) {
const TrackId trackId = getTrackId(trackLocation);
const bool trackAlreadyInLibrary = trackId.isValid();
if (pAlreadyInLibrary) {
*pAlreadyInLibrary = trackAlreadyInLibrary;
*pAlreadyInLibrary = trackId.isValid();
}

TrackPointer pTrack;
if (trackAlreadyInLibrary) {
pTrack = getTrack(trackId);
if (trackId.isValid()) {
pTrack = getTrackById(trackId);
if (!pTrack) {
qWarning() << "Failed to load track"
<< trackLocation;
Expand All @@ -1996,15 +1966,12 @@ TrackPointer TrackDAO::getOrAddTrack(const QString& trackLocation,
<< trackLocation;
return pTrack;
}
DEBUG_ASSERT(pTrack);
// If the track wasn't in the library already then it has not yet been
// checked for cover art. If processCoverArt is true then we should request
// cover processing via CoverArtCache asynchronously.
if (processCoverArt) {
CoverArtCache* pCache = CoverArtCache::instance();
if (pCache != nullptr) {
pCache->requestGuessCover(pTrack);
}
// If the track wasn't in the library already then it has not yet
// been checked for cover art.
CoverArtCache* pCache = CoverArtCache::instance();
if (pCache) {
// Process cover art asynchronously
pCache->requestGuessCover(pTrack);
}
}

Expand Down
Loading