APE: AudioProperties improvements

Add lengthInSeconds(), lengthInMilliseconds() properties. (#503)
Enable to read bit depth from older version files. (#360)
Remove some data members which are not needed to carry.
Add some tests for audio properties.
Add some supplementary comments.

diff --git a/taglib/ape/apeproperties.cpp b/taglib/ape/apeproperties.cpp
index cf829fe593db0446a18e51d7e95ac64d2687820b..89a11be392c928baa52a9f4432e7536c267e9a9a 100644 (file)
--- a/taglib/ape/apeproperties.cpp
+++ b/taglib/ape/apeproperties.cpp
@@ -39,16 +39,14 @@ using namespace TagLib;
 class APE::Properties::PropertiesPrivate
 {
 public:
-  PropertiesPrivate(File *file, long streamLength) :
+  PropertiesPrivate() :
     length(0),
     bitrate(0),
     sampleRate(0),
     channels(0),
     version(0),
     bitsPerSample(0),
-    sampleFrames(0),
-    file(file),
-    streamLength(streamLength) {}
+    sampleFrames(0) {}
 
   int length;
   int bitrate;
@@ -57,18 +55,17 @@ public:
   int version;
   int bitsPerSample;
   uint sampleFrames;
-  File *file;
-  long streamLength;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 // public members
 ////////////////////////////////////////////////////////////////////////////////
 
-APE::Properties::Properties(File *file, ReadStyle style) : AudioProperties(style)
+APE::Properties::Properties(File *file, ReadStyle style) :
+  AudioProperties(style),
+  d(new PropertiesPrivate())
 {
-  d = new PropertiesPrivate(file, file->length());
-  read();
+  read(file);
 }
 
 APE::Properties::~Properties()
@@ -77,6 +74,16 @@ APE::Properties::~Properties()
 }
 
 int APE::Properties::length() const
+{
+  return lengthInSeconds();
+}
+
+int APE::Properties::lengthInSeconds() const
+{
+  return d->length / 1000;
+}
+
+int APE::Properties::lengthInMilliseconds() const
 {
   return d->length;
 }
@@ -115,36 +122,47 @@ TagLib::uint APE::Properties::sampleFrames() const
 // private members
 ////////////////////////////////////////////////////////////////////////////////
 
-
-void APE::Properties::read()
+void APE::Properties::read(File *file)
 {
   // First we are searching the descriptor
-  long offset = findDescriptor();
+  const long offset = findDescriptor(file);
   if(offset < 0)
     return;
 
   // Then we read the header common for all versions of APE
-  d->file->seek(offset);
-  ByteVector commonHeader = d->file->readBlock(6);
-  if(!commonHeader.startsWith("MAC "))
+  file->seek(offset);
+  const ByteVector commonHeader = file->readBlock(6);
+  if(commonHeader.size() < 6) {
+    debug("APE::Properties::read() -- header is too short.");
     return;
-  d->version = commonHeader.toUShort(4, false);
+  }
 
-  if(d->version >= 3980) {
-    analyzeCurrent();
+  if(!commonHeader.startsWith("MAC ")) {
+    debug("APE::Properties::read() -- invalid header signiture.");
+    return;
   }
-  else {
-    analyzeOld();
+
+  d->version = commonHeader.toUShort(4, false);
+
+  if(d->version >= 3980)
+    analyzeCurrent(file);
+  else
+    analyzeOld(file);
+
+  if(d->sampleFrames > 0 && d->sampleRate > 0) {
+    const double length = d->sampleFrames * 1000.0 / d->sampleRate;
+    d->length  = static_cast<int>(length + 0.5);
+    d->bitrate = static_cast<int>(file->length() * 8.0 / length + 0.5);
   }
 }
 
-long APE::Properties::findDescriptor()
+long APE::Properties::findDescriptor(File *file)
 {
-  long ID3v2Location = findID3v2();
+  const long ID3v2Location = findID3v2(file);
   long ID3v2OriginalSize = 0;
   bool hasID3v2 = false;
   if(ID3v2Location >= 0) {
-    ID3v2::Tag tag(d->file, ID3v2Location);
+    const ID3v2::Tag tag(file, ID3v2Location);
     ID3v2OriginalSize = tag.header()->completeTagSize();
     if(tag.header()->tagSize() > 0)
       hasID3v2 = true;
@@ -152,9 +170,9 @@ long APE::Properties::findDescriptor()
 
   long offset = 0;
   if(hasID3v2)
-    offset = d->file->find("MAC ", ID3v2Location + ID3v2OriginalSize);
+    offset = file->find("MAC ", ID3v2Location + ID3v2OriginalSize);
   else
-    offset = d->file->find("MAC ");
+    offset = file->find("MAC ");
 
   if(offset < 0) {
     debug("APE::Properties::findDescriptor() -- APE descriptor not found");
@@ -164,49 +182,63 @@ long APE::Properties::findDescriptor()
   return offset;
 }
 
-long APE::Properties::findID3v2()
+long APE::Properties::findID3v2(File *file)
 {
-  if(!d->file->isValid())
+  if(!file->isValid())
     return -1;
 
-  d->file->seek(0);
+  file->seek(0);
 
-  if(d->file->readBlock(3) == ID3v2::Header::fileIdentifier())
+  if(file->readBlock(3) == ID3v2::Header::fileIdentifier())
     return 0;
 
   return -1;
 }
 
-void APE::Properties::analyzeCurrent()
+void APE::Properties::analyzeCurrent(File *file)
 {
   // Read the descriptor
-  d->file->seek(2, File::Current);
-  ByteVector descriptor = d->file->readBlock(44);
+  file->seek(2, File::Current);
+  const ByteVector descriptor = file->readBlock(44);
+  if(descriptor.size() < 44) {
+    debug("APE::Properties::analyzeCurrent() -- descriptor is too short.");
+    return;
+  }
+
   const uint descriptorBytes = descriptor.toUInt(0, false);
 
-  if ((descriptorBytes - 52) > 0)
-    d->file->seek(descriptorBytes - 52, File::Current);
+  if((descriptorBytes - 52) > 0)
+    file->seek(descriptorBytes - 52, File::Current);
 
   // Read the header
-  ByteVector header = d->file->readBlock(24);
+  const ByteVector header = file->readBlock(24);
+  if(header.size() < 24) {
+    debug("APE::Properties::analyzeCurrent() -- MAC header is too short.");
+    return;
+  }
 
   // Get the APE info
   d->channels      = header.toShort(18, false);
   d->sampleRate    = header.toUInt(20, false);
   d->bitsPerSample = header.toShort(16, false);
-  //d->compressionLevel =
 
-  const uint totalFrames      = header.toUInt(12, false);
+  const uint totalFrames = header.toUInt(12, false);
+  if(totalFrames == 0)
+    return;
+
   const uint blocksPerFrame   = header.toUInt(4, false);
   const uint finalFrameBlocks = header.toUInt(8, false);
-  d->sampleFrames = totalFrames > 0 ? (totalFrames -  1) * blocksPerFrame + finalFrameBlocks : 0;
-  d->length = d->sampleRate > 0 ? d->sampleFrames / d->sampleRate : 0;
-  d->bitrate = d->length > 0 ? ((d->streamLength * 8L) / d->length) / 1000 : 0;
+  d->sampleFrames = (totalFrames - 1) * blocksPerFrame + finalFrameBlocks;
 }
 
-void APE::Properties::analyzeOld()
+void APE::Properties::analyzeOld(File *file)
 {
-  ByteVector header = d->file->readBlock(26);
+  const ByteVector header = file->readBlock(26);
+  if(header.size() < 26) {
+    debug("APE::Properties::analyzeOld() -- MAC header is too short.");
+    return;
+  }
+
   const uint totalFrames = header.toUInt(18, false);
 
   // Fail on 0 length APE files (catches non-finalized APE files)
@@ -222,19 +254,25 @@ void APE::Properties::analyzeOld()
   else
     blocksPerFrame = 9216;
 
+  // Get the APE info
   d->channels   = header.toShort(4, false);
   d->sampleRate = header.toUInt(6, false);
 
   const uint finalFrameBlocks = header.toUInt(22, false);
+  d->sampleFrames = (totalFrames - 1) * blocksPerFrame + finalFrameBlocks;
 
-  uint totalBlocks = 0;
-  if(totalFrames > 0)
-    totalBlocks = (totalFrames - 1) * blocksPerFrame + finalFrameBlocks;
+  // Seek the RIFF chunk and get the bit depth
+  long offset = file->tell();
+  offset = file->find("WAVEfmt ", offset);
+  if(offset < 0)
+    return;
 
-  if(d->sampleRate > 0)
-    d->length = totalBlocks / d->sampleRate;
+  file->seek(offset + 12);
+  const ByteVector fmt = file->readBlock(16);
+  if(fmt.size() < 16) {
+    debug("APE::Properties::analyzeOld() -- fmt header is too short.");
+    return;
+  }
 
-  if(d->length > 0)
-    d->bitrate = ((d->streamLength * 8L) / d->length) / 1000;
+  d->bitsPerSample = fmt.toShort(14, false);
 }
-

diff --git a/taglib/ape/apeproperties.h b/taglib/ape/apeproperties.h
index f154ec34249b9b4c93c9385efbcd778e8ee33af8..ce31f84cc37dea348f422cdae8d46f0b2f8572ac 100644 (file)
--- a/taglib/ape/apeproperties.h
+++ b/taglib/ape/apeproperties.h
@@ -60,17 +60,56 @@ 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 number of bits per sample.
+       * Returns the number of bits per audio sample.
        */
       int bitsPerSample() const;
+
+      /*!
+       * Returns the total number of audio samples in file.
+       */
       uint sampleFrames() const;
 
       /*!
@@ -82,13 +121,13 @@ namespace TagLib {
       Properties(const Properties &);
       Properties &operator=(const Properties &);
 
-      void read();
+      void read(File *file);
 
-      long findDescriptor();
-      long findID3v2();
+      long findDescriptor(File *file);
+      long findID3v2(File *file);
 
-      void analyzeCurrent();
-      void analyzeOld();
+      void analyzeCurrent(File *file);
+      void analyzeOld(File *file);
 
       class PropertiesPrivate;
       PropertiesPrivate *d;

diff --git a/tests/data/mac-399.ape b/tests/data/mac-399.ape
index ae895ba21cb8d6ca9561db087d4373d976503e3e..3b0661ee130f83142c18229bd5184e8b9f58ffd7 100644 (file)
Binary files a/tests/data/mac-399.ape and b/tests/data/mac-399.ape differ

diff --git a/tests/test_ape.cpp b/tests/test_ape.cpp
index 2f842f650a691f7062f741a1352daf6a714b5b0e..c2b0d3dc93063f99ebdc6d9967d15d21495498cd 100644 (file)
--- a/tests/test_ape.cpp
+++ b/tests/test_ape.cpp
@@ -25,28 +25,46 @@ public:
   void testProperties399()
   {
     APE::File f(TEST_FILE_PATH_C("mac-399.ape"));
+    CPPUNIT_ASSERT(f.audioProperties());
     CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->length());
-    CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrate());
+    CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->lengthInSeconds());
+    CPPUNIT_ASSERT_EQUAL(3550, f.audioProperties()->lengthInMilliseconds());
+    CPPUNIT_ASSERT_EQUAL(192, f.audioProperties()->bitrate());
     CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
     CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+    CPPUNIT_ASSERT_EQUAL(16, f.audioProperties()->bitsPerSample());
+    CPPUNIT_ASSERT_EQUAL(156556U, f.audioProperties()->sampleFrames());
+    CPPUNIT_ASSERT_EQUAL(3990, f.audioProperties()->version());
   }
 
   void testProperties396()
   {
     APE::File f(TEST_FILE_PATH_C("mac-396.ape"));
+    CPPUNIT_ASSERT(f.audioProperties());
     CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->length());
+    CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->lengthInSeconds());
+    CPPUNIT_ASSERT_EQUAL(3685, 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(16, f.audioProperties()->bitsPerSample());
+    CPPUNIT_ASSERT_EQUAL(162496U, f.audioProperties()->sampleFrames());
+    CPPUNIT_ASSERT_EQUAL(3960, f.audioProperties()->version());
   }
 
   void testProperties390()
   {
     APE::File f(TEST_FILE_PATH_C("mac-390-hdr.ape"));
+    CPPUNIT_ASSERT(f.audioProperties());
     CPPUNIT_ASSERT_EQUAL(15, f.audioProperties()->length());
+    CPPUNIT_ASSERT_EQUAL(15, f.audioProperties()->lengthInSeconds());
+    CPPUNIT_ASSERT_EQUAL(15630, 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(16, f.audioProperties()->bitsPerSample());
+    CPPUNIT_ASSERT_EQUAL(689262U, f.audioProperties()->sampleFrames());
+    CPPUNIT_ASSERT_EQUAL(3900, f.audioProperties()->version());
   }
 
   void testFuzzedFile1()