Cover Art Copy Worker

CMakeLists.txt

@@ -733,6 +733,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
   src/library/dlgtrackinfo.cpp
   src/library/dlgtrackinfo.ui
   src/library/dlgtrackmetadataexport.cpp
+  src/library/export/coverartcopyworker.cpp
   src/library/export/dlgtrackexport.ui
   src/library/export/trackexportdlg.cpp
   src/library/export/trackexportwizard.cpp
@@ -950,6 +951,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
   src/util/duration.cpp
   src/util/experiment.cpp
   src/util/file.cpp
+  src/util/imagefiledata.cpp
   src/util/fileaccess.cpp
   src/util/fileinfo.cpp
   src/util/filename.cpp

src/library/export/coverartcopyworker.cpp

@@ -0,0 +1,107 @@
+#include "library/export/coverartcopyworker.h"
+
+#include <QDebug>
+#include <QFileInfo>
+#include <QMessageBox>
+
+#include "util/fileaccess.h"
+#include "util/imagefiledata.h"
+#include "util/safelywritablefile.h"
+
+void CoverArtCopyWorker::run() {
+    auto coverArtFileInfo = mixxx::FileInfo(m_selectedCoverArtFilePath);
+    auto selectedCoverFileAccess = mixxx::FileAccess(coverArtFileInfo);
+    if (!selectedCoverFileAccess.isReadable()) {
+        if (Sandbox::askForAccess(&coverArtFileInfo)) {
+            selectedCoverFileAccess = mixxx::FileAccess(coverArtFileInfo);
+        } else {
+            return;
+        }
+    }
+
+    ImageFileData imageFileData = ImageFileData::fromFilePath(m_selectedCoverArtFilePath);
+    if (imageFileData.isNull()) {
+        // TODO(rryan): feedback
+        return;
+    }
+
+    m_coverInfo.type = CoverInfo::FILE;
+    m_coverInfo.source = CoverInfo::USER_SELECTED;
+    m_coverInfo.coverLocation = m_selectedCoverArtFilePath;
+    m_coverInfo.setImage(imageFileData);
+
+    if (QFileInfo(m_oldCoverArtFilePath).canonicalPath() ==
+            QFileInfo(m_selectedCoverArtFilePath).canonicalPath()) {
+        qDebug() << "Track and selected cover art are in the same path:"
+                 << QFileInfo(m_selectedCoverArtFilePath).canonicalPath()
+                 << "Cover art updated without copying";
+        emit coverArtUpdated(m_coverInfo);
+        return;
+    }
+
+    copyFile(m_selectedCoverArtFilePath, m_oldCoverArtFilePath);
+}
+
+void CoverArtCopyWorker::copyFile(
+        const QString& m_selectedCoverArtFilePath,
+        const QString& m_oldCoverArtFilePath) {
+    QFileInfo coverArtPathFileInfo(m_oldCoverArtFilePath);
+    ImageFileData imageFileData = ImageFileData::fromFilePath(m_selectedCoverArtFilePath);
+
+    if (coverArtPathFileInfo.exists()) {
+        switch (makeOverwriteRequest(m_oldCoverArtFilePath)) {
+        case OverwriteAnswer::Cancel:
+            return;
+        case OverwriteAnswer::Overwrite:
+            break;
+        }
+
+        mixxx::SafelyWritableFile safelyWritableFile(m_oldCoverArtFilePath,
+                mixxx::SafelyWritableFile::SafetyMode::Replace);
+
+        DEBUG_ASSERT(!safelyWritableFile.fileName().isEmpty());
+        if (imageFileData.saveFile(safelyWritableFile.fileName())) {
+            qDebug() << "Cover art"
+                     << m_oldCoverArtFilePath
+                     << "copied successfully";
+            safelyWritableFile.commit();
+        } else {
+            qWarning() << "Error while copying the cover art to" << safelyWritableFile.fileName();
+        }
+    } else {
+        if (imageFileData.saveFile(m_oldCoverArtFilePath)) {
+            qDebug() << "Cover art"
+                     << m_oldCoverArtFilePath
+                     << "copied successfully";
+        } else {
+            qWarning() << "Error while copying the cover art to" << m_oldCoverArtFilePath;
+        }
+    }
+    emit coverArtUpdated(m_coverInfo);
+    quit();
+}
+
+CoverArtCopyWorker::OverwriteAnswer CoverArtCopyWorker::makeOverwriteRequest(
+        const QString& filename) {
+    QScopedPointer<std::promise<OverwriteAnswer>> mode_promise(
+            new std::promise<OverwriteAnswer>());
+    std::future<OverwriteAnswer> mode_future = mode_promise->get_future();
+    emit askOverwrite(filename, mode_promise.data());
+
+    mode_future.wait();
+
+    if (!mode_future.valid()) {
+        qWarning() << "CoverArtCopyWorker::askOverwrite invalid answer from future";
+        return OverwriteAnswer::Cancel;
+    }
+
+    OverwriteAnswer answer = mode_future.get();
+    switch (answer) {
+    case OverwriteAnswer::Cancel:
+        qDebug() << "Cover art overwrite declined";
+        break;
+    default:;
+    }
+
+    return answer;
+}

src/library/export/coverartcopyworker.h

@@ -0,0 +1,50 @@
+#pragma once
+
+#include <QObject>
+#include <QScopedPointer>
+#include <QString>
+#include <QThread>
+#include <future>
+
+#include "library/coverart.h"
+#include "library/coverartutils.h"
+#include "util/fileinfo.h"
+#include "util/imagefiledata.h"
+
+// This is a QThread class for copying the cover art.
+
+class CoverArtCopyWorker : public QThread {
+    Q_OBJECT
+  public:
+    enum class OverwriteAnswer {
+        Overwrite,
+        Cancel = -1,
+    };
+
+    CoverArtCopyWorker(const QString& selectedCoverArtFilePath,
+            const QString& oldCoverArtFilePath)
+            : m_selectedCoverArtFilePath(selectedCoverArtFilePath),
+              m_oldCoverArtFilePath(oldCoverArtFilePath) {
+        qRegisterMetaType<CoverInfoRelative>("CoverInfoRelative");
+    }
+
+    ~CoverArtCopyWorker() override = default;
+
+  signals:
+    void askOverwrite(const QString& filename,
+            std::promise<CoverArtCopyWorker::OverwriteAnswer>* promise);
+    void coverArtUpdated(const CoverInfoRelative& coverInfo);
+
+  protected:
+    void run() override;
+
+  private:
+    void copyFile(const QString& m_selectedCoverArtFilePath,
+            const QString& m_oldCoverArtFilePath);
+
+    OverwriteAnswer makeOverwriteRequest(const QString& filename);
+
+    CoverInfoRelative m_coverInfo;
+    const QString m_selectedCoverArtFilePath;
+    const QString m_oldCoverArtFilePath;
+};

src/sources/metadatasourcetaglib.cpp

