Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ios support play background and native media control bar #2130

Merged
merged 11 commits into from Mar 8, 2013

Conversation

Projects
None yet
7 participants
Contributor

ulion commented Jan 28, 2013

  1. support ios native media control panel on lock screen and left of multi-task bar
  2. continue play audio after lock screen with dark screen without stop.
  3. support play audio in background (need user tap the play button in the media control panel of ios after xbmc enter background state)
  4. support continue play after ios audio interrupt, like an alarm message form other app, or a phone call
  5. support show current playing item info on ios' lock screen, thumb also supported (this is an feature need ios5+)

do not support play video background yet (when user init the video play/continue, default will pause video when enter background), since I not known how to drop frames.

Owner

Memphiz commented Jan 28, 2013

Nice work. But i would like you to change 2 things if possible.

  1. All printsignature and elog changes should go into a seperate commit (something like "improved errorlogging" or so)
  2. Since this is a real bunch of code - do you think you could split out all the audiocontrolstuff/delegate from XBMCController into a seperate class?

@davilla what do you think
@huceke anything you can say about the audioengine changes?

Contributor

davilla commented Jan 28, 2013

  1. I generally dislike magic macros like PRINT_SIGNATURE() ..., What is the reason for changing the logging besides style ? Also, there's a ton of development logging going on, clean it up and only log what you need to see for user debugging.
  2. overall, looks great.
Owner

Memphiz commented Jan 28, 2013

http://pastebin.com/NmPrVnxD <- introduces this compiler warning - would be nice if you could catch that.

Owner

Memphiz commented Jan 28, 2013

Given it a quick runtest - man this rocks! :D

Contributor

ulion commented Jan 28, 2013

the compile warning is harmless it will be ok in xcode4, but if you are using xcode3, the result code works same way. if you surely want to remove it, I can call performSelector to do that work

about the logging, these magic logging macro is only used for osx/ios/xcode debug with console nslog output, do not related to xbmc logging system and xbmc logging system can not print object-c content either..., and no-op when compile in release mode, since currently the commit is in testing stage, I'd left them until ready to merge, when we can clean them if it is surely needed.

about split class, it's a little hard to do, I will think about it.

Contributor

davilla commented Jan 28, 2013

move the logging changes to a separate commit.

Contributor

ulion commented Jan 28, 2013

commit splited

Owner

Memphiz commented Jan 28, 2013

nice split - makes review easier ;o)

Contributor

ulion commented Jan 29, 2013

updated:

  1. handle interrupt when xbmc app inactive or background playing correctly, suspend audio engine and resume do the work without pausing.
  2. prevent from ios auto-suspend network after screen dark.
Contributor

ulion commented Feb 5, 2013

rebased on current master and little updated the nowPlaying item commit, added an AutoPool for the announce bridge.
I found not all Announce calls come from xbmc main thread, the OnStop comes from dvdplayer in my test, which need an auto-release pool for object-c code.

Contributor

ulion commented Mar 8, 2013

rebased, I'd like to merge this within this merge window, any objection? @Memphiz @davilla

Contributor

davilla commented Mar 8, 2013

any issues with other platforms ?

Contributor

ulion commented Mar 8, 2013

I think no, the only code changed which also used by other platform is in ulion/xbmc@1297e20
the last place, adjust Resume() before an audio stream operation, I think it's good, how about it? @huceke

Memphiz commented on d2232df Mar 8, 2013

Mhh any reason we don't rename MEDIA_PAUSE to MEDIA_PLAYPAUSE and name the real pause MEDIA_PAUSE instead? Based on your comment this sounds logical.

Owner

ulion replied Mar 8, 2013

Memphiz commented on 165be20 Mar 8, 2013

This last TODO drains battery if we are not playing and are backgrounded - right? ^^

Owner

ulion replied Mar 8, 2013

I'm working on it, update soon.

Owner

Memphiz commented Mar 8, 2013

Looks good to me. But there are some xcode project changes i am not able to look at with Internet Explorer @work. Will have another look at home...

Contributor

ulion commented Mar 8, 2013

the TODO is resolved. and also added seek forward and rewind support (hold the next/prev button of ios media control bar to trigger), use 4X speed works well with music seeking.

@Memphiz Memphiz added a commit that referenced this pull request Mar 8, 2013

@Memphiz Memphiz Merge pull request #2130 from ulion/ios_play_background
ios support play background and native media control bar
643ba7a

@Memphiz Memphiz merged commit 643ba7a into xbmc:master Mar 8, 2013

@ulion do we really need those? What was the intention for this? I am working on the ActiveAE sink and there the Engine doesn't come up again when suspended here. But the sink itself registers for interruption callbacks and stops the audiounit during interruption. So now i need to adapt this stuff for ActiveAE and we need a different approach here. Any ideas?

Contributor

ulion replied Feb 17, 2014

the ios media player bar, the play or pause button status depends on the audio engine's state, without this, ios media bar or the home screen media button will still show the 'pausing' button, not indicate that the music already paused which need display the 'play' button which can only be done by suspend the under audio engine.

what's happened on the audio engine, the code did work in the past, didn't it?

Member

fritsch replied Feb 18, 2014

We are Engine driven now and not workaround driven :-)

Owner

Memphiz replied Feb 18, 2014

@ulion - well the sink handles the Interruption now - atm it only stops the audio unit - as you pointed out ios still shows the Pause Button - so i guess i need to deactivate and reactivate the audiosession in the sink aswell during Interruption?

While Old ae worked good on ios it didn't on osx and now we are aiming for an unification for both (using activeae on all platforms then and only maintaining the sinks).

Thx for clarification - i think i will find a solution.

On another Note - did you Test Background Playback on ios7? Does it work for you? (I mean current Master code with Old ae)

Owner

Memphiz replied Feb 18, 2014

Mhh why did you suspend ae on Pause and in stop then and not just in the interruption Callback?

Contributor

ulion replied Feb 18, 2014

Contributor

ulion replied Feb 18, 2014

Member

FernetMenta replied Feb 18, 2014

what exactly do you want to achieve? release the audio device on pause/stop?

Contributor

ulion replied Feb 18, 2014

Owner

Memphiz replied Feb 18, 2014

@ulion btw on our old audioengine this was the cause on loosing gui sounds after first playback. I always wondered why this happens ;). Well imo this is problem. There is definitly no other way to signal ios that we are playing something right?

Member

FernetMenta replied Feb 18, 2014

let me know if you find a better way to inform the OS. if not we should only suspend the sink, not the entire engine. (requires some extra code in activeae)

Contributor

ulion replied Feb 18, 2014

Owner

Memphiz replied Feb 18, 2014

Yeah its ugly :( ... i got it that far that background playing works (but the ios button always stays as "pause" - but beside that everything works). At least on ios6 ... on ios7 i can't control XBMCs player. Well i fear this will go in as a regression during beta. I need to find some time to dig more into it. BTW if you need a raw dev device? I have my ipod 4g left unused atm and can send it to you. (though its ios6.1 only).

Sry i don't want to destroy this nice feature - but atm its just hard to figure it out cause i can't find so much documentation on that part.

Contributor

ulion replied Feb 19, 2014

Owner

Memphiz replied Feb 20, 2014

@ulion - lets move the discussion about possible regressions to the forum here:

http://forum.xbmc.org/showthread.php?tid=186994

@jmarshallnz fyi ping

this creates a problem if a modal dialog is open and application is minimized. what is this flag good for? this can be done by dirty region logic and frame limited. see issue #4617

Owner

Memphiz replied Apr 28, 2014

I guess @ulion added this for ios - you should not render ever when ios has pushed the app into the background. But lets wait on his comment.

Member

FernetMenta replied Apr 28, 2014

regardless of what the answer may be, this should not handled in application. if a platform has an issue with rendering when backgrounded, it should handle this in its renderer.

Contributor

ulion replied Apr 28, 2014

on windows the flag is set since 872d58c

Member

FernetMenta replied Apr 29, 2014

please move the background flag into iOS renderer if you need it there. Note that the high CPU issue also does exist for iOS. those flags at the application level taint design.

Member

FernetMenta replied Apr 29, 2014

btw: we already have the m_renderGUI flag at the application level.

Contributor

ulion replied Apr 29, 2014

Member

FernetMenta replied Apr 29, 2014

we already have a flag for background mode m_renderGUI. Why did you implement another?

Contributor

ulion replied Apr 29, 2014

Member

FernetMenta replied Apr 29, 2014

m_renderGUI was intruduced Oct. 2012 d72df19
the ios flag 2013

platforms like Linux, Windows, OSX use m_renderGUI via call to g_application.SetRenderGUI. I suggest we stick with this and elimiinate m_bInBackground

Member

wsoltys replied Apr 29, 2014

Member

wsoltys replied Apr 29, 2014

Contributor

ulion replied Apr 29, 2014

Member

FernetMenta replied Apr 30, 2014

again, it is wrong that every platform add its flags to application. platform specific action need to be done at lower levels like renderer, you can skip rendering there.
What special requirements do yo see on iOS not covered by m_renderGUI

Contributor

ulion replied Apr 30, 2014

Member

FernetMenta replied Apr 30, 2014

I can do if you answer my question: What special requirements do yo see on iOS not covered by m_renderGUI

Contributor

ulion replied Apr 30, 2014

Member

FernetMenta replied Apr 30, 2014

that is exactly what the m_renderGUI flag is supposed to do:

  1. skip Render
    https://github.com/xbmc/xbmc/blob/master/xbmc/XBApplicationEx.cpp#L175
  2. prevent ScreenSaver, I bring this bahavior back
    https://github.com/FernetMenta/xbmc/blob/03a9b198caf2efaa40d5ba87e8f57bc58d744bb2/xbmc/Application.cpp#L5213

could you please try the renderGUI flag on iOS. if something is missing we should try to add it without introducing a new flag.

Contributor

ulion replied Apr 30, 2014

Owner

Memphiz replied Apr 30, 2014

I am on vacation for a week now. So hopefully no sad decisions will be made here for Gotham which would result in breaking the iOS background music playback which i worked quiet a while on since ios7 changed alot of stuff. I am happy to test anything after my vacation though.

So what do i need? Removing that m_bInBackground flag and call "SetRenderGUI" instead?

Member

FernetMenta replied Apr 30, 2014

So what do i need? Removing that m_bInBackground flag and call "SetRenderGUI" instead?

yes, I would try this.

Owner

Memphiz replied May 12, 2014

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