Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gui] Kill g_audioManager #14198

Merged
merged 1 commit into from
Jul 21, 2018
Merged

[gui] Kill g_audioManager #14198

merged 1 commit into from
Jul 21, 2018

Conversation

hudokkow
Copy link
Member

Motivation and Context

Kill a global following @FernetMenta's #13724 example.

How Has This Been Tested?

Runtime tested on Ubuntu and Win10 x64.

@hudokkow hudokkow added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Type: Improvement non-breaking change which improves existing functionality v18 Leia labels Jul 18, 2018
@garbear
Copy link
Member

garbear commented Jul 18, 2018

Looks good. There may be some places where we need to check CServiceBroker::GetGUI() != nullptr

@@ -52,7 +52,7 @@ JSONRPC_STATUS CInputOperations::SendAction(int actionID, bool wakeScreensaver /
if(!wakeScreensaver || !handleScreenSaver())
{
g_application.ResetSystemIdleTimer();
g_audioManager.PlayActionSound(actionID);
CServiceBroker::GetGUI()->GetAudioManager().PlayActionSound(actionID);

This comment was marked as spam.

@@ -42,7 +42,7 @@ bool CUISoundsResource::IsInUse() const
void CUISoundsResource::OnPostInstall(bool update, bool modal)
{
if (IsInUse())
g_audioManager.Load();
CServiceBroker::GetGUI()->GetAudioManager().Load();

This comment was marked as spam.

@@ -697,11 +697,11 @@ bool CInputManager::ExecuteInputAction(const CAction &action)
{
bResult = g_application.OnAction(action);
if (bResult)
g_audioManager.PlayActionSound(action);
CServiceBroker::GetGUI()->GetAudioManager().PlayActionSound(action);

This comment was marked as spam.

}
else
{
g_audioManager.PlayActionSound(action);
CServiceBroker::GetGUI()->GetAudioManager().PlayActionSound(action);

This comment was marked as spam.

@FernetMenta
Copy link
Contributor

all calls from non gui components should test for nullptr

@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Jul 19, 2018
@jenkins4kodi jenkins4kodi removed the Rebase needed PR that does not apply/merge cleanly to current base branch label Jul 19, 2018
@hudokkow
Copy link
Member Author

Done. Thanks for the review.

@@ -2096,7 +2096,7 @@ bool CApplication::OnAction(const CAction &action)
if (!m_appPlayer.IsPaused() && m_appPlayer.GetPlaySpeed() != 1)
m_appPlayer.SetPlaySpeed(1);

g_audioManager.Enable(m_appPlayer.IsPaused());
CServiceBroker::GetGUI()->GetAudioManager().Enable(m_appPlayer.IsPaused());

This comment was marked as spam.

@hudokkow hudokkow force-pushed the audiomanager branch 4 times, most recently from 9640667 to 248e4cf Compare July 19, 2018 12:41
@Rechi Rechi requested a review from FernetMenta July 20, 2018 20:06
@@ -2156,7 +2158,7 @@ bool CApplication::OnAction(const CAction &action)
{
// unpause, and set the playspeed back to normal
m_appPlayer.Pause();
g_audioManager.Enable(m_appPlayer.IsPaused());
CServiceBroker::GetGUI()->GetAudioManager().Enable(m_appPlayer.IsPaused());

This comment was marked as spam.

@@ -2869,7 +2871,7 @@ void CApplication::Stop(int exitCode)
UnregisterActionListener(&m_appPlayer.GetSeekHandler());
UnregisterActionListener(&CPlayerController::GetInstance());

g_audioManager.DeInitialize();
CServiceBroker::GetGUI()->GetAudioManager().DeInitialize();

This comment was marked as spam.

@@ -3205,7 +3207,7 @@ bool CApplication::PlayFile(CFileItem item, const std::string& player, bool bRes
m_appPlayer.SetMute(m_muted);

#if !defined(TARGET_POSIX)
g_audioManager.Enable(false);
CServiceBroker::GetGUI()->GetAudioManager().Enable(false);

This comment was marked as spam.

@@ -3218,7 +3220,7 @@ void CApplication::PlaybackCleanup()
{
if (!m_appPlayer.IsPlaying())
{
g_audioManager.Enable(true);
CServiceBroker::GetGUI()->GetAudioManager().Enable(true);

This comment was marked as spam.

@FernetMenta
Copy link
Contributor

finally player will not depend on GUI. player can also be used for transcoding

@hudokkow
Copy link
Member Author

Done. Thanks for the guidance! 👍
Transcoding sounds nice... 😉

@MartijnKaijser MartijnKaijser added this to the Leia 18.0-alpha3 milestone Jul 21, 2018
@Rechi Rechi merged commit 472e96e into xbmc:master Jul 21, 2018
@hudokkow hudokkow deleted the audiomanager branch July 21, 2018 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Type: Improvement non-breaking change which improves existing functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants