Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

ActiveAE: fix for iOS - suspend sink when paused and app is not focused #4292

Merged
merged 2 commits into from

6 participants

@FernetMenta
Collaborator

see title

@Memphiz as discussed on irc, IDispResource has a new method you can call from windowing

@fritsch
Collaborator

jenkins build this please

@Memphiz
Owner

Nice - will have time to test it after the weekend i think.

@Memphiz
Owner

Well works as intended i guess - but its still not the final goal for my use case :)

  1. When app is not focussed (i.e. its backgrounded) - after i hit "pause" in the player the engine waits the keep alive time until itdeinits the sink - meaning the playbutton in the ios control center appears after that (longish) time
  2. When i turn off keep alive - it still needs 10 secs after pause until the sink gets deinitialized (this is still longish for this use case from a user experience pov) - though the drain is instantly.

Not sure how my sink should behave now - should it stop the audio output after drain? (atm it continues to put out silence).

@FernetMenta
Collaborator

looks like ActiveAE does not get the focussed event. Where is your code? I want to have a look. Did you enable IDispResource for iOS?

@Memphiz
Owner

Yes i have ... but it seems i need to tell the code so:

https://github.com/xbmc/xbmc/blob/master/xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp#L168

I try with making this a bit less restrictive ;)

@Memphiz
Owner

Now that activeae registeres itself for the idispresource on ios too its working like a champ :D - nice work :)

@FernetMenta
Collaborator

great to hear that it works. now you, @t-nelson , and @jmarshallnz have control over this pr.

@Memphiz
Owner

yep ... fyi - those are the changes which are ment to be PR'ed by me - after this PR is in ...

https://github.com/Memphiz/xbmc/commits/iosbackgroundmusic

@Memphiz
Owner

Just some more background for the RMs - this is needed for restoring the background music playback feature on ios after switching to ActiveAE. (so - fixing a regression here).

xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp
@@ -235,6 +235,9 @@ void CActiveAE::StateMachine(int signal, Protocol *port, Message *msg)
return;
case CActiveAEControlProtocol::DISPLAYRESET:
return;
+ case CActiveAEControlProtocol::APPFOCUSSED:
@t-nelson
t-nelson added a note

Too many S's :)

@FernetMenta Collaborator

grr, I was uncertain and looked up the word: http://www.dict.cc/?s=focussed

better trust a native speaker, I will change it

@t-nelson
t-nelson added a note

Haha yeah apparently this one is contended enough to justify its own webpage.

http://www.future-perfect.co.uk/grammar-tip/is-it-focussed-or-focused/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone
xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp
@@ -458,13 +461,20 @@ void CActiveAE::StateMachine(int signal, Protocol *port, Message *msg)
case CActiveAEControlProtocol::PAUSESTREAM:
CActiveAEStream *stream;
stream = *(CActiveAEStream**)msg->data;
+ bool streaming;
@t-nelson
t-nelson added a note

I think you're creating some odd scoping issues here. You only get a new stack frame in a case statement if you declare a context in it with {}. I think this instance of bool streaming is being used in the next case statement as well.

I'd either declare the contexts and redeclare the local for each, or just move the declaration to the top of the function.

@fritsch Collaborator
fritsch added a note

