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 NDK r18 Android x86 ffmpeg dependency build #14615

Merged
merged 1 commit into from Oct 22, 2018

Conversation

@djp952
Copy link
Contributor

commented Oct 15, 2018

Description

After the switch to NDK r18, the Android x86 ffmpeg dependency no longer builds properly. The ffmpeg source code isn't entirely compatible with the r18 clang toolset.

Motivation and Context

This PR adds a couple extra CFLAGs to the Android x86 ffmpeg Makefile to disable use of the clang integrated assembler and deal with an additional API level 21 incompatibility.

How Has This Been Tested?

Tested on Ubuntu 16.04 using the environment as specified in the Android Build README.md. After the change all dependencies, including ffmpeg 4.0.2, build successfully for the x86 platform.

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
@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Oct 15, 2018

@djp952 is that the only thing needed for x86 to build? We have disabled these builds for a long time and last time i tried it was broken (at least for jenkins)

@djp952

This comment has been minimized.

Copy link
Contributor Author

commented Oct 15, 2018

So far so good, the main Kodi application compiled perfectly fine for me for x86. I didn't build the binary addons, though. I am moving the APK over to another system to test it out on an Atom-based Android emulator image.

@djp952

This comment has been minimized.

Copy link
Contributor Author

commented Oct 15, 2018

This may be academic regardless … I looked into the text relocation message and to get rid of it ffmpeg would have to be compiled with --disable-asm or be moved back to a target SDK of 22. Neither of which are desirable (I assume). --disable-asm is reported as introducing "crippling" performance problems on x86. ffmpeg has no plans to change around their x86/x64 code to support this, and Google similarly has no plans on allowing it on x86/x64 anymore.

I suggest keeping these build changes so that folks like me can still self-build for Android x86 as needed, but it doesn't seem like you'd be able to put a viable build in front of an end user. Unless of course you guys can get Google or ffmpeg to budge on their respective positions about x86 text relocations.

@Rechi
Rechi approved these changes Oct 20, 2018
@Rechi

This comment has been minimized.

Copy link
Member

commented Oct 20, 2018

@MartijnKaijser last time you tried toolchain for the used NDK version wasn't installed.

@MartijnKaijser MartijnKaijser added this to the Leia 18.0-beta4 milestone Oct 22, 2018
@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Oct 22, 2018

Only touches Android x86 which we don't build and and ARM build just fine

@MartijnKaijser MartijnKaijser merged commit d114666 into xbmc:master Oct 22, 2018
1 check failed
1 check failed
default Sorry, building this PR failed. Please check the logs for the errors.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.