-
Notifications
You must be signed in to change notification settings - Fork 496
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] Store image/audio/video in FileProvider due to Android 11 updates #215
base: master
Are you sure you want to change the base?
Conversation
… storage, which will not be possible for app to access since Android 11
Cordova-android AndroidX reference: apache/cordova-android#1052 |
…e to AndroidX when cordova-android 10 is release, if necessary
@andreycruz16 I can see that the recorded video is still saved in external storage. Could you make sure that you pulled the code from this PR? It should save the video into a FileProvider, and the video uri should look like this. |
@andreycruz16 , @chriskhongqarma On android 11, Lineage OS 18.1, the video gets saved at path:
but I cannot find any file there (using a root explorer), therefore I am getting error 1 (not found) when trying to play it or copy it. Works fine in older Android versions Any suggestions? |
@mirko77 As I explained in this PR, |
Just FYI: this PR isn't being ignored, just busy preparing It's my goal to have all of these plugins ready to be released at the same time, or shortly after |
@breautek any updates? |
pendingRequests.resolveWithSuccess(req); | ||
} else { | ||
// still need to capture more videos | ||
captureVideo(req); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should createMediaFile(Uri data)
be removed? It doesn't appear to be used / replaced with createMediaFileWithAbsolutePath(String path)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chriskhongqarma Sorry if this is the incorrect place for this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 as createMediaFile(Uri data)
is not used anymore here, fully replaced by createMediaFileWithAbsolutePath(String path)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and other unused elements now like queryImgDB
, checkForDuplicateImage
, whichContentStore
, numPics
that should be removed too
Would it be possible to add search terms / more text to this issue? I spent the better part of a day tracking down an error that appears to be directly addressed by this PR (thank you by the way!!) In the 3.0.3 code, within Then, later in
and the plugin crashes, along with our app. Request: Could some portion of the logcat error be added to the search terms on this page/issue? Maybe I'm wrong, but this might be a nice way for others who might see this connect that error, with this issue / PR. Sorry if this is a dumb question. Thank you so much again for this PR!! |
Hey folks, now that I gave this a test on a couple Android 10-12 devices and it seemed to work, though I see this isn't using AndroidX yet. Let me know if there's anything I can do to help move this along. |
// are reported as video/3gpp. I'm doing this hacky check of the URI to see if it | ||
// is stored in the audio or video content store. | ||
if (fp.getAbsoluteFile().toString().endsWith(".3gp") || fp.getAbsoluteFile().toString().endsWith(".3gpp")) { | ||
obj.put("type", VIDEO_3GPP); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why just this line? was previously
if (data.toString().contains("/audio/")) {
obj.put("type", AUDIO_3GPP);
} else {
obj.put("type", VIDEO_3GPP);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the target file is always defined now with file extension/type? (video always as avi/mp4, and audio as wav) so this block about .3gpp files may be removed?
and only rely on the default obj.put("type", FileHelper.getMimeType(Uri.fromFile(fp), cordova));
?
@@ -488,6 +507,62 @@ private JSONObject createMediaFile(Uri data) { | |||
return obj; | |||
} | |||
|
|||
/** | |||
* Creates a JSONObject that represents a File from the Uri |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from the absolute path
@@ -234,6 +246,17 @@ private void captureAudio(Request req) { | |||
try { | |||
Intent intent = new Intent(android.provider.MediaStore.Audio.Media.RECORD_SOUND_ACTION); | |||
|
|||
String timeStamp = new SimpleDateFormat("yyyyMMddHHmmssSSS").format(new Date()); | |||
String fileName = "AUDIO_" + timeStamp + ".wav"; | |||
File audio = new File(getTempDirectoryPath(), fileName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(for all 3 capture types)
I would drop the use of getTempDirectoryPath
to not write in app temp dir, because this path is cleaned up by the system while such captured audio/image/video should be available afterward in the device media dirs (correct Environment.DIRECTORY_..) as a common practice.
See the Taking photos doc : https://developer.android.com/training/camera/photobasics#TaskPath, really similar to what you have done
- use
File.createTempFile
- with specific values and correct Environment.DIRECTORY_.. for each of our 3 types
- and possibly more
<external-files-path
entries inside mediacapture_provider_paths.xml to match that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Android training link I shared gives example of using getExternalFilesDir
(with corresponding paths.xml) but this writes to the app private dirs and thus requires to "Add the photo to a gallery" to be similar to the auto-indexing of previous Android and iOS mediacapture code!
Note: If you saved your photo to the directory provided by getExternalFilesDir(), the media scanner cannot access the files because they are private to your app.
Do you agree? Or to which dirs will we agree to write by default here?
Any way to capture and write all 3 types to public dirs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://developer.android.com/guide/topics/media/camera.html#saving-media
gives example of mediaStorageDir
defined using Environment.getExternalStoragePublicDirectory
but deprecated since API 29: what to use? are this SO or other correct? with pre and post Android Q code? through ContentResolver? through ACTION_CREATE_DOCUMENT? ...
@chriskhongqarma should be rebased to resolve conflicts since #192 has been merged into master branch yesterday |
@@ -0,0 +1,21 @@ | |||
<?xml version="1.0" encoding="utf-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cordova plugins usually store res files into src/android/res
: so use src/android/res/xml/mediacapture_provider_paths.xml
instead of src/android/xml/mediacapture_provider_paths.xml
audio); | ||
this.audioAbsolutePath = audio.getAbsolutePath(); | ||
intent.putExtra(android.provider.MediaStore.EXTRA_OUTPUT, audioUri); | ||
intent.addFlags(Intent.FLAG_GRANT_WRITE_URI_PERMISSION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this intent.addFlags(Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
?
- the Android training https://developer.android.com/training/camera/photobasics#TaskPath does not have this line
- seems to run fine without it in my test app
@@ -301,6 +328,16 @@ private void captureVideo(Request req) { | |||
PermissionHelper.requestPermission(this, req.requestCode, Manifest.permission.CAMERA); | |||
} else { | |||
Intent intent = new Intent(android.provider.MediaStore.ACTION_VIDEO_CAPTURE); | |||
String timeStamp = new SimpleDateFormat("yyyyMMddHHmmssSSS").format(new Date()); | |||
String fileName = "VID_" + timeStamp + ".avi"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest ".mp4"
instead of ".avi"
as it the file extension I get for video capture with previous code without these FileProvider changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it save to assume file extension to begin with? I imagine this could differ depending on the underlying camera app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@breautek I think you're right :/
"but" one more link in Android training docs should be interesting to follow : https://developer.android.com/guide/topics/media/camera.html#saving-media
for its getOutputMediaFile()
, with output switch for MEDIA_TYPE_IMAGE or MEDIA_TYPE_VIDEO = "IMG_x.jpg" // "VID_x.mp4"
a similar output extension of .mp4
can be found in CameraX training too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, well if the docs shows an assumption of .mp4
I guess it's fine to assume!
|
||
// create a file object from the uri | ||
if(data == null) { | ||
if(this.videoAbsolutePath != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to remove the check on videoAbsolutePath != null
; no such check for audio or image, and the absolutePath is always defined when this method is called
@@ -369,10 +406,8 @@ else if (resultCode == Activity.RESULT_CANCELED) { | |||
|
|||
|
|||
public void onAudioActivityResult(Request req, Intent intent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can remove the Intent intent
param for the 3 onAudio/Image/VideoActivityResult
not used anymore
I tested the 4.0 release, and it still has an error for recording a video. Taking a picture still works. The fork works for both video and pictures. |
@chriskhongqarma because
this is fixed by using which is what the camera plugin is doing as well (@breautek)-> apache/cordova-plugin-camera#827 (review) |
Can this branch use for android 12 13 issue? |
@shivamsharmanps yes but you need to apply the changes I mentioned, or use this fork |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest rebase against current master branch, resolve conflicts, and apply suggestions that everyone pointed out.
@@ -34,7 +34,7 @@ xmlns:android="http://schemas.android.com/apk/res/android" | |||
<engine name="cordova-android" version=">=6.3.0" /> | |||
</engines> | |||
|
|||
<dependency id="cordova-plugin-file" version="^6.0.0" /> | |||
<dependency id="cordova-plugin-file" version="^7.0.0" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this, master branch has newer dependency requirements
<preference name="ANDROID_SUPPORT_V4_VERSION" default="27.+"/> | ||
<framework src="com.android.support:support-v4:$ANDROID_SUPPORT_V4_VERSION"/> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this. We do not support the older Android Support Library anymore
@@ -57,6 +59,8 @@ Licensed to the Apache Software Foundation (ASF) under one | |||
import android.net.Uri; | |||
import android.os.Environment; | |||
import android.provider.MediaStore; | |||
import android.support.v4.content.FileProvider; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace this with AndroidX
@@ -234,6 +246,17 @@ private void captureAudio(Request req) { | |||
try { | |||
Intent intent = new Intent(android.provider.MediaStore.Audio.Media.RECORD_SOUND_ACTION); | |||
|
|||
String timeStamp = new SimpleDateFormat("yyyyMMddHHmmssSSS").format(new Date()); | |||
String fileName = "AUDIO_" + timeStamp + ".wav"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String fileName = "AUDIO_" + timeStamp + ".wav"; | |
String fileName = "cdv_media_capture_audio_" + timeStamp + ".m4a"; |
When I use the recording software, I get m4a files. I dont suggest changing format.
imageUri = contentResolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, cv); | ||
LOG.d(LOG_TAG, "Taking a picture and saving to: " + imageUri.toString()); | ||
String timeStamp = new SimpleDateFormat("yyyyMMddHHmmssSSS").format(new Date()); | ||
String fileName = "IMG_" + timeStamp + ".jpg"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String fileName = "IMG_" + timeStamp + ".jpg"; | |
String fileName = "cdv_media_capture_image_" + timeStamp + ".jpg"; |
@@ -301,6 +328,16 @@ private void captureVideo(Request req) { | |||
PermissionHelper.requestPermission(this, req.requestCode, Manifest.permission.CAMERA); | |||
} else { | |||
Intent intent = new Intent(android.provider.MediaStore.ACTION_VIDEO_CAPTURE); | |||
String timeStamp = new SimpleDateFormat("yyyyMMddHHmmssSSS").format(new Date()); | |||
String fileName = "VID_" + timeStamp + ".avi"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String fileName = "VID_" + timeStamp + ".avi"; | |
String fileName = "cdv_media_capture_video_" + timeStamp + ".mp4"; |
|
||
<paths xmlns:android="http://schemas.android.com/apk/res/android"> | ||
<cache-path name="cache_files" path="." /> | ||
</paths> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
</paths> | |
</paths> | |
Add blank line
Android
Motivation and Context
Since the Storage updates in Android 11, it would be impossible for app to get access to files (image, audio, video) saved into MediaStore external storage, as how it is implemented now. Starting from 2021 August, Android apps are required to target Android API level 30 (Android 11), which means it is critical that we need a new way to store files for media-capture plugin. More details are included in the issue #210.
Here is a suggested implementation for the above issue.
Description
The idea is to use a ContentProvider (a FileProvider, in this case) as a common database to store files, so that both our app and other apps can get access to.
An empty file (audio, image, video) with unique file name (which is generated using current timestamp) and corresponding file format will be created before capturing. Unique file name prevents the file from being duplicated. We use FileProvider to create a Uri for the file, and send it through capturing intents as
EXTRA_OUTPUT
, so that the created file will be saved with the above Uri. Meanwhile, we store the file absolute path as a global variable, so that it can be used when the intent returns results.As the file is saved into FileProvider, the app will get access and have control over the created file.
Testing
captureAudio, captureImage, captureVideo features are tested over various Android devices (from Android 7 to Android 11 are covered) using browserstack app live. The app then has full access over the generated media file (move, delete, transcode, etc).
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)