billthefarmer / mididriver

Android midi driver using Sonivox EAS library
176 stars 52 forks source link

arm64-v8a is not included in built artifact (published in jitpack) #26

Closed amitayd closed 7 years ago

amitayd commented 7 years ago

From build.gradle:

ndkBuild {
                abiFilters 'armeabi', 'armeabi-v7a', 'mips', 'x86'
            }

It's missing arm64-v8a ABI. This is causing java.lang.UnsatisfiedLinkError with: [...] couldn't find "libmidi.so".

(Perhaps there's a reason for omitting this target?).

amitayd commented 7 years ago

Also not clear right now whether the mididriver always uses the built packaged sonivox-lib, or if it uses the vendor "built-in" library.

Is there any reason not to always use the sonivox lib built as part of mididriver?

billthefarmer commented 7 years ago

There is a good reason for that - see #24. Also see the note in the latest release: https://github.com/billthefarmer/mididriver/releases/tag/v1.13. You are welcome to use version 1.11, but be aware of the limitations.

Note: Devices running 4.2.2 (API 17) and below don't appear to be able to load a second native library unless it's built in. Devices running 7.0 (API 24) and above don't allow the use of unsupported built in native libraries. Devices running 64 bit 5.01 or 5.1.1 may have a broken 64 bit version of the sonivox native library and may use the broken built in version rather then the fixed version in the driver.

amitayd commented 7 years ago

Thank you for the explanation. I saw the note but wasn't sure of the implications.

To clarify, as long as I use API 18 and above, I should be able to bundled driver both for 64bit and 32bit (as long as I modify the build to target the ABIs)?

Also, on 64bit devices running android 7.0+, would loading the library fail or it's expected to use the 32bit lib?

I manage to run the miditest app on android 7.1 64bit, but crash with the linkage error on my app, not sure yet why.

amitayd commented 7 years ago

I think the reason I can't use it is since I use other 64bit libraries, so it installs and uses only the native libs under the 64 bit directory (where libmidi.so and libsonivox.so can not be found). See http://stackoverflow.com/questions/30782848/how-to-use-32-bit-native-libraries-on-64-bit-android-device .

billthefarmer commented 7 years ago

Yes, there's an issue about that as well: #15.

amitayd commented 7 years ago

Thanks again for the explanation and your work! The updated sonivox source in mididriver are with the 64bit fixes though, if I understand correctly.

Perhaps client apps using mididriver can the platforms ABIs used themselves through abiFilter - I checked on midiTest and applying the filters 3rd party dependencies as well.

Another option might be a separate jitpack package that will include the 64bit ABIs (which will target android API 18+). Not sure about how to do it, but can look it up later.

Currently since i'm using other 3rd party 64bit native libs, I can only build mididriver myself from a fork (I need the volume feature introduced in v1.13).

billthefarmer commented 7 years ago

The problem with that is that there are devices out there running 64 bit 5.01 or 5.1.1 that may have a broken 64 bit version of the sonivox native library and may use the broken built in version rather then the fixed version in the driver. @rodneyryan found a Galaxy S6 running 5.1.1 that did this. It is possible to publish build variants on jitpack, I haven't worked out yet how to apply it here.

API Which library 64 bit
<17 built in only no
17-22 built in if present possibly broken
22+ app possibly
amitayd commented 7 years ago

Was about to start working on a 64bit-included build variant, and so you already have a branch for it. Is any help needed with testing or completing it? Do you plan to release it to jitpack?

Thanks for all your work!

amitayd commented 7 years ago

If it's helpful, I've managed to create a release based on the variants branch (with no additional changes), build it on Jitpack, and compile using it, and everything worked well. The build.gradle dependency I used was:

compile 'com.github.joytunes:mididriver:v1.14variants:64Release@aar'
billthefarmer commented 7 years ago

The problem with variants on jitpack is that it is not clearly documented how to make a particular variant the default, they just refer to an example which isn't documented either. Otherwise, as you found, it works.

amitayd commented 7 years ago

Seems to be covered on gradle android plugin ( http://tools.android.com/tech-docs/new-build-system/user-guide#TOC-Library-Publication ), and jitpack documentation seems not very comprehensive on other aspects.

Do you think you will create future releases with multiple variants?

I think it would offer a good solution to projects that have other native 64bit libs which are forced to branch now, keeping the support for the existing configuration.

On Jun 11, 2017 8:35 PM, "Bill Farmer" notifications@github.com wrote:

The problem with variants on jitpack is that it is not clearly documented how to make a particular variant the default, they just refer to an example which isn't documented either. Otherwise, as you found, it works.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/billthefarmer/mididriver/issues/26#issuecomment-307644349, or mute the thread https://github.com/notifications/unsubscribe-auth/ACjQgGdr5CzUexkaBUjVEaaLuRcwqLW9ks5sDCVegaJpZM4NFZmm .

billthefarmer commented 7 years ago

Well I tried it and it sort of worked, but I've thought of a simpler solution I should have thought of months ago, and I'm surprised that it hasn't been suggested.

If I build the sonivox library as a static library and build the midi library against it, that should work for all devices. There's only one native library, it's not called libsonivox.so, so no devices can use a possibly broken build in version and all ABIs can be built and included. I just tried it and it works.

amitayd commented 7 years ago

Version 1.14 seems to solve this issue (we can use the same dependency for both 32bit and 64bit on a project that has other libraries which are 64bit). Thanks for your efforts!

billthefarmer commented 7 years ago

I did a bit more testing with variants. It seems you can either have a defaultPublishConfig or publishNonDefault but not both on JitPack. It works locally.