From bcf965462f07e7772754777aeb64dab9591ba0b0 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 1 Sep 2010 14:04:10 +0100 Subject: Fixed buffer overrun in Symbian QAudioInput backend Task-number: QTBUG-13058 Reviewed-by: Derick Hawcroft --- src/multimedia/audio/qaudioinput_symbian_p.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/multimedia/audio/qaudioinput_symbian_p.cpp b/src/multimedia/audio/qaudioinput_symbian_p.cpp index 9d240ca..9ae4368 100644 --- a/src/multimedia/audio/qaudioinput_symbian_p.cpp +++ b/src/multimedia/audio/qaudioinput_symbian_p.cpp @@ -373,7 +373,8 @@ qint64 QAudioInputPrivate::read(char *data, qint64 len) TDesC8 &inputBuffer = buffer->Data(); - const qint64 inputBytes = bytesReady(); + Q_ASSERT(inputBuffer.Length() >= m_devSoundBufferPos); + const qint64 inputBytes = inputBuffer.Length() - m_devSoundBufferPos; const qint64 outputBytes = len - bytesRead; const qint64 copyBytes = outputBytes < inputBytes ? outputBytes : inputBytes; @@ -384,7 +385,7 @@ qint64 QAudioInputPrivate::read(char *data, qint64 len) data += copyBytes; bytesRead += copyBytes; - if (!bytesReady()) + if (inputBytes == copyBytes) bufferEmptied(); } @@ -403,13 +404,14 @@ void QAudioInputPrivate::pullData() TDesC8 &inputBuffer = buffer->Data(); - const qint64 inputBytes = bytesReady(); + Q_ASSERT(inputBuffer.Length() >= m_devSoundBufferPos); + const qint64 inputBytes = inputBuffer.Length() - m_devSoundBufferPos; const qint64 bytesPushed = m_sink->write( (char*)inputBuffer.Ptr() + m_devSoundBufferPos, inputBytes); m_devSoundBufferPos += bytesPushed; - if (!bytesReady()) + if (inputBytes == bytesPushed) bufferEmptied(); if (!bytesPushed) -- cgit v0.12 From 496f49b4398f8ff70e50236a71b1b221183171ca Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 8 Sep 2010 13:41:54 +0100 Subject: Discard all DevSound buffers held when QAudioInput::suspend() called Resuming recording causes buffers previously provided to the client (via MDevSoundObserver::BufferToBeEmptied()) to be invalidated. The buffers therefore must be discarded when recording is suspended. Task-number: QTBUG-13058 Reviewed-by: Derick Hawcroft --- src/multimedia/audio/qaudioinput_symbian_p.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/multimedia/audio/qaudioinput_symbian_p.cpp b/src/multimedia/audio/qaudioinput_symbian_p.cpp index 9ae4368..b837055 100644 --- a/src/multimedia/audio/qaudioinput_symbian_p.cpp +++ b/src/multimedia/audio/qaudioinput_symbian_p.cpp @@ -178,10 +178,9 @@ void QAudioInputPrivate::suspend() const qint64 samplesRecorded = getSamplesRecorded(); m_totalSamplesRecorded += samplesRecorded; - if (m_devSoundBuffer) { - m_devSoundBufferQ.append(m_devSoundBuffer); - m_devSoundBuffer = 0; - } + m_devSoundBuffer = 0; + m_devSoundBufferQ.clear(); + m_devSoundBufferPos = 0; setState(SymbianAudio::SuspendedState); } -- cgit v0.12 From 7ed96ae38b3eaee5988ac4d4717ff5c7611d0f15 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 1 Sep 2010 13:47:16 +0100 Subject: Permit QAudioOutput::processedUSecs() to be called immediately after start() If QAudioOutput::processedUSecs() is called very soon after QAudioOutput::start(), the DevSound instance owned by the Symbian backend may still be initializing. This patch causes the function to return zero, rather than failing an assertion. Task-number: QTBUG-13059 Reviewed-by: Derick Hawcroft --- src/multimedia/audio/qaudio_symbian_p.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/multimedia/audio/qaudio_symbian_p.cpp b/src/multimedia/audio/qaudio_symbian_p.cpp index 4522c5c..ae58b94 100644 --- a/src/multimedia/audio/qaudio_symbian_p.cpp +++ b/src/multimedia/audio/qaudio_symbian_p.cpp @@ -432,15 +432,16 @@ bool DevSoundWrapper::isFormatSupported(const QAudioFormat &format) const int DevSoundWrapper::samplesProcessed() const { - Q_ASSERT(StateInitialized == m_state); int result = 0; - switch (m_mode) { - case QAudio::AudioInput: - result = m_devsound->SamplesRecorded(); - break; - case QAudio::AudioOutput: - result = m_devsound->SamplesPlayed(); - break; + if (StateInitialized == m_state) { + switch (m_mode) { + case QAudio::AudioInput: + result = m_devsound->SamplesRecorded(); + break; + case QAudio::AudioOutput: + result = m_devsound->SamplesPlayed(); + break; + } } return result; } -- cgit v0.12 From 1f3c8e102c026b7bfb163234a8f87cc0b6863246 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 1 Sep 2010 13:49:54 +0100 Subject: Discard empty buffer on call to QAudioOutput::resume() The Symbian backend holds buffers which are passed to it from DevSound, exposing the memory of these buffers via the QIODevice interface. When QAudioOutput::resume() is called, the backend re-starts data flow by passing the buffer it holds back to DevSound. Previously, this would not happen if the buffer was empty, potentially causing playback to stall. This patch ensures that the buffer is always sent back to DevSound, and data flow therefore always resumes. Task-number: QTBUG-13059 Reviewed-by: Derick Hawcroft --- src/multimedia/audio/qaudiooutput_symbian_p.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/multimedia/audio/qaudiooutput_symbian_p.cpp b/src/multimedia/audio/qaudiooutput_symbian_p.cpp index 5098469..5054ae3 100644 --- a/src/multimedia/audio/qaudiooutput_symbian_p.cpp +++ b/src/multimedia/audio/qaudiooutput_symbian_p.cpp @@ -210,7 +210,7 @@ void QAudioOutputPrivate::suspend() void QAudioOutputPrivate::resume() { if (SymbianAudio::SuspendedState == m_internalState) { - if (!m_pullMode && m_devSoundBuffer && m_devSoundBuffer->Data().Length()) + if (!m_pullMode && m_devSoundBuffer) bufferFilled(); startPlayback(); } -- cgit v0.12 From 5b89aac0a2e317f59c3437c7a9d1a9ee51182714 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 1 Sep 2010 13:52:46 +0100 Subject: Suppress overflow errors raised by Symbian DevSound during playback When QAudioOutput::suspend() and QAudioOutput::resume() are called repeatedly, with a short delay between each call, DevSound occasionally raises a KErrOverflow error. The backend previously translated this into QAudio::IOError, causing the object to transition into the QAudio::Stopped state. This error can be safely ignored, with playback resuming as soon as more audio data is provided to DevSound. This patch therefore suppresses the error. Task-number: QTBUG-13059 Reviewed-by: Derick Hawcroft --- src/multimedia/audio/qaudiooutput_symbian_p.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/multimedia/audio/qaudiooutput_symbian_p.cpp b/src/multimedia/audio/qaudiooutput_symbian_p.cpp index 5054ae3..c8b6ecf 100644 --- a/src/multimedia/audio/qaudiooutput_symbian_p.cpp +++ b/src/multimedia/audio/qaudiooutput_symbian_p.cpp @@ -371,6 +371,9 @@ void QAudioOutputPrivate::devsoundPlayError(int err) else setState(SymbianAudio::IdleState); break; + case KErrOverflow: + // Silently consume this error when in playback mode + break; default: setError(QAudio::IOError); break; -- cgit v0.12 From 15f6338e3984e103728a66de2f25e09e19f9077e Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 8 Sep 2010 15:18:06 +0100 Subject: Discard buffer pointer when DevSound is stopped Task-number: QTBUG-13504 Reviewed-by: Derick Hawcroft --- src/multimedia/audio/qaudiooutput_symbian_p.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/multimedia/audio/qaudiooutput_symbian_p.cpp b/src/multimedia/audio/qaudiooutput_symbian_p.cpp index c8b6ecf..cc32f9d 100644 --- a/src/multimedia/audio/qaudiooutput_symbian_p.cpp +++ b/src/multimedia/audio/qaudiooutput_symbian_p.cpp @@ -195,6 +195,7 @@ void QAudioOutputPrivate::suspend() // lost data with silence following a call to resume(), in order to // ensure that processedUSecs() returns the correct value. m_devSound->stop(); + m_devSoundBuffer = 0; m_totalSamplesPlayed += samplesPlayed; // Calculate the amount of data dropped -- cgit v0.12 From 82a0342fa076b4872c439079822bad61023af7da Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 14 Sep 2010 15:02:03 +0100 Subject: Implement QAudioInput::suspend() using CMMFDevSound::Stop() As with playback mode, lack of support for CMMFDevSound::Pause() in DevSound's recording mode causes problems on some devices. Specifically, while QAudioInput works fine on the Nokia 5800, this bug was observed on the Nokia N8. This fix means that suspending and resuming audio input will work correctly on all devices. Task-number: QTBUG-13506 Reviewed-by: Derick Hawcroft --- src/multimedia/audio/qaudio_symbian_p.cpp | 25 ++++++++++++-- src/multimedia/audio/qaudio_symbian_p.h | 15 +++++++-- src/multimedia/audio/qaudioinput_symbian_p.cpp | 30 +++++++++++------ src/multimedia/audio/qaudiooutput_symbian_p.cpp | 45 +++++++++++-------------- 4 files changed, 73 insertions(+), 42 deletions(-) diff --git a/src/multimedia/audio/qaudio_symbian_p.cpp b/src/multimedia/audio/qaudio_symbian_p.cpp index ae58b94..59fc05f 100644 --- a/src/multimedia/audio/qaudio_symbian_p.cpp +++ b/src/multimedia/audio/qaudio_symbian_p.cpp @@ -313,7 +313,8 @@ QAudio::State stateNativeToQt(State nativeState) return QAudio::ActiveState; case IdleState: return QAudio::IdleState; - case SuspendedState: + case SuspendedPausedState: + case SuspendedStoppedState: return QAudio::SuspendedState; default: Q_ASSERT_X(false, Q_FUNC_INFO, "Invalid state"); @@ -476,10 +477,22 @@ bool DevSoundWrapper::start() return (KErrNone == err); } -void DevSoundWrapper::pause() +bool DevSoundWrapper::pause() { Q_ASSERT(StateInitialized == m_state); - m_devsound->Pause(); + const bool canPause = isResumeSupported(); + if (canPause) + m_devsound->Pause(); + else + stop(); + return canPause; +} + +void DevSoundWrapper::resume() +{ + Q_ASSERT(StateInitialized == m_state); + Q_ASSERT(isResumeSupported()); + // TODO: QTBUG-13625 } void DevSoundWrapper::stop() @@ -558,6 +571,12 @@ void DevSoundWrapper::populateCapabilities() } } +bool DevSoundWrapper::isResumeSupported() const +{ + // TODO: QTBUG-13625 + return false; +} + void DevSoundWrapper::InitializeComplete(TInt aError) { Q_ASSERT(StateInitializing == m_state); diff --git a/src/multimedia/audio/qaudio_symbian_p.h b/src/multimedia/audio/qaudio_symbian_p.h index 58ef192..84a93d1 100644 --- a/src/multimedia/audio/qaudio_symbian_p.h +++ b/src/multimedia/audio/qaudio_symbian_p.h @@ -81,7 +81,10 @@ enum State { , InitializingState , ActiveState , IdleState - , SuspendedState + // QAudio is suspended; DevSound is paused + , SuspendedPausedState + // QAudio is suspended; DevSound is stopped + , SuspendedStoppedState }; /** @@ -117,7 +120,14 @@ public: int samplesProcessed() const; bool setFormat(const QAudioFormat &format); bool start(); - void pause(); + + // If DevSound implementation supports pause, calls pause and returns true. + // Otherwise calls stop and returns false. In this case, all DevSound buffers + // currently held by the backend must be discarded. + bool pause(); + + void resume(); + void stop(); void bufferProcessed(); @@ -140,6 +150,7 @@ signals: private: void getSupportedCodecs(); void populateCapabilities(); + bool isResumeSupported() const; private: const QAudio::Mode m_mode; diff --git a/src/multimedia/audio/qaudioinput_symbian_p.cpp b/src/multimedia/audio/qaudioinput_symbian_p.cpp index b837055..485c695 100644 --- a/src/multimedia/audio/qaudioinput_symbian_p.cpp +++ b/src/multimedia/audio/qaudioinput_symbian_p.cpp @@ -174,22 +174,30 @@ void QAudioInputPrivate::suspend() || SymbianAudio::IdleState == m_internalState) { m_notifyTimer->stop(); m_pullTimer->stop(); - m_devSound->pause(); const qint64 samplesRecorded = getSamplesRecorded(); m_totalSamplesRecorded += samplesRecorded; - m_devSoundBuffer = 0; - m_devSoundBufferQ.clear(); - m_devSoundBufferPos = 0; - - setState(SymbianAudio::SuspendedState); + const bool paused = m_devSound->pause(); + if (paused) { + if (m_devSoundBuffer) + m_devSoundBufferQ.append(m_devSoundBuffer); + m_devSoundBuffer = 0; + setState(SymbianAudio::SuspendedPausedState); + } else { + m_devSoundBuffer = 0; + m_devSoundBufferQ.clear(); + m_devSoundBufferPos = 0; + setState(SymbianAudio::SuspendedStoppedState); + } } } void QAudioInputPrivate::resume() { - if (SymbianAudio::SuspendedState == m_internalState) { - if (!m_pullMode && !bytesReady()) + if (QAudio::SuspendedState == m_externalState) { + if (SymbianAudio::SuspendedPausedState == m_internalState) + m_devSound->resume(); + else m_devSound->start(); startDataTransfer(); } @@ -245,7 +253,7 @@ int QAudioInputPrivate::notifyInterval() const qint64 QAudioInputPrivate::processedUSecs() const { int samplesPlayed = 0; - if (m_devSound && SymbianAudio::SuspendedState != m_internalState) + if (m_devSound && QAudio::SuspendedState != m_externalState) samplesPlayed = getSamplesRecorded(); // Protect against division by zero @@ -334,7 +342,7 @@ void QAudioInputPrivate::startDataTransfer() if (!m_pullMode) pushData(); } else { - if (SymbianAudio::SuspendedState == m_internalState) + if (QAudio::SuspendedState == m_externalState) setState(SymbianAudio::ActiveState); else setState(SymbianAudio::IdleState); @@ -442,7 +450,7 @@ void QAudioInputPrivate::devsoundBufferToBeEmptied(CMMFBuffer *baseBuffer) m_totalBytesReady += buffer->Data().Length(); - if (SymbianAudio::SuspendedState == m_internalState) { + if (SymbianAudio::SuspendedPausedState == m_internalState) { m_devSoundBufferQ.append(buffer); } else { // Will be returned to DevSoundWrapper by bufferProcessed(). diff --git a/src/multimedia/audio/qaudiooutput_symbian_p.cpp b/src/multimedia/audio/qaudiooutput_symbian_p.cpp index cc32f9d..ea14e19 100644 --- a/src/multimedia/audio/qaudiooutput_symbian_p.cpp +++ b/src/multimedia/audio/qaudiooutput_symbian_p.cpp @@ -180,40 +180,33 @@ void QAudioOutputPrivate::suspend() || SymbianAudio::IdleState == m_internalState) { m_notifyTimer->stop(); m_underflowTimer->stop(); - const qint64 samplesWritten = SymbianAudio::Utils::bytesToSamples( m_format, m_bytesWritten); - const qint64 samplesPlayed = getSamplesPlayed(); - - m_bytesWritten = 0; - - // CMMFDevSound::Pause() is not guaranteed to work correctly in all - // implementations, for play-mode DevSound sessions. We therefore - // have to implement suspend() by calling CMMFDevSound::Stop(). - // Because this causes buffered data to be dropped, we replace the - // lost data with silence following a call to resume(), in order to - // ensure that processedUSecs() returns the correct value. - m_devSound->stop(); - m_devSoundBuffer = 0; m_totalSamplesPlayed += samplesPlayed; - - // Calculate the amount of data dropped - const qint64 paddingSamples = samplesWritten - samplesPlayed; - Q_ASSERT(paddingSamples >= 0); - m_bytesPadding = SymbianAudio::Utils::samplesToBytes(m_format, - paddingSamples); - - setState(SymbianAudio::SuspendedState); + m_bytesWritten = 0; + const bool paused = m_devSound->pause(); + if (paused) { + setState(SymbianAudio::SuspendedPausedState); + } else { + m_devSoundBuffer = 0; + // Calculate the amount of data dropped + const qint64 paddingSamples = samplesWritten - samplesPlayed; + Q_ASSERT(paddingSamples >= 0); + m_bytesPadding = SymbianAudio::Utils::samplesToBytes(m_format, + paddingSamples); + setState(SymbianAudio::SuspendedStoppedState); + } } } void QAudioOutputPrivate::resume() { - if (SymbianAudio::SuspendedState == m_internalState) { - if (!m_pullMode && m_devSoundBuffer) - bufferFilled(); - startPlayback(); + if (QAudio::SuspendedState == m_externalState) { + if (SymbianAudio::SuspendedPausedState == m_internalState) + m_devSound->resume(); + else + startPlayback(); } } @@ -271,7 +264,7 @@ int QAudioOutputPrivate::notifyInterval() const qint64 QAudioOutputPrivate::processedUSecs() const { int samplesPlayed = 0; - if (m_devSound && SymbianAudio::SuspendedState != m_internalState) + if (m_devSound && QAudio::SuspendedState != m_externalState) samplesPlayed = getSamplesPlayed(); // Protect against division by zero -- cgit v0.12