From cb1e2843dee85ed6b8b72f91d03471e296f5324f Mon Sep 17 00:00:00 2001 From: sukibaby <163092272+sukibaby@users.noreply.github.com> Date: Thu, 6 Jun 2024 15:35:04 +0000 Subject: [PATCH] Minor RageSound improvements Nothing major here - updating C style casts, moving repeated function calls into variables, changing lrint's... **RageSound.cpp** - Initialize m_pSource to nullptr in the member initializer list, rather than in the body of the constructor - Define an undefined variable `iSourceFrame` - Change a `lrint` to a `static_cast+0.5` - Implement missing error handling in `SetStopModeFromString` with a log message **RageSoundManager** - Combined the iterator increment and the erase operation for `Update` into one line to prevent needing to create the `next` variable, since GameLoop calls this method frequently **RageSoundReader** - This method is called from RageSound just after making sure iFrames isn't equal to 0, so it's not needed for RageSoundReader to do it again. - We will never fail to read a file 100 times. If we do, it's because of I/O errors, so make that more clear. --- src/RageSound.cpp | 24 +++++++++++++----------- src/RageSoundManager.cpp | 14 ++++++-------- src/RageSoundReader.cpp | 10 +++++----- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/RageSound.cpp b/src/RageSound.cpp index d26c9c9db6..072dca0d7a 100644 --- a/src/RageSound.cpp +++ b/src/RageSound.cpp @@ -66,12 +66,11 @@ RageSound::~RageSound() RageSound::RageSound( const RageSound &cpy ): RageSoundBase( cpy ), - m_Mutex( "RageSound" ) + m_Mutex( "RageSound" ), + m_pSource( nullptr ) { ASSERT(SOUNDMAN != nullptr); - m_pSource = nullptr; - *this = cpy; } @@ -283,7 +282,7 @@ int RageSound::GetDataToPlay( float *pBuffer, int iFrames, std::int64_t &iStream while( iFrames > 0 ) { float fRate = 1.0f; - int iSourceFrame; + int iSourceFrame = 0; /* Read data from our source. */ int iGotFrames = m_pSource->RetriedRead( pBuffer + (iFramesStored * m_pSource->GetNumChannels()), iFrames, &iSourceFrame, &fRate ); @@ -291,6 +290,7 @@ int RageSound::GetDataToPlay( float *pBuffer, int iFrames, std::int64_t &iStream if( iGotFrames == RageSoundReader::ERROR ) { m_sError = m_pSource->GetError(); + // This error probably indicates an I/O error, rather than a decoding error. LOG->Warn( "Decoding %s failed: %s", GetLoadedFilePath().c_str(), m_sError.c_str() ); } @@ -331,7 +331,7 @@ void RageSound::StartPlaying() ASSERT( !m_bPlaying ); // Move to the start position. - SetPositionFrames( std::lrint(m_Param.m_StartSecond * samplerate()) ); + SetPositionFrames( static_cast((m_Param.m_StartSecond * samplerate())) + 0.5 ); /* If m_StartTime is in the past, then we probably set a start time but took too * long loading. We don't want that; log it, since it can be unobvious. */ @@ -510,8 +510,9 @@ float RageSound::GetPositionSeconds( bool *bApproximate, RageTimer *pTimestamp ) *bApproximate = false; /* If we're not playing, just report the static position. */ + float sampleRate = static_cast(samplerate()); if( !IsPlaying() ) - return m_iStoppedSourceFrame / float(samplerate()); + return m_iStoppedSourceFrame / sampleRate; /* If we don't yet have any position data, CommitPlayingPosition hasn't yet been called at all, * so guess what we think the real time is. */ @@ -520,11 +521,11 @@ float RageSound::GetPositionSeconds( bool *bApproximate, RageTimer *pTimestamp ) // LOG->Trace( "no data yet; %i", m_iStoppedSourceFrame ); if( bApproximate ) *bApproximate = true; - return m_iStoppedSourceFrame / float(samplerate()); + return m_iStoppedSourceFrame / sampleRate; } int iSourceFrame = GetSourceFrameFromHardwareFrame( iCurrentHardwareFrame, bApproximate ); - return iSourceFrame / float(samplerate()); + return iSourceFrame / sampleRate; } @@ -539,16 +540,17 @@ bool RageSound::SetPositionFrames( int iFrames ) } int iRet = m_pSource->SetPosition( iFrames ); + RString filePath = GetLoadedFilePath(); if( iRet == -1 ) { m_sError = m_pSource->GetError(); - LOG->Warn( "SetPositionFrames: seek %s failed: %s", GetLoadedFilePath().c_str(), m_sError.c_str() ); + LOG->Warn( "SetPositionFrames: seek %s failed: %s", filePath.c_str(), m_sError.c_str() ); } else if( iRet == 0 ) { /* Seeked past EOF. */ LOG->Warn( "SetPositionFrames: %i samples is beyond EOF in %s", - iFrames, GetLoadedFilePath().c_str() ); + iFrames, filePath.c_str() ); } else { @@ -642,7 +644,7 @@ void RageSound::SetStopModeFromString( const RString &sStopMode ) } else { - // error + LOG->Warn("Invalid stop mode \"%s\" for sound \"%s\"", sStopMode.c_str(), m_sFilePath.c_str()); } } diff --git a/src/RageSoundManager.cpp b/src/RageSoundManager.cpp index 1723d32915..42533b4ee8 100644 --- a/src/RageSoundManager.cpp +++ b/src/RageSoundManager.cpp @@ -112,20 +112,18 @@ void RageSoundManager::Update() /* Scan m_mapPreloadedSounds for sounds that are no longer loaded, and delete them. */ g_SoundManMutex.Lock(); /* lock for access to m_mapPreloadedSounds, owned_sounds */ { - std::map::iterator it, next; - it = m_mapPreloadedSounds.begin(); - - while( it != m_mapPreloadedSounds.end() ) + for( auto it = m_mapPreloadedSounds.begin(); it != m_mapPreloadedSounds.end(); ) { - next = it; ++next; if( it->second->GetReferenceCount() == 1 ) { LOG->Trace( "Deleted old sound \"%s\"", it->first.c_str() ); delete it->second; - m_mapPreloadedSounds.erase( it ); + it = m_mapPreloadedSounds.erase(it); + } + else + { + ++it; } - - it = next; } } diff --git a/src/RageSoundReader.cpp b/src/RageSoundReader.cpp index 1e3f983dc9..0a9d45a55a 100644 --- a/src/RageSoundReader.cpp +++ b/src/RageSoundReader.cpp @@ -8,13 +8,10 @@ REGISTER_CLASS_TRAITS( RageSoundReader, pCopy->Copy() ); /* Read(), handling the STREAM_LOOPED and empty return cases. */ int RageSoundReader::RetriedRead( float *pBuffer, int iFrames, int *iSourceFrame, float *fRate ) { - if( iFrames == 0 ) - return 0; - /* pReader may return 0, which means "try again immediately". As a failsafe, * only try this a finite number of times. Use a high number, because in * principle each filter in the stack may cause this. */ - int iTries = 100; + int iTries = 10; while( --iTries ) { if( fRate ) @@ -29,9 +26,12 @@ int RageSoundReader::RetriedRead( float *pBuffer, int iFrames, int *iSourceFrame if( iGotFrames != 0 ) return iGotFrames; + + // If the user is having I/O issues, give them a hint in the logs. + LOG->Warn( "Read() failed, retrying..." ); } - LOG->Warn( "Read() busy looping" ); + LOG->Warn( "Read() returned a failure status after 10 attempts to read the file; likely an I/O error" ); /* Pretend we got EOF. */ return RageSoundReader::END_OF_FILE;