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

use jniLibs to work with targetSdkVersion=29 #130

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

alexcohn
Copy link

Fixing #126

@darrinps
Copy link

Just curious why this hasn't been merged in. Seems straightforward and needed.

@alexcohn
Copy link
Author

@darrinps there are some good reasons why this was not merged. One, as you noticed, is that this still lacks 64 bit, which makes this solution unsatisfactory for majority of the people who need to target Android Q. Second, people may consider renaming the executable to lib…so to be a dirty trick. Third, this removes a lot of code people had wrote, debugged, and invested into. Not a very nice feeling for a maintainer.

@HiroyukiTamura
Copy link

I'm strongly looking forward speedy marge and release, and next one as completely fix.

@pcm2a
Copy link

pcm2a commented Sep 26, 2019

I would like to target Q as well but cannot until ffmpeg is working with it. Is this code in a fork with a release that can be included?

@alexcohn
Copy link
Author

@pcm2a what is your incentive to target Q? Remember, you can use Q APIs in a project that targets earlier SDK. At any rate, if you don't set min supported SDK to Q, you have to check the availability of these APIs before using them.

@pcm2a
Copy link

pcm2a commented Sep 29, 2019

@alexcohn just planning ahead. When R is released in 2020 all apps will be forced to target Q I plan on being ready in advance vs rushing to the finish line.

@alexcohn
Copy link
Author

alexcohn commented Oct 1, 2019

@pcm2a if you think this PR is important, feel free to vote for it.

@kartik1225
Copy link

Waiting for this merge

@HBiSoft
Copy link

HBiSoft commented Feb 3, 2020

Besides this fix, for this library to work when targeting Q, we need an option to pass a FileDescriptor instead of having to pass the file path.

For example, instead of:

String[] command = {"-i", mStringFilePath, "-crf", "18", "-c:v", "libx264", "-c:a", "copy", pathToAppDirectory};
FFmpeg.getInstance(this).execute(command, new ExecuteBinaryResponseHandler()...

We could be given an option to pass a FileDescriptor, like this:

FileDescriptor fd = getContentResolver().openFileDescriptor(Uri, "r").getFileDescriptor();
//Note that there is no input path provided below
String[] command = {"-crf", "18", "-c:v", "libx264", "-c:a", "copy", pathToAppDirectory};
FFmpeg.getInstance(this).execute(command, fd, new ExecuteBinaryResponseHandler()...
//and when the developer doesn't want to use the FileDescriptor
FFmpeg.getInstance(this).execute(command, null, new ExecuteBinaryResponseHandler()

@alexcohn
Copy link
Author

alexcohn commented Feb 3, 2020

@HBiSoft Passing FileDescriptor still won't let you work around the scoped storage. You need significant changes on the ffmpeg binary as well, to handle this: pass these file descriptors via Unix sockets.

If you switch to an ffmpeg in-process library solution, e.g. https://github.com/tanersener/mobile-ffmpeg, you can transparently pass your FileDescriptor with the /proc/self/fd/%d trick or pipe: protocol or use it through a custom AVIOContext.

@HBiSoft
Copy link

HBiSoft commented Feb 3, 2020

@alexcohn The owner of FFmpegMediaMetadataRetriever did something similar, have a look here:

https://github.com/wseemann/FFmpegMediaMetadataRetriever/blob/b42986fb4044c54131379407598d8ac0ff1a3326/gradle/fmmr-library/library/src/main/jni/metadata/ffmpeg_mediametadataretriever.c#L304

You can also have a look at my post on the mobile-ffmpeg library which is related to this - tanersener/mobile-ffmpeg#334 (comment)

I believe the owner of this library can easily implement this as well.

@alexcohn
Copy link
Author

alexcohn commented Feb 3, 2020

@HBiSoft they have their struct State that can handle such URI, that's not how FFmpeg-Android is built.

@HBiSoft
Copy link

HBiSoft commented Feb 4, 2020

@alexcohn
I was able to get it working on mobile-ffmpeg using the pipe protocol.

What suggestions do you have if we want to achieve the same using this library?

@alexcohn
Copy link
Author

alexcohn commented Feb 4, 2020

@HBiSoft on the one hand, ffmpeg supports the pipe protocol out-of-the-box, and I believe that the Android port did not damage this functionality. On the other hand, the pipe protocol is not seekable, which may be a significant limitation for some ffmpeg file formats and some applications. I won't propose it as a panacea.

@HBiSoft
Copy link

HBiSoft commented Feb 4, 2020

@alexcohn
I tested it with this library and it didn't work. Even if it does, it's not the solution.
This library will either have to be rebuild in a way that excepts Uri's and can handle them properly or I can't see it working after developers are forced to target API level 29 (which will probably be within the next few months).

@alexcohn
Copy link
Author

alexcohn commented Feb 4, 2020

@HBiSoft

I tested it with this library and it didn't work.

I am confused. Did you test pipes or the /proc/self/fd trick?

@HBiSoft
Copy link

HBiSoft commented Feb 4, 2020

@alexcohn I tested with pipe ->

String safUriToFFmpegPath(final Uri uri) {
    try {
        ParcelFileDescriptor parcelFileDescriptor = getContentResolver().openFileDescriptor(uri, "r");
        return String.format(Locale.getDefault(), "pipe:%d", parcelFileDescriptor.getFd());
    } catch (FileNotFoundException e) {
        return "";
    }
}

@alexcohn
Copy link
Author

alexcohn commented Feb 8, 2020

The pipe: protocol has its own limitations, but in my limited experiments, the /proc/…/fd/… trick worked as expected. Only note that you cannot use /proc/self/fd here, you must put the actual Process.myPid() instead.

@alexcohn
Copy link
Author

alexcohn commented Feb 8, 2020

@HBiSoft please see alexcohn@686c4b5 – for me it resolves the problem with Android Q. I only care about input, but I have no doubt you can follow the same approach to use an output directory out of the app sandbox.

@HBiSoft
Copy link

HBiSoft commented Feb 9, 2020

@alexcohn
Thank you for your reply.

I have tested the above and I get /proc/4759/fd/70: Permission denied when selecting from the SD Card.

I see you mentioned this:

Work around the API 29 new [restrictions on use of external storage]

@alexcohn
Copy link
Author

I have tested the above and I get /proc/4759/fd/70: Permission denied when selecting from the SD Card.

Yes, it seems to only work for Downloads.

@alexcohn
Copy link
Author

alexcohn commented Mar 8, 2020

Thanks to @Le-Dinh-Nam #126 comment, I could clean up a bit more. It seems that later platform versions extract all files from lib folder, not filtering by the lib.so pattern, as earlier.

UPDATE no, it does not, see Google Issue. I was forced to revert this change.

@Aditya94A
Copy link

Looks good! @Brianvdb Please merge this.

@maxirosson
Copy link

Any update about this?

@magicseal
Copy link

waiting for this to be merged..

This reverts commit e50d1c4
See https://issuetracker.google.com/issues/152645643 for discussion: this is not supposed to work, wrap.sh is a special well-known name. Also, older devices (e.g. API 24) don't install files from libs directory if they don't match the expected "lib.*\.so" pattern.
@serkanucar
Copy link

Any update about this?

@SoluLabLive
Copy link

Any update about this? When can I expect a new version?

@maxirosson
Copy link

Any update about this?

@Ahmed-Basalib10
Copy link

any update ??

@GauravCreed
Copy link

Hi @alexcohn

Any update...? Then please help me..
Thanks(_)

@alexcohn
Copy link
Author

@GauravCreed help you what?

@GauravCreed
Copy link

Hi @alexcohn

getting ffmpeg error code 13 in android 10 permission denied in android 10 device.
so i am using https://github.com/tanersener/mobile-ffmpeg then problem is apk size increase.

@Aditya94A
Copy link

Looks like this isn't going to be merged, is it safe to use the fork directly? @alexcohn Any gotchas? Does this fully work with Android Q/R in the way we expect bravobit/FFmpeg-Android to work?

@HBiSoft
Copy link

HBiSoft commented Nov 13, 2020

@AdityaAnand1 I would recommend using (https://github.com/tanersener/mobile-ffmpeg). You get more control over what arch you want to build and you can pass it a Uri.

@pratik-tech4
Copy link

pratik-tech4 commented Nov 23, 2020

Hi, @alexcohn

I am using this library,

https://github.com/tanersener/mobile-ffmpeg
implementation 'com.arthenica:mobile-ffmpeg-full:4.2.2.LTS'

this is working on android 10 too ,
But the problem is i added this library my apk size is increased too much, almost 50mb increased.

Could you please help me with this?

@alexcohn
Copy link
Author

@pratik-tech4 you definitely don't need the 'full' flavor of mobile-ffmpeg: it includes extra libraries that are not part of bravobit. You can make the right choice, or even build a bespoke mixture yourself, only including the external libs that you really need (and maybe revisit your commands to avoid non-necessary external libs, too). Also, if you don't use AAB and don't use split APK, the APK now includes all ABIs. For production, you only care about arm64 and armv7-a as separate binaries.

@GauravCreed
Copy link

HI @alexcohn

How about when I am using both audio and video both, i want to add text and image both in video,

implementation 'com.arthenica:mobile-ffmpeg-audio:4.4'
implementation 'com.arthenica:mobile-ffmpeg-video:4.4'

it will gives some error like duplicate classes.

Capture

@pratik-tech4
Copy link

Hi @alexcohn

as @GauravCreed said same way i tried but same error.

If i add audio and video version both it gives me same error as @GauravCreed faced. Thats why i am using full version.

I am using it to cut audio then merge it and adding audio to video. So if possible which version i should use to work these functionality.

@alexcohn
Copy link
Author

@GauravCreed, @pratik-tech4: no, you cannot and should not add two ffmpeg-mobile packages.

I would suggest to start with min, and find out what is missing for you. Only if you find that you desperately need more, consider another package. Note that the audio package does contain basic video capabilities, while the video package still provides the essential audio functions.

@HBiSoft
Copy link

HBiSoft commented Nov 28, 2020

@GauravCreed, @pratik-tech4
You can build it with custom options like this:

  • In android.sh look for DISPLAY_HELP="" and set it to DISPLAY_HELP="yes".
  • Then right-click on android.sh and select run.
  • You will be prompted with the following:
    Screenshot 2020-09-06 at 08 25 26
  • Copy the path at the top, mine for example is /Users/Hagen/Desktop/mobile-ffmpeg-master/android.sh
  • Then paste it at the bottom
  • You can add all the options you need next to it, in your case I would suggest the following - /Users/Hagen/Desktop/mobile-ffmpeg-master/android.sh -l --enable-gpl --disable-x86 --disable-x86-64 --enable-x264
  • Go back to android.sh and set DISPLAY_HELP="yes" back to DISPLAY_HELP="".
  • Right-click on android.sh and select run.
  • Wait till it completed and you will find it in the prebuild folder.

Use the .aar in your project and do a few tests. If there are missing libraries (in my case zlib) build the project again and add the missing lib.


By doing this, you have full control over which libs you want based on what you need.

@pratik-tech4
Copy link

hi, @HBiSoft , @alexcohn

Thank you for your response,

for now i used
implementation 'com.arthenica:mobile-ffmpeg-full:4.4.LTS' in this my apk size is around 74MB.

so, I added ndk filters like below

ndk { abiFilters "armeabi-v7a","arm64-v8a" }

by this two filters apk size is around 48MB.

So now i have one doubt is that

if i make sign apk with these two filters "armeabi-v7a","arm64-v8a", is it approve in play store? as in play store apk must have 64-bit support.

So by these two filter it will approve or no? And it will run in older devices? like which are 32 bit.

Thanks in advance!

@HBiSoft
Copy link

HBiSoft commented Nov 28, 2020

@pratik-tech4
Yes it will be approved. armeabi-v7a is 32bit and arm64-v8a is 64bit.

I would rather suggest using splits (as shown below) instead of using abiFilter. By splitting the apk`s you will only install the necessary apk. Using abiFilter, you will include all the archs, which increases your apk size unnecessarily.

android.splits {
    abi {
        enable true
        reset()
        include 'armeabi-v7a', 'arm64-v8a'
        universalApk false
    }
}

@pratik-tech4
Copy link

@pratik-tech4
Yes it will be approved. armeabi-v7a is 32bit and arm64-v8a is 64bit.

I would rather suggest using splits (as shown below) instead of using abiFilter. By splitting the apk`s you will only install the necessary apk. Using abiFilter, you will include all the archs, which increases your apk size unnecessarily.

android.splits {
    abi {
        enable true
        reset()
        include 'armeabi-v7a', 'arm64-v8a'
        universalApk false
    }
}

@HBiSoft
Thanks

by doing this changes both apk is build separately app_armeabi-v7a.apk, app_arm64-v8a.apk. So which one need to upload in play store?

@AxitaKathiriya
Copy link

When will you release new version of library with this merge? Please release this as soon as possible. I am waiting for this..

@Shabinder
Copy link

Shabinder commented Sep 3, 2021

@alexcohn Hey thanks for this PR and it works fine when building and using debug apk,

but as soon as I use a release apk after signing, it doesn't save ffmpeg & ffprobe in /data/app/xxxx/libs folder.

I have tried various methods like enabling/disabling android:extractNativeLibs="true" in App manifest but no luck

However migrating libs to jniLibs folder as lib..ffmpeg.so worked , no idea why....
if anybody does, I am curious

@alexcohn
Copy link
Author

alexcohn commented Sep 5, 2021

@Shabinder that's true. Android system APK installer extracts only libraries that follow the strict libxxxx.so naming convention, but makes exception for debug APKs (so that you can embed your debug scripts, like wrap.sh).

When I published this PR, I was not aware of this weird behavior.

@Shabinder
Copy link

When I published this PR, I was not aware of this weird behavior.

Yeah the most weird about it is that there is no flag to keep such resources when building release apk.
Anyway thanks for your contributions they helped a lot 👍

@alexcohn
Copy link
Author

alexcohn commented Sep 5, 2021

At any rate, I strongly recommend you to switch from a poorly supported executable-based solution to a library-based one, e.g. https://github.com/tanersener/ffmpeg-kit.

@Shabinder
Copy link

Shabinder commented Sep 5, 2021

At any rate, I strongly recommend you to switch from a poorly supported executable-based solution to a library-based one, e.g. https://github.com/tanersener/ffmpeg-kit.

I did went to initially configure ffmpeg-kit, but I needed very much control over ffmpeg-build configuration and wanted to keep the lib size to as low as possible, hence I decided to build my own ffmpeg libs and include them in-app,
and as a result Adding FFmpeg just increased just around 2.9 mb in my apk (All 4 arch arm64, x86, arm7, x86_64 ) , I was surprised too.
if you want to explore, project is here: https://github.com/Shabinder/SpotiFlyer

Adding FFmpeg-kit as a submodule was not a solution while configuring it custom as It was giving some error that said that produced aar wont be included in parent app(I don't remember the proper wording but there was a limitation) and I didn't wanted to go to the route of maintaining my fork and using mavenCentral as the source

@alexcohn
Copy link
Author

alexcohn commented Sep 6, 2021

You cannot rely on Android Studio to build and copy the ffmpeg aar into your apk. You can include any custom build steps in your build.gradle, though. Or simply copy the aar into your project, the way you handle the executables. But even if you work with executables, it's worth the hassle to split APKs or build a Bundle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet