Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

[ios] - restore the music background playback ability #4350

Merged
merged 6 commits into from

5 participants

@Memphiz
Owner

This is the final Pull Request for restoring the background music playback feature on iOS (which was broken since new iossink was introduced).

It relies on the now merged PR #4292

Please wait for my go (will do some paranoia tests again)

@Memphiz Memphiz added the Gotham label
@jmarshallnz jmarshallnz commented on the diff
xbmc/windowing/osx/WinSystemIOS.mm
@@ -318,6 +320,28 @@
return rtn;
}
+void CWinSystemIOS::Register(IDispResource *resource)
+{
+ CSingleLock lock(m_resourceSection);
+ m_resources.push_back(resource);
+}
+
+void CWinSystemIOS::Unregister(IDispResource* resource)
+{
+ CSingleLock lock(m_resourceSection);
+ std::vector<IDispResource*>::iterator i = find(m_resources.begin(), m_resources.end(), resource);
+ if (i != m_resources.end())
+ m_resources.erase(i);
@jmarshallnz Owner

In case you haven't heard of it, std::remove() can be useful in these situations. There's a bit of a gotcha, however, in that you need to combine it with std::erase() to get rid of unneeded items at the end of the list.

Note that it's not a requirement at all to use it, but it is designed for just this case.

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

Sry line commenting is broken on iPhone - That erase thingy is 1:1 c&p from winsystemosx ;)

@Memphiz
Owner

Ok - i tested this once more and it works on ios5, ios6 and ios7.

Sometimes i encoutered crashs in CGUITextureManager::FreeUnusedTextures when glDeleteTextures(1, (GLuint*) &m_unusedHwTextures[i]); was called.

It seems that when we are backgrounded we are supposed to completely shutdown rendering - we do so but we hold references to unused textures here which might get deleted by ios when we are backgrounded. I figured its sane to check if those are valid textures before deleting them.

@jmarshallnz is this call to glIsTexture safe on all gl implementations?

@jmarshallnz
Owner

I think it should be OK.

@popcornmix
Collaborator

glIsTexture causes a gl pipeline flush and response from GPU, so is a slow operation (definitely on Raspberry Pi, but likely on other GL implementations).
Now maybe this is rare enough not to be an issue, but it seems possible this will cause visible stutters.
Would a quirk (EGL_TEXTURES_MAY_BE_DELETED) be safer?

@Memphiz
Owner

As far as i remember from the stacktrace the call to deleteunusedtextures was run from ProcessSlow (so each 500ms) and only for unused textures. But i am happy to quirk it to TARGET_DARWIN_IOS?

Also @popcornmix fyi - i got that call from

https://github.com/xbmc/xbmc/blob/master/xbmc/cores/VideoRenderers/LinuxRendererGLES.cpp#L2163

So maybe it has some performance penalties there too?

@Memphiz
Owner

pull requests updated ...

@FernetMenta
Collaborator

So maybe it has some performance penalties there too?

that's during cleanup so a flush does not harm.
Even if called only every 500ms it may cause stutter. Do you have a chance and check if XBMC is in background and call glIsTexture only for this case?

@t-nelson

This is tagged for Gotham. Where does it stand?

@Memphiz
Owner

