Demo Spectrum wav data size is not calculated correctly

- Grab length of audio data from Data chunk size
- Read from the device no greater than the audio Data chunk size

Task-number: QTBUG-13779
Reviewed-by: Andrew den Exter

Author: Justin McPherson

demos/spectrum/app/wavfile.cpp

@@ -76,80 +76,84 @@ struct CombinedHeader
 {
     RIFFHeader  riff;
     WAVEHeader  wave;
-    DATAHeader  data;
 };
 
-static const int HeaderLength = sizeof(CombinedHeader);
 
 
 WavFile::WavFile(const QAudioFormat &format, qint64 dataLength)
-    :   m_format(format)
-    ,   m_dataLength(dataLength)
+    : m_format(format)
+    , m_dataLength(dataLength)
+    , m_dataPosition(0)
 {
-
 }
 
 bool WavFile::readHeader(QIODevice &device)
 {
-    bool result = true;
-
-    if (!device.isSequential())
-        result = device.seek(0);
-    // else, assume that current position is the start of the header
-
-    if (result) {
-        CombinedHeader header;
-        result = (device.read(reinterpret_cast<char *>(&header), HeaderLength) == HeaderLength);
-        if (result) {
-            if ((memcmp(&header.riff.descriptor.id, "RIFF", 4) == 0
-                || memcmp(&header.riff.descriptor.id, "RIFX", 4) == 0)
-                && memcmp(&header.riff.type, "WAVE", 4) == 0
-                && memcmp(&header.wave.descriptor.id, "fmt ", 4) == 0
-                && header.wave.audioFormat == 1 // PCM
-            ) {
-                if (memcmp(&header.riff.descriptor.id, "RIFF", 4) == 0)
-                    m_format.setByteOrder(QAudioFormat::LittleEndian);
-                else
-                    m_format.setByteOrder(QAudioFormat::BigEndian);
-
-                m_format.setChannels(qFromLittleEndian<quint16>(header.wave.numChannels));
-                m_format.setCodec("audio/pcm");
-                m_format.setFrequency(qFromLittleEndian<quint32>(header.wave.sampleRate));
-                m_format.setSampleSize(qFromLittleEndian<quint16>(header.wave.bitsPerSample));
-
-                switch(header.wave.bitsPerSample) {
-                case 8:
-                    m_format.setSampleType(QAudioFormat::UnSignedInt);
-                    break;
-                case 16:
-                    m_format.setSampleType(QAudioFormat::SignedInt);
-                    break;
-                default:
-                    result = false;
-                }
-
-                m_dataLength = device.size() - HeaderLength;
-            } else {
-                result = false;
-            }
+    if (!device.isSequential()) {
+        if (!device.seek(0))
+            return false;
+        // XXX: else, assume that current position is the start of the header
+    }
+
+    CombinedHeader header;
+    if (device.read(reinterpret_cast<char *>(&header), sizeof(CombinedHeader)) != sizeof(CombinedHeader))
+        return false;
+
+    if ((memcmp(&header.riff.descriptor.id, "RIFF", 4) == 0
+        || memcmp(&header.riff.descriptor.id, "RIFX", 4) == 0)
+        && memcmp(&header.riff.type, "WAVE", 4) == 0
+        && memcmp(&header.wave.descriptor.id, "fmt ", 4) == 0
+        && (header.wave.audioFormat == 1 || header.wave.audioFormat == 0)) {
+
+        // Read off remaining header information
+        DATAHeader dataHeader;
+
+        if (qFromLittleEndian<quint32>(header.wave.descriptor.size) > sizeof(WAVEHeader)) {
+            // Extended data available
+            quint16 extraFormatBytes;
+            if (device.peek((char*)&extraFormatBytes, sizeof(quint16)) != sizeof(quint16))
+                return false;
+            const qint64 throwAwayBytes = sizeof(quint16) + qFromLittleEndian<quint16>(extraFormatBytes);
+            if (device.read(throwAwayBytes).size() != throwAwayBytes)
+                return false;
         }
+
+        if (device.read((char*)&dataHeader, sizeof(DATAHeader)) != sizeof(DATAHeader))
+            return false;
+
+        // Establish format
+        if (memcmp(&header.riff.descriptor.id, "RIFF", 4) == 0)
+            m_format.setByteOrder(QAudioFormat::LittleEndian);
+        else
+            m_format.setByteOrder(QAudioFormat::BigEndian);
+
+        int bps = qFromLittleEndian<quint16>(header.wave.bitsPerSample);
+        m_format.setChannels(qFromLittleEndian<quint16>(header.wave.numChannels));
+        m_format.setCodec("audio/pcm");
+        m_format.setFrequency(qFromLittleEndian<quint32>(header.wave.sampleRate));
+        m_format.setSampleSize(qFromLittleEndian<quint16>(header.wave.bitsPerSample));
+        m_format.setSampleType(bps == 8 ? QAudioFormat::UnSignedInt : QAudioFormat::SignedInt);
+
+        m_dataLength = qFromLittleEndian<quint32>(dataHeader.descriptor.size);
+        m_dataPosition = 0;
     }
 
-    return result;
+    return true;
 }
 
 bool WavFile::writeHeader(QIODevice &device)
 {
     CombinedHeader header;
+    DATAHeader dataHeader;
 
-    memset(&header, 0, HeaderLength);
+    memset(&header, 0, sizeof(CombinedHeader));
 
     // RIFF header
     if (m_format.byteOrder() == QAudioFormat::LittleEndian)
         strncpy(&header.riff.descriptor.id[0], "RIFF", 4);
     else
         strncpy(&header.riff.descriptor.id[0], "RIFX", 4);
-    qToLittleEndian<quint32>(quint32(m_dataLength + HeaderLength - 8),
+    qToLittleEndian<quint32>(quint32(m_dataLength + sizeof(CombinedHeader) + sizeof(DATAHeader) - sizeof(chunk)),
                              reinterpret_cast<unsigned char*>(&header.riff.descriptor.size));
     strncpy(&header.riff.type[0], "WAVE", 4);
 
@@ -171,11 +175,12 @@ bool WavFile::writeHeader(QIODevice &device)
                              reinterpret_cast<unsigned char*>(&header.wave.bitsPerSample));
 
     // DATA header
-    strncpy(&header.data.descriptor.id[0], "data", 4);
+    strncpy(dataHeader.descriptor.id, "data", 4);
     qToLittleEndian<quint32>(quint32(m_dataLength),
-                             reinterpret_cast<unsigned char*>(&header.data.descriptor.size));
+                             reinterpret_cast<unsigned char*>(&dataHeader.descriptor.size));
 
-    return (device.write(reinterpret_cast<const char *>(&header), HeaderLength) == HeaderLength);
+    return device.write(reinterpret_cast<const char *>(&header), sizeof(CombinedHeader)) == sizeof(CombinedHeader)
+           && device.write(reinterpret_cast<const char*>(&dataHeader), sizeof(DATAHeader)) == sizeof(DATAHeader);
 }
 
 const QAudioFormat& WavFile::format() const
@@ -190,7 +195,7 @@ qint64 WavFile::dataLength() const
 
 qint64 WavFile::headerLength()
 {
-    return HeaderLength;
+    return sizeof(CombinedHeader);
 }
 
 bool WavFile::writeDataLength(QIODevice &device, qint64 dataLength)
@@ -205,42 +210,47 @@ bool WavFile::writeDataLength(QIODevice &device, qint64 dataLength)
     return result;
 }
 
-#include <QFile>
-#include <QTextStream>
-
 qint64 WavFile::readData(QIODevice &device, QByteArray &buffer,
                          QAudioFormat outputFormat)
 {
-    if (QAudioFormat() == outputFormat)
+    // Sanity checks
+    if (!outputFormat.isValid())
         outputFormat = m_format;
 
+    if (!isPCMS16LE(outputFormat) || !isPCMS16LE(m_format))
+        return 0;
+
+    if (m_dataPosition == m_dataLength)
+        return 0;
+
+    // Process
     qint64 result = 0;
 
-    QFile file("wav.txt");
-    file.open(QIODevice::WriteOnly | QIODevice::Text);
-    QTextStream stream;
-    stream.setDevice(&file);
-
-    if (isPCMS16LE(outputFormat) && isPCMS16LE(m_format)) {
-        QVector<char> inputSample(2 * m_format.channels());
-
-        qint16 *output = reinterpret_cast<qint16*>(buffer.data());
-
-        while (result < buffer.size()) {
-            if (device.read(inputSample.data(), inputSample.count())) {
-                int inputIdx = 0;
-                for (int outputIdx = 0; outputIdx < outputFormat.channels(); ++outputIdx) {
-                    const qint16* input = reinterpret_cast<const qint16*>(inputSample.data() + 2 * inputIdx);
-                    *output++ = qFromLittleEndian<qint16>(*input);
-                    result += 2;
-                    if (inputIdx < m_format.channels())
-                        ++inputIdx;
-                }
-            } else {
-                break;
+    const int frameSize = 2 * m_format.channels(); // 16 bit samples
+    QVector<char> inputSample(frameSize);
+
+    qint16 *output = reinterpret_cast<qint16*>(buffer.data());
+
+    while (result < buffer.size()) {
+        if (m_dataPosition == m_dataLength)
+            break;
+
+        // XXX only working with particular alignments
+        if (device.read(inputSample.data(), inputSample.count())) {
+            int inputIdx = 0;
+            for (int outputIdx = 0; outputIdx < outputFormat.channels(); ++outputIdx) {
+                const qint16* input = reinterpret_cast<const qint16*>(inputSample.data() + 2 * inputIdx);
+                *output++ = qFromLittleEndian<qint16>(*input);
+                result += 2;
+                if (inputIdx < m_format.channels())
+                    ++inputIdx;
             }
+            m_dataPosition += frameSize;
+        } else {
+            break;
         }
     }
+
     return result;
 }
 

demos/spectrum/app/wavfile.h

@@ -77,6 +77,7 @@ class WavFile
 private:
     QAudioFormat m_format;
     qint64 m_dataLength;
+    qint64 m_dataPosition;
 };
 
 #endif