[pvr] Rewrite of PVR window integration #4753

xhaggi · 2014-05-19T11:41:21Z

As pointed out on IRC i'm working on a rewrite of the current PVR window integration. I've refactored all "sub windows"
(channels, guide, recordings, timers, seach) out of CGUIWindowPVR to be normal windows which inherit from a CGUIWindowPVRBase class
(same it is done on other parts of XBMC). Each window has its own new window id depends on the type tv or radio (WINDOW_TV_CHANNELS, WINDOW_RADIO_CHANNELS, WINDOW_TV_GUIDE etc.)
and is mapped to an action command (TVChannels, RadioChannels, TVGuide etc.) within the CButtonTranslator. You could activate such a window by calling
ActivateWindow(TVGuide) or ActivateWindow(RadioGuide).
I also changed the implementation of views to be dynamic such as the video views are, except the guide views which need static ID refs (10 = epggrid, 11 = now, 12 = next, 13 = channel epg).
But the skinners will have control over which views are used by adding them to the <views> node. So now it's possible to add different views for channel list,
recordings etc. or for example only provide the guide grid view for the guide window by adding the ID 11 to <views> only.

There are several other changes, so here is complete overview:

new windows ID's: WINDOW_TV_CHANNELS, WINDOW_TV_GUIDE, WINDOW_TV_RECORDINGS, WINDOW_TV_TIMERS, WINDOW_TV_SEARCH, WINDOW_RADIO_CHANNELS, WINDOW_RADIO_GUIDE, WINDOW_RADIO_RECORDINGS, WINDOW_RADIO_TIMERS, WINDOW_RADIO_SEARCH
new window action mappings: TVChannels, TVGuide, TVRecordings, TVTimers, TVSearch, RadioChannels, RadioGuide, RadioRecordings, RadioTimers, RadioSearch
use of ActivateWindow(TVGuide) instead of ActivateWindowAndFocus(MyPVR, 31,0, 10,0)
get rid of static button references for activating a pvr window (heavily used on the sideblade menu), instead <onclick>ActivateWindow(TVGuide)</onclick> is used.
new group selection dialog (Button control ID: 28)
use of the "View As" button (ID: 2) to switch between existing views.
new gui info properties PVR.HasTVChannels and PVR.HasRadioChannels to determine if we have tv and/or radio channels and hide the tv/radio item in home menu.
new window property IsRadio

It would be nice to get some feedback especially from @jmarshallnz for the gui part, @opdenkamp and @Jalle19 for the PVR related parts and @ronie for the changes I made to confluence.

ronie · 2014-05-24T20:32:19Z

skin changes look ok to me.
nice work xhaggi, things will make the pvr section far more useable.

xbmc/windows/GUIMediaWindow.cpp

+     iWindow != WINDOW_PVR_TIMERS ||
+     iWindow != WINDOW_PVR_SEARCH ||
+    (iWindow == WINDOW_PVR_RECORDINGS && StringUtils::StartsWith(m_vecItems->GetPath(), "pvr://recordings/")))
+     m_history.AddPath(m_vecItems->GetPath(), m_strFilterPath);


xhaggi · 2014-05-25T10:19:21Z

@ronie thanks for reviewing the confluence part, removed the include and unused language keys.

opdenkamp · 2014-05-26T18:35:14Z

i've reviewed the first commit, but don't have time to review the rest right now. in general: great work, but try to split the changes in smaller commits instead of a huge one next time, since this makes it nearly impossible to review.

xhaggi · 2014-05-26T18:37:28Z

thanks so far .. this was a rework of my old PR and i'm lazy to split things up.

xhaggi · 2014-05-26T21:35:05Z

@opdenkamp i've now added the unique window ids (WINDOW_TV_CHANNELS, WINDOW_RADIO_CHANNELS) + action mappings (tvchannels, radiochannels) for each type of pvr window.

xhaggi · 2014-05-27T08:27:23Z

@ronie could you please review the changes made to confluence 9a1e085. Now we use unique window id's instead of one for tv and radio.

xbmc/pvr/windows/GUIWindowPVRChannels.h

-    CPVRChannelGroupPtr m_selectedGroup;
-    bool              m_bShowHiddenChannels;
-    bool              m_bRadio;
+    bool m_bShowHiddenChannels = false;


xhaggi · 2014-05-28T20:26:01Z

@opdenkamp i've done most of the stuff you chalked up. would be nice if you take a look at the rest of this PR, so i can rebase against master

xhaggi · 2014-05-29T08:04:50Z

@opdenkamp done, let me know if i could start a rebase.

xbmc/pvr/timers/PVRTimers.cpp

@@ -421,8 +419,11 @@ bool CPVRTimers::GetDirectory(const CStdString& strPath, CFileItemList &items) c
      for (vector<CPVRTimerInfoTagPtr>::const_iterator timerIt = it->second->begin(); timerIt != it->second->end(); timerIt++)
      {
        CPVRTimerInfoTagPtr current = *timerIt;
-        item.reset(new CFileItem(*current));
-        items.Add(item);
+        if(bRadio == current->m_bIsRadio)


Jalle19 · 2014-07-12T08:47:23Z

Apart from the minor stuff I suggested I believe this is okay, like others have said it's so big that it's hard to get a grip of what's changed or not (especially since some files show as completely removed and completely added respectively). I say we get this in sooner rather than later and fix whatever issues crop up after that as we go. We desperately need a more solid codebase to work with in this area, it's hard to fix bugs and change behavior without it.

jmarshallnz · 2014-07-12T10:39:28Z

Righto, been long enough. @xhaggi please create a new branch, squash in fixes where appropriate, rebase to master, PR and jenkins build, then hit the button.

@opdenkamp if you do have time to do any further review, this branch will remain available - anything you find can always be fixed in master - no big deal.

opdenkamp · 2014-07-12T11:01:38Z

I've talked to @xhaggi on irc, will review the rest this week end. Little patience please.

jmarshallnz · 2014-07-12T11:06:49Z

Much appreciated.

xbmc/cores/dvdplayer/DVDPlayer.cpp

-  if (pWindow)
-    pWindow->SetInvalid();
+  /* make sure the pvr windows are updated */
+  g_PVRManager.SetWindowsInvalid();


xbmc/pvr/PVRManager.cpp

@@ -1581,3 +1622,14 @@ bool CPVRManager::CreateChannelEpgs(void)
  m_bEpgsCreated = m_channelGroups->CreateChannelEpgs();
  return m_bEpgsCreated;
 }
+
+void CPVRManager::SetWindowsInvalid(void)


opdenkamp · 2014-07-13T11:06:21Z

right, for as far as it is reviewable like this, it looks okay. let's do the magic numbers later and get this rebased and squashed. could you try removing the hack from dvdplayer that i commented about, and check if it selects the correct channel that you're playing in the window. would be nice if that one could go.

other than that, press the button.

xbmc#4759)

xhaggi · 2014-07-13T12:40:06Z

@opdenkamp thanks for your time ;) .. i've removed the dvd/omxplayer hack, sqashed and rebased everything.

jenkins build this please

[pvr] Rewrite of PVR window integration

xhaggi mentioned this pull request May 19, 2014

[pvr] Separate windows for TV and Radio #3585

Closed

xhaggi added PVR labels May 19, 2014

MartijnKaijser added this to the Pending for inclusion milestone May 21, 2014

xhaggi mentioned this pull request May 22, 2014

Additional funtions in PVR channels contextmenu #2223

Closed

jmarshallnz reviewed May 24, 2014
View reviewed changes

opdenkamp self-assigned this May 24, 2014

xhaggi reviewed May 27, 2014
View reviewed changes

xbmc/pvr/windows/GUIWindowPVRChannels.h

CPVRChannelGroupPtr m_selectedGroup;

bool m_bShowHiddenChannels;

bool m_bRadio;

bool m_bShowHiddenChannels = false;

This comment was marked as spam.

Sign in to view

xhaggi changed the title ~~[WIP] Rewrite of PVR window integration~~ [pvr] Rewrite of PVR window integration Jul 11, 2014

