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

Implement Apple TV Platform #16860

Merged
merged 9 commits into from
Jan 10, 2020
Merged

Implement Apple TV Platform #16860

merged 9 commits into from
Jan 10, 2020

Conversation

fuzzard
Copy link
Contributor

@fuzzard fuzzard commented Nov 2, 2019

Description

Implement the AppleTV platform for ATV4/4k

Motivation and Context

Well here we are, years in the making. May as well say thanks to everyone to get it this far, Memphiz, Davilla (Team Mrmc), Sylvain, Kambala, Phunkyfish, Pogar, and anyone else who has helped that ive missed.

Notes:
PR is not in buildable individual commits. We have broken it up like this for, hopefully, for easier review, particularly the non platform code that is touched. Before Merge, the intent is to squash most into a single commit.
Feel free to update Labels as you see relevant.

Todo:
Currently a lot of refactoring work is being done around darwin_embedded platform as a whole. This will obviously bleed into tvos, but will tackle it later. Feel free to comment on any architecture related issues, but keep in mind, that will most likely go on the todo list for now.

How Has This Been Tested?

ATV dev's, and some external users who have helped with testing

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • 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
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

Copy link
Member

@Rechi Rechi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are not following the code guidelines.

@fuzzard fuzzard force-pushed the master-ATV branch 3 times, most recently from 592a903 to ea72f99 Compare November 3, 2019 09:16
@fuzzard
Copy link
Contributor Author

fuzzard commented Nov 3, 2019

Should be better now rechi. The 2 files that don't conform are 3rd party code we use for a gzip library

tools/depends/target/python3/darwin_embedded.patch Outdated Show resolved Hide resolved
tools/depends/target/python3/darwin_embedded.patch Outdated Show resolved Hide resolved
tools/depends/target/python3/darwin_embedded.patch Outdated Show resolved Hide resolved
tools/depends/target/python3/darwin_embedded.patch Outdated Show resolved Hide resolved
tools/depends/target/python3/darwin_embedded.patch Outdated Show resolved Hide resolved
tools/depends/target/python3/darwin_embedded.patch Outdated Show resolved Hide resolved
tools/depends/target/python3/darwin_embedded.patch Outdated Show resolved Hide resolved
tools/depends/target/python3/darwin_embedded.patch Outdated Show resolved Hide resolved
Copy link
Member

@Rechi Rechi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the most commits in this PR I don't know what changes to expect as it is the directory path of the changed files.

cmake/platform/darwin_embedded/tvos.cmake Outdated Show resolved Hide resolved
tools/buildsteps/tvos/prepare-xbmc Outdated Show resolved Hide resolved
xbmc/settings/DisplaySettings.cpp Show resolved Hide resolved
xbmc/platform/darwin/ios-common/DarwinNSUserDefaults.h Outdated Show resolved Hide resolved
xbmc/platform/darwin/ios-common/DarwinNSUserDefaults.mm Outdated Show resolved Hide resolved
Comment on lines +83 to +96
# Create xcode target that allows to build binary-addons.
if(CORE_PLATFORM_NAME_LC STREQUAL tvos)
if(ADDONS_TO_BUILD)
set(_addons "ADDONS=${ADDONS_TO_BUILD}")
endif()
add_custom_target(binary-addons
COMMAND $(MAKE) -C ${CMAKE_SOURCE_DIR}/tools/depends/target/binary-addons clean
COMMAND $(MAKE) -C ${CMAKE_SOURCE_DIR}/tools/depends/target/binary-addons VERBOSE=1 V=99
INSTALL_PREFIX="${CMAKE_BINARY_DIR}/addons" CROSS_COMPILING=yes ${_addons})
if(ENABLE_XCODE_ADDONBUILD)
add_dependencies(${APP_NAME_LC} binary-addons)
endif()
unset(_addons)
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only add this for tvOS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was put in the cmakebuildsys PR earlier, but was asked to be pulled out for "discussion".
Because of timelines, i felt it easier to attribute to tvos only, and if its desired for ios, i can remove the platform check at a later date.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't related to the previous PR and still isn't to this one. If you want this feature, open a separate PR, which implements it for all platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a feature to ease building for the apple tv platform, how is that not relevant here? Just because it could be used for ios is not the point. We are working towards updating ios stuff as well, and thats all still to come, why not add a feature for THIS platform now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a feature for tvOS only and also not for tvOS and iOS only, it is a general feature.
The reason I ask for doing it right from the beginning is, that otherwise it might get left behind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly i dont have time to vet it for macos right now. I know it works for ios, but again, i PR'd this early knowing there would be feedback, and i understand where you are coming from regarding "forgotten" catch up features for other platforms, but the point is to get this in for Alpha no? Not everything will be perfect, and theres plenty of todo's.

