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

[pvr][fix] users are trapped in the splash screen while start into a pvr window + no connection to backend #7664

Merged

Conversation

xhaggi
Copy link
Member

@xhaggi xhaggi commented Jul 30, 2015

bildschirmfoto_2015-07-30_um_09 31 26

This happens if you choose a PVR window as startup window and Kodi can't connect to the PVR backend. You are trapped in the splash screen and you need to kill the app and your GUI settings to reset the startup window configuration. It happens because the PVR Manager will handle the activation of the startup window after it has been started, but this fails (no connected clients). This PR will remove the startup window activation stuff from the PVR manager and let CApplication handle it like it's done for every other window. To activate a PVR window only if the PVR manager has been started, i've introduced a busy dialog into CGUIWindowPVRBase and wait until the manager has been started. You are able to cancel the waiting process and you'll be returned to the previous window (usually the home window).

The mouse pointer issue is related to the splash window issue we already addressed with #7650.

@ksooo @Jalle19 mind taking a look and should we consider a backport?

@xhaggi xhaggi added Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality labels Jul 30, 2015
@xhaggi xhaggi force-pushed the pvr-window-busy-until-manager-started branch from ac9b1f5 to 7c078f3 Compare July 30, 2015 09:57
@xhaggi
Copy link
Member Author

xhaggi commented Jul 30, 2015

I've pushed an alternative way (bases on progress dialog) to inform the user about the PVR manager startup if he enters a PVR window because the busy dialog can't display a message.

screenshot003

@xhaggi xhaggi force-pushed the pvr-window-busy-until-manager-started branch 2 times, most recently from b51f905 to 55e7540 Compare July 30, 2015 14:59
@xhaggi
Copy link
Member Author

xhaggi commented Jul 30, 2015

@ronie mind taking a look at the commit for confluence. We are able to specify if we want to display/hide the progressbar in the progress dialog with CGUIDialogProgress::ShowProgressBar(bool bOnOff) but confluence is missing the id attribute.

@phil65
Copy link
Contributor

phil65 commented Jul 30, 2015

@xhaggi wouldnt DialogOK be more appropriate to display a short message or do you need the progress bar?

@xhaggi
Copy link
Member Author

xhaggi commented Jul 30, 2015

nope won't work because you can't specify what happen if you click on the ok button. And it will block the gfx render thread/lock gfx context which is a no-go too.

edit: to be clear what happens here - the progress dialog automatically disappear if the pvr manager is ready or you can interact and cancel the waiting process

@@ -39,6 +40,7 @@
#include "pvr/dialogs/GUIDialogPVRTimerSettings.h"
#include "pvr/timers/PVRTimers.h"
#include "epg/Epg.h"
#include "epg/EpgContainer.h"

This comment was marked as spam.

This comment was marked as spam.

@ksooo
Copy link
Member

ksooo commented Jul 31, 2015

Not runtime-tested, but the code changes look reasonable to me. If I find the time I will do a runtime test tonight or during the weekend.

@xhaggi xhaggi force-pushed the pvr-window-busy-until-manager-started branch from 55e7540 to 64b7f5f Compare July 31, 2015 19:17
@xhaggi
Copy link
Member Author

xhaggi commented Jul 31, 2015

jenkins build this please

@ksooo
Copy link
Member

ksooo commented Jul 31, 2015

