S3M: unit tests and bug fixes (NUL char. is not optional in S3M strings)

diff --git a/taglib/s3m/s3mfile.cpp b/taglib/s3m/s3mfile.cpp
index c786d0fc1623b5cd22d35874782ab506b6b5be70..a739ea524cbd7c9be9f046ff9c60b0ac09f617d2 100644 (file)
--- a/taglib/s3m/s3mfile.cpp
+++ b/taglib/s3m/s3mfile.cpp
@@ -82,7 +82,9 @@ bool S3M::File::save()
   // note: if title starts with "Extended Module: "
   // the file would look like an .xm file
   seek(0);
-  writeString(d->tag.title(), 28);
+  writeString(d->tag.title(), 27);
+  // string terminating NUL is not optional:
+  writeByte(0);
 
   seek(32);
 
@@ -120,9 +122,11 @@ bool S3M::File::save()
     seek(((long)instrumentOffset << 4) + 48);
 
     if(i < lines.size())
-      writeString(lines[i], 28);
+      writeString(lines[i], 27);
     else
-      writeString(String::null, 28);
+      writeString(String::null, 27);
+    // string terminating NUL is not optional:
+    writeByte(0);
   }
   return true;
 }
@@ -163,6 +167,8 @@ void S3M::File::read(bool)
   // I've seen players who call the next two bytes
   // "ultra click" and "use panning values" (if == 0xFC).
   // I don't see them in any spec, though.
+  // Hm, but there is "UltraClick-removal" and some other
+  // variables in ScreamTracker IIIs GUI.
 
   seek(12, Current);
 

diff --git a/taglib/s3m/s3mproperties.cpp b/taglib/s3m/s3mproperties.cpp
index 30264f44358d8402bde4cc58c666dbae5e713c12..d7f40c8f570e1b701f0478a10957bdb3950211a8 100644 (file)
--- a/taglib/s3m/s3mproperties.cpp
+++ b/taglib/s3m/s3mproperties.cpp
@@ -30,7 +30,7 @@ public:
   PropertiesPrivate() :
     tableLength(0),
     channels(0),
-    stereo(0),
+    stereo(false),
     sampleCount(0),
     patternCount(0),
     flags(0),

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 28f621fae1b44dd2ba83c74eca1e843f4fa14be7..5cb828198e1c02f19e8fba0bb884dbc4bfabffbb 100644 (file)
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -52,6 +52,7 @@ SET(test_runner_SRCS
   test_wav.cpp
   test_wavpack.cpp
   test_mod.cpp
+  test_s3m.cpp
   test_xm.cpp
 )
 IF(WITH_MP4)

diff --git a/tests/data/test.s3m b/tests/data/test.s3m
index ee3b6d7afe9ab127b48ea98d1e5cb6489898dad8..668250bb7876f31762bb3c2d4a2cafb93cff2f9c 100644 (file)
Binary files a/tests/data/test.s3m and b/tests/data/test.s3m differ

diff --git a/tests/test_s3m.cpp b/tests/test_s3m.cpp
new file mode 100644 (file)
index 0000000..0fa0cb8
--- /dev/null
+++ b/tests/test_s3m.cpp
@@ -0,0 +1,128 @@
+/***************************************************************************
+    copyright           : (C) 2011 by Mathias Panzenböck
+    email               : grosser.meister.morti@gmx.net
+ ***************************************************************************/
+
+/***************************************************************************
+ *   This library is free software; you can redistribute it and/or modify  *
+ *   it  under the terms of the GNU Lesser General Public License version  *
+ *   2.1 as published by the Free Software Foundation.                     *
+ *                                                                         *
+ *   This library is distributed in the hope that it will be useful, but   *
+ *   WITHOUT ANY WARRANTY; without even the implied warranty of            *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU     *
+ *   Lesser General Public License for more details.                       *
+ *                                                                         *
+ *   You should have received a copy of the GNU Lesser General Public      *
+ *   License along with this library; if not, write to the Free Software   *
+ *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,            *
+ *   MA  02110-1301  USA                                                   *
+ ***************************************************************************/
+
+#include <cppunit/extensions/HelperMacros.h>
+#include <s3mfile.h>
+#include "utils.h"
+
+using namespace std;
+using namespace TagLib;
+
+static const String titleBefore("test song name");
+static const String titleAfter("changed title");
+
+static const String commentBefore(
+  "This is an instrument name.\n"
+  "Module file formats\n"
+  "abuse instrument names\n"
+  "as multiline comments.\n"
+  " ");
+
+static const String newComment(
+  "This is an instrument name!\n"
+  "Module file formats\n"
+  "abuse instrument names\n"
+  "as multiline comments.\n"
+  "-----------------------------------\n"
+  "This line will be dropped and the previous is truncated.");
+
+static const String commentAfter(
+  "This is an instrument name!\n"
+  "Module file formats\n"
+  "abuse instrument names\n"
+  "as multiline comments.\n"
+  "---------------------------");
+
+class TestS3M : public CppUnit::TestFixture
+{
+  CPPUNIT_TEST_SUITE(TestS3M);
+  CPPUNIT_TEST(testReadTags);
+  CPPUNIT_TEST(testWriteTags);
+  CPPUNIT_TEST_SUITE_END();
+
+public:
+  void testReadTags()
+  {
+    testRead(TEST_FILE_PATH_C("test.s3m"), titleBefore, commentBefore);
+  }
+
+  void testWriteTags()
+  {
+    ScopedFileCopy copy("test", ".s3m");
+    {
+      S3M::File file(copy.fileName().c_str());
+      CPPUNIT_ASSERT(file.tag() != 0);
+      file.tag()->setTitle(titleAfter);
+      file.tag()->setComment(newComment);
+      file.tag()->setTrackerName("won't be saved");
+      CPPUNIT_ASSERT(file.save());
+    }
+    String cmd("cp ");
+    cmd += copy.fileName();
+    cmd += " /tmp/copy.s3m";
+    CPPUNIT_ASSERT_EQUAL(0, system(cmd.data(String::Latin1).data()));
+
+    testRead(copy.fileName().c_str(), titleAfter, commentAfter);
+    CPPUNIT_ASSERT(fileEqual(
+      copy.fileName(),
+      TEST_FILE_PATH_C("changed.s3m")));
+  }
+
+private:
+  void testRead(FileName fileName, const String &title, const String &comment)
+  {
+    S3M::File file(fileName);
+
+    CPPUNIT_ASSERT(file.isValid());
+
+    S3M::Properties *p = file.audioProperties();
+    Mod::Tag *t = file.tag();
+    
+    CPPUNIT_ASSERT(0 != p);
+    CPPUNIT_ASSERT(0 != t);
+
+    CPPUNIT_ASSERT_EQUAL( 0, p->length());
+    CPPUNIT_ASSERT_EQUAL( 0, p->bitrate());
+    CPPUNIT_ASSERT_EQUAL( 0, p->sampleRate());
+    CPPUNIT_ASSERT_EQUAL(16, p->channels());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  0, p->tableLength());
+    CPPUNIT_ASSERT_EQUAL(false, p->stereo());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)   5, p->sampleCount());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)   1, p->patternCount());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)   0, p->flags());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)4896, p->trackerVersion());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)   2, p->fileFormatVersion());
+    CPPUNIT_ASSERT_EQUAL((TagLib::uchar) 64, p->globalVolume());
+    CPPUNIT_ASSERT_EQUAL((TagLib::uchar) 48, p->masterVolume());
+    CPPUNIT_ASSERT_EQUAL((TagLib::uchar)125, p->tempo());
+    CPPUNIT_ASSERT_EQUAL((TagLib::uchar)  6, p->bpmSpeed());
+    CPPUNIT_ASSERT_EQUAL(title, t->title());
+    CPPUNIT_ASSERT_EQUAL(String::null, t->artist());
+    CPPUNIT_ASSERT_EQUAL(String::null, t->album());
+    CPPUNIT_ASSERT_EQUAL(comment, t->comment());
+    CPPUNIT_ASSERT_EQUAL(String::null, t->genre());
+    CPPUNIT_ASSERT_EQUAL(0U, t->year());
+    CPPUNIT_ASSERT_EQUAL(0U, t->track());
+    CPPUNIT_ASSERT_EQUAL(String("ScreamTracker III"), t->trackerName());
+  }
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestS3M);