We had that yesterday. The compiler will create a "lable" streaming. When you ever access that one with only assigning, e.g. streaming = false or streaming = true, then it's safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@FernetMenta FernetMenta commented on the diff
xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp
@@ -392,13 +395,14 @@ void CActiveAE::StateMachine(int signal, Protocol *port, Message *msg)
case AE_TOP_CONFIGURED:
if (port == &m_controlPort)
{
+ bool streaming;
@FernetMenta Collaborator

@t-nelson I moved it up here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@FernetMenta
Collaborator

@t-nelson updated to single 's' focused

xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp
((5 lines not shown))
switch (signal)
{
case CActiveAEControlProtocol::RECONFIGURE:
if (m_streams.empty())
{
- bool silence = false;
- m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::SILENCEMODE, &silence, sizeof(bool));
+ bool streaming = false;
@t-nelson
t-nelson added a note

Do you still want a declaration here?

@FernetMenta Collaborator

certainly not, grr :) force push in a second

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@t-nelson

It kinda seems like this is tainting your active/actor model a bit. But I have no better suggestion and it's yours to maintain. I won't hold this up once we're happy with the scoping in the switches.

@FernetMenta
Collaborator

I think I should put {} after each case statement but better do this after release.

@Memphiz
Owner

@t-nelson @jmarshallnz this needs a gotham tag (needed for fixing the regression on ios music background playing)

@FernetMenta FernetMenta added the Gotham label
@FernetMenta
Collaborator

@t-nelson @jmarshallnz any reason why this is not pulled in?

@jmarshallnz
Owner

jenkins build this please

@jmarshallnz jmarshallnz merged commit 28f60ed into from
@jmarshallnz jmarshallnz removed the Gotham label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
27 xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp
@@ -235,6 +235,9 @@ void CActiveAE::StateMachine(int signal, Protocol *port, Message *msg)
return;
case CActiveAEControlProtocol::DISPLAYRESET:
return;
+ case CActiveAEControlProtocol::APPFOCUSED:
+ m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::APPFOCUSED, msg->data, sizeof(bool));
+ return;
default:
break;
}
@@ -392,13 +395,14 @@ void CActiveAE::StateMachine(int signal, Protocol *port, Message *msg)
case AE_TOP_CONFIGURED:
if (port == &m_controlPort)
{
+ bool streaming;
@FernetMenta Collaborator

@t-nelson I moved it up here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
switch (signal)
{
case CActiveAEControlProtocol::RECONFIGURE:
if (m_streams.empty())
{
- bool silence = false;
- m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::SILENCEMODE, &silence, sizeof(bool));
+ streaming = false;
+ m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::STREAMING, &streaming, sizeof(bool));
}
LoadSettings();
ChangeResamplers();
@@ -459,12 +463,18 @@ void CActiveAE::StateMachine(int signal, Protocol *port, Message *msg)
CActiveAEStream *stream;
stream = *(CActiveAEStream**)msg->data;
if (stream->m_paused != true && m_streams.size() == 1)
+ {
FlushEngine();
+ streaming = false;
+ m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::STREAMING, &streaming, sizeof(bool));
+ }
stream->m_paused = true;
return;
case CActiveAEControlProtocol::RESUMESTREAM:
stream = *(CActiveAEStream**)msg->data;
stream->m_paused = false;
+ streaming = true;
+ m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::STREAMING, &streaming, sizeof(bool));
m_extTimeout = 0;
return;
case CActiveAEControlProtocol::FLUSHSTREAM:
@@ -973,8 +983,8 @@ void CActiveAE::Configure(AEAudioFormat *desiredFmt)
sinkInputFormat = inputFormat;
m_internalFormat = inputFormat;
- bool silence = false;
- m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::SILENCEMODE, &silence, sizeof(bool));
+ bool streaming = false;
+ m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::STREAMING, &streaming, sizeof(bool));
delete m_encoder;
m_encoder = NULL;
@@ -998,8 +1008,8 @@ void CActiveAE::Configure(AEAudioFormat *desiredFmt)
// resample buffers for streams
else
{
- bool silence = true;
- m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::SILENCEMODE, &silence, sizeof(bool));
+ bool streaming = true;
+ m_sink.m_controlPort.SendOutMessage(CSinkControlProtocol::STREAMING, &streaming, sizeof(bool));
AEAudioFormat outputFormat;
if (m_mode == MODE_RAW)
@@ -2353,6 +2363,11 @@ void CActiveAE::OnResetDevice()
m_controlPort.SendOutMessage(CActiveAEControlProtocol::DISPLAYRESET);
}
+void CActiveAE::OnAppFocusChange(bool focus)
+{
+ m_controlPort.SendOutMessage(CActiveAEControlProtocol::APPFOCUSED, &focus, sizeof(focus));
+}
+
//-----------------------------------------------------------------------------
// Utils
//-----------------------------------------------------------------------------
View
2  xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.h
@@ -89,6 +89,7 @@ class CActiveAEControlProtocol : public Protocol
GETSTATE,
DISPLAYLOST,
DISPLAYRESET,
+ APPFOCUSED,
KEEPCONFIG,
TIMEOUT,
};
@@ -233,6 +234,7 @@ class CActiveAE : public IAE, private CThread
virtual void OnLostDevice();
virtual void OnResetDevice();
+ virtual void OnAppFocusChange(bool focus);
protected:
void PlaySound(CActiveAESound *sound);
View
42 xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAESink.cpp
@@ -177,6 +177,7 @@ void CActiveAESink::StateMachine(int signal, Protocol *port, Message *msg)
}
m_extError = false;
m_extSilenceTimer = 0;
+ m_extStreaming = false;
ReturnBuffers();
OpenSink();
@@ -216,6 +217,12 @@ void CActiveAESink::StateMachine(int signal, Protocol *port, Message *msg)
msg->Reply(CSinkControlProtocol::ACC);
return;
+ case CSinkControlProtocol::APPFOCUSED:
+ m_extAppFocused = *(bool*)msg->data;
+ SetSilenceTimer();
+ m_extTimeout = 0;
+ return;
+
default:
break;
}
@@ -275,19 +282,14 @@ void CActiveAESink::StateMachine(int signal, Protocol *port, Message *msg)
{
switch (signal)
{
- case CSinkControlProtocol::SILENCEMODE:
- bool silencemode;
- silencemode = *(bool*)msg->data;
- if (silencemode)
- m_extSilenceTimeout = XbmcThreads::EndTime::InfiniteValue;
- else
- m_extSilenceTimeout = CSettings::Get().GetInt("audiooutput.streamsilence") * 60000;
- m_extSilenceTimer.Set(m_extSilenceTimeout);
+ case CSinkControlProtocol::STREAMING:
+ m_extStreaming = *(bool*)msg->data;
+ SetSilenceTimer();
if (!m_extSilenceTimer.IsTimePast())
{
m_state = S_TOP_CONFIGURED_SILENCE;
- m_extTimeout = 0;
}
+ m_extTimeout = 0;
return;
case CSinkControlProtocol::VOLUME:
m_volume = *(float*)msg->data;
@@ -339,7 +341,10 @@ void CActiveAESink::StateMachine(int signal, Protocol *port, Message *msg)
{
switch (signal)
{
- case CSinkControlProtocol::SILENCEMODE:
+ case CSinkControlProtocol::STREAMING:
+ m_extStreaming = *(bool*)msg->data;
+ SetSilenceTimer();
+ m_extTimeout = 0;
return;
case CSinkControlProtocol::VOLUME:
m_volume = *(float*)msg->data;
@@ -427,7 +432,10 @@ void CActiveAESink::StateMachine(int signal, Protocol *port, Message *msg)
{
m_sink->Drain();
m_state = S_TOP_CONFIGURED_IDLE;
- m_extTimeout = 10000;
+ if (m_extAppFocused)
+ m_extTimeout = 10000;
+ else
+ m_extTimeout = 0;
}
return;
default:
@@ -477,6 +485,7 @@ void CActiveAESink::Process()
m_state = S_TOP_UNCONFIGURED;
m_extTimeout = 1000;
m_bStateMachineSelfTrigger = false;
+ m_extAppFocused = true;
while (!m_bStop)
{
@@ -905,3 +914,14 @@ void CActiveAESink::GenerateNoise()
convertFn(noise, nb_floats, m_sampleOfSilence.pkt->data[0]);
_aligned_free(noise);
}
+
+void CActiveAESink::SetSilenceTimer()
+{
+ if (m_extStreaming)
+ m_extSilenceTimeout = XbmcThreads::EndTime::InfiniteValue;
+ else if (m_extAppFocused)
+ m_extSilenceTimeout = CSettings::Get().GetInt("audiooutput.streamsilence") * 60000;
+ else
+ m_extSilenceTimeout = 0;
+ m_extSilenceTimer.Set(m_extSilenceTimeout);
+}
View
6 xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAESink.h
@@ -57,7 +57,8 @@ class CSinkControlProtocol : public Protocol
{
CONFIGURE,
UNCONFIGURE,
- SILENCEMODE,
+ STREAMING,
+ APPFOCUSED,
VOLUME,
FLUSH,
TIMEOUT,
@@ -108,6 +109,7 @@ class CActiveAESink : private CThread
void GetDeviceFriendlyName(std::string &device);
void OpenSink();
void ReturnBuffers();
+ void SetSilenceTimer();
unsigned int OutputSamples(CSampleBuffer* samples);
void ConvertInit(CSampleBuffer* samples);
@@ -123,6 +125,8 @@ class CActiveAESink : private CThread
int m_extTimeout;
bool m_extError;
unsigned int m_extSilenceTimeout;
+ bool m_extAppFocused;
+ bool m_extStreaming;
XbmcThreads::EndTime m_extSilenceTimer;
CSampleBuffer m_sampleOfSilence;
View
1  xbmc/guilib/DispResource.h
@@ -27,6 +27,7 @@ class IDispResource
virtual ~IDispResource() {};
virtual void OnLostDevice() {};
virtual void OnResetDevice() {};
+ virtual void OnAppFocusChange(bool focus) {};
};
#endif
Something went wrong with that request. Please try again.