I would rather put it through in its known tested state, and when time permits and proper testing is done, open up the platform check to others.

@fuzzard fuzzard force-pushed the master-ATV branch 2 times, most recently from 676cf58 to 1c9553b Compare November 4, 2019 21:43
@fuzzard
Copy link
Contributor Author

fuzzard commented Nov 4, 2019

For the most commits in this PR I don't know what changes to expect as it is the directory path of the changed files.

Not sure what you mean by this? is this for the cmake treedata split for tvos/ios

@Rechi
Copy link
Member

Rechi commented Nov 5, 2019

E.g. the commit message [tvOS] system, [tvOS] xbmc or [tvOS] xbmc/addons. I have no idea what it changes, without looking at it.
Actually I see no reason to split the changes for tvOS per xbmc subdirectory, because they do not work / compile stand-alone.

@fuzzard
Copy link
Contributor Author

fuzzard commented Nov 5, 2019

Notes:
PR is not in buildable individual commits. We have broken it up like this for, hopefully, for easier review, particularly the non platform code that is touched. Before Merge, the intent is to squash most into a single commit.

Thats a quote from the PR message. The intent is for reviewers to see non platform code at a glance for reviewers relevant areas of expertise. It will not merge with this many commits, it WILL be squashed. If you want it squashed now, thats fine by me, it will be a 3 commits.

@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Nov 7, 2019
@jenkins4kodi jenkins4kodi removed the Rebase needed PR that does not apply/merge cleanly to current base branch label Nov 9, 2019
@fuzzard fuzzard force-pushed the master-ATV branch 3 times, most recently from 7645479 to 791398d Compare November 18, 2019 00:07
@fuzzard fuzzard force-pushed the master-ATV branch 2 times, most recently from 788bad9 to b5f3375 Compare January 5, 2020 20:36
@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 5, 2020

Jenkins build this please

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 6, 2020

jenkins build this please

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 6, 2020

Jenkins build this please

3 similar comments
@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 7, 2020

Jenkins build this please

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 7, 2020

Jenkins build this please

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 8, 2020

Jenkins build this please

fuzzard and others added 9 commits January 8, 2020 19:08
implement WIN_SYSTEM_CLASS define to refactor SettingOptionsMonitorsFiller winSystem
object creation. Less duplicate code.
Currently no use outside of tvos. Can be used for ios, so will put in ios-common
waitpid undef not required. Functions correctly with waitpid on both ios/tvos
…latformVersionDecoded

Changes to a generic code path for GetBuildTargetPlatformVersionDecoded that
utilises GetBuildTargetPlatformVersion to do the macro for the specific platform (ios
and future tvos)
Initial commit for TVOS platform.
binary-addon building added to xcode project for tvOS.
The default behaviour is to add all addons to the project, but not add as a dependency.
This means the addons will NOT be built be default.
Documentation updated and shows how to selectively build addons if required and how to
add the auto building dependecy to the Xcode project.
Implement some todo's regarding sleep/wake workflow.
Stop/start ActiveAE on sleep/wake
Stop/start All network services on sleep/wake
combine and streamline methods involved with sleep/wake
@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 8, 2020

Jenkins build this please

```
make -C tools/depends/target/binary-addons ADDONS="audioencoder.flac pvr.vdr.vnsi audiodecoder.snesapu"
```
**TIP:** BUILD_DIR can be omitted, and project will be created in $HOME/kodi/build
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried it out: The project has been created in ./build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I see the problem?

All conventions in the readme follow on from prior steps, so if you set it up as described, Kodi source gets cloned into $HOME/Kodi, which is where you execute the cmakebuildsys command. This is then going to be ./build, as you are in the directory, or long form $HOME/Kodi/build

Copy link

@chkpnt chkpnt Jan 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I'm sorry. I've missunderstood the sentence, thought $HOME/kodi/build was hard coded somewhere.

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 9, 2020

Jenkins build this please

2 similar comments
@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 10, 2020

Jenkins build this please

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 10, 2020

Jenkins build this please

@fuzzard fuzzard merged commit fd07663 into xbmc:master Jan 10, 2020
@Memphiz
Copy link
Member

Memphiz commented Jan 10, 2020

Yay - congrats - thx and well done to all involved!

@pogarek
Copy link

pogarek commented Jan 10, 2020

Indeed. Well done and congrats. Finally it happened !

@fuzzard fuzzard deleted the master-ATV branch August 16, 2023 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants