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

Added Dialogs for direct Audiostream and Subtitle switching #16825

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions xbmc/guilib/GUIWindowManager.cpp
Expand Up @@ -127,6 +127,8 @@
#include "dialogs/GUIDialogPlayEject.h"
#include "dialogs/GUIDialogMediaFilter.h"
#include "video/dialogs/GUIDialogSubtitles.h"
#include "video/dialogs/GUIDialogSubtitleSelect.h"
#include "video/dialogs/GUIDialogAudioSelect.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong include order


#include "peripherals/dialogs/GUIDialogPeripherals.h"
#include "peripherals/dialogs/GUIDialogPeripheralSettings.h"
Expand Down Expand Up @@ -241,6 +243,8 @@ void CGUIWindowManager::CreateWindows()

Add(new CGUIDialogMediaFilter);
Add(new CGUIDialogSubtitles);
Add(new CGUIDialogSubtitleSelect);
Add(new CGUIDialogAudioSelect);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Destruction of the new dialog instances inCGUIWindowManager::DestroyWindows() is missing.


Add(new CGUIWindowMusicPlayList);
Add(new CGUIWindowMusicNav);
Expand Down
2 changes: 2 additions & 0 deletions xbmc/guilib/WindowIDs.h
Expand Up @@ -92,6 +92,8 @@
#define WINDOW_DIALOG_INFOPROVIDER_SETTINGS 10158
#define WINDOW_DIALOG_SUBTITLE_OSD_SETTINGS 10159
#define WINDOW_DIALOG_BUSY_NOCANCEL 10160
#define WINDOW_DIALOG_SUBTITLE_SELECT 10161
#define WINDOW_DIALOG_AUDIO_SELECT 10162

#define WINDOW_MUSIC_PLAYLIST 10500
#define WINDOW_MUSIC_NAV 10502
Expand Down
2 changes: 2 additions & 0 deletions xbmc/input/WindowTranslator.cpp
Expand Up @@ -91,6 +91,8 @@ const CWindowTranslator::WindowMapByName CWindowTranslator::WindowMappingByName
{ "osdvideosettings" , WINDOW_DIALOG_VIDEO_OSD_SETTINGS },
{ "osdaudiosettings" , WINDOW_DIALOG_AUDIO_OSD_SETTINGS },
{ "osdsubtitlesettings" , WINDOW_DIALOG_SUBTITLE_OSD_SETTINGS },
{ "osdsubtitleselect" , WINDOW_DIALOG_SUBTITLE_SELECT },
{ "osdaudioselect" , WINDOW_DIALOG_AUDIO_SELECT },
{ "videobookmarks" , WINDOW_DIALOG_VIDEO_BOOKMARKS },
{ "filebrowser" , WINDOW_DIALOG_FILE_BROWSER },
{ "networksetup" , WINDOW_DIALOG_NETWORK_SETUP },
Expand Down
7 changes: 5 additions & 2 deletions xbmc/video/dialogs/CMakeLists.txt
Expand Up @@ -6,7 +6,9 @@ set(SOURCES GUIDialogAudioSettings.cpp
GUIDialogVideoBookmarks.cpp
GUIDialogVideoInfo.cpp
GUIDialogVideoOSD.cpp
GUIDialogVideoSettings.cpp)
GUIDialogVideoSettings.cpp
GUIDialogSubtitleSelect.cpp
GUIDialogAudioSelect.cpp)

set(HEADERS GUIDialogAudioSettings.h
GUIDialogFullScreenInfo.h
Expand All @@ -16,7 +18,8 @@ set(HEADERS GUIDialogAudioSettings.h
GUIDialogVideoBookmarks.h
GUIDialogVideoInfo.h
GUIDialogVideoOSD.h
GUIDialogVideoSettings.h)
GUIDialogVideoSettings.h
GUIDialogAudioSelect.h)

if(OPENGL_FOUND OR CORE_SYSTEM_NAME STREQUAL windows OR CORE_SYSTEM_NAME STREQUAL windowsstore)
list(APPEND SOURCES GUIDialogCMSSettings.cpp)
Expand Down
142 changes: 142 additions & 0 deletions xbmc/video/dialogs/GUIDialogAudioSelect.cpp
@@ -0,0 +1,142 @@
/*
* Copyright (C) 2005-2019 Team Kodi
* This file is part of Kodi - https://kodi.tv
*
* SPDX-License-Identifier: GPL-2.0-or-later
* See LICENSES/README.md for more information.
*/

#include "GUIDialogAudioSelect.h"

#include "Application.h"
#include "GUIUserMessages.h"
#include "LangInfo.h"
#include "ServiceBroker.h"
#include "URL.h"
#include "Util.h"
#include "addons/AddonManager.h"
#include "cores/IPlayer.h"
#include "dialogs/GUIDialogKaiToast.h"
#include "dialogs/GUIDialogSelect.h"
#include "filesystem/AddonsDirectory.h"
#include "filesystem/Directory.h"
#include "filesystem/File.h"
#include "filesystem/SpecialProtocol.h"
#include "filesystem/StackDirectory.h"
#include "guilib/GUIComponent.h"
#include "guilib/GUIKeyboardFactory.h"
#include "guilib/GUIWindowManager.h"
#include "guilib/LocalizeStrings.h"
#include "input/Key.h"
#include "settings/Settings.h"
#include "settings/lib/Setting.h"
#include "utils/JobManager.h"
#include "utils/LangCodeExpander.h"
#include "utils/StringUtils.h"
#include "utils/URIUtils.h"
#include "utils/Variant.h"
#include "utils/log.h"
#include "video/VideoDatabase.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt that all these headers are actually needed. Example: utils/log.h included, but I cannot see any use of CLog


#include "system.h"

using namespace ADDON;
using namespace XFILE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this using statements.


#define STRING_AUDIO_HEADER 460
#define STRING_TOAST_TEXT_ERROR 416

CGUIDialogAudioSelect::CGUIDialogAudioSelect(void) : CGUIDialog(WINDOW_DIALOG_AUDIO_SELECT, "")
{
m_loadType = KEEP_IN_MEMORY;
}

CGUIDialogAudioSelect::~CGUIDialogAudioSelect(void)
{
}

bool CGUIDialogAudioSelect::OnMessage(CGUIMessage& message)
{
return CGUIDialog::OnMessage(message);
}

void CGUIDialogAudioSelect::OnInitWindow()
{
ShowAudioSelect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure it is correct to show the dialog in OnInitWindow - while the dialog is not yet fully initialized? Did you copy this pattern from an existing dialog implementation?


CGUIWindow::OnInitWindow();
}

