From 18975aef57250eb2822bf474cc8ded1334ba40e6 Mon Sep 17 00:00:00 2001 From: Craig Hansen-Sturm Date: Wed, 10 Sep 2014 16:48:35 -0700 Subject: [PATCH 1/3] log audio device framing errors, copy as much data as we can, but don't assert --- libraries/audio/src/AudioBuffer.h | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/libraries/audio/src/AudioBuffer.h b/libraries/audio/src/AudioBuffer.h index 2ef39430c1..0c82339dfd 100644 --- a/libraries/audio/src/AudioBuffer.h +++ b/libraries/audio/src/AudioBuffer.h @@ -110,8 +110,16 @@ public: if ( !_frameBuffer || !frames) { return; } - assert(channelCount <= _channelCountMax); - assert(frameCount <= _frameCountMax); + + if (channelCount >_channelCountMax || frameCount >_frameCountMax) { + qDebug() << "Audio framing error: _channelCount=" << _channelCount << "channelCountMax=" << _channelCountMax; + qDebug() << "Audio framing error: _frameCount=" << _frameCount << "frameCountMax=" << _frameCountMax; + + // This condition should never happen; however, we do our best to recover here copying as many frames + // as we have allocated + _channelCount = std::min(_channelCount,_channelCountMax); + _frameCount = std::min(_frameCount,_frameCountMax); + } _frameCount = frameCount; // we allow copying fewer frames than we've allocated _channelCount = channelCount; // we allow copying fewer channels that we've allocated From 0d5f28c7df9e51e1c290e512d4f9ca40b6a2a946 Mon Sep 17 00:00:00 2001 From: Craig Hansen-Sturm Date: Thu, 11 Sep 2014 11:05:27 -0700 Subject: [PATCH 2/3] audio buffer framing issue / comments --- libraries/audio/src/AudioBuffer.h | 40 ++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/libraries/audio/src/AudioBuffer.h b/libraries/audio/src/AudioBuffer.h index 0c82339dfd..1e52f8aa21 100644 --- a/libraries/audio/src/AudioBuffer.h +++ b/libraries/audio/src/AudioBuffer.h @@ -111,19 +111,41 @@ public: return; } - if (channelCount >_channelCountMax || frameCount >_frameCountMax) { - qDebug() << "Audio framing error: _channelCount=" << _channelCount << "channelCountMax=" << _channelCountMax; - qDebug() << "Audio framing error: _frameCount=" << _frameCount << "frameCountMax=" << _frameCountMax; + if (channelCount <=_channelCountMax && frameCount <=_frameCountMax) { + // We always allow copying fewer frames than we have allocated + _frameCount = frameCount; + _channelCount = channelCount; + } + else { + // + // However we do not attempt to copy more frames than we've allocated ;-) This is a framing error caused by either + // a/ the platform audio driver not queuing and smoothing out device IO capture frames -or- + // b/ our IO processing thread (currently running on a Qt GUI thread) has been delayed/scheduled too late. + // + // The fix is not to make the problem worse by allocating more on this thread, rather, it is to handle dynamic + // re-sizing off the IO processing thread. While a/ is not iin our control, we will address the off thread + // re-sizing and Qt GUI IO thread issue in later releases. + // + // For now, we log this condition, and do our best to recover by copying as many frames as we have allocated. + // Unfortunately, this will result (temporarily), in a discontinuity. + // + // If you repeatedly receive this error, contact craig@highfidelity.io and send me what audio device you are using, + // what audio-stack you are using (pulse/alsa, core audio, ...), what OS, and what the reported frame/channel + // counts are. + qDebug() << "Audio framing error: _channelCount=" + << _channelCount + << "channelCountMax=" + << _channelCountMax + << "_frameCount=" + << _frameCount + << "frameCountMax=" + << _frameCountMax; + - // This condition should never happen; however, we do our best to recover here copying as many frames - // as we have allocated _channelCount = std::min(_channelCount,_channelCountMax); _frameCount = std::min(_frameCount,_frameCountMax); } - - _frameCount = frameCount; // we allow copying fewer frames than we've allocated - _channelCount = channelCount; // we allow copying fewer channels that we've allocated - + if (copyOut) { S* dst = frames; From 893d4686db1913b79c8cf2f609f0cb27d6dd7106 Mon Sep 17 00:00:00 2001 From: Craig Hansen-Sturm Date: Thu, 11 Sep 2014 11:18:05 -0700 Subject: [PATCH 3/3] comments --- libraries/audio/src/AudioBuffer.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/libraries/audio/src/AudioBuffer.h b/libraries/audio/src/AudioBuffer.h index 1e52f8aa21..863bbdf22a 100644 --- a/libraries/audio/src/AudioBuffer.h +++ b/libraries/audio/src/AudioBuffer.h @@ -119,19 +119,21 @@ public: else { // // However we do not attempt to copy more frames than we've allocated ;-) This is a framing error caused by either - // a/ the platform audio driver not queuing and smoothing out device IO capture frames -or- + // a/ the platform audio driver not correctly queuing and regularly smoothing device IO capture frames -or- // b/ our IO processing thread (currently running on a Qt GUI thread) has been delayed/scheduled too late. // - // The fix is not to make the problem worse by allocating more on this thread, rather, it is to handle dynamic - // re-sizing off the IO processing thread. While a/ is not iin our control, we will address the off thread - // re-sizing and Qt GUI IO thread issue in later releases. + // The fix is not to make the problem worse by allocating additional frames on this thread, rather, it is to handle + // dynamic re-sizing off the IO processing thread. While a/ is not in our control, we will address the off thread + // re-sizing,, as well as b/, in later releases. // // For now, we log this condition, and do our best to recover by copying as many frames as we have allocated. - // Unfortunately, this will result (temporarily), in a discontinuity. + // Unfortunately, this will result (temporarily), in an audible discontinuity. // // If you repeatedly receive this error, contact craig@highfidelity.io and send me what audio device you are using, // what audio-stack you are using (pulse/alsa, core audio, ...), what OS, and what the reported frame/channel - // counts are. + // counts are. In addition, any information about what you were doing at the time of the discontinuity, would be + // useful (e.g., accessing any client features/menus) + // qDebug() << "Audio framing error: _channelCount=" << _channelCount << "channelCountMax="