-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
refactor(android): replace image path usage with image uris #906
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
breautek
force-pushed
the
refactor/image-file-paths
branch
from
October 25, 2024 21:39
42babc5
to
7be7422
Compare
5 tasks
breautek
force-pushed
the
refactor/image-file-paths
branch
from
October 26, 2024 04:01
7be7422
to
314928e
Compare
erisu
approved these changes
Oct 28, 2024
Exigerr
pushed a commit
to PebblePad/cordova-plugin-camera
that referenced
this pull request
Jan 21, 2025
… / audio) files from the gallery picker. Changes from apache#906 resulted in all gallery picker results being read in full into a byte array. This works fine when dealing with images, however when larger files (video / audio) are selected it will nearly always result in an out of memory error (reading too much data in one go). With videos no transformation is required the URI, the URI simply need to be returned.
Exigerr
pushed a commit
to PebblePad/cordova-plugin-camera
that referenced
this pull request
Jan 21, 2025
…) files from the gallery picker. Changes from apache#906 resulted in all gallery picker results being read in full into a byte array. This works fine when dealing with images, however when larger files (videos for example) are selected it will nearly always result in an out of memory error (reading too much data in one go). With videos no transformation is required, the URI simply need to be returned.
Exigerr
pushed a commit
to PebblePad/cordova-plugin-camera
that referenced
this pull request
Jan 21, 2025
…large media (video) files from the gallery picker. Changes from apache#906 resulted in all gallery picker results being read in full into a byte array. This works fine when dealing with images, however when larger files (videos for example) are selected it will nearly always result in an out of memory error (reading too much data in one go). With videos no transformation is required, the URI simply need to be returned.
5 tasks
KarinBerg
added a commit
to MobisysGmbH/mobisys-cordova-plugin-camera
that referenced
this pull request
Feb 4, 2025
* chore: bump plugin version 7.0.0-dev (apache#845) * dep(dev)!: bump @cordova/eslint-config@5.0.0 (apache#846) * dep(dev)!: bump @cordova/eslint-config@5.0.0 * chore: apply automatic lint fix * feat(android)!: Android 13 support (apache#844) * feat(android)!: Android 13 support * refactor(android): simplify getPermissions logic * feat(android)!: bump cordova-android requirement to >=12.0.0 * feat(android): update saveAlbumPermission to include Android 9 and below use case --------- Co-authored-by: ochakov <evgeny@ochakov.com> * chore: Update SUPPORT_QUESTION.md template (apache#849) * fix!: remove deprecated platforms (apache#848) * chore: remove windows/osx from plugin.xml (apache#850) * ci(gh-action): sync with paramedic configs (apache#851) * release(camera-v7.0.0): updated version and RELEASENOTES.md * chore: bump version 7.0.1-dev * ci(android): Update Android CI to be compatible with cordova-android@13 (apache#890) * chore: Added npmrc * ci: sync workflow with paramedic (apache#895) * chore: Update eslint config to 5.1.0 (apache#898) * chore: Update package to 8.0.0-dev (apache#899) * Remove media permissions to make complaint with Android 14 requirements (apache#889) Co-authored-by: Ravi Yakasiri <ravi.yakasiri@planonsoftware.com> * fix(android): Isolate provider access to a subdirectory (apache#901) * fix(android): Use VERSION_CODES instead of hard-coded API literals (apache#904) * fix(android): improper cache path construction during image manipulation (apache#905) * fix(android): Improper serialization of image uri in save instance state (apache#903) * fix(android): Return data uris as an URI (apache#910) * fix: return content uris when possible when selecting from gallery (apache#902) * fix(browser): Make data uri be returned as actual URI strings (apache#912) * fix(ios): Sync camera API return to match Android changes (apache#911) * refactor(android): replace image path usage with image uris (apache#906) * refactor(android): clean up image file path usages * removed references of image paths in log messages * refactor(android): remove query img usage (apache#907) * refactor: remove unnecessary duplicate image checks and queryImgDb usage * remove unused imageType parameter, because it's a private API anyway * docs: Revisions for v8 public API changes with the return string formats of getPicture (apache#913) * refactor(android): Make WRITE_EXTERNAL_STORAGE optional (apache#909) * refactor(android): Rework permission management to make WRITE_EXTERNAL_STORAGE optional * removed unused getPermissions API * Proper error if WRITE_EXTERNAL_STORAGE is required but missing the declaration * removed obsolete hasPermissions API * fix: Remove WRITE_EXTERNAL_PERMISSION (apache#915) * deprecation: allowEdit (apache#914) * deprecation: allowEdit * applied suggestions to verbiage * chore: version 8.0.0 * chore: 8.0.1-dev * chore: remove trailing whitespace (apache#921) * Change "allowedPublishingBranches" from "refs/heads/master" to "refs/heads/release" * ci: Publish only the file "mobisys-internal-cordova-plugin-camera-*.tgz" to AzureDevOps * ci: Use "release/v8" of "devops-templates" * Add new section "Branches" to the README.md --------- Co-authored-by: エリス <erisu@users.noreply.github.com> Co-authored-by: ochakov <evgeny@ochakov.com> Co-authored-by: jcesarmobile <jcesarmobile@gmail.com> Co-authored-by: Erisu <erisu@apache.org> Co-authored-by: Norman Breau <norman@breautek.com> Co-authored-by: ravi-yk <107058879+ravi-yk@users.noreply.github.com> Co-authored-by: Ravi Yakasiri <ravi.yakasiri@planonsoftware.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Platforms affected
Android
Motivation and Context
Note this contains #902 will rebase after merge.
Raw system file paths are dangerous to use nowadays since they are often restricted, especially when dealing with external filesystems or filesystems that might not even be on-device (e.g. Google Drive).
Therefore all usages of
imageFilePath
was removed and replaced withimageUri
.Code that used
imageFilePath
generally now accepts aUri
instead or anInputStream
.Description
readData / getPageSize
Utility methods to read data into memory. getPageSize is an optimization to read a page of memory, which is usually 4kb but Android 15 devices may be shipped with 16kb page size devices.
There is a lot of disk usage when processing images which might be necessary back in the day when RAM is limited but modern devices have plenty of RAM to deal with image content. It's far more efficient to read the data into memory once and provide several instances of in-memory data streams instead of constantly reading data from disk. (See notes on
getScaledAndRotatedBitmap
for more details)ExifHelper
A new method exposed to instantiate via
InputStream
.getScaledAndRotatedBitmap
No longer accepts a file path. Instead it accepts the raw binary data. It no longer opens or manages it's own stream.
It will create
ByteArrayInputStream
which is a stream that doesn't require to be closed, as it operates on in-memory data. It doesn't need to manage kernel objects like file descriptors.Older code used to create several read streams and read the input source from disk several times. It also created temporary files which was deemed a requirement when dealing with "off-device" files such as Google Drive. There is some truth to this, but temporary files isn't necessary. Data can be read from in-memory instead saving the need to constantly read and write from disk. Content Resolver APIs can also obtain some data like mime type.
Therefore lots of stream management code was removed, which allowed us to remove a lot of try/catches as well. Using the exif changes, we use a
ByteArrayInputStream
instead virtually everywheresfileStream
was previously used.Note that this is used when sourcing an image from a gallery, it has an unrelated issue that is out of scope of this PR that causes it to not use the result of the modifications and instead returns the original image anyway.
Sourcing an image from camera and applying modifications via
targetWidth
ortargetHeight
does work as expected.on save instance / on restore instance
the image file path key and serialization/restoration were removed.
processResultFromCamera
sourcePath
was replaced withInputStream input
.croppedFilePath
is still used and is wrapped by aFileInputStream
. Refactoring this file path is out of scope of this particular PR, which is only applicable ifallowEdit
is enabled.Otherwise
imageUri
is used and resolved by the content resolver.input
is then asserted for non null, and throwsIOException
otherwise.The input is then read into memory and stored in
sourceData
which is fed intogetScaledAndRotatedBitmap
. See notes above.Other Changes
I realise there's a ton of other "format" changes, which
Android Studio IDE is responsible for and I do plan on cleaning them up in another PR.
Testing
Manual testing on API 28 (for non-scoped access framework) & 34 (for scoped access framework).
Paramedic tests passes.
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)