bool CGUIDialogAudioSelect::ShowAudioSelect()
{
//Only display when playing
if (!g_application.GetAppPlayer().HasPlayer())
{
CGUIDialogKaiToast::QueueNotification(
CGUIDialogKaiToast::Info, g_localizeStrings.Get(STRING_AUDIO_HEADER),
g_localizeStrings.Get(STRING_TOAST_TEXT_ERROR), TOAST_DISPLAY_TIME, false);
return false;
}

CGUIDialogSelect* dialog =
CServiceBroker::GetGUI()->GetWindowManager().GetWindow<CGUIDialogSelect>(
WINDOW_DIALOG_SELECT);
if (dialog == NULL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (dialog == nullptr) or if (!dialog)

return false;

CFileItemList options;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would define vars where they are actually first used.


int audioStreamCount = g_application.GetAppPlayer().GetAudioStreamCount();
int currentAudio = g_application.GetAppPlayer().GetAudioStream();

// cycle through each subtitle and add it to our entry list
for (int i = 0; i < audioStreamCount; ++i)
{
std::string strItem;
std::string strLanguage;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would define vars where they are actually first used.


AudioStreamInfo info;
g_application.GetAppPlayer().GetAudioStreamInfo(i, info);

if (!g_LangCodeExpander.Lookup(info.language, strLanguage))
strLanguage = g_localizeStrings.Get(13205); // Unknown

if (info.name.length() == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (info.name.empty())

strItem = strLanguage;
else
strItem = StringUtils::Format("%s - %s", strLanguage.c_str(), info.name.c_str());

CFileItemPtr item(new CFileItem(strItem.c_str()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::make_shared<CFileItem>(strItem);

item->SetProperty("value", i);

if (i == currentAudio)
item->Select(true);

options.Add(item);
}

if (options.Size() < 2)
{
CGUIDialogKaiToast::QueueNotification(
CGUIDialogKaiToast::Info, g_localizeStrings.Get(STRING_AUDIO_HEADER),
g_localizeStrings.Get(STRING_TOAST_TEXT_ERROR), TOAST_DISPLAY_TIME, false);
return true;
}

dialog->Reset();
dialog->SetHeading(g_localizeStrings.Get(STRING_AUDIO_HEADER).c_str());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.c_str()not needed.

dialog->SetItems(options);
dialog->SetMultiSelection(false);
dialog->Open();

if (!dialog->IsConfirmed())
return true;

int selected = dialog->GetSelectedItem();
if (selected != currentAudio)
{
g_application.GetAppPlayer().SetAudioStream(selected);
}

return true;
}
28 changes: 28 additions & 0 deletions xbmc/video/dialogs/GUIDialogAudioSelect.h
@@ -0,0 +1,28 @@
#pragma once

/*
* Copyright (C) 2005-2019 Team Kodi
* This file is part of Kodi - https://kodi.tv
*
* SPDX-License-Identifier: GPL-2.0-or-later
* See LICENSES/README.md for more information.
*/

#include "guilib/GUIDialog.h"

#include <string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

include not needed.


class CFileItem;
class CFileItemList;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These forward decls are not needed.


class CGUIDialogAudioSelect : public CGUIDialog
{
public:
CGUIDialogAudioSelect(void);
~CGUIDialogAudioSelect(void) override;
bool OnMessage(CGUIMessage& message) override;
void OnInitWindow() override;

protected:
bool ShowAudioSelect();
};
164 changes: 164 additions & 0 deletions xbmc/video/dialogs/GUIDialogSubtitleSelect.cpp
@@ -0,0 +1,164 @@
/*
* Copyright (C) 2005-2019 Team Kodi
* This file is part of Kodi - https://kodi.tv
*
* SPDX-License-Identifier: GPL-2.0-or-later
* See LICENSES/README.md for more information.
*/

#include "GUIDialogSubtitleSelect.h"

#include "Application.h"
#include "GUIUserMessages.h"
#include "LangInfo.h"
#include "ServiceBroker.h"
#include "URL.h"
#include "Util.h"
#include "addons/AddonManager.h"
#include "cores/IPlayer.h"
#include "dialogs/GUIDialogKaiToast.h"
#include "dialogs/GUIDialogSelect.h"
#include "filesystem/AddonsDirectory.h"
#include "filesystem/Directory.h"
#include "filesystem/File.h"
#include "filesystem/SpecialProtocol.h"
#include "filesystem/StackDirectory.h"
#include "guilib/GUIComponent.h"
#include "guilib/GUIKeyboardFactory.h"
#include "guilib/GUIWindowManager.h"
#include "guilib/LocalizeStrings.h"
#include "input/Key.h"
#include "settings/Settings.h"
#include "settings/lib/Setting.h"
#include "utils/JobManager.h"
#include "utils/LangCodeExpander.h"
#include "utils/StringUtils.h"
#include "utils/URIUtils.h"
#include "utils/Variant.h"
#include "utils/log.h"
#include "video/VideoDatabase.h"

#include "system.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt that all these headers are actually needed. Example: utils/log.h included, but I cannot see any use of CLog


using namespace ADDON;
using namespace XFILE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this using statements.


#define STRING_SUBTITLES_HEADER 287
#define STRING_TOAST_TEXT_ERROR 24109

CGUIDialogSubtitleSelect::CGUIDialogSubtitleSelect(void)
: CGUIDialog(WINDOW_DIALOG_SUBTITLE_SELECT, "")
{
m_loadType = KEEP_IN_MEMORY;
}

CGUIDialogSubtitleSelect::~CGUIDialogSubtitleSelect(void)
{
}

bool CGUIDialogSubtitleSelect::OnMessage(CGUIMessage& message)
{
return CGUIDialog::OnMessage(message);
}

void CGUIDialogSubtitleSelect::OnInitWindow()
{
ShowSubtitleSelect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure it is correct to show the dialog in OnInitWindow - while the dialog is not yet fully initialized? Did you copy this pattern from an existing dialog implementation?


CGUIWindow::OnInitWindow();
}

bool CGUIDialogSubtitleSelect::ShowSubtitleSelect()
{
//Only display when playing
if (!g_application.GetAppPlayer().HasPlayer())
{
CGUIDialogKaiToast::QueueNotification(
CGUIDialogKaiToast::Info, g_localizeStrings.Get(STRING_SUBTITLES_HEADER),
g_localizeStrings.Get(STRING_TOAST_TEXT_ERROR), TOAST_DISPLAY_TIME, false);
return false;
}

CGUIDialogSelect* dialog =
CServiceBroker::GetGUI()->GetWindowManager().GetWindow<CGUIDialogSelect>(
WINDOW_DIALOG_SELECT);
if (dialog == NULL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (dialog == nullptr) or if (!dialog)

return false;

CFileItemList options;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would define vars where they are actually first used.


int subtitleStreamCount = g_application.GetAppPlayer().GetSubtitleCount();
int currentSubtitle = g_application.GetAppPlayer().GetSubtitle();
if (!g_application.GetAppPlayer().GetSubtitleVisible())
currentSubtitle = -1;

//Add the Disabled Element
{
CFileItemPtr item(new CFileItem(g_localizeStrings.Get(1223).c_str()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::make_shared<CFileItem>(g_localizeStrings.Get(1223));

item->SetProperty("value", -1);
if (currentSubtitle == -1)
item->Select(true);
options.Add(item);
}

// cycle through each subtitle and add it to our entry list
for (int i = 0; i < subtitleStreamCount; ++i)
{
SubtitleStreamInfo info;
g_application.GetAppPlayer().GetSubtitleStreamInfo(i, info);

std::string strItem;
std::string strLanguage;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would define vars where they are actually first used.


if (!g_LangCodeExpander.Lookup(info.language, strLanguage))
strLanguage = g_localizeStrings.Get(13205); // Unknown

if (info.name.length() == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (info.name.empty())

strItem = strLanguage;
else
strItem = StringUtils::Format("%s - %s", strLanguage.c_str(), info.name.c_str());

CFileItemPtr item(new CFileItem(strItem.c_str()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::make_shared<CFileItem>(strItem);

item->SetProperty("value", i);

if (i == currentSubtitle)
item->Select(true);

options.Add(item);
}

if (options.Size() < 2)
{
CGUIDialogKaiToast::QueueNotification(
CGUIDialogKaiToast::Info, g_localizeStrings.Get(STRING_SUBTITLES_HEADER),
g_localizeStrings.Get(STRING_TOAST_TEXT_ERROR), TOAST_DISPLAY_TIME, false);
return true;
}

dialog->Reset();
dialog->SetHeading(g_localizeStrings.Get(STRING_SUBTITLES_HEADER).c_str());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.c_str()not needed.

dialog->SetItems(options);
dialog->SetMultiSelection(false);
dialog->Open();

if (!dialog->IsConfirmed())
return true;

int selected = dialog->GetSelectedItem() - 1;
if (selected == currentSubtitle)
{
if (!g_application.GetAppPlayer().GetSubtitleVisible())
g_application.GetAppPlayer().SetSubtitleVisible(true);
}
else if (selected == -1)
{
g_application.GetAppPlayer().SetSubtitleVisible(false);
}
else
{
g_application.GetAppPlayer().SetSubtitle(selected);
g_application.GetAppPlayer().SetSubtitleVisible(true);
}

return true;
}