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

Fixes and code improvements from game branch #11013

Merged
merged 8 commits into from Nov 30, 2016

Conversation

Projects
None yet
4 participants
@garbear
Copy link
Member

commented Nov 30, 2016

Just some improvements I made in the course of my retroplayer work. The bug fixes here don't affect Krypton, so they don't need to be backported.

For when master is branched for v18 development.

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)
@popcornmix

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

Aren't the DVDCodecUtils patches real bug fixes that are wanted for Krypton?
Ping @fritsch

@garbear

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2016

It's dead code in Krypton, though I've been using it in RetroPlayer for years. I guess we could backport them.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

I don't think we should patch dead code. Better remove that entire method and embed it into retro player.

@garbear

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2016

it's only dead code in master. in my branch I've been maintaining it. it would be nice if we could save refactoring until after I merge the upcoming retroplayer pr due to the heavy load I'm under every time I rebase.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

I progress VP development on a separate brach like you do with retroplayer. When I merge VP changes back to master this dead code might be disappear. Please don't shift the rebase burdon to my side :)
This function is yours, please take ownership on your branch and delete the dead code here. This will eliminate any further merge conflicts.

@garbear

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2016

Let's race ;) if you can wait two days to remove this dead code, I'll get retroplayer in and you can safely rebase.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

I can wait much longer than 2 days. But please don't make retroplayer dependent on code in VP.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

in other words, I am fine with this :)

@garbear

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2016

RetroPlayer only depends on a small bit of VP code that you're trying to separate anyways, so it'll work well to highlight what we can compartmentalize out of the two players.

Here's the PR I'm opening later today: https://github.com/garbear/xbmc/compare/retroplayer~8...garbear:retroplayer?expand=1. The player core's commit will be labeled 0006.

@garbear

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2016

jenkins build this please

@garbear garbear merged commit afa567b into xbmc:master Nov 30, 2016

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
jenkins4kodi You did a great job. Have a cookie.
Details

@garbear garbear deleted the garbear:retroplayer-fixes branch Nov 30, 2016

@hudokkow hudokkow added this to the L 18.0-alpha1 milestone Dec 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.