Skip to content

Loading…

AE: Fix segfault after indirection patches #2289

Merged
merged 1 commit into from

4 participants

@fritsch
Team Kodi member

After the indirection patches, we don't wait unitl OpenSink() is finished. So there can be race conditions, when we remove the master stream. Basically, when removing such a stream and deleting it, it must be set to NULL - as every other check in e.g. outputrawstage won't work correctly.

The first idea to use "boolean guards" (TM) was not really smart and only made it harder to reproduce - as it helped sometimes.

Basically:
FreeStream has a streamlock, it deletes / removes the stream there, sets it to NULL. OutputRawstream has the same lock, so comes in before or afterwards.

Having a dangeling pointer, e.g. not setting m_masterstream = NULL was just bad.
A nice good morning ping from germany goes to @davilla @wsnipex @pike @ronie

@ghost

do you plan on fxing up pap after your endevours? its back to eating a full core..

@fritsch
Team Kodi member

Hehe - i am pretty new to this stuff. I have some things on my todo list for AE. The last patch series enables Sleep Possibility on Alsa - so you should see really improved idle power usage. When the sink gets unloaded, it basically stays in SoftSuspend, e.g. ProcessSuspend(); and sleeps quite long naps.

Can you be more concrete on your mentioned issue and how to reproduce? As I did not realize increased CPU usage.

@fritsch
Team Kodi member
@pike2k
Team Kodi member

seems to fix the switching issue here on intel

@wsnipex
Team Kodi member

works for me.

edit: confirmed unrelated to this PR:
while testing I noticed that switching audio output from the audio OSD(while playing a video) does not work. I can cycle through hdmi, spdif, analog, but output device stays the same as configured in settings.

@davilla davilla merged commit 9a5b151 into xbmc:master
@fritsch
Team Kodi member

@davilla: Please also for frodo.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 23, 2013
  1. @fritsch
Showing with 5 additions and 3 deletions.
  1. +5 −3 xbmc/cores/AudioEngine/Engines/SoftAE/SoftAE.cpp
View
8 xbmc/cores/AudioEngine/Engines/SoftAE/SoftAE.cpp
@@ -728,7 +728,6 @@ void CSoftAE::PauseStream(CSoftAEStream *stream)
CSingleLock streamLock(m_streamLock);
RemoveStream(m_playingStreams, stream);
stream->m_paused = true;
- streamLock.Leave();
m_reOpen = true;
m_wake.Set();
@@ -874,7 +873,10 @@ IAEStream *CSoftAE::FreeStream(IAEStream *stream)
RemoveStream(m_streams , (CSoftAEStream*)stream);
// Reopen is old behaviour. Not opening when masterstream stops means clipping on S/PDIF.
if(!m_isSuspended && (m_masterStream == stream))
+ {
m_reOpen = true;
+ m_masterStream = NULL;
+ }
delete (CSoftAEStream*)stream;
return NULL;
@@ -1058,11 +1060,11 @@ void CSoftAE::Run()
bool restart = false;
/* with the new non blocking implementation - we just reOpen here, when it tells reOpen */
- if (!m_reOpen && (this->*m_outputStageFn)(hasAudio) > 0)
+ if ((this->*m_outputStageFn)(hasAudio) > 0)
hasAudio = false; /* taken some audio - reset our silence flag */
/* if we have enough room in the buffer */
- if (!m_reOpen && m_buffer.Free() >= m_frameSize)
+ if (m_buffer.Free() >= m_frameSize)
{
/* take some data for our use from the buffer */
uint8_t *out = (uint8_t*)m_buffer.Take(m_frameSize);
Something went wrong with that request. Please try again.