MusePak: AudioProperties improvements

Add lengthInSeconds(), lengthInMilliseconds() properties. (#503)
Remove some data members which are not needed to carry.
Add some tests for audio properties.
Add some supplementary comments.

diff --git a/taglib/mpc/mpcproperties.cpp b/taglib/mpc/mpcproperties.cpp
index 128eff42361e639c961c3a5e7a3eb4b647862cdf..2eb2f2a69960161675c239921b632912638813c8 100644 (file)
--- a/taglib/mpc/mpcproperties.cpp
+++ b/taglib/mpc/mpcproperties.cpp
@@ -36,9 +36,7 @@ using namespace TagLib;
 class MPC::Properties::PropertiesPrivate
 {
 public:
-  PropertiesPrivate(long length, ReadStyle s) :
-    streamLength(length),
-    style(s),
+  PropertiesPrivate() :
     version(0),
     length(0),
     bitrate(0),
@@ -51,8 +49,6 @@ public:
     albumGain(0),
     albumPeak(0) {}
 
-  long streamLength;
-  ReadStyle style;
   int version;
   int length;
   int bitrate;
@@ -71,23 +67,25 @@ public:
 // public members
 ////////////////////////////////////////////////////////////////////////////////
 
-MPC::Properties::Properties(const ByteVector &data, long streamLength, ReadStyle style) : AudioProperties(style)
+MPC::Properties::Properties(const ByteVector &data, long streamLength, ReadStyle style) :
+  AudioProperties(style),
+  d(new PropertiesPrivate())
 {
-  d = new PropertiesPrivate(streamLength, style);
-  readSV7(data);
+  readSV7(data, streamLength);
 }
 
-MPC::Properties::Properties(File *file, long streamLength, ReadStyle style) : AudioProperties(style)
+MPC::Properties::Properties(File *file, long streamLength, ReadStyle style) :
+  AudioProperties(style),
+  d(new PropertiesPrivate())
 {
-  d = new PropertiesPrivate(streamLength, style);
   ByteVector magic = file->readBlock(4);
   if(magic == "MPCK") {
     // Musepack version 8
-    readSV8(file);
+    readSV8(file, streamLength);
   }
   else {
     // Musepack version 7 or older, fixed size header
-    readSV7(magic + file->readBlock(MPC::HeaderSize - 4));
+    readSV7(magic + file->readBlock(MPC::HeaderSize - 4), streamLength);
   }
 }
 
@@ -97,6 +95,16 @@ MPC::Properties::~Properties()
 }
 
 int MPC::Properties::length() const
+{
+  return lengthInSeconds();
+}
+
+int MPC::Properties::lengthInSeconds() const
+{
+  return d->length / 1000;
+}
+
+int MPC::Properties::lengthInMilliseconds() const
 {
   return d->length;
 }
@@ -189,11 +197,14 @@ unsigned long readSize(const ByteVector &data, TagLib::uint &pos)
   return size;
 }
 
-// This array looks weird, but the same as original MusePack code found at:
-// https://www.musepack.net/index.php?pg=src
-static const unsigned short sftable [8] = { 44100, 48000, 37800, 32000, 0, 0, 0, 0 };
+namespace
+{
+  // This array looks weird, but the same as original MusePack code found at:
+  // https://www.musepack.net/index.php?pg=src
+  const unsigned short sftable [8] = { 44100, 48000, 37800, 32000, 0, 0, 0, 0 };
+}
 
-void MPC::Properties::readSV8(File *file)
+void MPC::Properties::readSV8(File *file, long streamLength)
 {
   bool readSH = false, readRG = false;
 
@@ -236,7 +247,7 @@ void MPC::Properties::readSV8(File *file)
         break;
       }
 