Jalle19 reviewed Jul 12, 2014
View reviewed changes

opdenkamp reviewed Jul 13, 2014
View reviewed changes

xbmc/cores/dvdplayer/DVDPlayer.cpp

if (pWindow)

pWindow->SetInvalid();

/* make sure the pvr windows are updated */

g_PVRManager.SetWindowsInvalid();

This comment was marked as spam.

Sign in to view

opdenkamp reviewed Jul 13, 2014
View reviewed changes

xbmc/pvr/PVRManager.cpp

@@ -1581,3 +1622,14 @@ bool CPVRManager::CreateChannelEpgs(void)

m_bEpgsCreated = m_channelGroups->CreateChannelEpgs();

return m_bEpgsCreated;

}

void CPVRManager::SetWindowsInvalid(void)

This comment was marked as spam.

Sign in to view

xhaggi added 12 commits July 13, 2014 13:32

[sort] adds sorting by channel number

e9fd428

[pvr] rewrite of pvr windows

ba35108

[win32] update VS2010 project files

7da7573

[osx] update xcode project

f2f6a21

[keymaps] update to fit new pvr windows

3980038

[confluence] rewrite of pvr windows

156f600

[gui] new gui info properties PVR.HasTVChannels and PVR.HasRadioChannels

8b21450

[confluence] hide TV/Radio menu entry if no channels exist

4d8e819

[gui] bump gui version to 5.2.0

cd6b4ae

[confluence] bump version to 2.4.0

0f61db4

[pvr] fixes deadlocks during notifications from observables (adapt from

668cf18

xbmc#4759)

[keymaps] adjust MyTV section to fit new window structure

cb1bbfc

xhaggi added a commit that referenced this pull request Jul 13, 2014

Merge pull request #4753 from xhaggi/pvr-window-rewrite

0d6f7f7

[pvr] Rewrite of PVR window integration

xhaggi merged commit 0d6f7f7 into xbmc:master Jul 13, 2014

MartijnKaijser modified the milestones: Helix 14.0-alpha1, Pending for inclusion Jul 19, 2014

xhaggi deleted the pvr-window-rewrite branch August 7, 2014 11:46

xhaggi restored the pvr-window-rewrite branch August 7, 2014 11:46

xhaggi deleted the pvr-window-rewrite branch April 20, 2015 10:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pvr] Rewrite of PVR window integration #4753

[pvr] Rewrite of PVR window integration #4753

xhaggi commented May 19, 2014

ronie commented May 24, 2014

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

xhaggi commented May 25, 2014

opdenkamp commented May 26, 2014

xhaggi commented May 26, 2014

xhaggi commented May 26, 2014

xhaggi commented May 27, 2014

This comment was marked as spam.

xhaggi commented May 28, 2014

xhaggi commented May 29, 2014

This comment was marked as spam.

Jalle19 commented Jul 12, 2014

jmarshallnz commented Jul 12, 2014

opdenkamp commented Jul 12, 2014

jmarshallnz commented Jul 12, 2014

This comment was marked as spam.

This comment was marked as spam.

opdenkamp commented Jul 13, 2014

xhaggi commented Jul 13, 2014

[pvr] Rewrite of PVR window integration #4753

[pvr] Rewrite of PVR window integration #4753

Conversation

xhaggi commented May 19, 2014

ronie commented May 24, 2014

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

xhaggi commented May 25, 2014

opdenkamp commented May 26, 2014

xhaggi commented May 26, 2014

xhaggi commented May 26, 2014

xhaggi commented May 27, 2014

This comment was marked as spam.

xhaggi commented May 28, 2014

xhaggi commented May 29, 2014

This comment was marked as spam.

Jalle19 commented Jul 12, 2014

jmarshallnz commented Jul 12, 2014

opdenkamp commented Jul 12, 2014

jmarshallnz commented Jul 12, 2014

This comment was marked as spam.

This comment was marked as spam.

opdenkamp commented Jul 13, 2014

xhaggi commented Jul 13, 2014