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

Fix for a hangup when viewing a PVR recording and pressing next/previ… #12210

Merged
merged 1 commit into from
Jun 5, 2017
Merged

Fix for a hangup when viewing a PVR recording and pressing next/previ… #12210

merged 1 commit into from
Jun 5, 2017

Conversation

hugegreenbug
Copy link
Contributor

@hugegreenbug hugegreenbug commented Jun 1, 2017

…ous (#17476)

This fix skips channel up/down events for saved recordings to prevent the hang

Description

Fixes a hang when pressing previous or next while viewing a saved recording

http://trac.kodi.tv/ticket/17476

Motivation and Context

How Has This Been Tested?

Compiled on MacOS. Binary addons are not showing up with the latest master, so I haven't been able to test fully.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [X ] My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • [X ] I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed

@ksooo
Copy link
Member

ksooo commented Jun 1, 2017

I bet that you have not tested this, because it will not even compile. In master, there is no symbol g_PVRManager anymore. Come on, you can do better.

Hint: CServiceBroker::GetPVRManager()

@ksooo ksooo added Type: Fix non-breaking change which fixes an issue Component: PVR v18 Leia labels Jun 1, 2017
@ksooo ksooo added this to the L 18.0-alpha1 milestone Jun 1, 2017
@hugegreenbug
Copy link
Contributor Author

Yeah, I said I didn't test it yet. I'm working on it.

@ksooo
Copy link
Member

ksooo commented Jun 1, 2017

Yeah, I said I didn't test it yet. I'm working on it.

It is no good practice to submit a PR that has not been compiled/tested locally on at least one platform before. Submitting a PR draws attention of other people and I hate (!) spending my time reviewing a PR and detecting syntax errors, you know. This is the job of compilers before a human beeing gets bothered with the code. ;-)

@hugegreenbug
Copy link
Contributor Author

hugegreenbug commented Jun 1, 2017 via email

@ksooo
Copy link
Member

ksooo commented Jun 1, 2017

Nono, let's keep it open now that things are sorted out. My goal was to engage you to do better next time. ;-)

@ksooo ksooo reopened this Jun 1, 2017
@pollito05
Copy link

yes

@hugegreenbug
Copy link
Contributor Author

@ksooo I have changed g_PVRManager to CServiceBroker::GetPVRManager() and compiled it locally. I am unable to test fully since the binary addons are not showing up in my build. I was told it may be due to recent changes.

@MartijnKaijser MartijnKaijser requested a review from ksooo June 4, 2017 10:29
@ksooo
Copy link
Member

ksooo commented Jun 4, 2017

@hugegreenbug can you please squash changes into a single commit.

 - This fix skips channel up/down events for saved recordings to prevent the hang
 - Bug #17476
@hugegreenbug hugegreenbug reopened this Jun 5, 2017
@hugegreenbug
Copy link
Contributor Author

@ksooo Ok, I committed it as a single commit.

@ksooo
Copy link
Member

ksooo commented Jun 5, 2017

Thank you

jenkins build and merge

@jenkins4kodi jenkins4kodi merged commit 51c5350 into xbmc:master Jun 5, 2017
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 v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants