Refactoring of the Musepack SV8 properties code

diff --git a/taglib/mpc/mpcfile.cpp b/taglib/mpc/mpcfile.cpp
index 7e33fdb4eb0990c5b36a1514c660334e3a64059a..519a04675502f5a9faee689a8c341ff1cfc6b623 100644 (file)
--- a/taglib/mpc/mpcfile.cpp
+++ b/taglib/mpc/mpcfile.cpp
@@ -310,18 +310,7 @@ void MPC::File::read(bool readProperties, Properties::ReadStyle /* propertiesSty
   // Look for MPC metadata
 
   if(readProperties) {
-    // Checking MPC version
-    ByteVector magic = readBlock(4);
-    if (magic == MPC::V8MagicTitle) {
-      // Musepack version 8 or newer, we should find "SH" packet and "RG" packet
-      d->properties = new Properties(ByteVector("MPCK")+findHeaderPacket(),
-                                   length() - d->ID3v2Size - d->APESize);
-    }
-    else {
-      seek(tell()-4);
-      d->properties = new Properties(readBlock(MPC::HeaderSize),
-                                   length() - d->ID3v2Size - d->APESize);
-    }
+    d->properties = new Properties(this, length() - d->ID3v2Size - d->APESize);
   }
 }
 
@@ -369,42 +358,3 @@ long MPC::File::findID3v2()
 
   return -1;
 }
-
-/*
-The structure of MPC v8 packet:
-key   16 bit
-size  n*8 bit, 0<n<10
-data  size*8
-
-This function returns only data for corresponding key
-*/
-ByteVector MPC::File::findHeaderPacket()
-{
-  ByteVector packet;
-  if(!isValid())
-    return packet;
-
-  ByteVector key = readBlock(2);
-  uint sizelength=0;
-  long size = readSize(sizelength);
-
-  if (key=="SH")
-    return readBlock(size-2-sizelength+12); //12 is the size of the next packet
-                                            //that contains replaygain info
-
-  return packet;
-}
-
-long MPC::File::readSize(uint &sizelength)
-{
-  unsigned char tmp;
-  long size = 0;
-
-  do {
-    ByteVector b = readBlock(1);
-    tmp = b[0];
-    size = (size << 7) | (tmp & 0x7F);
-    sizelength++;
-  } while((tmp & 0x80));
-  return size;
-}
\ No newline at end of file

diff --git a/taglib/mpc/mpcfile.h b/taglib/mpc/mpcfile.h
index e5d1085691155fb18a35663e7c1204864db538c2..61ac6d67e5a8021ded10a8fdf26edcba2135e6d1 100644 (file)
--- a/taglib/mpc/mpcfile.h
+++ b/taglib/mpc/mpcfile.h
@@ -191,8 +191,6 @@ namespace TagLib {
       long findAPE();
       long findID3v1();
       long findID3v2();
-      ByteVector findHeaderPacket();
-      long readSize(uint &sizelength);
 
       class FilePrivate;
       FilePrivate *d;

diff --git a/taglib/mpc/mpcproperties.cpp b/taglib/mpc/mpcproperties.cpp
index 1afa4da0634be6c53bc281bdfc359f7ab3bac0b4..7046db99742e4e584a3150e537d78b1eaea56898 100644 (file)
--- a/taglib/mpc/mpcproperties.cpp
+++ b/taglib/mpc/mpcproperties.cpp
@@ -36,8 +36,7 @@ using namespace TagLib;
 class MPC::Properties::PropertiesPrivate
 {
 public:
-  PropertiesPrivate(const ByteVector &d, long length, ReadStyle s) :
-    data(d),
+  PropertiesPrivate(long length, ReadStyle s) :
     streamLength(length),
     style(s),
     version(0),
@@ -52,7 +51,6 @@ public:
     albumGain(0),
     albumPeak(0) {}
 
-  ByteVector data;
   long streamLength;
   ReadStyle style;
   int version;
@@ -75,8 +73,22 @@ public:
 
 MPC::Properties::Properties(const ByteVector &data, long streamLength, ReadStyle style) : AudioProperties(style)
 {
-  d = new PropertiesPrivate(data, streamLength, style);
-  read();
+  d = new PropertiesPrivate(streamLength, style);
+  readSV7(data);
+}
+
+MPC::Properties::Properties(File *file, long streamLength, ReadStyle style) : AudioProperties(style)
+{
+  d = new PropertiesPrivate(streamLength, style);
+  ByteVector magic = file->readBlock(4);
+  if(magic == "MPCK") {
+    // Musepack version 8
+    readSV8(file);
+  }
+  else {
+    // Musepack version 7 or older, fixed size header
+    readSV7(magic + file->readBlock(MPC::HeaderSize - 4));
+  }
 }
 
 MPC::Properties::~Properties()
@@ -109,12 +121,12 @@ int MPC::Properties::mpcVersion() const
   return d->version;
 }
 
-uint MPC::Properties::totalFrames() const
+TagLib::uint MPC::Properties::totalFrames() const
 {
   return d->totalFrames;
 }
 
-uint MPC::Properties::sampleFrames() const
+TagLib::uint MPC::Properties::sampleFrames() const
 {
   return d->sampleFrames;
 }
@@ -143,107 +155,134 @@ int MPC::Properties::albumPeak() const
 // private members
 ////////////////////////////////////////////////////////////////////////////////
 
-
-ulong readSize(const ByteVector &b, uint &sizelength)
+unsigned long readSize(File *file, TagLib::uint &sizelength)
 {
   unsigned char tmp;
-  ulong size = 0;
-  sizelength = 0;
+  unsigned long size = 0;
 
   do {
-    tmp = b[sizelength];
+    ByteVector b = file->readBlock(1);
+    tmp = b[0];
     size = (size << 7) | (tmp & 0x7F);
     sizelength++;
   } while((tmp & 0x80));
   return size;
 }
 
-static const unsigned short sftable [4] = { 44100, 48000, 37800, 32000 };
-
-void MPC::Properties::read()
+unsigned long readSize(const ByteVector &data, TagLib::uint &sizelength)
 {
-  if (d->data.startsWith("MPCK")) {
-    //no checksum checking so far, this code isn't working
-    //quint16 crc=qChecksum(b.mid(4).data(), b.length()-4);
-    //uint crc1=d->data.mid(0,2).toUInt(false);
-
-
-    int pos=8;
-    d->version = d->data[pos];
-    pos+=1;
-    uint sizelength=0;
-    d->sampleFrames = readSize(d->data.mid(pos), sizelength);
-    pos+=sizelength;
-    ulong begSilence = readSize(d->data.mid(pos), sizelength);
-    pos+=sizelength;
-
-    std::bitset<16> flags(TAGLIB_CONSTRUCT_BITSET(d->data.mid(pos,2).toUShort(true)));
-    pos+=2;
+  unsigned char tmp;
+  unsigned long size = 0;
+  unsigned long pos = 0;
 
-    d->sampleRate = sftable[flags[15] * 4 + flags[14]*2 + flags[13]];
-    d->channels = flags[7] * 8 + flags[6]*4 + flags[5]*2+flags[4] + 1;
+  do {
+    tmp = data[pos++];
+    size = (size << 7) | (tmp & 0x7F);
+    sizelength++;
+  } while((tmp & 0x80) && (pos < data.size()));
+  return size;
+}
 
-    //bool msUsed = flags[3];
-    //int audioBlockFrames = (flags[2]*4 + flags[1]*2 + flags[0]) * 2;
+static const unsigned short sftable [4] = { 44100, 48000, 37800, 32000 };
 
-    if ((d->sampleFrames - begSilence) != 0)
-      d->bitrate = d->streamLength * 8.0 * d->sampleRate/(d->sampleFrames-begSilence);
+void MPC::Properties::readSV8(File *file)
+{
+  bool readSH = false, readRG = false;
+
+  while(!readSH && !readRG) {
+    ByteVector packetType = file->readBlock(2);
+    uint packetSizeLength = 0;
+    unsigned long packetSize = readSize(file, packetSizeLength);
+    unsigned long dataSize = packetSize - 2 - packetSizeLength;
+
+    if(packetType == "SH") {
+      // Stream Header
+      // http://trac.musepack.net/wiki/SV8Specification#StreamHeaderPacket
+      ByteVector data = file->readBlock(dataSize);
+      readSH = true;
+
+      TagLib::uint pos = 4;
+      d->version = data[pos];
+      pos += 1;
+      d->sampleFrames = readSize(data.mid(pos), pos);
+      ulong begSilence = readSize(data.mid(pos), pos);
+
+      std::bitset<16> flags(TAGLIB_CONSTRUCT_BITSET(data.mid(pos, 2).toUShort(true)));
+      pos += 2;
+
+      d->sampleRate = sftable[flags[15] * 4 + flags[14] * 2 + flags[13]];
+      d->channels = flags[7] * 8 + flags[6] * 4 + flags[5] * 2 + flags[4] + 1;
+
+      if((d->sampleFrames - begSilence) != 0)
+        d->bitrate = d->streamLength * 8.0 * d->sampleRate / (d->sampleFrames - begSilence);
       d->bitrate = d->bitrate / 1000;
 
-    d->length = (d->sampleFrames - begSilence) / d->sampleRate;
-
-    // Replaygain info scanning
-    pos = d->data.find(ByteVector("RG"),pos);
-    if (pos>=0) {
-      int replayGainVersion = d->data[pos+3];
-      if (replayGainVersion==1) {
-        d->trackGain = d->data.mid(pos+4,2).toUInt(true);
-        d->trackPeak = d->data.mid(pos+6,2).toUInt(true);
-        d->albumGain = d->data.mid(pos+8,2).toUInt(true);
-        d->albumPeak = d->data.mid(pos+10,2).toUInt(true);
+      d->length = (d->sampleFrames - begSilence) / d->sampleRate;
+    }
+
+    else if (packetType == "RG") {
+      // Replay Gain
+      // http://trac.musepack.net/wiki/SV8Specification#ReplaygainPacket
+      ByteVector data = file->readBlock(dataSize);
+      readRG = true;
+
+      int replayGainVersion = data[0];
+      if(replayGainVersion == 1) {
+        d->trackGain = data.mid(1, 2).toUInt(true);
+        d->trackPeak = data.mid(3, 2).toUInt(true);
+        d->albumGain = data.mid(5, 2).toUInt(true);
+        d->albumPeak = data.mid(7, 2).toUInt(true);
       }
     }
 
-    return;
-  }
+    else if(packetType == "SE") {
+      break;
+    }
 
-  if(!d->data.startsWith("MP+"))
-    return;
+    else {
+      file->seek(dataSize, File::Current);
+    }
+  }
+}
 
-  d->version = d->data[3] & 15;
+void MPC::Properties::readSV7(const ByteVector &data)
+{
+  if(data.startsWith("MP+")) {
+    d->version = data[3] & 15;
+    if(d->version < 7)
+      return;
 
-  if(d->version >= 7) {
-    d->totalFrames = d->data.mid(4, 4).toUInt(false);
+    d->totalFrames = data.mid(4, 4).toUInt(false);
 
-    std::bitset<32> flags(TAGLIB_CONSTRUCT_BITSET(d->data.mid(8, 4).toUInt(false)));
+    std::bitset<32> flags(TAGLIB_CONSTRUCT_BITSET(data.mid(8, 4).toUInt(false)));
     d->sampleRate = sftable[flags[17] * 2 + flags[16]];
     d->channels = 2;
 
-    uint gapless = d->data.mid(5, 4).toUInt(false);
+    uint gapless = data.mid(5, 4).toUInt(false);
 
-    d->trackGain = d->data.mid(14,2).toShort(false);
-    d->trackPeak = d->data.mid(12,2).toUInt(false);
-    d->albumGain = d->data.mid(18,2).toShort(false);
-    d->albumPeak = d->data.mid(16,2).toUInt(false);
+    d->trackGain = data.mid(14,2).toShort(false);
+    d->trackPeak = data.mid(12,2).toUInt(false);
+    d->albumGain = data.mid(18,2).toShort(false);
+    d->albumPeak = data.mid(16,2).toUInt(false);
 
     // convert gain info
-    if (d->trackGain != 0) {
+    if(d->trackGain != 0) {
       int tmp = (int)((64.82 - (short)d->trackGain / 100.) * 256. + .5);
-      if (tmp >= (1 << 16) || tmp < 0) tmp = 0;
+      if(tmp >= (1 << 16) || tmp < 0) tmp = 0;
       d->trackGain = tmp;
     }
 
-  if (d->albumGain != 0) {
-    int tmp = (int)((64.82 - d->albumGain / 100.) * 256. + .5);
-    if (tmp >= (1 << 16) || tmp < 0) tmp = 0;
-    d->albumGain = tmp;
-  }
+    if(d->albumGain != 0) {
+      int tmp = (int)((64.82 - d->albumGain / 100.) * 256. + .5);
+      if(tmp >= (1 << 16) || tmp < 0) tmp = 0;
+      d->albumGain = tmp;
+    }
 
-  if (d->trackPeak != 0)
-    d->trackPeak = (int) (log10(d->trackPeak) * 20 * 256 + .5);
+    if (d->trackPeak != 0)
+      d->trackPeak = (int)(log10(d->trackPeak) * 20 * 256 + .5);
 
-  if (d->albumPeak != 0)
-    d->albumPeak = (int) (log10(d->albumPeak) * 20 * 256 + .5);
+    if (d->albumPeak != 0)
+      d->albumPeak = (int)(log10(d->albumPeak) * 20 * 256 + .5);
 
     bool trueGapless = (gapless >> 31) & 0x0001;
     if(trueGapless) {
@@ -254,7 +293,7 @@ void MPC::Properties::read()
       d->sampleFrames = d->totalFrames * 1152 - 576;
   }
   else {
-    uint headerData = d->data.mid(0, 4).toUInt(false);
+    uint headerData = data.mid(0, 4).toUInt(false);
 
     d->bitrate = (headerData >> 23) & 0x01ff;
     d->version = (headerData >> 11) & 0x03ff;
@@ -262,9 +301,9 @@ void MPC::Properties::read()
     d->channels = 2;
 
     if(d->version >= 5)
-      d->totalFrames = d->data.mid(4, 4).toUInt(false);
+      d->totalFrames = data.mid(4, 4).toUInt(false);
     else
-      d->totalFrames = d->data.mid(6, 2).toUInt(false);
+      d->totalFrames = data.mid(6, 2).toUInt(false);
 
     d->sampleFrames = d->totalFrames * 1152 - 576;
   }
@@ -274,3 +313,4 @@ void MPC::Properties::read()
   if(!d->bitrate)
     d->bitrate = d->length > 0 ? ((d->streamLength * 8L) / d->length) / 1000 : 0;
 }
+

diff --git a/taglib/mpc/mpcproperties.h b/taglib/mpc/mpcproperties.h
index f67d434d4243ddef950df8ed6b91986315549f04..adf40d83996a6305c52e4a76388cb5bc567d3271 100644 (file)
--- a/taglib/mpc/mpcproperties.h
+++ b/taglib/mpc/mpcproperties.h
@@ -36,7 +36,6 @@ namespace TagLib {
     class File;
 
     static const uint HeaderSize = 8*7;
-    static const char * V8MagicTitle = "MPCK";
 
     //! An implementation of audio property reading for MPC
 
@@ -51,9 +50,17 @@ namespace TagLib {
       /*!
        * Create an instance of MPC::Properties with the data read from the
        * ByteVector \a data.
+       *
+       * This constructor is deprecated. It only works for MPC version up to 7.
        */
       Properties(const ByteVector &data, long streamLength, ReadStyle style = Average);
 
+      /*!
+       * Create an instance of MPC::Properties with the data read directly
+       * from a MPC::File.
+       */
+      Properties(File *file, long streamLength, ReadStyle style = Average);
+
       /*!
        * Destroys this MPC::Properties instance.
        */
@@ -103,7 +110,8 @@ namespace TagLib {
       Properties(const Properties &);
       Properties &operator=(const Properties &);
 
-      void read();
+      void readSV7(const ByteVector &data);
+      void readSV8(File *file);
 
       class PropertiesPrivate;
       PropertiesPrivate *d;

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 67b295814274fb2fd5de00b344db0dfb52122ee3..d916d10e0168dc696911de0f9e2efe9082ad8180 100644 (file)
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -9,6 +9,7 @@ INCLUDE_DIRECTORIES(
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mpeg/id3v2
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mpeg/id3v2/frames
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mpeg
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mpc
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mp4
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/riff
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/riff/aiff
@@ -59,6 +60,7 @@ SET(test_runner_SRCS
   test_s3m.cpp
   test_it.cpp
   test_xm.cpp
+  test_mpc.cpp
 )
 
 ADD_EXECUTABLE(test_runner ${test_runner_SRCS})

diff --git a/tests/data/sv4_header.mpc b/tests/data/sv4_header.mpc
new file mode 100644 (file)
index 0000000..214f7ac
Binary files /dev/null and b/tests/data/sv4_header.mpc differ

diff --git a/tests/data/sv5_header.mpc b/tests/data/sv5_header.mpc
new file mode 100644 (file)
index 0000000..6d17e65
Binary files /dev/null and b/tests/data/sv5_header.mpc differ

diff --git a/tests/data/sv8_header.mpc b/tests/data/sv8_header.mpc
new file mode 100644 (file)
index 0000000..3405545
Binary files /dev/null and b/tests/data/sv8_header.mpc differ

diff --git a/tests/test_mpc.cpp b/tests/test_mpc.cpp
new file mode 100644 (file)
index 0000000..758263c
--- /dev/null
+++ b/tests/test_mpc.cpp
@@ -0,0 +1,62 @@
+#include <cppunit/extensions/HelperMacros.h>
+#include <string>
+#include <stdio.h>
+#include <tag.h>
+#include <tstringlist.h>
+#include <tbytevectorlist.h>
+#include <mpcfile.h>
+#include "utils.h"
+
+using namespace std;
+using namespace TagLib;
+
+class TestMPC : public CppUnit::TestFixture
+{
+  CPPUNIT_TEST_SUITE(TestMPC);
+  CPPUNIT_TEST(testPropertiesSV8);
+  CPPUNIT_TEST(testPropertiesSV7);
+  CPPUNIT_TEST(testPropertiesSV5);
+  CPPUNIT_TEST(testPropertiesSV4);
+  CPPUNIT_TEST_SUITE_END();
+
+public:
+
+  void testPropertiesSV8()
+  {
+    MPC::File f(TEST_FILE_PATH_C("sv8_header.mpc"));
+    CPPUNIT_ASSERT_EQUAL(1, f.audioProperties()->length());
+    CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrate());
+    CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
+    CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+  }
+
+  void testPropertiesSV7()
+  {
+    MPC::File f(TEST_FILE_PATH_C("click.mpc"));
+    CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->length());
+    CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrate());
+    CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
+    CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+  }
+
+  void testPropertiesSV5()
+  {
+    MPC::File f(TEST_FILE_PATH_C("sv5_header.mpc"));
+    CPPUNIT_ASSERT_EQUAL(26, f.audioProperties()->length());
+    CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrate());
+    CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
+    CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+  }
+
+  void testPropertiesSV4()
+  {
+    MPC::File f(TEST_FILE_PATH_C("sv4_header.mpc"));
+    CPPUNIT_ASSERT_EQUAL(26, f.audioProperties()->length());
+    CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrate());
+    CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
+    CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+  }
+
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestMPC);