Permalink
Browse files

fixed, rework logic in SoftAE::ProcessSuspend to not consume CPU, dam…

…n vampires
  • Loading branch information...
davilla committed Dec 26, 2012
1 parent a6058c0 commit 79372867c6f1d5e14d6ca45b2f1267fef859f233
Showing with 14 additions and 7 deletions.
  1. +14 −7 xbmc/cores/AudioEngine/Engines/SoftAE/SoftAE.cpp
@@ -1388,15 +1388,21 @@ inline void CSoftAE::RemoveStream(StreamList &streams, CSoftAEStream *stream)
inline void CSoftAE::ProcessSuspend()
{
bool sinkIsSuspended = false;
+ unsigned int curSystemClock = 0;
- if (m_playingStreams.empty() && m_playing_sounds.empty() &&
- !m_softSuspend && !g_advancedSettings.m_streamSilence)
+ if (!m_softSuspend && m_playingStreams.empty() && m_playing_sounds.empty() &&
+ !g_advancedSettings.m_streamSilence)

This comment has been minimized.

Show comment Hide comment
@DDDamian

DDDamian Dec 26, 2012

Was it the order? ;) Does the fix in general allow you to remove the ifdefs around the call to ProcessSuspend added by TheUni as a workaround?

@DDDamian

DDDamian Dec 26, 2012

Was it the order? ;) Does the fix in general allow you to remove the ifdefs around the call to ProcessSuspend added by TheUni as a workaround?

This comment has been minimized.

Show comment Hide comment
@davilla

davilla Dec 26, 2012

Owner

Not sure, I changed the order because m_softSuspend check is faster than the .empty() checks. Since all these are &&'ed together, do the fast ones 1st.

I suspect the added else m_softSuspend = false or the double SystemClockMillis system call, the 1st would happen all the time which is silly. Both only need to happen if we are testing to soft suspend.

And yes, The ifdef removal around the call to ProcessSuspend would be a separate commit.

See here for what pivos is running.
Pivosgroup/xbmc@80763ee

@davilla

davilla Dec 26, 2012

Owner

Not sure, I changed the order because m_softSuspend check is faster than the .empty() checks. Since all these are &&'ed together, do the fast ones 1st.

I suspect the added else m_softSuspend = false or the double SystemClockMillis system call, the 1st would happen all the time which is silly. Both only need to happen if we are testing to soft suspend.

And yes, The ifdef removal around the call to ProcessSuspend would be a separate commit.

See here for what pivos is running.
Pivosgroup/xbmc@80763ee

This comment has been minimized.

Show comment Hide comment
@DDDamian

DDDamian Dec 26, 2012

On a quick test here it never falls into the suspend loop below as curSystemClock never exceeds m_softSuspendTimer - i.e. we never pass the 10-second countdown to drop into the wait state.

Gotta run out but will test more tonight.

@DDDamian

DDDamian Dec 26, 2012

On a quick test here it never falls into the suspend loop below as curSystemClock never exceeds m_softSuspendTimer - i.e. we never pass the 10-second countdown to drop into the wait state.

Gotta run out but will test more tonight.

This comment has been minimized.

Show comment Hide comment
@davilla

davilla Dec 26, 2012

Owner

quite possible I broke the auto suspend feature :)

@davilla

davilla Dec 26, 2012

Owner

quite possible I broke the auto suspend feature :)

{
m_softSuspend = true;
- m_softSuspendTimer = XbmcThreads::SystemClockMillis() + 10000; //10.0 second delay for softSuspend
+ // only one call to SystemClockMillis, please.
+ curSystemClock = XbmcThreads::SystemClockMillis();
+ m_softSuspendTimer = curSystemClock + 10000; //10.0 second delay for softSuspend
+ }
+ else
+ {
+ // some hole in the logic, set this or we will consume cpu and bork audio.
+ m_softSuspend = false;
}
-
- unsigned int curSystemClock = XbmcThreads::SystemClockMillis();
/* idle while in Suspend() state until Resume() called */
/* idle if nothing to play and user hasn't enabled */
@@ -1419,8 +1425,9 @@ inline void CSoftAE::ProcessSuspend()
sinkLock.Leave();
}
- /* idle for platform-defined time */
- m_wake.WaitMSec(SOFTAE_IDLE_WAIT_MSEC);
+ /* idle for platform-defined time if streams or sounds are empty */
+ if (m_playingStreams.empty() && m_playing_sounds.empty())
+ m_wake.WaitMSec(SOFTAE_IDLE_WAIT_MSEC);
/* check if we need to resume for stream or sound */
if (!m_isSuspended && (!m_playingStreams.empty() || !m_playing_sounds.empty()))

0 comments on commit 7937286

Please sign in to comment.