comment writing support and more tests for mod and xm

taglib/mod/modfile.cpp

@@ -79,7 +79,19 @@ bool Mod::File::save()
   }
   seek(0);
   writeString(d->tag.title(), 20);
-  // TODO: write comment as instrument names
+  StringList lines = d->tag.comment().split("\n");
+  uint n = std::min(lines.size(), d->properties.instrumentCount());
+  for(uint i = 0; i < n; ++ i)
+  {
+    writeString(lines[i], 22);
+    seek(8, Current);
+  }
+
+  for(uint i = n; i < d->properties.instrumentCount(); ++ i)
+  {
+    writeString(String::null, 22);
+    seek(8, Current);
+  }
   return true;
 }
 
@@ -92,8 +104,8 @@ void Mod::File::read(bool)
   ByteVector modId = readBlock(4);
   READ_ASSERT(modId.size() == 4);
 
-  int channels    =  4;
-  int instruments = 31;
+  int  channels    =  4;
+  uint instruments = 31;
   if(modId == "M.K." || modId == "M!K!" || modId == "M&K!" || modId == "N.T.")
   {
     d->tag.setTrackerName("ProTracker");
@@ -143,7 +155,7 @@ void Mod::File::read(bool)
   READ_STRING(d->tag.setTitle, 20);
 
   StringList comment;
-  for(int i = 0; i < instruments; ++ i)
+  for(uint i = 0; i < instruments; ++ i)
   {
     READ_STRING_AS(instrumentName, 22);
     // value in words, * 2 (<< 1) for bytes:
@@ -151,7 +163,7 @@ void Mod::File::read(bool)
 
     READ_BYTE_AS(fineTuneByte);
     int fineTune = fineTuneByte & 0xF;
-    // > 7 means nagative value
+    // > 7 means negative value
     if(fineTune > 7) fineTune -= 16;
 
     READ_BYTE_AS(volume);

taglib/mod/modfilebase.cpp

@@ -19,6 +19,7 @@
  *   MA  02110-1301  USA                                                   *
  ***************************************************************************/
 
+#include "tdebug.h"
 #include "modfilebase.h"
 
 using namespace TagLib;

taglib/mod/modfilebase.h

@@ -25,22 +25,26 @@
 #include "taglib.h"
 #include "tfile.h"
 #include "tstring.h"
+#include "tlist.h"
 #include "taglib_export.h"
 
+#include <algorithm>
+
 namespace TagLib {
   namespace Mod {
-    class TAGLIB_EXPORT FileBase : public TagLib::File {
-      protected:
-        FileBase(FileName file);
-        FileBase(IOStream *stream);
+    class TAGLIB_EXPORT FileBase : public TagLib::File
+    {
+    protected:
+      FileBase(FileName file);
+      FileBase(IOStream *stream);
 
-        void writeString(const String &s, ulong size, char padding = 0);
-        bool readString(String &s, ulong size);
-        bool readByte(uchar &byte);
-        bool readU16L(ushort &number);
-        bool readU32L(ulong &number);
-        bool readU16B(ushort &number);
-        bool readU32B(ulong &number);
+      void writeString(const String &s, ulong size, char padding = 0);
+      bool readString(String &s, ulong size);
+      bool readByte(uchar &byte);
+      bool readU16L(ushort &number);
+      bool readU32L(ulong &number);
+      bool readU16B(ushort &number);
+      bool readU32B(ulong &number);
     };
   }
 }

taglib/toolkit/tstring.cpp

@@ -26,6 +26,7 @@
 #include "tstring.h"
 #include "unicode.h"
 #include "tdebug.h"
+#include "tstringlist.h"
 
 #include <ostream>
 
@@ -304,6 +305,26 @@ int String::rfind(const String &s, int offset) const
     return -1;
 }
 
+StringList String::split(const String &separator) const
+{
+  StringList list;
+  for(int index = 0;;)
+  {
+    int sep = find(separator, index);
+    if(sep < 0)
+    {
+      list.append(substr(index, size() - index));
+      break;
+    }
+    else
+    {
+      list.append(substr(index, sep - index));
+      index = sep + separator.size();
+    }
+  }
+  return list;
+}
+
 bool String::startsWith(const String &s) const
 {
   if(s.length() > length())

taglib/toolkit/tstring.h

@@ -56,6 +56,8 @@
 
 namespace TagLib {
 
+  class StringList;
+
   //! A \e wide string class suitable for unicode.
 
   /*!
@@ -239,6 +241,11 @@ namespace TagLib {
      */
     int rfind(const String &s, int offset = -1) const;
 
+    /*!
+     * Splits the string on each occurrence of \a separator.
+     */
+    StringList split(const String &separator = " ") const;
+
     /*!
      * Returns true if the strings starts with the substring \a s.
      */

taglib/xm/xmfile.cpp

@@ -24,10 +24,327 @@
 #include "xmfile.h"
 #include "modfileprivate.h"
 
+#include <string.h>
 #include <algorithm>
 
 using namespace TagLib;
 using namespace XM;
+using TagLib::uint;
+using TagLib::ushort;
+using TagLib::ulong;
+
+/*!
+ * The *Reader classes are helpers to make handling of the stripped XM
+ * format more easy. In the stripped XM format certain headersizes might
+ * be smaller than one would expect. The fields that are not included
+ * are then just some predefined valued (e.g. 0).
+ *
+ * Using these classes this code:
+ *
+ *   if(headerSize >= 4)
+ *   {
+ *     if(!readU16L(value1)) ERROR();
+ *     if(headerSize >= 8)
+ *     {
+ *       if(!readU16L(value2)) ERROR();
+ *       if(headerSize >= 12)
+ *       {
+ *         if(!readString(value3, 22)) ERROR();
+ *         ...
+ *       }
+ *     }
+ *   }
+ *
+ * Becomes:
+ *
+ *   StructReader header;
+ *   header.u16L(value1).u16L(value2).string(value3, 22). ...;
+ *   if(header.read(*this, headerSize) < std::min(header.size(), headerSize))
+ *     ERROR();
+ *
+ * Maybe if this is useful to other formats this cleasses can be moved to
+ * their onw public files.
+ */
+class Reader
+{
+public:
+  virtual ~Reader()
+  {
+  }
+  
+  /*!
+   * Reads associated values from \a file, but never reads more
+   * then \a limit bytes.
+   */
+  virtual uint read(TagLib::File &file, uint limit) = 0;
+
+  /*!
+   * Returns the number of bytes this reader would like to read.
+   */
+  virtual uint size() const = 0;
+};
+
+class SkipReader : public Reader
+{
+public:
+  SkipReader(uint size) : m_size(size)
+  {
+  }
+  
+  uint read(TagLib::File &file, uint limit)
+  {
+    uint count = std::min(m_size, limit);
+    file.seek(count, TagLib::File::Current);
+    return count;
+  }
+
+  uint size() const
+  {
+    return m_size;
+  }
+
+private:
+  uint m_size;
+};
+
+template<typename T>
+class ValueReader : public Reader
+{
+public:
+  ValueReader(T &value) : value(value)
+  {
+  }
+
+protected:
+  T &value;
+};
+
+class StringReader : public ValueReader<String>
+{
+public:
+  StringReader(String &string, uint size) :
+    ValueReader<String>(string), m_size(size)
+  {
+  }
+  
+  uint read(TagLib::File &file, uint limit)
+  {
+    ByteVector data = file.readBlock(std::min(m_size,limit));
+    uint count = data.size();
+    int index = data.find((char) 0);
+    if(index > -1)
+    {
+      data.resize(index);
+    }
+    data.replace((char) 0xff, ' ');
+    value = data;
+    return count;
+  }
+
+  uint size() const
+  {
+    return m_size;
+  }
+
+private:
+  uint m_size;
+};
+
+class ByteReader : public ValueReader<uchar>
+{
+public:
+  ByteReader(uchar &byte) : ValueReader<uchar>(byte) {}
+  
+  uint read(TagLib::File &file, uint limit)
+  {
+    ByteVector data = file.readBlock(std::min(1U,limit));
+    if(data.size() > 0)
+    {
+      value = data[0];
+    }
+    return data.size();
+  }
+
+  uint size() const
+  {
+    return 1;
+  }
+};
+
+template<typename T>
+class NumberReader : public ValueReader<T>
+{
+public:
+  NumberReader(T &value, bool bigEndian) :
+    ValueReader<T>(value), bigEndian(bigEndian)
+  {
+  }
+
+protected:
+  bool bigEndian;
+};
+
+class U16Reader : public NumberReader<ushort>
+{
+public:
+  U16Reader(ushort &value, bool bigEndian)
+  : NumberReader<ushort>(value, bigEndian) {}
+
+  uint read(TagLib::File &file, uint limit)
+  {
+    ByteVector data = file.readBlock(std::min(2U,limit));
+    value = data.toUShort(bigEndian);
+    return data.size();
+  }
+
+  uint size() const
+  {
+    return 2;
+  }
+};
+
+class U32Reader : public NumberReader<ulong>
+{
+public:
+  U32Reader(ulong &value, bool bigEndian = true) :
+    NumberReader<ulong>(value, bigEndian)
+  {
+  }
+
+  uint read(TagLib::File &file, uint limit)
+  {
+    ByteVector data = file.readBlock(std::min(4U,limit));
+    value = data.toUInt(bigEndian);
+    return data.size();
+  }
+  
+  uint size() const
+  {
+    return 4;
+  }
+};
+
+class StructReader : public Reader
+{
+public:
+  StructReader()
+  {
+    m_readers.setAutoDelete(true);
+  }
+
+  /*!
+   * Add a nested reader. This reader takes ownership.
+   */
+  StructReader &reader(Reader *reader)
+  {
+    m_readers.append(reader);
+    return *this;
+  }
+
+  /*!
+   * Don't read anything but skip \a size bytes.
+   */
+  StructReader &skip(uint size)
+  {
+    m_readers.append(new SkipReader(size));
+    return *this;
+  }
+  
+  /*!
+   * Read a string of \a size characters (bytes) into \a string.
+   */
+  StructReader &string(String &string, uint size)
+  {
+    m_readers.append(new StringReader(string, size));
+    return *this;
+  }
+
+  /*!
+   * Read a byte into \a byte.
+   */
+  StructReader &byte(uchar &byte)
+  {
+    m_readers.append(new ByteReader(byte));
+    return *this;
+  }
+  
+  /*!
+   * Read a unsigned 16 Bit integer into \a number. The byte order
+   * is controlled by \a bigEndian.
+   */
+  StructReader &u16(ushort &number, bool bigEndian)
+  {
+    m_readers.append(new U16Reader(number, bigEndian));
+    return *this;
+  }
+  
+  /*!
+   * Read a unsigned 16 Bit little endian integer into \a number.
+   */
+  StructReader &u16L(ushort &number)
+  {
+    return u16(number, false);
+  }
+  
+  /*!
+   * Read a unsigned 16 Bit big endian integer into \a number.
+   */
+  StructReader &u16B(ushort &number)
+  {
+    return u16(number, true);
+  }
+  
+  /*!
+   * Read a unsigned 32 Bit integer into \a number. The byte order
+   * is controlled by \a bigEndian.
+   */
+  StructReader &u32(ulong &number, bool bigEndian)
+  {
+    m_readers.append(new U32Reader(number, bigEndian));
+    return *this;
+  }
+  
+  /*!
+   * Read a unsigned 32 Bit little endian integer into \a number.
+   */
+  StructReader &u32L(ulong &number)
+  {
+    return u32(number, false);
+  }
+  
+  /*!
+   * Read a unsigned 32 Bit big endian integer into \a number.
+   */
+  StructReader &u32B(ulong &number)
+  {
+    return u32(number, true);
+  }
+
+  uint size() const
+  {
+    uint size = 0;
+    for(List<Reader*>::ConstIterator i = m_readers.begin(); i != m_readers.end(); ++ i)
+    {
+      size += (*i)->size();
+    }
+    return size;
+  }
+
+  uint read(TagLib::File &file, uint limit)
+  {
+    uint sumcount = 0;
+    for(List<Reader*>::Iterator i = m_readers.begin(); limit > 0 && i != m_readers.end(); ++ i)
+    {
+      uint count = (*i)->read(file, limit);
+      limit    -= count;
+      sumcount += count;
+    }
+    return sumcount;
+  }
+
+private:
+  List<Reader*> m_readers;
+};
 
 class XM::File::FilePrivate
 {
@@ -83,7 +400,105 @@ bool XM::File::save()
   writeString(d->tag.title(), 20);
   seek(1, Current);
   writeString(d->tag.trackerName(), 20);
-  // TODO: write comment as instrument and sample names
+  seek(2, Current);
+  ulong headerSize = 0;
+  if(!readU32L(headerSize))
+    return false;
+  seek(2+2+2, Current);
+
+  ushort patternCount = 0;
+  ushort instrumentCount = 0;
+  if(!readU16L(patternCount) || !readU16L(instrumentCount))
+    return false;
+
+  seek(60 + headerSize);
+
+  // need to read patterns again in order to seek to the instruments:
+  for(ushort i = 0; i < patternCount; ++ i)
+  {
+    ulong patternHeaderLength = 0;
+    if(!readU32L(patternHeaderLength) || patternHeaderLength < 4)
+      return false;
+    
+    ushort dataSize = 0;
+    StructReader pattern;
+    pattern.skip(3).u16L(dataSize);
+
+    uint count = pattern.read(*this, patternHeaderLength - 4U);
+    if(count != std::min(patternHeaderLength - 4U, (ulong)pattern.size()))
+      return false;
+
+    seek(patternHeaderLength - (4 + count) + dataSize, Current);
+  }
+
+  StringList lines = d->tag.comment().split("\n");  
+  uint sampleNameIndex = instrumentCount;
+  for(ushort i = 0; i < instrumentCount; ++ i)
+  {
+    ulong instrumentHeaderSize = 0;
+    if(!readU32L(instrumentHeaderSize) || instrumentHeaderSize < 4)
+      return false;
+
+    uint len = std::min(22UL, instrumentHeaderSize - 4U);
+    if(i > lines.size())
+      writeString(String::null, len);
+    else
+      writeString(lines[i], len);
+
+    ushort sampleCount = 0;
+    long offset = 0;
+    if(instrumentHeaderSize >= 29U)
+    {
+      seek(1, Current);
+      if(!readU16L(sampleCount))
+        return false;
+
+      if(sampleCount > 0)
+      {
+        ulong sampleHeaderSize = 0;
+        if(instrumentHeaderSize < 33U || !readU32L(sampleHeaderSize))
+          return false;
+        // skip unhandeled header proportion:
+        seek(instrumentHeaderSize - 33, Current);
+        
+        for(ushort j = 0; j < sampleCount; ++ j)
+        {
+          if(sampleHeaderSize > 4U)
+          {
+            ulong length = 0;
+            if(!readU32L(length))
+              return false;
+            offset += length;
+
+            seek(std::min(sampleHeaderSize, 14UL), Current);
+            if(sampleHeaderSize > 18U)
+            {
+              uint len = std::min(sampleHeaderSize - 18U, 22UL);
+              if(sampleNameIndex >= lines.size())
+                writeString(String::null, len);
+              else
+                writeString(lines[sampleNameIndex ++], len);
+              seek(sampleHeaderSize - (18U + len), Current);
+            }
+          }
+          else
+          {
+            seek(sampleHeaderSize, Current);
+          }
+        }
+      }
+      else
+      {
+        offset = instrumentHeaderSize - 29;
+      }
+    }
+    else
+    {
+      offset = instrumentHeaderSize - (4 + len);
+    }
+    seek(offset, Current);
+  }
+  
   return true;
 }
 
@@ -92,97 +507,143 @@ void XM::File::read(bool)
   if(!isOpen())
     return;
 
-  READ_ASSERT(readBlock(17) == "Extended Module: ");
+  seek(0);
+  ByteVector magic = readBlock(17);
+  // it's all 0x00 for stripped XM files:
+  READ_ASSERT(magic == "Extended Module: " || magic == ByteVector(17, 0));
 
   READ_STRING(d->tag.setTitle, 20);
-  READ_BYTE_AS(mark);
-  READ_ASSERT(mark == 0x1A);
+  READ_BYTE_AS(escape);
+  // in stripped XM files this is 0x00:
+  READ_ASSERT(escape == 0x1A || escape == 0x00);
   
   READ_STRING(d->tag.setTrackerName, 20);
   READ_U16L(d->properties.setVersion);
+  
   READ_U32L_AS(headerSize);
-  READ_U16L(d->properties.setTableLength);
-  READ_U16L(d->properties.setRestartPosition);
-  READ_U16L(d->properties.setChannels);
-  READ_U16L_AS(patternCount);
+  READ_ASSERT(headerSize >= 4);
+
+  ushort tableLength     = 0;
+  ushort restartPosition = 0;
+  ushort channels        = 0;
+  ushort patternCount    = 0;
+  ushort instrumentCount = 0;
+  ushort flags    = 0;
+  ushort tempo    = 0;
+  ushort bpmSpeed = 0;
+
+  StructReader header;
+  header.u16L(tableLength)
+        .u16L(restartPosition)
+        .u16L(channels)
+        .u16L(patternCount)
+        .u16L(instrumentCount)
+        .u16L(flags)
+        .u16L(tempo)
+        .u16L(bpmSpeed);
+
+  uint count = header.read(*this, headerSize - 4U);
+  uint size = std::min(headerSize - 4U, (ulong)header.size());
+  
+  READ_ASSERT(count == size);
+
+  d->properties.setTableLength(tableLength);
+  d->properties.setRestartPosition(restartPosition);
+  d->properties.setChannels(channels);
   d->properties.setPatternCount(patternCount);
-  READ_U16L_AS(instrumentCount);
   d->properties.setInstrumentCount(instrumentCount);
-  READ_U16L(d->properties.setFlags);
-  READ_U16L(d->properties.setTempo);
-  READ_U16L(d->properties.setBpmSpeed);
+  d->properties.setFlags(flags);
+  d->properties.setTempo(tempo);
+  d->properties.setBpmSpeed(bpmSpeed);
 
   seek(60 + headerSize);
-    
+  
+  // read patterns:
   for(ushort i = 0; i < patternCount; ++ i)
   {
     READ_U32L_AS(patternHeaderLength);
-    READ_BYTE_AS(patternType);
-    READ_U16L_AS(rowCount);
-    READ_U16L_AS(patternDataSize);
+    READ_ASSERT(patternHeaderLength >= 4);
+    
+    uchar  packingType = 0;
+    ushort rowCount = 0;
+    ushort dataSize = 0;
+    StructReader pattern;
+    pattern.byte(packingType).u16L(rowCount).u16L(dataSize);
 
-    seek(patternHeaderLength - (4+1+2+2) + patternDataSize, Current);
+    uint count = pattern.read(*this, patternHeaderLength - 4U);
+    READ_ASSERT(count == std::min(patternHeaderLength - 4U, (ulong)pattern.size()));
+
+    seek(patternHeaderLength - (4 + count) + dataSize, Current);
   }
     
   StringList intrumentNames;
   StringList sampleNames;
+
+  // read instruments:
   for(ushort i = 0; i < instrumentCount; ++ i)
   {
-    long pos = tell();
-    READ_U32L_AS(instrumentSize);
+    READ_U32L_AS(instrumentHeaderSize);
+    READ_ASSERT(instrumentHeaderSize >= 4);
 
     String instrumentName;
-    uchar instrumentType = 0;
+    uchar  instrumentType = 0;
     ushort sampleCount = 0;
-      
-    if(instrumentSize > 4)
-    {
-      READ_ASSERT(readString(instrumentName, std::min(22UL, instrumentSize-4)));
 
-      if(instrumentSize >= (4+22+1))
-      {
-        READ_ASSERT(readByte(instrumentType));
+    StructReader instrument;
+    instrument.string(instrumentName, 22).byte(instrumentType).u16L(sampleCount);
 
-        if(instrumentSize >= (4+22+1+2))
-        {
-          READ_ASSERT(readU16L(sampleCount));
-        }
-      }
-    }
+    // 4 for instrumentHeaderSize
+    uint count = 4 + instrument.read(*this, instrumentHeaderSize - 4U);
+    READ_ASSERT(count == std::min(instrumentHeaderSize, (ulong)instrument.size() + 4));
 
-    ulong sumSampleLength = 0;
     ulong sampleHeaderSize = 0;
+    long offset = 0;
     if(sampleCount > 0)
     {
-      if(!readU32L(sampleHeaderSize))
-      {
-        setValid(false);
-        return;
-      }
+      // wouldn't know which header size to assume otherwise:
+      READ_ASSERT(instrumentHeaderSize >= count + 4 && readU32L(sampleHeaderSize));
+      // skip unhandeled header proportion:
+      seek(instrumentHeaderSize - count - 4, Current);
 
-      seek(pos + instrumentSize);
-
-      long sampleheaderPos = tell();
       for(ushort j = 0; j < sampleCount; ++ j)
       {
-        seek(sampleheaderPos + sampleHeaderSize * j);
-        READ_U32L_AS(length);
-        READ_U32L_AS(loopStart);
-        READ_U32L_AS(loopLength);
-        READ_BYTE_AS(volume);
-        READ_BYTE_AS(finetune);
-        READ_BYTE_AS(sampleType);
-        READ_BYTE_AS(panning);
-        READ_BYTE_AS(noteNumber);
-        READ_BYTE_AS(compression);
-        READ_STRING_AS(sampleName, 22);
-        
-        sumSampleLength += length;
+        ulong length      = 0;
+        ulong loopStart   = 0;
+        ulong loopLength  = 0;
+        uchar volume      = 0;
+        uchar finetune    = 0;
+        uchar sampleType  = 0;
+        uchar panning     = 0;
+        uchar noteNumber  = 0;
+        uchar compression = 0;
+        String sampleName;
+        StructReader sample;
+        sample.u32L(length)
+              .u32L(loopStart)
+              .u32L(loopLength)
+              .byte(volume)
+              .byte(finetune)
+              .byte(sampleType)
+              .byte(panning)
+              .byte(noteNumber)
+              .byte(compression)
+              .string(sampleName, 22);
+
+        uint count = sample.read(*this, sampleHeaderSize);
+        READ_ASSERT(count == std::min(sampleHeaderSize, (ulong)sample.size()));
+        // skip unhandeled header proportion:
+        seek(sampleHeaderSize - count, Current);
+
+        offset += length;
         sampleNames.append(sampleName);
       }
     }
+    else
+    {
+      offset = instrumentHeaderSize - count;
+    }
     intrumentNames.append(instrumentName);
-    seek(pos + instrumentSize + sampleHeaderSize * sampleCount + sumSampleLength);
+    seek(offset, Current);
   }
 
   String comment(intrumentNames.toString("\n"));

tests/test_mod.cpp

@@ -20,43 +20,63 @@
  ***************************************************************************/
 
 #include <cppunit/extensions/HelperMacros.h>
-#include <string>
 #include <modfile.h>
 #include "utils.h"
 
 using namespace std;
 using namespace TagLib;
 
+static const String titleBefore("title of song");
+static const String titleAfter("changed title");
+
+static const String commentBefore(
+  "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");
+
+static const String newComment(
+  "This line will be truncated because it is too long for a mod instrument name.\n"
+  "This line is ok.");
+
+static const String commentAfter(
+  "This line will be trun\n"
+  "This line is ok.\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\n\n");
+
 class TestMod : public CppUnit::TestFixture
 {
   CPPUNIT_TEST_SUITE(TestMod);
-  CPPUNIT_TEST(testRead);
-  CPPUNIT_TEST(testChangeTitle);
+  CPPUNIT_TEST(testReadTags);
+  CPPUNIT_TEST(testWriteTags);
   CPPUNIT_TEST_SUITE_END();
 
 public:
-  void testRead()
+  void testReadTags()
   {
-    testRead(TEST_FILE_PATH_C("test.mod"), "title of song");
+    testRead(TEST_FILE_PATH_C("test.mod"), titleBefore, commentBefore);
   }
 
-  void testChangeTitle()
+  void testWriteTags()
   {
     ScopedFileCopy copy("test", ".mod");
     {
       Mod::File file(copy.fileName().c_str());
       CPPUNIT_ASSERT(file.tag() != 0);
-      file.tag()->setTitle("changed title");
+      file.tag()->setTitle(titleAfter);
+      file.tag()->setComment(newComment);
       CPPUNIT_ASSERT(file.save());
     }
-    testRead(copy.fileName().c_str(), "changed title");
+    testRead(copy.fileName().c_str(), titleAfter, commentAfter);
     CPPUNIT_ASSERT(fileEqual(
       copy.fileName(),
-      TEST_FILE_PATH_C("changed_title.mod")));
+      TEST_FILE_PATH_C("changed.mod")));
   }
 
 private:
-  void testRead(FileName fileName, const String &title)
+  void testRead(FileName fileName, const String &title, const String &comment)
   {
     Mod::File file(fileName);
 
@@ -77,14 +97,7 @@ private:
     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(comment, t->comment());
     CPPUNIT_ASSERT_EQUAL(String::null, t->genre());
     CPPUNIT_ASSERT_EQUAL(0U, t->year());
     CPPUNIT_ASSERT_EQUAL(0U, t->track());

tests/test_xm.cpp

@@ -21,42 +21,148 @@
 
 #include <cppunit/extensions/HelperMacros.h>
 #include <string>
+#include <sstream>
 #include <xmfile.h>
 #include "utils.h"
 
 using namespace std;
 using namespace TagLib;
 
+static const String titleBefore("title of song");
+static const String titleAfter("changed title");
+
+static const String trackerNameBefore("MilkyTracker        ");
+static const String trackerNameAfter("TagLib");
+
+static const String commentBefore(
+  "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\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\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\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\n\n\n\n\n\n\n\n\n\n\n\n\n\n"
+  "\n\n\n"
+  "Sample\n"
+  "names\n"
+  "are sometimes\n"
+  "also abused as\n"
+  "comments.");
+
+static const String newCommentShort(
+  "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\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\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\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\n\n\n\n\n\n\n\n\n\n\n\n\n\n"
+  "\n\n\n"
+  "Sample names\n"
+  "are sometimes\n"
+  "also abused as\n"
+  "comments.");
+
+static const String newCommentLong(
+  "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\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\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\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\n\n\n\n\n\n\n\n\n\n\n\n\n\n"
+  "\n\n\n"
+  "Sample names\n"
+  "are sometimes\n"
+  "also abused as\n"
+  "comments.\n"
+  "\n\n\n\n\n\n\n"
+  "TEST");
+
+static const String commentAfter(
+  "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\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\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\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\n\n\n\n\n\n\n\n\n\n\n\n\n\n"
+  "\n\n\n"
+  "Sample names\n"
+  "are sometimes\n"
+  "also abused as\n"
+  "comments.\n");
+
 class TestXM : public CppUnit::TestFixture
 {
   CPPUNIT_TEST_SUITE(TestXM);
-  CPPUNIT_TEST(testRead);
-  CPPUNIT_TEST(testChangeTitle);
+  CPPUNIT_TEST(testReadTags);
+  CPPUNIT_TEST(testReadStrippedTags);
+  CPPUNIT_TEST(testWriteTagsShort);
+  CPPUNIT_TEST(testWriteTagsLong);
   CPPUNIT_TEST_SUITE_END();
 
 public:
-  void testRead()
+  void testReadTags()
   {
-    testRead(TEST_FILE_PATH_C("test.xm"), "title of song");
+    testRead(TEST_FILE_PATH_C("test.xm"), titleBefore,
+             commentBefore, trackerNameBefore);
   }
 
-  void testChangeTitle()
+  void testReadStrippedTags()
   {
-    ScopedFileCopy copy("test", ".xm");
-    {
-      XM::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");
-    CPPUNIT_ASSERT(fileEqual(
-      copy.fileName(),
-      TEST_FILE_PATH_C("changed_title.xm")));
+    XM::File file(TEST_FILE_PATH_C("stripped.xm"));
+    CPPUNIT_ASSERT(file.isValid());
+
+    XM::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((TagLib::ushort)  1, p->tableLength());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  0, p->version());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  0 , p->restartPosition());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  1, p->patternCount());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  0, p->instrumentCount());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  1, p->flags());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  6, p->tempo());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)125, p->bpmSpeed());
+    CPPUNIT_ASSERT_EQUAL(titleBefore, t->title());
+    CPPUNIT_ASSERT_EQUAL(String::null, t->artist());
+    CPPUNIT_ASSERT_EQUAL(String::null, t->album());
+    CPPUNIT_ASSERT_EQUAL(String::null, 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::null, t->trackerName());
+  }
+
+  void testWriteTagsShort()
+  {
+    testWriteTags(newCommentShort);
+  }
+
+  void testWriteTagsLong()
+  {
+    testWriteTags(newCommentLong);
   }
 
 private:
-  void testRead(FileName fileName, const String &title)
+  void testRead(FileName fileName, const String &title,
+                const String &comment, const String &trackerName)
   {
     XM::File file(fileName);
 
@@ -72,9 +178,9 @@ private:
     CPPUNIT_ASSERT_EQUAL(0, p->bitrate());
     CPPUNIT_ASSERT_EQUAL(0, p->sampleRate());
     CPPUNIT_ASSERT_EQUAL(8, p->channels());
-	CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  1, p->tableLength());
-	CPPUNIT_ASSERT_EQUAL((TagLib::ushort)260, p->version());
-	CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  0, p->restartPosition());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  1, p->tableLength());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)260, p->version());
+    CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  0, p->restartPosition());
     CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  1, p->patternCount());
     CPPUNIT_ASSERT_EQUAL((TagLib::ushort)128, p->instrumentCount());
     CPPUNIT_ASSERT_EQUAL((TagLib::ushort)  1, p->flags());
@@ -83,27 +189,29 @@ private:
     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\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\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\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\n\n\n\n\n\n\n\n\n\n\n\n\n\n"
-	  "\n\n\n"
-	  "Sample\n"
-	  "names\n"
-	  "are sometimes\n"
-	  "also abused as\n"
-	  "comments."
-    ), t->comment());
+    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("MilkyTracker        "), t->trackerName());
+    CPPUNIT_ASSERT_EQUAL(trackerName, t->trackerName());
+  }
+  
+  void testWriteTags(const String &comment)
+  {
+    ScopedFileCopy copy("test", ".xm");
+    {
+      XM::File file(copy.fileName().c_str());
+      CPPUNIT_ASSERT(file.tag() != 0);
+      file.tag()->setTitle(titleAfter);
+      file.tag()->setComment(comment);
+      file.tag()->setTrackerName(trackerNameAfter);
+      CPPUNIT_ASSERT(file.save());
+    }
+    testRead(copy.fileName().c_str(), titleAfter,
+             commentAfter, trackerNameAfter);
+    CPPUNIT_ASSERT(fileEqual(
+      copy.fileName(),
+      TEST_FILE_PATH_C("changed.xm")));
   }
 };