I didn't ever see a CR on this, so I looked at it. Changes look good. Eric
> -----Original Message----- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Liam Murray > Sent: Tuesday, March 22, 2005 8:49 PM > To: [EMAIL PROTECTED] > Cc: [EMAIL PROTECTED] > Subject: [Audio-dev] CR: Fix symbian audio device crash > > Summary > ========= > > Fixes crash in CHXAudioDevice::RunL(). (Thanks to Rajesh for > helping with > this.) > > Overview > ======== > > A crash would occur in RunL() on the following line. > > m_iTimer.After(iStatus, 100*1000 ); > > There were two problems (identified by Rajesh). 1) We were > calling this > when RunL() was being called with KErrCancel. The cancel was > generated from > a call to Pause() which cancels the timer. 2) We were not > checking to see > if the object was already active (the timer set). Above this > line call > OnTimeSync() and that call in some cases can result in a call back to > Resume(), which restarts the timer. > > My fix is (based on suggestions from Rajesh) : 1) only reset > the timer if > that status in RunL() is not an error or cancel. 2) ensure > the object is > not active before issueing the timer request. > > Additional changes: > > - added HXLOG (converted DPRINTF now that HXLOG works on Symbian) > > - added TIMESYNC_CALLBACK_INTERVAL_MS = 100 for magic number > > - detect and return oom condition in > HXSymbianAudioServerContext::Run() > > Branch > ====== > HEAD, Cay150 > > File modified > =========== > audio/device/auddevlib > audio/device/platform/symbian/audsymbian.cpp > audio/device/platform/symbian/audiosvr/audio_svr_cntxt.cpp > > Builds tested > =========== > Wins, Thumb > > - Liam > > > Index: auddevlib > =================================================================== > RCS file: /cvsroot/audio/device/auddevlib,v > retrieving revision 1.4 > diff -u -w -r1.4 auddevlib > --- auddevlib 13 Sep 2004 18:55:34 -0000 1.4 > +++ auddevlib 23 Mar 2005 01:02:38 -0000 > @@ -41,6 +41,7 @@ > project.AddModuleIncludes("common/include", > "common/container/pub", > "common/util/pub", > + "common/log/logutil/pub", > "common/system/pub", > "common/dbgtool/pub", > "common/runtime/pub", > Index: platform/symbian/audsymbian.cpp > =================================================================== > RCS file: /cvsroot/audio/device/platform/symbian/audsymbian.cpp,v > retrieving revision 1.30 > diff -u -w -r1.30 audsymbian.cpp > --- platform/symbian/audsymbian.cpp 14 Mar 2005 19:43:21 > -0000 1.30 > +++ platform/symbian/audsymbian.cpp 23 Mar 2005 01:02:38 -0000 > @@ -64,6 +64,9 @@ > #include "audsymbian.h" > static UINT32 Scale(UINT32 v, UINT32 f0, UINT32 f1, UINT32 > t0, UINT32 t1); > > +// interval between generated calls to OnTimeSync > +const UINT32 TIMESYNC_CALLBACK_INTERVAL_MS = 100; > + > CHXAudioDevice::CHXAudioDevice() > : CActive(EPriorityHigh), > m_lRefCount(0), > @@ -278,9 +281,10 @@ > m_pAudioStream->Play(); > } > > + //Resume timer that drives calls to OnTimeSync > if( !IsActive() ) > { > - m_iTimer.After(iStatus, 1000*100); > + m_iTimer.After(iStatus, > TIMESYNC_CALLBACK_INTERVAL_MS * 1000); > SetActive(); > } > > @@ -299,6 +303,7 @@ > m_pAudioStream->Pause(); > } > > + //Cancel timer so we stop calling OnTimeSync > m_iTimer.Cancel(); > > return res; > @@ -343,19 +348,29 @@ > > void CHXAudioDevice::RunL() > { > - if (iStatus != KErrCancel) > + if (KErrNone == iStatus.Int()) > { > - // call back the response object to update time > + //Check the time and call OnTimeSync > if( m_pDeviceResponse && !m_paused) > { > ULONG32 ulAudioTime = 0; > GetCurrentAudioTime(ulAudioTime); > m_pDeviceResponse->OnTimeSync(ulAudioTime); > } > - } > > - m_iTimer.After(iStatus, 100*1000 ); > + //Set timer for next OnTimeSync callback > + if (!IsActive()) > + { > + m_iTimer.After(iStatus, > TIMESYNC_CALLBACK_INTERVAL_MS * 1000); > SetActive(); > + } > + } > + else > + { > + //Errors (other than KErrCancel) not handled. We need > + //a well-defined way to notify higher-level code. > + HX_ASSERT(KErrCancel == iStatus.Int()); > + } > } > > void CHXAudioDevice::DoCancel() > Index: platform/symbian/audiosvr/audio_svr_cntxt.cpp > =================================================================== > RCS file: > /cvsroot/audio/device/platform/symbian/audiosvr/audio_svr_cntxt.cpp,v > retrieving revision 1.6 > diff -u -w -r1.6 audio_svr_cntxt.cpp > --- platform/symbian/audiosvr/audio_svr_cntxt.cpp 10 Feb > 2005 17:47:34 > -0000 1.6 > +++ platform/symbian/audiosvr/audio_svr_cntxt.cpp 23 Mar > 2005 01:02:38 -0000 > @@ -56,15 +56,11 @@ > * > */ > #include "hlxclib/stdlib.h" > -//#include "globals/hxglobals.h" > #include "hxglobalmgr_inst.h" > #include "audio_svr_cntxt.h" > #include "audio_svr.h" > #include "hxassert.h" > - > -#include "debug.h" > - > -#define D_SYMAUDIO D_INFO > +#include "hxtlogutil.h" > > > // arguments passed to audio server thread entry function > @@ -92,7 +88,7 @@ > HXSymbianAudioServerContext::~HXSymbianAudioServerContext() > { > // make sure server is stopped > - DPRINTF(D_SYMAUDIO, > ("HXSymbianAudioServerContext::~HXSymbianAudioServerContext()\n")); > + HXLOGL3(HXLOG_ADEV, > "HXSymbianAudioServerContext::~HXSymbianAudioServerContext()"); > Stop(); > > m_handle.Close(); > @@ -110,7 +106,7 @@ > // > void HXSymbianAudioServerContext::Start() > { > - DPRINTF(D_SYMAUDIO, > ("HXSymbianAudioServerContext::Start(): running = > %ld\n", (m_running ? 1 : 0))); > + HXLOGL3(HXLOG_ADEV, > "HXSymbianAudioServerContext::Start(): running = > %ld\n", m_running ? 1 : 0); > if (!m_running) > { > m_startSem.CreateLocal(0); > @@ -126,7 +122,7 @@ > // > void HXSymbianAudioServerContext::Stop() > { > - DPRINTF(D_SYMAUDIO, > ("HXSymbianAudioServerContext::Stop(): running = > %ld\n", (m_running ? 1 : 0))); > + HXLOGL3(HXLOG_ADEV, > "HXSymbianAudioServerContext::Stop(): running = > %ld\n", m_running ? 1 : 0); > if (m_running) > { > m_startSem.Wait(); > @@ -141,9 +137,10 @@ > > TInt HXSymbianAudioServerContext::Run() > { > + TInt err = KErrNone; > if (!m_running) > { > - DPRINTF(D_SYMAUDIO, ("HXSymbianAudioServerContext::Run(): > creating\n")); > + HXLOGL3(HXLOG_ADEV, > "HXSymbianAudioServerContext::Run(): creating"); > // close handle if previously opened > if (m_handleOpen) > { > @@ -152,24 +149,31 @@ > } > > AudioServerThreadArgs* pArgs = new AudioServerThreadArgs(); > - HX_ASSERT(pArgs); //XXXLCM > + if(pArgs) > + { > pArgs->pCtx = this; > pArgs->pGM = HXGlobalManInstance::GetInstance(); > > > - if (KErrNone == m_handle.Create(kHXSymbianAudioServer, > + err = m_handle.Create(kHXSymbianAudioServer, > > HXSymbianAudioServerContext::_Main, > kDefaultStack, > - /*NULL*/&User::Heap(), > pArgs, EOwnerThread)) > + /*NULL*/&User::Heap(), pArgs, EOwnerThread); > + if (KErrNone == err) > { > - DPRINTF(D_SYMAUDIO, > ("HXSymbianAudioServerContext::Run(): created > audio server thread\n")); > + HXLOGL3(HXLOG_ADEV, > "HXSymbianAudioServerContext::Run(): > created audio server thread"); > m_handleOpen = true; > m_handle.Resume(); > m_running = true; > } > } > + else > + { > + err = KErrNoMemory; > + } > + } > > - return m_running ? KErrNone : KErrGeneral; > + return err; > } > > // > @@ -182,7 +186,7 @@ > { > TRAPD(leaveCode, StartServerL()); > > - DPRINTF(D_SYMAUDIO, > ("HXSymbianAudioServerContext::Main(): leave code > = %ld\n", leaveCode)); > + HXLOGL3(HXLOG_ADEV, > "HXSymbianAudioServerContext::Main(): leave code = > %ld\n", leaveCode); > > if (leaveCode != KErrNone) > { > @@ -229,7 +233,7 @@ > AudioServerThreadArgs* pArgs = (AudioServerThreadArgs*)obj; > HX_ASSERT(pArgs); > > - //Install a handle to the global manager for this thread. > + // install a handle to the global manager for this thread. > HXGlobalManInstance::SetInstance(pArgs->pGM); > > > > > _______________________________________________ > Audio-dev mailing list > [email protected] > http://lists.helixcommunity.org/mailman/listinfo/audio-dev > _______________________________________________ Audio-dev mailing list [email protected] http://lists.helixcommunity.org/mailman/listinfo/audio-dev
