Fix m3u list parsing with /r/n end of line

src/library/parser.h

@@ -41,7 +41,7 @@ class Parser : public QObject {
     Note for developers:
     This function should return an empty PtrList
      or 0 in order for the trackimporter to function**/
-    virtual QList<QString> parse(const QString&) = 0;
+    virtual QList<QString> parse(const QString& sFilename, bool keepMissingFiles) = 0;
 
   protected:
     // Pointer to the parsed Filelocations

src/library/parsercsv.cpp

@@ -27,7 +27,8 @@ ParserCsv::ParserCsv() : Parser() {
 ParserCsv::~ParserCsv() {
 }
 
-QList<QString> ParserCsv::parse(const QString& sFilename) {
+QList<QString> ParserCsv::parse(const QString& sFilename, bool keepMissingFiles) {
+    Q_UNUSED(keepMissingFiles);
     QFile file(sFilename);
     QString basepath = sFilename.section('/', 0, -2);
 

src/library/parsercsv.h

@@ -25,16 +25,13 @@ class ParserCsv : public Parser
     Q_OBJECT
 public:
     ParserCsv();
-    virtual ~ParserCsv();
-    /**Overwriting function parse in class Parser**/
-    QList<QString> parse(const QString&);
+    ~ParserCsv() override;
+    QList<QString> parse(const QString&, bool keepMissingFiles) override;
     // Playlist Export
     static bool writeCSVFile(const QString &file, BaseSqlTableModel* pPlaylistTableModel, bool useRelativePath);
     // Readable Text export
     static bool writeReadableTextFile(const QString &file, BaseSqlTableModel* pPlaylistTableModel,  bool writeTimestamp);
 private:
-    /**Reads a line from the file and returns filepath if a valid file**/
-    QList<QList<QString> > tokenize(const QByteArray& str, char delimiter);
-
-
+  // Reads a line from the file and returns filepath if a valid file
+  QList<QList<QString> > tokenize(const QByteArray& str, char delimiter);
 };

src/library/parserm3u.cpp

@@ -15,6 +15,7 @@
 
 #include <QDir>
 #include <QMessageBox>
+#include <QRegularExpression>
 #include <QTextCodec>
 #include <QUrl>
 #include <QtDebug>
@@ -24,7 +25,11 @@
 namespace {
 // according to http://en.wikipedia.org/wiki/M3U the default encoding of m3u is Windows-1252
 // see also http://tools.ietf.org/html/draft-pantos-http-live-streaming-07
-const char* kStandardM3uTextEncoding = "Windows-1250";
+const char kStandardM3uTextEncoding[] = "Windows-1250";
+const char kM3uHeader[] = "#EXTM3U";
+const char kM3uCommentPrefix[] = "#";
+// Note: The RegEx pattern is compiled, when first used the first time
+const auto kUniveralEndOfLineRegEx = QRegularExpression(QStringLiteral("\r\n|\r|\n"));
 } // anonymous namespace
 
 /**
@@ -51,7 +56,7 @@ ParserM3u::~ParserM3u()
 
 }
 
-QList<QString> ParserM3u::parse(const QString& filename) {
+QList<QString> ParserM3u::parse(const QString& filename, bool keepMissingFiles) {
     QList<QString> paths;
 
     QFile file(filename);
@@ -62,21 +67,7 @@ QList<QString> ParserM3u::parse(const QString& filename) {
         return paths;
     }
 
-    // Unfortunately QTextStream does not handle <CR> (=\r or asci value 13) line breaks.
-    // This is important on OS X where iTunes, e.g., exports M3U playlists using <CR>
-    // rather that <LF>.
-    //
-    // Using QFile::readAll() we obtain the complete content of the playlist as a ByteArray.
-    // We replace any '\r' with '\n' if applicaple
-    // This ensures that playlists from iTunes on OS X can be parsed
     QByteArray byteArray = file.readAll();
-    //detect encoding
-    bool isCRLF_encoded = byteArray.contains("\r\n");
-    bool isCR_encoded = byteArray.contains("\r");
-    if (isCR_encoded && !isCRLF_encoded) {
-        byteArray.replace('\r', '\n');
-    }
-
     QString fileContents;
     if (isUtf8(byteArray.constData())) {
         fileContents = QString::fromUtf8(byteArray);
@@ -86,11 +77,19 @@ QList<QString> ParserM3u::parse(const QString& filename) {
                                ->toUnicode(byteArray);
     }
 
+    if (!fileContents.startsWith(kM3uHeader)) {
+        qWarning() << "M3U playlist file" << filename << "does not start with" << kM3uHeader;
+    }
+
     QFileInfo fileInfo(filename);
-    const QStringList fileLines = fileContents.split('\n');
+    const QStringList fileLines = fileContents.split(kUniveralEndOfLineRegEx);
     for (const QString& line : fileLines) {
+        if (line.startsWith(kM3uCommentPrefix)) {
+            // Skip lines with comments
+            continue;
+        }
         auto trackFile = playlistEntryToFileInfo(line, fileInfo.canonicalPath());
-        if (trackFile.checkFileExists()) {
+        if (keepMissingFiles || trackFile.checkFileExists()) {
             paths.append(trackFile.location());
         } else {
             qInfo() << "File" << trackFile.location() << "from M3U playlist"

src/library/parserm3u.h

@@ -24,9 +24,8 @@ class ParserM3u : public Parser
     Q_OBJECT
 public:
     ParserM3u();
-    ~ParserM3u();
-    /**Overwriting function parse in class Parser**/
-    QList<QString> parse(const QString&);
+    ~ParserM3u() override;
+    QList<QString> parse(const QString&, bool keepMissingFiles) override;
     //Playlist Export
     static bool writeM3UFile(const QString &file_str, const QList<QString> &items, bool useRelativePath, bool useUtf8);
     static bool writeM3UFile(const QString &file, const QList<QString> &items, bool useRelativePath);

src/library/parserpls.cpp

@@ -32,7 +32,8 @@ ParserPls::ParserPls() : Parser() {
 ParserPls::~ParserPls() {
 }
 
-QList<QString> ParserPls::parse(const QString& sFilename) {
+QList<QString> ParserPls::parse(const QString& sFilename, bool keepMissingFiles) {
+    Q_UNUSED(keepMissingFiles);
     //long numEntries =0;
     QFile file(sFilename);
     const auto basePath = sFilename.section('/', 0, -2);

src/library/parserpls.h

@@ -23,7 +23,7 @@ class ParserPls : public Parser {
     ParserPls();
     virtual ~ParserPls();
     /**Can be called to parse a pls file**/
-    QList<QString> parse(const QString&);
+    QList<QString> parse(const QString& sFilename, bool keepMissingFiles);
     //Playlist Export
     static bool writePLSFile(const QString &file, const QList<QString> &items, bool useRelativePath);
 

src/library/trackset/baseplaylistfeature.cpp

@@ -454,7 +454,7 @@ void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlist_file) {
     }
 
     if (playlist_parser) {
-        QStringList entries = playlist_parser->parse(playlist_file);
+        QStringList entries = playlist_parser->parse(playlist_file, false);
 
         // Iterate over the List that holds URLs of playlist entries
         m_pPlaylistTableModel->addTracks(QModelIndex(), entries);

src/library/trackset/crate/cratefeature.cpp

@@ -610,11 +610,11 @@ void CrateFeature::slotImportPlaylistFile(const QString& playlist_file) {
     if (playlist_file.endsWith(".m3u", Qt::CaseInsensitive) ||
             playlist_file.endsWith(".m3u8", Qt::CaseInsensitive)) {
         // .m3u8 is Utf8 representation of an m3u playlist
-        entries = ParserM3u().parse(playlist_file);
+        entries = ParserM3u().parse(playlist_file, false);
     } else if (playlist_file.endsWith(".pls", Qt::CaseInsensitive)) {
-        entries = ParserPls().parse(playlist_file);
+        entries = ParserPls().parse(playlist_file, false);
     } else if (playlist_file.endsWith(".csv", Qt::CaseInsensitive)) {
-        entries = ParserCsv().parse(playlist_file);
+        entries = ParserCsv().parse(playlist_file, false);
     } else {
         return;
     }

src/test/playlisttest.cpp

@@ -1,14 +1,19 @@
 #include <gtest/gtest.h>
 
+#include <QDataStream>
 #include <QDebug>
+#include <QTemporaryFile>
 #include <QUrl>
 #include <QtGlobal>
 
 #include "library/parser.h"
+#include "library/parserm3u.h"
 
 class DummyParser : public Parser {
   public:
-    QList<QString> parse(const QString&) override {
+    QList<QString> parse(const QString& sFilename, bool keepMissingFiles) override {
+        Q_UNUSED(sFilename);
+        Q_UNUSED(keepMissingFiles);
         return QList<QString>();
     }
 
@@ -56,3 +61,25 @@ TEST_F(PlaylistTest, Relative) {
     EXPECT_EQ(QString("base/folder/../../bar.mp3"),
             parser.playlistEntryToFilePath("../../bar.mp3", "base/folder"));
 }
+
+TEST_F(PlaylistTest, m3uEndOfLine) {
+    QTemporaryFile m3uFile;
+    ASSERT_TRUE(m3uFile.open());
+    m3uFile.write("crlf.mp3\r\n");
+    m3uFile.write("cr.mp3\r");
+    m3uFile.write("lf.mp3\n");
+    // Check for Windows-1250 Euro Sign
+    m3uFile.write("EuroSign\x80.mp3\n");
+    m3uFile.write("end.mp3");
+    m3uFile.close();
+
+    QList<QString> entries = ParserM3u().parse(m3uFile.fileName(), true);
+    EXPECT_EQ(entries.size(), 5);
+    if (entries.size() == 5) {
+        EXPECT_TRUE(entries[0].endsWith(QStringLiteral("crlf.mp3")));
+        EXPECT_TRUE(entries[1].endsWith(QStringLiteral("cr.mp3")));
+        EXPECT_TRUE(entries[2].endsWith(QStringLiteral("lf.mp3")));
+        EXPECT_TRUE(entries[3].endsWith(QStringLiteral("EuroSign\u20AC.mp3")));
+        EXPECT_TRUE(entries[4].endsWith(QStringLiteral("end.mp3")));
+    }
+}

src/util/dnd.cpp

@@ -117,13 +117,13 @@ QList<mixxx::FileInfo> DragAndDropHelper::supportedTracksFromUrls(
 
         if (acceptPlaylists && (file.endsWith(".m3u") || file.endsWith(".m3u8"))) {
             QScopedPointer<ParserM3u> playlist_parser(new ParserM3u());
-            QList<QString> track_list = playlist_parser->parse(file);
+            QList<QString> track_list = playlist_parser->parse(file, false);
             foreach (const QString& playlistFile, track_list) {
                 addFileToList(mixxx::FileInfo(playlistFile), &fileInfos);
             }
         } else if (acceptPlaylists && url.toString().endsWith(".pls")) {
             QScopedPointer<ParserPls> playlist_parser(new ParserPls());
-            QList<QString> track_list = playlist_parser->parse(file);
+            QList<QString> track_list = playlist_parser->parse(file, false);
             foreach (const QString& playlistFile, track_list) {
                 addFileToList(mixxx::FileInfo(playlistFile), &fileInfos);
             }