Set TV as startup window, disabled tvheadend . Runtime tested. Crashes reliably :-( Crash log: http://paste.ubuntu.com/11975859/

@xhaggi
Copy link
Member Author

xhaggi commented Aug 1, 2015

thanks will take a look at it.

@xhaggi
Copy link
Member Author

xhaggi commented Aug 1, 2015

I can't really reproduce the issue. I wonder which version you run, because in master we don't have the method CGUIDialog::DoModal(int, std::string const&) anymore.

#0  0x0000000000cbafd8 in PVR::CGUIWindowPVRChannels::UpdateButtons() ()
#1  0x0000000000b2dae2 in CGUIMediaWindow::OnFilterItems(std::string const&) ()
#2  0x0000000000b2e474 in CGUIMediaWindow::Update(std::string const&, bool) ()
#3  0x0000000000cbaed5 in PVR::CGUIWindowPVRChannels::Update(std::string const&, bool) ()
#4  0x0000000000b2ad14 in CGUIMediaWindow::Refresh(bool) ()
#5  0x0000000000b314f6 in CGUIMediaWindow::OnMessage(CGUIMessage&) ()
#6  0x0000000000cbb365 in PVR::CGUIWindowPVRChannels::OnMessage(CGUIMessage&) ()
#7  0x00000000008629b7 in CGUIWindowManager::SendMessage(CGUIMessage&) ()
#8  0x0000000000862c4e in CGUIWindowManager::DispatchThreadMessages() ()
#9  0x0000000000b4a054 in CApplication::Process() ()
#10 0x00000000008614ec in CGUIWindowManager::ProcessRenderLoop(bool) ()
#11 0x0000000000880ee2 in CGUIDialog::DoModal_Internal(int, std::string const&) ()
#12 0x0000000000880a21 in CGUIDialog::DoModal(int, std::string const&) ()
#13 0x0000000000cb8821 in PVR::CGUIWindowPVRBase::OnInitWindow() ()

@ksooo
Copy link
Member

ksooo commented Aug 1, 2015

Latest 15.1 which the PR backport.

@xhaggi
Copy link
Member Author

xhaggi commented Aug 1, 2015

could you please try the master.

@ksooo
Copy link
Member

ksooo commented Aug 1, 2015

Look at the crashing method. It seems to access PVR stuff that is not yet available at that moment?

for me, the crash occurs not sporadically, but on every startup.

@xhaggi
Copy link
Member Author

xhaggi commented Aug 1, 2015

sure we can check for if (g_PVRManager.IsStarted()) in UpdateButtons() and return if not started. But i want to know if it also occurs on master.

@xhaggi
Copy link
Member Author

xhaggi commented Aug 1, 2015

we need to know which message is triggered in CApplication::Process() this message refresh the list :/

@ksooo
Copy link
Member

ksooo commented Aug 1, 2015

I do very sorry. I have no time today.

@xhaggi
Copy link
Member Author

xhaggi commented Aug 1, 2015

np, let me know if you found time. have a nice day 😎

@ksooo
Copy link
Member

ksooo commented Aug 1, 2015

Just a short update (will be offline after writing until late night/tomorrow ;-) Happens also on master, same callstack. Crash gets caused here: https://github.com/xbmc/xbmc/blob/master/xbmc/windows/GUIMediaWindow.cpp#L337 No idea though, why message GUI_MSG_REMOVED_MEDIA is generated on startup. I definitely do not remove any media myself on startup. ;-)

@xhaggi
Copy link
Member Author

xhaggi commented Aug 1, 2015

thanks for the investigation. that explain why i can't reproduce the issue. we neee to protect the update method and do a earlier return if pvr manager is not started. this should fix it.

@xhaggi
Copy link
Member Author

xhaggi commented Aug 1, 2015

@ksooo I've pushed a new commit which will protect the pvr windows against illegal gui messages if the manager is not started. It would be nice if you take a look and do some tests thanks ;)

@xhaggi xhaggi force-pushed the pvr-window-busy-until-manager-started branch from b12f9d8 to bced7e9 Compare August 1, 2015 16:49
@ksooo
Copy link
Member

ksooo commented Aug 1, 2015

Confirming that it works now.

Another story is that I find the dialog in front of the half populated PVR window is very ugly. See @xhaggi's screenshot. It really looks exactly like this while one is waiting for PVR to get ready. But as I have no ides how to do it better, I'd rather shut up, I guess.

@xhaggi
Copy link
Member Author

xhaggi commented Aug 1, 2015

@ksooo better as trapped in the splash and you can't interrupt it ;)

jenkins build this please

@xhaggi xhaggi force-pushed the pvr-window-busy-until-manager-started branch from bced7e9 to 97f912a Compare August 2, 2015 10:35
@xhaggi
Copy link
Member Author

xhaggi commented Aug 2, 2015

is blocked by #7667

@xhaggi xhaggi force-pushed the pvr-window-busy-until-manager-started branch from 97f912a to b0961a2 Compare August 3, 2015 15:52
@xhaggi
Copy link
Member Author

xhaggi commented Aug 3, 2015

jenkins build and merge

@jenkins4kodi jenkins4kodi merged commit a59d037 into xbmc:master Aug 3, 2015
@hudokkow hudokkow added this to the J**** 16.0-alpha2 milestone Aug 3, 2015
@xhaggi xhaggi deleted the pvr-window-busy-until-manager-started branch August 6, 2015 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport: Done Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants