Ogg Speex: AudioProperties improvements

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

taglib/ogg/speex/speexproperties.cpp

@@ -41,21 +41,19 @@ using namespace TagLib::Ogg;
 class Speex::Properties::PropertiesPrivate
 {
 public:
-  PropertiesPrivate(File *f, ReadStyle s) :
-    file(f),
-    style(s),
+  PropertiesPrivate() :
     length(0),
     bitrate(0),
+    bitrateNominal(0),
     sampleRate(0),
     channels(0),
     speexVersion(0),
     vbr(false),
     mode(0) {}
 
-  File *file;
-  ReadStyle style;
   int length;
   int bitrate;
+  int bitrateNominal;
   int sampleRate;
   int channels;
   int speexVersion;
@@ -67,10 +65,11 @@ public:
 // public members
 ////////////////////////////////////////////////////////////////////////////////
 
-Speex::Properties::Properties(File *file, ReadStyle style) : AudioProperties(style)
+Speex::Properties::Properties(File *file, ReadStyle style) :
+  AudioProperties(style),
+  d(new PropertiesPrivate())
 {
-  d = new PropertiesPrivate(file, style);
-  read();
+  read(file);
 }
 
 Speex::Properties::~Properties()
@@ -79,13 +78,28 @@ Speex::Properties::~Properties()
 }
 
 int Speex::Properties::length() const
+{
+  return lengthInSeconds();
+}
+
+int Speex::Properties::lengthInSeconds() const
+{
+  return d->length / 1000;
+}
+
+int Speex::Properties::lengthInMilliseconds() const
 {
   return d->length;
 }
 
 int Speex::Properties::bitrate() const
 {
-  return int(float(d->bitrate) / float(1000) + 0.5);
+  return d->bitrate;
+}
+
+int Speex::Properties::bitrateNominal() const
+{
+  return d->bitrateNominal;
 }
 
 int Speex::Properties::sampleRate() const
@@ -107,11 +121,15 @@ int Speex::Properties::speexVersion() const
 // private members
 ////////////////////////////////////////////////////////////////////////////////
 
-void Speex::Properties::read()
+void Speex::Properties::read(File *file)
 {
   // Get the identification header from the Ogg implementation.
 
-  ByteVector data = d->file->packet(0);
+  const ByteVector data = file->packet(0);
+  if(data.size() < 64) {
+    debug("Speex::Properties::read() -- data is too short.");
+    return;
+  }
 
   uint pos = 28;
 
@@ -138,7 +156,7 @@ void Speex::Properties::read()
   pos += 4;
 
   // bitrate;                /**< Bit-rate used */
-  d->bitrate = data.toUInt(pos, false);
+  d->bitrateNominal = data.toUInt(pos, false);
   pos += 4;
 
   // frame_size;             /**< Size of frames */
@@ -152,19 +170,32 @@ void Speex::Properties::read()
   // frames_per_packet;      /**< Number of frames stored per Ogg packet */
   // unsigned int framesPerPacket = data.mid(pos, 4).toUInt(false);
 
-  const Ogg::PageHeader *first = d->file->firstPageHeader();
-  const Ogg::PageHeader *last = d->file->lastPageHeader();
+  const Ogg::PageHeader *first = file->firstPageHeader();
+  const Ogg::PageHeader *last  = file->lastPageHeader();
 
   if(first && last) {
-    long long start = first->absoluteGranularPosition();
-    long long end = last->absoluteGranularPosition();
+    const long long start = first->absoluteGranularPosition();
+    const long long end   = last->absoluteGranularPosition();
 
-    if(start >= 0 && end >= 0 && d->sampleRate > 0)
-      d->length = (int) ((end - start) / (long long) d->sampleRate);
-    else
+    if(start >= 0 && end >= 0 && d->sampleRate > 0) {
+      const long long frameCount = end - start;
+
+      if(frameCount > 0) {
+        const double length = frameCount * 1000.0 / d->sampleRate;
+        d->length  = static_cast<int>(length + 0.5);
+        d->bitrate = static_cast<int>(file->length() * 8.0 / length + 0.5);
+      }
+    }
+    else {
       debug("Speex::Properties::read() -- Either the PCM values for the start or "
             "end of this file was incorrect or the sample rate is zero.");
+    }
   }
   else
     debug("Speex::Properties::read() -- Could not find valid first and last Ogg pages.");
+
+  // Alternative to the actual average bitrate.
+
+  if(d->bitrate == 0 && d->bitrateNominal > 0)
+    d->bitrate = static_cast<int>(d->bitrateNominal / 1000.0 + 0.5);
 }

taglib/ogg/speex/speexproperties.h

@@ -61,11 +61,51 @@ 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 nominal bit rate as read from the Speex header in kb/s.
+         */
+        int bitrateNominal() const;
+
+        /*!
+         * Returns the sample rate in Hz.
+         */
         virtual int sampleRate() const;
+
+        /*!
+         * Returns the number of audio channels.
+         */
         virtual int channels() const;
 
         /*!
@@ -77,7 +117,7 @@ namespace TagLib {
         Properties(const Properties &);
         Properties &operator=(const Properties &);
 
-        void read();
+        void read(File *file);
 
         class PropertiesPrivate;
         PropertiesPrivate *d;

tests/CMakeLists.txt

@@ -65,6 +65,7 @@ SET(test_runner_SRCS
   test_xm.cpp
   test_mpc.cpp
   test_opus.cpp
+  test_speex.cpp
 )
 
 INCLUDE_DIRECTORIES(${CPPUNIT_INCLUDE_DIR})

tests/test_speex.cpp

@@ -0,0 +1,31 @@
+#include <speexfile.h>
+#include <cppunit/extensions/HelperMacros.h>
+#include "utils.h"
+
+using namespace std;
+using namespace TagLib;
+
+class TestSpeex : public CppUnit::TestFixture
+{
+  CPPUNIT_TEST_SUITE(TestSpeex);
+  CPPUNIT_TEST(testAudioProperties);
+  CPPUNIT_TEST_SUITE_END();
+
+public:
+
+  void testAudioProperties()
+  {
+    Ogg::Speex::File f(TEST_FILE_PATH_C("empty.spx"));
+    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(53, f.audioProperties()->bitrate());
+    CPPUNIT_ASSERT_EQUAL(-1, f.audioProperties()->bitrateNominal());
+    CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels());
+    CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+  }
+
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestSpeex);