-
-
Notifications
You must be signed in to change notification settings - Fork 94
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
improve memory consumption while image processing #2681
Comments
background figured out at deltachat/deltachat-ios#1330 (comment): the image in question has about 120 MB raw data - so the mentioned 250 MB needed for recoding seems reasonable. however, as recoding fails we do not really know how if the 250 MB would be sufficient - would be interested to know the memory consumption for an image with say, 16mb raw data (2000px * 2000px) - if that is 32mb then, i'd say, core cannot do much upon that. |
There are ways to load only tiles of an image, scale them and write them back to the memory before loading the next chunk and probably other even more efficient ways I'm not just aware of. Fixing this issue would give a UX boost for sharing larger images on iOS and generally it would help to not being killed under memory pressure. |
I have profiled memory consumption of the test suite. When you run
in the first few lines. This way I find the binary name (
At the end of testing,
JPEG-related functions in image recoding are near the top of the memory peak flamegraph: |
thanks for the profiling - and sorry for coming back so late. i am currently on trying to find a solution for deltachat/deltachat-ios#1330 - the core issue seems to be that iOS extensions must not use more than 120 mb in no means
tbh, i have no good idea what that should tell me :) which image size does the benchmark try to decode? a typical panorama may be sth as 16382 x 3690 pixels (tested on iphone7, iphone13) - so 180mb raw data assuming 3 bytes/pixel - i assume that is fully loaded to memory before trying to recode to sth. as 1280 x 288 would be? can you confirm the 3 bytes/pixel? unless the recoding in core is achieved smarter, my idea is to not pass such large images to core from the share extension but show a message to the user, saying, you can do this in the app (which allows using up to 2000mb peak). alternatively, we could try to pre-scale the image using iOS, that seems to work better (eg. the thumbnails of the huge panoramas is displayed) there is probably not much that can be improved in core about that and we could close this issue then (which is also no bug then) |
Recent report from @lk108 A friend of mine is running 1.48.3-gplay on Android. Sending large images does not work, the message composer aborts (?, probably need to ask for details again) and the log says
So the problem also exists on Android, iOS workaround deltachat/deltachat-ios#2020 only helped on iOS. |
Maybe related |
The solution for this is to check the dimensions of image before resizing by reading its header. If dimensions are unknown or too large, we should not try to load the image into memory as bitmap and resize it, but send it as is. |
Probably a UI setting is needed then. It's not clear if the user wants to send big images generating much traffic, maybe they would prefer to get an error by default and retry to send an image as file if needed. |
The friend I mentioned expects DC to automatically resize big images. They know it from other messengers, and it's mentioned in the DC FAQ that "For performance, images are optimized and sent at a smaller size by default". So just sending a 25 MB image as is without warning would be unexpected. Getting an error would be better, but it would still be perceived as a feature that is missing despite being advertised, at least by this one person. Besides, other apps are able to resize the image on their phone, so they wonder why other apps can do it, but DC can't. Maybe it can't be helped at the moment, but if it depends on the mentioned |
I don't propose to disable resizing completely, only not try to resize if it has unreasonable resolution like some astronomic images. Anything with resolution produced by phone cameras should be resized, but if someone sends 100000x100000 entirely black PNG with some image in the corner, we should send it as is even if it is slightly larger than normal size because |
I agree that we should look at the image dimensions before trying to resize it, but allowing "slightly larger" (in byte size) images seems not helpful. Rather such a "strange" image would either fit into the byte size limit or exceed it overly, so allowing some extra room here would only add uncertainty (from the user's point of view) about which images can be sent by Delta Chat and which can't. So i'd say we should just return an error for images with big dimensions. If the user actually wants to send it, they can send it as file. |
Well, maybe return an error if bitmap size (width x height x bit depth) is higher than 120 mb, referencing https://forums.developer.apple.com/forums/thread/115259 as the source of the limit in the comment. |
So for the user that reported the issue, the image will not be resized, but an error will be shown? They weren't trying to attach something from an external camera, but an image taken by their phone (granted, a high-end model with resolution set to max). |
I think that as a temporary solution it's the best choice. Then we maybe can try to fix it in the |
This probably requires not only max resolution, but also making a panorama rather than a normal photo. They can still send it by attaching "as a file" or somehow resize it using their phone tools. We will see if users complain about this error once the limit is there. |
Using the current iOS app 1.22.1 with core 1.60.0 an image of 9.5 mb sent by deltachat consumes more than 250 MB RAM.
This leads to crashes of the share extension app as described here deltachat/deltachat-ios#1330.
I wonder if the image processing can be memory optimized.
Moreover it would help if there was an API to define a fixed memory limit an image processing task can allocate and if it exceeds to raise an error, the UI can catch. This way we could avoid the situation that iOS throws an
EXC_RESOURCE RESOURCE_TYPE_MEMORY
exception, which can't be handled by try/catch.The text was updated successfully, but these errors were encountered: