NO-GO: scoped storage JNI #1
Conversation
only .url_open function is different from file: protocol, and even that is almost identical
ffmpeg may need suffix and/or some other info that can be extracted from SAF intent data also, use dup() to avoid [fdsan issues](https://developer.android.com/about/versions/10/features#fdsan)
with slight modifications to allow more flexible choice of output see also https://stackoverflow.com/a/62252329/192373 The idea is to demonstrate how the saf: protocol can be used with ffmpeg (avformat) API.
using saf: protocol for input
working with ParcelFileDescriptor via JNI
done by receiving the file name from a content: Uri also, adds error checking and required cleanup to JNI calls
cleanup JNI calls, error checking and logging wrap the changes to file.c in #ifdef ANDROID
the previous method did not work when 'content:' Uri had provider path with dots
|
i'm getting cpu features failed while building for LTS version Build command: |
|
Thanks, let me see what went wrong here. |
keep latest filename for reuse
|
@JayParikh20 I tried your command line on a jpg file, and it produced correct results for regular output path, but empty jpg output on Android when I tried scoped storage for output. The video did work correctly. Note that I pushed a quick fix that removes the debug logging that had tons of |
@alexcohn hey, commit worked, images are now processing super fast.
For some reason that command with hardcoded values does not work with scoped storage. You can ignore it, it is working with my original command, working fine for me in both cases (file path & scoped storage)
video files are still not working for me. What is your video selection method? |
|
Could you please try a different location (e.g. Downloads)? Are there any logs marked |
|
So I change my loglevel to AV_LOG_TRACE, and found out that the command stops working after preset option of x264. removing preset option everything works !! (using codec as libx264) I'm sorry I should have looked into this before |
|
Thanks for this update! |
alexcohn
force-pushed
the
feature/scoped-storage
branch
2 times, most recently
from
4ca25d6 to
609d0b9
Compare
|
@JayParikh20 would you like to try the new https://github.com/alexcohn/mobile-ffmpeg/tree/feature/scoped-storage branch for this? I believe that now the balance between ease of use and minimum changes to ffmpeg core is better. The question is, does it effect the performance. |
|
@alexcohn hey, I tried the branch. Did not work for me Using this as an input, am I doing it right?
|
|
@JayParikh20 Sorry, I found it was my fault: it only worked to read documents. Fixed now. But soon I hope to release another change that will work without modifying ffmpeg at all. |
|
@alexcohn thanks, tried with the latest commit, I am still getting the same error. I also have tried it on multiple devices.
In which branch would this be? |
|
The (almost) final version is on https://github.com/alexcohn/mobile-ffmpeg/tree/feature/scoped-storage, it is in sync with https://github.com/tanersener/mobile-ffmpeg/tree/development and this removes my patch to ffmpeg core (libavformat/file.c). This means, you need a full rebuild after you pull this version. |
alexcohn
force-pushed
the
feature/scoped-storage
branch
from
be9c88c to
97b2581
Compare
|
This isn't working for me on Android 9: My command is |
|
@peter9870: you should use |
|
@alexcohn no problem! The issue was the quotes around the input file. The other issue is it'll only take one image. Normally I'd just do |
|
@peter9870: for assorted images, you can use EXTRA_ALLOW_MULTIPLE. For numbered images in the same directory, try ACTION_OPEN_DOCUMENT_TREE. But if these images were created by your app, you probably have full access to them without SAF intents. |
|
@alexcohn String videoPath = Config.getSafParameterForRead(this, mUri);
String testCommand = "-i "+videoPath+" /storage/emulated/0/test.mp4";Then it works as expected. But when I use: String videoPath = Config.getSafParameterForRead(this, mUri);
String[] testCommand = {"-i", videoPath, "/storage/emulated/0/test.mp4"};Then I get Please note that passing a file path as an input works: String[] testCommand = {"-i", "/storage/emulated/0/videoInput.mp4", "/storage/emulated/0/test.mp4"};I'm not sure why this is because I could not see a difference between: and: I'm not sure if it could be me doing something wrong, but I triple checked. |
|
@HBiSoft Thanks for testing. The cause is here I wanted to make It happens so, I didn't think about this use case. But I also want the most basic method to work correctly even for files that have spaces in their names, so I will provide a different workaround, at the price of mangling the original name a bit. |
|
@alexcohn |
|
@alexcohn Thanks, that fixed it. |
|
@alexcohn Edit: |
This is strange. Did you use a string array API, or a single-string API? On the test app, you can see that it works correctly with |
|
@alexcohn |
|
It's very strange indeed. The demo app uses String API, and for me it works. The command is logged before execution. Could you upload the log please? UPDATE: as you use SAF for input, there may be some subtle difference. Could you also post your code that constructs the command String? |
|
hi @HBiSoft what's the status with this |
|
@alexcohn This only happens when calling final String ffprobeCommand = "-hide_banner " +Config.getSafParameterForRead(this, uri);
int rc = FFprobe.execute(ffprobeCommand);
if (rc == RETURN_CODE_SUCCESS) {
//SUCCESS
}When I change it to the following, then it works fine: final String[] ffprobeCommand = {"-hide_banner", Config.getSafParameterForRead(this, uri)};
int rc = FFprobe.execute(ffprobeCommand);
if (rc == RETURN_CODE_SUCCESS) {
//SUCCESS
}Here is the entire log: When I rename the file to Please let me know if there is anything else you need. |
|
@alexcohn Here are the two scenarios I found: return "\"saf:" + fd + "/" + displayName + "\"";
return "saf:" + fd + "/" + displayName.replace(' ', ' ');
What I've done to avoid this is use a EDIT: So, I think the real issue is with |
|
@HBiSoft I am afraid somewhere in your git+build process, the |
|
@HBiSoft I will be very much obliged if you can try again after this change |
|
@alexcohn |
* introducing saf_wrapper - a transparent API to use the file descriptor via custom AVIO for saf: Url * demonstrate how use to use SAF with some demo tabs SAF is enabled for API 19 or higher SAF pickers are used for FFprobe on CommandTabFragment and for video output on PipeTabFragment * Revert "demonstrate how use to use SAF with some demo tabs" This reverts commit 97b2581 * add SAF tab with two buttons * change the getSafParameter handles filenames with spaces see alexcohn#1 (comment) * take advantage of Cursor being Closeable * provide NBSP in hexa, to make sure that git and editor don't destroy it * Avoid "rw" because it may not be supported (e.g. Google Drive). Also, some code cleanup in SafTabFragment. * don't run FFprobe after video playback * ACTION_GET_CONTENT is better than ACTION_OPEN_DOCUMENT: it gives us more options to choose. * add closeParcelFileDescriptor to fix working with Google Drive keep the temp files open, we must keep the ParcelFileDecriptor until the fd is closed
This PR is not going anywhere, at least for now. The current approach in https://github.com/alexcohn/mobile-ffmpeg/tree/feature/scoped-storage is that it does not patch the ffmpeg core at all. The potential advantages of working with
content:protocol in C dwarf in comparison with this.