-      ulong begSilence = readSize(data, pos);
+      const ulong begSilence = readSize(data, pos);
       if(pos > dataSize - 2) {
         debug("MPC::Properties::readSV8() - \"SH\" packet is corrupt.");
         break;
@@ -249,12 +260,12 @@ void MPC::Properties::readSV8(File *file)
       d->channels   = ((flags >> 4) & 0x0F) + 1;
 
       const uint frameCount = d->sampleFrames - begSilence;
-      if(frameCount != 0 && d->sampleRate != 0) {
-        d->bitrate = (int)(d->streamLength * 8.0 * d->sampleRate / frameCount / 1000);
-        d->length  = frameCount / d->sampleRate;
+      if(frameCount > 0 && d->sampleRate > 0) {
+        const double length = frameCount * 1000.0 / d->sampleRate;
+        d->length  = static_cast<int>(length + 0.5);
+        d->bitrate = static_cast<int>(streamLength * 8.0 / length + 0.5);
       }
     }
-
     else if (packetType == "RG") {
       // Replay Gain
       // http://trac.musepack.net/wiki/SV8Specification#ReplaygainPacket
@@ -266,7 +277,7 @@ void MPC::Properties::readSV8(File *file)
 
       readRG = true;
 
-      int replayGainVersion = data[0];
+      const int replayGainVersion = data[0];
       if(replayGainVersion == 1) {
         d->trackGain = data.toShort(1, true);
         d->trackPeak = data.toShort(3, true);
@@ -285,7 +296,7 @@ void MPC::Properties::readSV8(File *file)
   }
 }
 
-void MPC::Properties::readSV7(const ByteVector &data)
+void MPC::Properties::readSV7(const ByteVector &data, long streamLength)
 {
   if(data.startsWith("MP+")) {
     d->version = data[3] & 15;
@@ -294,11 +305,11 @@ void MPC::Properties::readSV7(const ByteVector &data)
 
     d->totalFrames = data.toUInt(4, false);
 
-    std::bitset<32> flags(TAGLIB_CONSTRUCT_BITSET(data.toUInt(8, false)));
-    d->sampleRate = sftable[flags[17] * 2 + flags[16]];
-    d->channels = 2;
+    const uint flags = data.toUInt(8, false);
+    d->sampleRate = sftable[(flags >> 16) & 0x03];
+    d->channels   = 2;
 
-    uint gapless = data.toUInt(5, false);
+    const uint gapless = data.toUInt(5, false);
 
     d->trackGain = data.toShort(14, false);
     d->trackPeak = data.toShort(12, false);
@@ -333,12 +344,12 @@ void MPC::Properties::readSV7(const ByteVector &data)
       d->sampleFrames = d->totalFrames * 1152 - 576;
   }
   else {
-    uint headerData = data.toUInt(0, false);
+    const uint headerData = data.toUInt(0, false);
 
-    d->bitrate = (headerData >> 23) & 0x01ff;
-    d->version = (headerData >> 11) & 0x03ff;
+    d->bitrate    = (headerData >> 23) & 0x01ff;
+    d->version    = (headerData >> 11) & 0x03ff;
     d->sampleRate = 44100;
-    d->channels = 2;
+    d->channels   = 2;
 
     if(d->version >= 5)
       d->totalFrames = data.toUInt(4, false);
@@ -348,9 +359,11 @@ void MPC::Properties::readSV7(const ByteVector &data)
     d->sampleFrames = d->totalFrames * 1152 - 576;
   }
 
-  d->length = d->sampleRate > 0 ? (d->sampleFrames + (d->sampleRate / 2)) / d->sampleRate : 0;
+  if(d->sampleFrames > 0 && d->sampleRate > 0) {
+    const double length = d->sampleFrames * 1000.0 / d->sampleRate;
+    d->length = static_cast<int>(length + 0.5);
 
-  if(!d->bitrate)
-    d->bitrate = d->length > 0 ? ((d->streamLength * 8L) / d->length) / 1000 : 0;
+    if(d->bitrate == 0)
+      d->bitrate = static_cast<int>(streamLength * 8.0 / length + 0.5);
+  }
 }
-

diff --git a/taglib/mpc/mpcproperties.h b/taglib/mpc/mpcproperties.h
index adf40d83996a6305c52e4a76388cb5bc567d3271..d6b066919f0432f0a9e5908dccdb2bd426f53b2c 100644 (file)
--- a/taglib/mpc/mpcproperties.h
+++ b/taglib/mpc/mpcproperties.h
@@ -66,17 +66,53 @@ namespace TagLib {
        */
       virtual ~Properties();
 
-      // Reimplementations.
-
+      /*!
+       * Returns the length of the file in seconds.  The length is rounded down to
+       * the nearest whole second.
+       *
+       * \note This method is just an alias of lengthInSeconds().
+       *
+       * \deprecated
+       */
       virtual int length() const;
+
+      /*!
+       * Returns the length of the file in seconds.  The length is rounded down to
+       * the nearest whole second.
+       *
+       * \see lengthInMilliseconds()
+       */
+      // BIC: make virtual
+      int lengthInSeconds() const;
+
+      /*!
+       * Returns the length of the file in milliseconds.
+       *
+       * \see lengthInSeconds()
+       */
+      // BIC: make virtual
+      int lengthInMilliseconds() const;
+
+      /*!
+       * Returns the average bit rate of the file in kb/s.
+       */
       virtual int bitrate() const;
+
+      /*!
+       * Returns the sample rate in Hz.
+       */
       virtual int sampleRate() const;
+
+      /*!
+       * Returns the number of audio channels.
+       */
       virtual int channels() const;
 
       /*!
        * Returns the version of the bitstream (SV4-SV8)
        */
       int mpcVersion() const;
+
       uint totalFrames() const;
       uint sampleFrames() const;
 
@@ -110,8 +146,8 @@ namespace TagLib {
       Properties(const Properties &);
       Properties &operator=(const Properties &);
 
-      void readSV7(const ByteVector &data);
-      void readSV8(File *file);
+      void readSV7(const ByteVector &data, long streamLength);
+      void readSV8(File *file, long streamLength);
 
       class PropertiesPrivate;
       PropertiesPrivate *d;

diff --git a/tests/test_mpc.cpp b/tests/test_mpc.cpp
index 0589740f80bd52c175fe9385531b22f265859a96..50a62d22a1b1ae1437604bb2bb9670cff445c4f0 100644 (file)
--- a/tests/test_mpc.cpp
+++ b/tests/test_mpc.cpp
@@ -28,41 +28,61 @@ public:
   void testPropertiesSV8()
   {
     MPC::File f(TEST_FILE_PATH_C("sv8_header.mpc"));
+    CPPUNIT_ASSERT(f.audioProperties());
     CPPUNIT_ASSERT_EQUAL(8, f.audioProperties()->mpcVersion());
     CPPUNIT_ASSERT_EQUAL(1, f.audioProperties()->length());
-    CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrate());
+    CPPUNIT_ASSERT_EQUAL(1, f.audioProperties()->lengthInSeconds());
+    CPPUNIT_ASSERT_EQUAL(1497, f.audioProperties()->lengthInMilliseconds());
+    CPPUNIT_ASSERT_EQUAL(1, f.audioProperties()->bitrate());
     CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
     CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+    CPPUNIT_ASSERT_EQUAL(66014U, f.audioProperties()->sampleFrames());
   }
 
   void testPropertiesSV7()
   {
     MPC::File f(TEST_FILE_PATH_C("click.mpc"));
+    CPPUNIT_ASSERT(f.audioProperties());
     CPPUNIT_ASSERT_EQUAL(7, f.audioProperties()->mpcVersion());
     CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->length());
-    CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrate());
+    CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->lengthInSeconds());
+    CPPUNIT_ASSERT_EQUAL(40, f.audioProperties()->lengthInMilliseconds());
+    CPPUNIT_ASSERT_EQUAL(318, f.audioProperties()->bitrate());
     CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
     CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+    CPPUNIT_ASSERT_EQUAL(1760U, f.audioProperties()->sampleFrames());
+    CPPUNIT_ASSERT_EQUAL(14221, f.audioProperties()->trackGain());
+    CPPUNIT_ASSERT_EQUAL(19848, f.audioProperties()->trackPeak());
+    CPPUNIT_ASSERT_EQUAL(14221, f.audioProperties()->albumGain());
+    CPPUNIT_ASSERT_EQUAL(19848, f.audioProperties()->albumPeak());
   }
 
   void testPropertiesSV5()
   {
     MPC::File f(TEST_FILE_PATH_C("sv5_header.mpc"));
+    CPPUNIT_ASSERT(f.audioProperties());
     CPPUNIT_ASSERT_EQUAL(5, f.audioProperties()->mpcVersion());
     CPPUNIT_ASSERT_EQUAL(26, f.audioProperties()->length());
+    CPPUNIT_ASSERT_EQUAL(26, f.audioProperties()->lengthInSeconds());
+    CPPUNIT_ASSERT_EQUAL(26371, f.audioProperties()->lengthInMilliseconds());
     CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrate());
     CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
     CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+    CPPUNIT_ASSERT_EQUAL(1162944U, f.audioProperties()->sampleFrames());
   }
 
   void testPropertiesSV4()
   {
     MPC::File f(TEST_FILE_PATH_C("sv4_header.mpc"));
+    CPPUNIT_ASSERT(f.audioProperties());
     CPPUNIT_ASSERT_EQUAL(4, f.audioProperties()->mpcVersion());
     CPPUNIT_ASSERT_EQUAL(26, f.audioProperties()->length());
+    CPPUNIT_ASSERT_EQUAL(26, f.audioProperties()->lengthInSeconds());
+    CPPUNIT_ASSERT_EQUAL(26371, f.audioProperties()->lengthInMilliseconds());
     CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrate());
     CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
     CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+    CPPUNIT_ASSERT_EQUAL(1162944U, f.audioProperties()->sampleFrames());
   }
 
   void testFuzzedFile1()