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

[Android][cmake] adjust macros according to the API we are targeting #23414

Merged
merged 2 commits into from Jun 17, 2023

Conversation

joseluismarti
Copy link
Contributor

Description

As we're targeting API level 21:

Requires the _LARGEFILE_SOURCE macro to be used to make fseeko and ftello available. It would not be necessary from API level 24 onwards.

Does not require the _LARGEFILE64_SOURCE macro to be used to make off64_t and corresponding functions available. These functions are available in the NDK at the current API level.

Also does not require the _FILE_OFFSET_BITS=64, these counterpart functions have been progressively incorporated into the NDK.

The __USE_FILE_OFFSET64 macro is the internal flag that should never be set by user code.

How has this been tested?

I've tested it on Fire TV (32 bits) and it works as usual, it plays files larger than 2GiB correctly.

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

@fuzzard fuzzard added this to the Omega 21.0 Alpha 3 milestone Jun 17, 2023
@fuzzard fuzzard added Type: Improvement non-breaking change which improves existing functionality Platform: Android v21 Omega labels Jun 17, 2023
@jenkins4kodi
Copy link
Contributor

I've made some formatting changes to meet the current code style. The diffs are available in the following links:

For more information please see our current code style guidelines.

@fuzzard fuzzard merged commit 8f774d1 into xbmc:master Jun 17, 2023
2 checks passed
@joseluismarti joseluismarti deleted the cmake-macros branch June 18, 2023 06:39
@neo1973 neo1973 mentioned this pull request Jul 3, 2023
7 tasks
@fritsch
Copy link
Member

fritsch commented Jul 3, 2023

Seems some the enforcing of the offset being 64 bit is still needed. This causes regressions. As on 32 bit userspace not enough bits.

@joseluismarti
Copy link
Contributor Author

I've tried reverting the commit locally and there is no difference in seeking performance when playing a video via SMB. Tested on Android armv7.

I don't think this is the cause, we will have to do more tests or wait for more feedback.

@neo1973
Copy link
Member

neo1973 commented Jul 3, 2023

@joseluismarti: Could you upload your local build somewhere so @MishaIsay could try it on their setup? Just to make sure this isn't the issue.

@joseluismarti
Copy link
Contributor Author

joseluismarti commented Jul 3, 2023

A test apk with the revert of this commit in case anyone needs to do some testing.

As the signature is different you must uninstall Kodi first. If necessary make a backup since uninstalling deletes everything.

Edited: I've checked it again and causes a regression when seeking.

@MishaIsay
Copy link

Seeking in that build works correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: Android Type: Improvement non-breaking change which improves existing functionality v21 Omega
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants