added unit test for one flavour of .mod files

taglib/it/itfile.cpp

@@ -27,11 +27,6 @@
 using namespace TagLib;
 using namespace IT;
 
-// Just copied this array from some example code.
-// I think this might be unneccesarry and only needed if
-// you convert IT to XM to keep your mod player more simple.
-static const uchar AUTOVIB_IT_TO_XM[] = {0, 3, 1, 4, 2, 0, 0, 0};
-
 class IT::File::FilePrivate
 {
 public:
@@ -170,20 +165,7 @@ void IT::File::read(bool)
     READ_BYTE_AS(vibratoDepth);
     READ_BYTE_AS(vibratoSweep);
     READ_BYTE_AS(vibratoType);
-
-    if(c4speed == 0)
-    {
-      c4speed = 8363;
-    }
-    else if(c4speed < 256)
-    {
-      c4speed = 256;
-    }
-      
-    vibratoDepth = vibratoDepth & 0x7F;
-    vibratoSweep = (vibratoSweep + 3) >> 2;
-    vibratoType  = AUTOVIB_IT_TO_XM[vibratoType & 0x07];
-
+    
     comment.append(sampleName);
   }
 

taglib/mod/modfile.cpp

@@ -78,7 +78,9 @@ bool Mod::File::save()
     return false;
   }
   seek(0);
-  writeString(d->tag.title(), 20, ' ');
+  // Even though the spec says the title is padded with space
+  // common tracker padd with '\0', so why shouldn't I?
+  writeString(d->tag.title(), 20);
   // TODO: write comment as instrument names
   return true;
 }
@@ -130,6 +132,8 @@ void Mod::File::read(bool)
   }
   else
   {
+    // Not sure if this is correct. I'd need a file
+    // created with NoiseTracker to check this.
     d->tag.setTrackerName("NoiseTracker"); // probably
     channels    =  4;
     instruments = 15;

tests/CMakeLists.txt

@@ -19,6 +19,10 @@ INCLUDE_DIRECTORIES(
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/ogg/flac
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/flac
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/wavpack
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mod
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/s3m
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/it
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/xm
 )
 
 SET(test_runner_SRCS
@@ -47,6 +51,7 @@ SET(test_runner_SRCS
   test_apetag.cpp
   test_wav.cpp
   test_wavpack.cpp
+  test_mod.cpp
 )
 IF(WITH_MP4)
    SET(test_runner_SRCS ${test_runner_SRCS}
@@ -67,5 +72,4 @@ ADD_CUSTOM_TARGET(check
     ./test_runner
     DEPENDS test_runner
 )
-
 endif(BUILD_TESTS)

tests/test_mod.cpp

@@ -0,0 +1,126 @@
+#include <cppunit/extensions/HelperMacros.h>
+#include <string>
+#include <stdio.h>
+#include <string.h>
+#include <modfile.h>
+#include <stdlib.h>
+#include "utils.h"
+
+using namespace std;
+using namespace TagLib;
+
+class TestMod : public CppUnit::TestFixture
+{
+	CPPUNIT_TEST_SUITE(TestMod);
+	CPPUNIT_TEST(testRead);
+	CPPUNIT_TEST(testChangeTitle);
+	CPPUNIT_TEST_SUITE_END();
+
+public:
+	void testRead()
+	{
+		testRead(TEST_FILE_PATH_C("test.mod"), "title of song");
+	}
+
+	void testChangeTitle()
+	{
+		ScopedFileCopy copy("test", ".mod");
+		{
+			Mod::File file(copy.fileName().c_str());
+			CPPUNIT_ASSERT(file.tag() != 0);
+			file.tag()->setTitle("changed title");
+			CPPUNIT_ASSERT(file.save());
+		}
+		{
+			testRead(copy.fileName().c_str(), "changed title");
+		}
+		{
+			assertFileEqual(
+				copy.fileName().c_str(),
+				TEST_FILE_PATH_C("changed_title.mod"));
+		}
+	}
+
+private:
+	class Closer
+	{
+	public:
+		Closer(FILE *stream) : m_stream(stream)
+		{
+		}
+
+		~Closer()
+		{
+			if (m_stream)
+			{
+				fclose(m_stream);
+			}
+		}
+	private:
+		FILE *m_stream;
+	};
+
+	void assertFileEqual(const char *file1, const char *file2)
+	{
+		char buf1[BUFSIZ];
+		char buf2[BUFSIZ];
+
+		FILE *stream1 = fopen(file1, "rb");
+		FILE *stream2 = fopen(file2, "rb");
+
+		Closer closer1(stream1);
+		Closer closer2(stream2);
+
+		CPPUNIT_ASSERT(stream1 != 0);
+		CPPUNIT_ASSERT(stream2 != 0);
+
+		for (;;)
+		{
+			size_t n1 = fread(buf1, 1, BUFSIZ, stream1);
+			size_t n2 = fread(buf2, 1, BUFSIZ, stream2);
+
+			CPPUNIT_ASSERT_EQUAL(n1, n2);
+
+			if (n1 == 0) break;
+
+			CPPUNIT_ASSERT(memcmp(buf1, buf2, n1) == 0);
+		}
+	}
+
+	void testRead(FileName fileName, const String &title)
+	{
+		Mod::File file(fileName);
+
+		CPPUNIT_ASSERT(file.isValid());
+
+		Mod::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(8, p->channels());
+		CPPUNIT_ASSERT_EQUAL(31U, p->instrumentCount());
+		CPPUNIT_ASSERT_EQUAL(1U, p->patternCount());
+		CPPUNIT_ASSERT_EQUAL(title, t->title());
+		CPPUNIT_ASSERT_EQUAL(String::null, t->artist());
+		CPPUNIT_ASSERT_EQUAL(String::null, t->album());
+		CPPUNIT_ASSERT_EQUAL(String(
+			"Instrument names\n"
+			"are abused as\n"
+			"comments in\n"
+			"module file formats.\n"
+			"-+-+-+-+-+-+-+-+-+-+-+\n"
+			"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n"
+		), 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("StarTrekker"), t->trackerName());
+	}
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestMod);