more correct S3M parsing and property naming

3 files changed, 79 insertions, 82 deletions

diff --git a/taglib/s3m/s3mfile.cpp b/taglib/s3m/s3mfile.cpp
index d48911d7..0752f15d 100644
--- a/taglib/s3m/s3mfile.cpp
+++ b/taglib/s3m/s3mfile.cpp
@@ -24,6 +24,8 @@
 #include "tdebug.h"
 #include "modfileprivate.h"
 
+#include <iostream>
+
 using namespace TagLib;
 using namespace S3M;
 
@@ -146,25 +148,24 @@ void S3M::File::read(bool)
 
   READ_U16L(d->properties.setPatternCount);
   READ_U16L(d->properties.setFlags);
-  READ_U16L(d->properties.setVersion);
-  READ_U16L(d->properties.setSamplesType);
+  READ_U16L(d->properties.setTrackerVersion);
+  READ_U16L(d->properties.setFileFormatVersion);
 
   READ_ASSERT(readBlock(4) == "SCRM");
 
-  READ_BYTE_AS(baseVolume);
-  d->properties.setBaseVolume((int)baseVolume << 1);
-
-  READ_BYTE(d->properties.setTempo);
+  READ_BYTE(d->properties.setGlobalVolume);
   READ_BYTE(d->properties.setBpmSpeed);
+  READ_BYTE(d->properties.setTempo);
 
-  READ_BYTE_AS(stereo);
-  d->properties.setStereo((stereo & 0x80) != 0);
-  READ_BYTE(d->properties.setUltraClick);
+  READ_BYTE_AS(masterVolume);
+  d->properties.setMasterVolume(masterVolume & 0x7f);
+  d->properties.setStereo((masterVolume & 0x80) != 0);
 
-  READ_BYTE_AS(usePanningValues);
-  d->properties.setUsePanningValues(usePanningValues == 0xFC);
+  // 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.
 
-  seek(10, Current);
+  seek(12, Current);
 
   int channels = 0;
   for(int i = 0; i < 32; ++ i)
@@ -179,30 +180,38 @@ void S3M::File::read(bool)
 
   seek(channels, Current);
 
+  // Note: The S3M spec mentions instruments, but I could
+  //       not figure out where these can be found. They are
+  //       similar to samples, though (SCRI instead of SCRS).
   StringList comment;
   for(ushort i = 0; i < sampleCount; ++ i)
   {
     seek(96L + length + ((long)i << 1));
 
-    READ_U16L_AS(instrumentOffset);
-    seek((long)instrumentOffset << 4);
+    READ_U16L_AS(sampleHeaderOffset);
+    seek((long)sampleHeaderOffset << 4);
 
     READ_BYTE_AS(sampleType);
     READ_STRING_AS(dosFileName, 13);
-    READ_U16L_AS(sampleOffset);
+    READ_U16L_AS(sampleDataOffset);
     READ_U32L_AS(sampleLength);
     READ_U32L_AS(repeatStart);
     READ_U32L_AS(repeatStop);
     READ_BYTE_AS(sampleVolume);
 
-    seek(2, Current);
+    seek(1, Current);
 
+    READ_BYTE_AS(packing);
     READ_BYTE_AS(sampleFlags);
     READ_U32L_AS(baseFrequency);
 
     seek(12, Current);
 
     READ_STRING_AS(sampleName, 28);
+    // The next 4 bytes should be "SCRS", but I've found
+    // otherwise ok files with 4 nils instead.
+    // READ_ASSERT(readBlock(4) == "SCRS");
+
     comment.append(sampleName);
   }
 
diff --git a/taglib/s3m/s3mproperties.cpp b/taglib/s3m/s3mproperties.cpp
index 88ed8797..30264f44 100644
--- a/taglib/s3m/s3mproperties.cpp
+++ b/taglib/s3m/s3mproperties.cpp
@@ -34,13 +34,14 @@ public:
     sampleCount(0),
     patternCount(0),
     flags(0),
-    version(0),
-    samplesType(0),
-    baseVolume(0),
+    trackerVersion(0),
+    fileFormatVersion(0),
+    globalVolume(0),
+    masterVolume(0),
     tempo(0),
-    bpmSpeed(0),
-    ultraClick(0),
-    usePanningValues(false) {}
+    bpmSpeed(0)
+  {
+  }
   
   ushort tableLength;
   int    channels;
@@ -48,13 +49,12 @@ public:
   ushort sampleCount;
   ushort patternCount;
   ushort flags;
-  ushort version;
-  ushort samplesType;
-  int    baseVolume;
+  ushort trackerVersion;
+  ushort fileFormatVersion;
+  uchar  globalVolume;
+  uchar  masterVolume;
   uchar  tempo;
   uchar  bpmSpeed;
-  uchar  ultraClick;
-  bool   usePanningValues;
 };
 
 S3M::Properties::Properties(AudioProperties::ReadStyle propertiesStyle) :
@@ -113,19 +113,24 @@ ushort S3M::Properties::flags() const
   return d->flags;
 }
 
-ushort S3M::Properties::version() const
+ushort S3M::Properties::trackerVersion() const
 {
-  return d->version;
+  return d->trackerVersion;
 }
 
-ushort S3M::Properties::samplesType() const
+ushort S3M::Properties::fileFormatVersion() const
 {
-  return d->samplesType;
+  return d->fileFormatVersion;
 }
 
-int S3M::Properties::baseVolume() const
+uchar S3M::Properties::globalVolume() const
 {
-  return d->baseVolume;
+  return d->globalVolume;
+}
+
+uchar S3M::Properties::masterVolume() const
+{
+  return d->masterVolume;
 }
 
 uchar S3M::Properties::tempo() const
@@ -138,16 +143,6 @@ uchar S3M::Properties::bpmSpeed() const
   return d->bpmSpeed;
 }
 
-uchar S3M::Properties::ultraClick() const
-{
-  return d->ultraClick;
-}
-
-bool S3M::Properties::usePanningValues() const
-{
-  return d->usePanningValues;
-}
-
 void S3M::Properties::setTableLength(ushort tableLength)
 {
   d->tableLength = tableLength;
@@ -178,19 +173,24 @@ void S3M::Properties::setFlags(ushort flags)
   d->flags = flags;
 }
 
-void S3M::Properties::setVersion(ushort version)
+void S3M::Properties::setTrackerVersion(ushort trackerVersion)
 {
-  d->version = version;
+  d->trackerVersion = trackerVersion;
 }
 
-void S3M::Properties::setSamplesType(ushort samplesType)
+void S3M::Properties::setFileFormatVersion(ushort fileFormatVersion)
 {
-  d->samplesType = samplesType;
+  d->fileFormatVersion = fileFormatVersion;
 }
 
-void S3M::Properties::setBaseVolume(int baseVolume)
+void S3M::Properties::setGlobalVolume(uchar globalVolume)
 {
-  d->baseVolume = baseVolume;
+  d->globalVolume = globalVolume;
+}
+
+void S3M::Properties::setMasterVolume(uchar masterVolume)
+{
+  d->masterVolume = masterVolume;
 }
 
 void S3M::Properties::setTempo(uchar tempo)
@@ -202,13 +202,3 @@ void S3M::Properties::setBpmSpeed(uchar bpmSpeed)
 {
   d->bpmSpeed = bpmSpeed;
 }
-
-void S3M::Properties::setUltraClick(uchar ultraClick)
-{
-  d->ultraClick = ultraClick;
-}
-
-void S3M::Properties::setUsePanningValues(bool usePanningValues)
-{
-  d->usePanningValues = usePanningValues;
-}
diff --git a/taglib/s3m/s3mproperties.h b/taglib/s3m/s3mproperties.h
index 80ab4aa2..87214770 100644
--- a/taglib/s3m/s3mproperties.h
+++ b/taglib/s3m/s3mproperties.h
@@ -38,34 +38,32 @@ namespace TagLib {
       int sampleRate() const;
       int channels()   const;
 
-      ushort tableLength()  const;
-      bool   stereo()       const;
-      ushort sampleCount()  const;
-      ushort patternCount() const;
-      ushort flags()        const;
-      ushort version()      const;
-      ushort samplesType()  const;
-      int    baseVolume()   const;
-      uchar  tempo()        const;
-      uchar  bpmSpeed()     const;
-      uchar  ultraClick()   const;
-      bool   usePanningValues() const;
+      ushort tableLength()       const;
+      bool   stereo()            const;
+      ushort sampleCount()       const;
+      ushort patternCount()      const;
+      ushort flags()             const;
+      ushort trackerVersion()    const;
+      ushort fileFormatVersion() const;
+      uchar  globalVolume()      const;
+      uchar  masterVolume()      const;
+      uchar  tempo()             const;
+      uchar  bpmSpeed()          const;
 
     protected:
       void setTableLength(ushort tableLength);
       void setChannels(int channels);
 
-      void setStereo      (bool stereo);
-      void setSampleCount (ushort sampleCount);
-      void setPatternCount(ushort patternCount);
-      void setFlags       (ushort flags);
-      void setVersion     (ushort version);
-      void setSamplesType (ushort samplesType);
-      void setBaseVolume  (int baseVolume);
-      void setTempo       (uchar tempo);
-      void setBpmSpeed    (uchar bpmSpeed);
-      void setUltraClick  (uchar ultraClick);
-      void setUsePanningValues(bool usePanningValues);
+      void setStereo           (bool stereo);
+      void setSampleCount      (ushort sampleCount);
+      void setPatternCount     (ushort patternCount);
+      void setFlags            (ushort flags);
+      void setTrackerVersion   (ushort trackerVersion);
+      void setFileFormatVersion(ushort fileFormatVersion);
+      void setGlobalVolume     (uchar globalVolume);
+      void setMasterVolume     (uchar masterVolume);
+      void setTempo            (uchar tempo);
+      void setBpmSpeed         (uchar bpmSpeed);
 
     private:
       Properties(const Properties&);