Well its Fine by me - What fernetmenta suggests (didn't get an Email - just read it now) - is possible - i will add it and reping you @t-nelson

@t-nelson

Sounds good. Kick off jenkins once you've pushed.

@Memphiz
Owner

Updated the last commit as suggested (only check via glIsTexture when app is backgrounded) - also rebased to master.

jenkins build this please

@t-nelson

@FernetMenta One last pass please.

@Memphiz Any squashing needed/wanted before push?

@Memphiz
Owner

Those commits are well split imo - if you don't mind i would get them in without squashing ...

@t-nelson

@Memphiz yeah sure. I just wanted to make sure they were as you wanted.

@jmarshallnz
Owner

jenkins build this please

@Memphiz Memphiz [guilib] - sanity check hw textures before deleting (on ios when back…
…grounded it seems that ios diddles with textures and might delete them away without xbmc knowing) - do the check only when ios windowing is backgrounded
8f87119
@Memphiz
Owner

args - that is the problem with only compiling the changed files and skipping the link test - jenkins build this please - i really wonder why i didn't get the email 6 days ago grrrr

@t-nelson

Ready here?

@Memphiz
Owner

Yes please - this pr is lingering :D

@t-nelson t-nelson merged commit fa2681d into from
@t-nelson t-nelson removed the Gotham label
@mossywell mossywell referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 23, 2014
  1. @Memphiz

    [ios] - remove the audiosession handling from the sink - its handled …

    Memphiz authored
    …in the UIApplication already
  2. @Memphiz

    [ios] - always send a speed change to ios when the player gets paused…

    Memphiz authored
    … (to stop the playback progress bar in the ios control center)
  3. @Memphiz
  4. @Memphiz
  5. @Memphiz
Commits on Apr 3, 2014
  1. @Memphiz

    [guilib] - sanity check hw textures before deleting (on ios when back…

    Memphiz authored
    …grounded it seems that ios diddles with textures and might delete them away without xbmc knowing) - do the check only when ios windowing is backgrounded
This page is out of date. Refresh to see the latest.
View
4 xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp
@@ -166,7 +166,7 @@ CActiveAE::~CActiveAE()
void CActiveAE::Dispose()
{
-#if defined(HAS_GLX) || defined(TARGET_DARWIN_OSX)
+#if defined(HAS_GLX) || defined(TARGET_DARWIN)
g_Windowing.Unregister(this);
#endif
@@ -2150,7 +2150,7 @@ bool CActiveAE::Initialize()
}
// hook into windowing for receiving display reset events
-#if defined(HAS_GLX) || defined(TARGET_DARWIN_OSX)
+#if defined(HAS_GLX) || defined(TARGET_DARWIN)
g_Windowing.Register(this);
#endif
View
2  xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.h
@@ -181,7 +181,7 @@ class CEngineStats
CCriticalSection m_lock;
};
-#if defined(HAS_GLX) || defined(TARGET_DARWIN_OSX)
+#if defined(HAS_GLX) || defined(TARGET_DARWIN)
class CActiveAE : public IAE, public IDispResource, private CThread
#else
class CActiveAE : public IAE, private CThread
View
52 xbmc/cores/AudioEngine/Sinks/AESinkDARWINIOS.cpp
@@ -114,14 +114,11 @@ class CAAudioUnitSink
static void sessionPropertyCallback(void *inClientData,
AudioSessionPropertyID inID, UInt32 inDataSize, const void *inData);
- static void sessionInterruptionCallback(void *inClientData, UInt32 inInterruption);
-
static OSStatus renderCallback(void *inRefCon, AudioUnitRenderActionFlags *ioActionFlags,
const AudioTimeStamp *inTimeStamp, UInt32 inOutputBusNumber, UInt32 inNumberFrames,
AudioBufferList *ioData);
bool m_setup;
- bool m_initialized;
bool m_activated;
AudioUnit m_audioUnit;
AudioStreamBasicDescription m_outputFormat;
@@ -142,8 +139,7 @@ class CAAudioUnitSink
};
CAAudioUnitSink::CAAudioUnitSink()
-: m_initialized(false)
-, m_activated(false)
+: m_activated(false)
, m_buffer(NULL)
, m_playing(false)
, m_playing_saved(false)
@@ -332,25 +328,12 @@ bool CAAudioUnitSink::setupAudio()
if (m_setup && m_audioUnit)
return true;
- // Audio Session Setup
- UInt32 sessionCategory = kAudioSessionCategory_MediaPlayback;
- status = AudioSessionSetProperty(kAudioSessionProperty_AudioCategory,
- sizeof(sessionCategory), &sessionCategory);
- if (status != noErr)
- {
- CLog::Log(LOGERROR, "%s error setting sessioncategory (error: %d)", __PRETTY_FUNCTION__, (int)status);
- return false;
- }
-
AudioSessionAddPropertyListener(kAudioSessionProperty_AudioRouteChange,
sessionPropertyCallback, this);
AudioSessionAddPropertyListener(kAudioSessionProperty_CurrentHardwareOutputVolume,
sessionPropertyCallback, this);
- if (AudioSessionSetActive(true) != noErr)
- return false;
-
// Audio Unit Setup
// Describe a default output unit.
AudioComponentDescription description = {};
@@ -455,17 +438,6 @@ bool CAAudioUnitSink::activateAudioSession()
{
if (!m_activated)
{
- if (!m_initialized)
- {
- OSStatus osstat = AudioSessionInitialize(NULL, kCFRunLoopDefaultMode, sessionInterruptionCallback, this);
- if (osstat == kAudioSessionNoError || osstat == kAudioSessionAlreadyInitialized)
- m_initialized = true;
- else
- {
- CLog::Log(LOGERROR, "%s error initializing audio session (error: %d)", __PRETTY_FUNCTION__, (int)osstat);
- return false;
- }
- }
if (checkAudioRoute() && setupAudio())
m_activated = true;
}
@@ -480,7 +452,6 @@ void CAAudioUnitSink::deactivateAudioSession()
pause();
AudioUnitUninitialize(m_audioUnit);
AudioComponentInstanceDispose(m_audioUnit), m_audioUnit = NULL;
- AudioSessionSetActive(false);
AudioSessionRemovePropertyListenerWithUserData(kAudioSessionProperty_AudioRouteChange,
sessionPropertyCallback, this);
AudioSessionRemovePropertyListenerWithUserData(kAudioSessionProperty_CurrentHardwareOutputVolume,
@@ -508,27 +479,6 @@ void CAAudioUnitSink::sessionPropertyCallback(void *inClientData,
}
}
-void CAAudioUnitSink::sessionInterruptionCallback(void *inClientData, UInt32 inInterruption)
-{
- CAAudioUnitSink *sink = (CAAudioUnitSink*)inClientData;
-
- if (inInterruption == kAudioSessionBeginInterruption)
- {
- CLog::Log(LOGDEBUG, "Bgn interuption");
- sink->m_playing_saved = sink->m_playing;
- sink->pause();
- }
- else if (inInterruption == kAudioSessionEndInterruption)
- {
- CLog::Log(LOGDEBUG, "End interuption");
- if (sink->m_playing_saved)
- {
- sink->m_playing_saved = false;
- sink->play(sink->m_mute);
- }
- }
-}
-
inline void LogLevel(unsigned int got, unsigned int wanted)
{
static unsigned int lastReported = INT_MAX;
View
2  xbmc/guilib/DispResource.h
@@ -20,7 +20,7 @@
#pragma once
-#if defined(HAS_GLX) || defined(TARGET_DARWIN_OSX)
+#if defined(HAS_GLX) || defined(TARGET_DARWIN)
class IDispResource
{
public:
View
12 xbmc/guilib/TextureManager.cpp
@@ -36,6 +36,10 @@
#include "URL.h"
#include <assert.h>
+#if defined(TARGET_DARWIN_IOS) && !defined(TARGET_DARWIN_IOS_ATV2)
+#include "windowing/WindowingFactory.h" // for g_Windowing in CGUITextureManager::FreeUnusedTextures
+#endif
+
using namespace std;
@@ -478,7 +482,13 @@ void CGUITextureManager::FreeUnusedTextures(unsigned int timeDelay)
#if defined(HAS_GL) || defined(HAS_GLES)
for (unsigned int i = 0; i < m_unusedHwTextures.size(); ++i)
{
- glDeleteTextures(1, (GLuint*) &m_unusedHwTextures[i]);
+ // on ios the hw textures might be deleted from the os
+ // when XBMC is backgrounded (e.x. for backgrounded music playback)
+ // sanity check before delete in that case.
+#if defined(TARGET_DARWIN_IOS) && !defined(TARGET_DARWIN_IOS_ATV2)
+ if (!g_Windowing.IsBackgrounded() || glIsTexture(m_unusedHwTextures[i]))
+#endif
+ glDeleteTextures(1, (GLuint*) &m_unusedHwTextures[i]);
}
#endif
m_unusedHwTextures.clear();
View
10 xbmc/osx/ios/XBMCController.mm
@@ -180,7 +180,7 @@ void AnnounceBridge(ANNOUNCEMENT::AnnouncementFlag flag, const char *sender, con
LOG(@"item: %@", item.description);
[g_xbmcController performSelectorOnMainThread:@selector(onPlay:) withObject:item waitUntilDone:NO];
}
- else if (msg == "OnSpeedChanged")
+ else if (msg == "OnSpeedChanged" || msg == "OnPause")
{
NSDictionary *item = [dict valueForKey:@"item"];
NSDictionary *player = [dict valueForKey:@"player"];
@@ -188,10 +188,8 @@ void AnnounceBridge(ANNOUNCEMENT::AnnouncementFlag flag, const char *sender, con
[item setValue:[NSNumber numberWithDouble:g_application.GetTime()] forKey:@"elapsed"];
LOG(@"item: %@", item.description);
[g_xbmcController performSelectorOnMainThread:@selector(OnSpeedChanged:) withObject:item waitUntilDone:NO];
- }
- else if (msg == "OnPause")
- {
- [g_xbmcController performSelectorOnMainThread:@selector(onPause:) withObject:[dict valueForKey:@"item"] waitUntilDone:NO];
+ if (msg == "OnPause")
+ [g_xbmcController performSelectorOnMainThread:@selector(onPause:) withObject:[dict valueForKey:@"item"] waitUntilDone:NO];
}
else if (msg == "OnStop")
{
@@ -1037,11 +1035,13 @@ - (void)enterBackground
m_isPlayingBeforeInactive = YES;
CApplicationMessenger::Get().MediaPauseIfPlaying();
}
+ g_Windowing.OnAppFocusChange(false);
}
- (void)enterForeground
{
PRINT_SIGNATURE();
+ g_Windowing.OnAppFocusChange(true);
// when we come back, restore playing if we were.
if (m_isPlayingBeforeInactive)
{
View
12 xbmc/windowing/osx/WinSystemIOS.h
@@ -27,6 +27,9 @@
#include "windowing/WinSystem.h"
#include "rendering/gles/RenderSystemGLES.h"
#include "utils/GlobalsHandling.h"
+#include "threads/CriticalSection.h"
+
+class IDispResource;
class CWinSystemIOS : public CWinSystemBase, public CRenderSystemGLES
{
@@ -56,12 +59,18 @@ class CWinSystemIOS : public CWinSystemBase, public CRenderSystemGLES
virtual bool BeginRender();
virtual bool EndRender();
+
+ virtual void Register(IDispResource *resource);
+ virtual void Unregister(IDispResource *resource);
+
virtual int GetNumScreens();
virtual int GetCurrentScreen();
void InitDisplayLink(void);
void DeinitDisplayLink(void);
double GetDisplayLinkFPS(void);
+ void OnAppFocusChange(bool focus);
+ bool IsBackgrounded() const { return m_bIsBackgrounded; }
protected:
virtual bool PresentRenderImpl(const CDirtyRegionList &dirty);
@@ -72,6 +81,9 @@ class CWinSystemIOS : public CWinSystemBase, public CRenderSystemGLES
bool m_bWasFullScreenBeforeMinimize;
CStdString m_eglext;
int m_iVSyncErrors;
+ CCriticalSection m_resourceSection;
+ std::vector<IDispResource*> m_resources;
+ bool m_bIsBackgrounded;
private:
bool GetScreenResolution(int* w, int* h, double* fps, int screenIdx);
View
26 xbmc/windowing/osx/WinSystemIOS.mm
@@ -34,6 +34,8 @@
#include "guilib/GraphicContext.h"
#include "guilib/Texture.h"
#include "utils/StringUtils.h"
+#include "guilib/DispResource.h"
+#include "threads/SingleLock.h"
#include <vector>
#undef BOOL
@@ -54,6 +56,7 @@
m_eWindowSystem = WINDOW_SYSTEM_IOS;
m_iVSyncErrors = 0;
+ m_bIsBackgrounded = false;
}
CWinSystemIOS::~CWinSystemIOS()
@@ -318,6 +321,29 @@
return rtn;
}
+void CWinSystemIOS::Register(IDispResource *resource)
+{
+ CSingleLock lock(m_resourceSection);
+ m_resources.push_back(resource);
+}
+
+void CWinSystemIOS::Unregister(IDispResource* resource)
+{
+ CSingleLock lock(m_resourceSection);
+ std::vector<IDispResource*>::iterator i = find(m_resources.begin(), m_resources.end(), resource);
+ if (i != m_resources.end())
+ m_resources.erase(i);
@jmarshallnz Owner

In case you haven't heard of it, std::remove() can be useful in these situations. There's a bit of a gotcha, however, in that you need to combine it with std::erase() to get rid of unneeded items at the end of the list.

Note that it's not a requirement at all to use it, but it is designed for just this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+}
+
+void CWinSystemIOS::OnAppFocusChange(bool focus)
+{
+ CSingleLock lock(m_resourceSection);
+ m_bIsBackgrounded = !focus;
+ CLog::Log(LOGDEBUG, "CWinSystemIOS::OnAppFocusChange: %d", focus ? 1 : 0);
+ for (std::vector<IDispResource *>::iterator i = m_resources.begin(); i != m_resources.end(); i++)
+ (*i)->OnAppFocusChange(focus);
+}
+
void CWinSystemIOS::InitDisplayLink(void)
{
}
Something went wrong with that request. Please try again.