@@ -18,9 +18,6 @@ namespace {
 
 Logger kLogger("MetadataSourceTagLib");
 
-// TODO(uklotzde): Add a configurable option in the user settings
-const bool kExportTrackMetadataIntoTemporaryFile = true;
-
 // Workaround for missing functionality in TagLib 1.11.x that
 // doesn't support to read text chunks from AIFF files.
 // See also:
@@ -613,7 +610,8 @@ MetadataSourceTagLib::exportTrackMetadata(
                     << "into file" << m_fileName
                     << "with type" << m_fileType;
 
-    SafelyWritableFile safelyWritableFile(m_fileName, kExportTrackMetadataIntoTemporaryFile);
+    SafelyWritableFile safelyWritableFile(m_fileName,
+            SafelyWritableFile::SafetyMode::Edit);
     if (!safelyWritableFile.isReady()) {
         kLogger.warning()
                 << "Unable to export track metadata into file"

src/util/safelywritablefile.cpp

@@ -19,6 +19,14 @@ const Logger kLogger("SafelyWritableFile");
 // Appended to the original file name of the temporary file used for writing
 const QString kSafelyWritableTempFileSuffix = QStringLiteral("_temp");
 
+// SafelyWritableFile is also used to write cover arts.
+// Cover art worker is suffix sensetive.
+// In order to have a successful writing we need a correct suffix.
+// So instead of suffix, we need a temp file prefix.
+// There is a bug related with the image format reported
+// See: https://bugreports.qt.io/browse/QTBUG-89022
+const QString kSafelyWritableTempFilePrefix = QStringLiteral("temp_");
+
 // Appended to the original file name for renaming and before deleting this
 // file. Should not be longer than kSafelyWritableTempFileSuffix to avoid
 // potential failures caused by exceeded path length.
@@ -31,7 +39,8 @@ const int kWindowsSharingViolationSleepBeforeNextRetryMillis = 100;
 
 } // namespace
 
-SafelyWritableFile::SafelyWritableFile(QString origFileName, bool useTemporaryFile) {
+SafelyWritableFile::SafelyWritableFile(QString origFileName,
+        SafelyWritableFile::SafetyMode safetyMode) {
     // Both file names remain uninitialized until all prerequisite operations
     // in the constructor have been completed successfully. Otherwise failure
     // to create the temporary file will not be handled correctly!
@@ -46,7 +55,8 @@ SafelyWritableFile::SafelyWritableFile(QString origFileName, bool useTemporaryFi
         // Abort constructor
         return;
     }
-    if (useTemporaryFile) {
+    switch (safetyMode) {
+    case SafetyMode::Edit: {
         QString tempFileName = origFileName + kSafelyWritableTempFileSuffix;
         QFile origFile(origFileName);
         if (!origFile.copy(tempFileName)) {
@@ -80,10 +90,24 @@ SafelyWritableFile::SafelyWritableFile(QString origFileName, bool useTemporaryFi
         // Successfully cloned original into temporary file for writing - finish initialization
         m_origFileName = std::move(origFileName);
         m_tempFileName = std::move(tempFileName);
-    } else {
-        // Directly write into original file - finish initialization
+        break;
+    }
+    case SafetyMode::Replace: {
+        QString origFilePath = QFileInfo(origFileName).canonicalPath();
+        QString origFileCompleteBasename = QFileInfo(origFileName).completeBaseName();
+        QString origFileSuffix = QFileInfo(origFileName).suffix();
+        QString tempFileCompleteBasename = kSafelyWritableTempFilePrefix +
+                origFileCompleteBasename + '.' + origFileSuffix;
+        QString tempFileName = origFilePath + '/' + tempFileCompleteBasename;
         m_origFileName = std::move(origFileName);
+        m_tempFileName = std::move(tempFileName);
+        break;
+    }
+    case SafetyMode::Direct: {
+        // Directly write into original file - finish initialization
         DEBUG_ASSERT(m_tempFileName.isNull());
+        m_origFileName = std::move(origFileName);
+    }
     }
 }
 

src/util/safelywritablefile.h

@@ -22,7 +22,16 @@ namespace mixxx {
 
 class SafelyWritableFile final {
   public:
-    SafelyWritableFile(QString origFileName, bool useTemporaryFile);
+    enum class SafetyMode {
+        // Bypass
+        Direct,
+        // Create a temp file with suffix
+        Edit,
+        // Use temp file name with prefix
+        Replace,
+    };
+    SafelyWritableFile(QString origFileName,
+            SafelyWritableFile::SafetyMode safetyMode);
     ~SafelyWritableFile();
 
     const QString& fileName() const;
@@ -34,7 +43,6 @@ class SafelyWritableFile final {
     void cancel();
 
   private:
-    bool m_UseTemporaryFile;
     QString m_origFileName;
     QString m_tempFileName;
 };