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] Recordings window: Respect setting SETTING_MYVIDEOS_SELECTACTIO… #10333

Merged
merged 1 commit into from Aug 25, 2016

Conversation

ksooo
Copy link
Member

@ksooo ksooo commented Aug 23, 2016

Recordings window: Respect setting SETTING_MYVIDEOS_SELECTACTION also for recordings. Fixes inconsistent 'select item' behavior wrt 'normal' videos and recordings.

Reported here: http://forum.kodi.tv/showthread.php?tid=249318

@Jalle19: next one for review, if you like.

@ksooo ksooo added Type: Fix non-breaking change which fixes an issue Component: PVR v17 Krypton labels Aug 23, 2016
@ksooo ksooo added this to the Krypton 17.0-beta2 milestone Aug 23, 2016
@ksooo ksooo force-pushed the pvr-fix-recordings-defaultselectaction branch from d714d80 to 21b7fd2 Compare August 23, 2016 21:49
bReturn = true;
break;
case SELECT_ACTION_PLAY_OR_RESUME:
PlayFile(m_vecItems->Get(iItem).get(), false, true);

This comment was marked as spam.

This comment was marked as spam.

…N also for recordings. Fixes inconsistent 'select item' behavior wrt 'normal' videos and recordings.
@ksooo ksooo force-pushed the pvr-fix-recordings-defaultselectaction branch from 21b7fd2 to 63198fd Compare August 24, 2016 12:27
@ksooo
Copy link
Member Author

ksooo commented Aug 24, 2016

@Jalle19 good to go now?

@@ -262,11 +262,34 @@ bool CGUIWindowPVRRecordings::OnMessage(CGUIMessage &message)
{
case ACTION_SELECT_ITEM:
case ACTION_MOUSE_LEFT_CLICK:
switch(CSettings::GetInstance().GetInt(CSettings::SETTING_MYVIDEOS_SELECTACTION))

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@ksooo
Copy link
Member Author

ksooo commented Aug 25, 2016

jenkins build this please

@ksooo ksooo merged commit d283360 into xbmc:master Aug 25, 2016
@ksooo ksooo deleted the pvr-fix-recordings-defaultselectaction branch August 25, 2016 08:03
@MilhouseVH
Copy link
Contributor

According to a forum comment, "recording folders don't seem to work under liveTV", which I'm assuming is due to this change (as there's no other relevant change since the last working build).

@ksooo
Copy link
Member Author

ksooo commented Aug 26, 2016

@MilhouseVH Thanks for the heads up. I will take look later today.

ksooo added a commit to ksooo/xbmc that referenced this pull request Aug 26, 2016
ksooo added a commit that referenced this pull request Aug 28, 2016
…ssion

[PVR] Recordings window: Fix recording groups not working (regression introduced by #10333)
@axbmcuser
Copy link
Contributor

With default select action set to "Show information", KODI now opens the Information Dialog of a recording (which is correct).

When i hit "play recording" within the info dialog, the dialog closes and opens up again, becuase Estuary does Dialog.close(PVRRecordingInfo,true) & Action(Select). I hope it hasn't been reported before. Thanks.

@ksooo
Copy link
Member Author

ksooo commented Aug 28, 2016

Not related to this PR. Open a TRACK ticket if you find a new issue, please.

@ksooo
Copy link
Member Author

ksooo commented Aug 28, 2016

On second thought this actually is related to this PR, although an Estuary problem. Thanks for reporting.

@axbmcuser
Copy link
Contributor

axbmcuser commented Aug 28, 2016

I noticed it in comparison while building right before and after this PR got merged - combined with the intention of this PR it made sense to me that it's most likely related.

Side note: Replacing Action(Select) with Action(Play) in Estuary did not solve the issue in my quick tests. (Nothing happens with Action(Play))
But that was just a lucky guess - most likely it's the wrong approach and someone got another fix in mind.

@ksooo
Copy link
Member Author

ksooo commented Aug 28, 2016

Using "Action(Play)" is the right approach; unfortunately currently this reveals another bug. I just tracked it down and will come up with a fix.

@axbmcuser
Copy link
Contributor

axbmcuser commented Aug 29, 2016

@ksooo

This time i can't tell if truly related, but something quite bad regarding the recordings-folder i noticed recently is:

Reproduce 1 (almost works every time in my case):

  • Play LiveTV (in my case via tvheadend dvb-c)
  • While playback in background, open the discussed "recording" folder
  • Background playback of LiveTv freezes (Complete KODI GUI still active, just the LiveTV playback is stopped - sometimes Estuary showes the buffering indicator in this state, but playback never resumes)

Reproduce 2 (if example 1 does not reproduce):

  • Play LiveTV (in my case via tvheadend)
  • While playback in background, open "recording" folder
  • Open Information Dialog on any recording
  • Background playback of LiveTv freezes (Complete KODI GUI still active, just the LiveTV playback is stopped - sometimes Estuary showes the buffering indicator in this state, but playback never resumes)

Debug log:
http://pastebin.com/raw/Gyk59ALa

Tract is down (404), i'm happy to move the report there, once it's back.

Additional Info: Tested on LibreELEC nightly (debug log), additional testing with MillhouseLibreELEC 0828 + Windows win32 nightly 608220d all with same error/result). All setups fresh/clean

@ksooo
Copy link
Member Author

ksooo commented Aug 29, 2016

This is definitely not related to this PR. please open a track ticket.

@ksooo
Copy link
Member Author

ksooo commented Aug 29, 2016

@FernetMenta fyi, i quickly debugged this. When opening the recordings window, a thumbextractor gets created, but not by pvr code! You see the consequences... The playing Live TV channel gets stopped:

(lldb) thread backtrace
* thread #35: tid = 0xd6ea2, 0x0000000116f04524 pvr.hts.3.4.6.dylib`::CloseLiveStream() + 4 at client.cpp:322, name = 'JobWorker', stop reason = breakpoint 1.1
  * frame #0: 0x0000000116f04524 pvr.hts.3.4.6.dylib`::CloseLiveStream() + 4 at client.cpp:322
    frame #1: 0x0000000100d6b6ea Kodi`PVR::CPVRClient::CloseStream(this=0x0000000109ba0800) + 58 at PVRClient.cpp:1817
    frame #2: 0x00000001015de583 Kodi`PVR::CPVRClients::CloseStream(this=0x0000000106e3a710) + 99 at PVRClients.cpp:1345
    frame #3: 0x00000001016b0dca Kodi`PVR::CPVRManager::CloseStream(this=0x000000010705f000) + 474 at PVRManager.cpp:1204
    frame #4: 0x00000001017911de Kodi`CDVDInputStreamPVRManager::Close(this=0x0000000108c35bd0) + 110 at DVDInputStreamPVRManager.cpp:233
    frame #5: 0x000000010178f062 Kodi`CDVDInputStreamPVRManager::~CDVDInputStreamPVRManager(this=0x0000000108c35bd0) + 66 at DVDInputStreamPVRManager.cpp:63
    frame #6: 0x000000010178f185 Kodi`CDVDInputStreamPVRManager::~CDVDInputStreamPVRManager(this=0x0000000108c35bd0) + 21 at DVDInputStreamPVRManager.cpp:62
    frame #7: 0x000000010178f1e9 Kodi`CDVDInputStreamPVRManager::~CDVDInputStreamPVRManager(this=0x0000000108c35bd0) + 25 at DVDInputStreamPVRManager.cpp:62
    frame #8: 0x0000000100bec1fc Kodi`CDVDFileInfo::GetFileStreamDetails(pItem=0x000000011b5eb3d0) + 1868 at DVDFileInfo.cpp:351
    frame #9: 0x00000001018f6b4c Kodi`CThumbExtractor::DoWork(this=0x000000011b5eb390) + 5068 at VideoThumbLoader.cpp:140
    frame #10: 0x0000000100cae9df Kodi`CJobWorker::Process(this=0x000000011639cf10) + 111 at JobManager.cpp:69
    frame #11: 0x000000010094b080 Kodi`CThread::Action(this=0x000000011639cf10) + 896 at Thread.cpp:221
    frame #12: 0x0000000100949789 Kodi`CThread::staticThread(data=0x000000011639cf10) + 969 at Thread.cpp:131
    frame #13: 0x0000000106cc1805 libsystem_pthread.dylib`_pthread_body + 131
    frame #14: 0x0000000106cc1782 libsystem_pthread.dylib`_pthread_start + 168
    frame #15: 0x0000000106cbefa1 libsystem_pthread.dylib`thread_start + 13
(lldb) 

@ksooo
Copy link
Member Author

ksooo commented Aug 29, 2016

@FernetMenta Happens only if you select a recording folder. Then, for this folder on the right half of the recordings window a "folder listing" is displayed. I think the directory provider behind this listing tries to extract the thumbnails.

screenshot002

@ksooo
Copy link
Member Author

ksooo commented Aug 29, 2016

@FernetMenta yep, the code defining the folder listing ui (around https://github.com/xbmc/xbmc/blob/master/addons/skin.estuary/1080i/MyPVRRecordings.xml#L252) is causing the trouble. I removed it for testing purposes and as expected Live TV no longer gets stopped when opening the recordings window.

Bad thing is that I don't have a good ida for a fix right now. Maybe we should not do the folder listing if pvr is playing live tv or a recording? What do you think?

@axbmcuser
Copy link
Contributor

@ksooo
Another "recordings"-folder issue i noticed (not sure if you get notified by tract since it's assigned to "opdenkamp")
http://trac.kodi.tv/ticket/16866
Maybe - if you got the time - you want to take a look. :) Thanks, as always.

@ksooo
Copy link
Member Author

ksooo commented Aug 30, 2016

@axbmcuser May I kindly ask you not to misuse random unrelated github PRs to contact me personally. Every github comment generates an email for every Team Kodi member. I check trac on a regular base, btw. Thanks.

@axbmcuser
Copy link
Contributor

Understood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: PVR Type: Fix non-breaking change which fixes an issue v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants