-
-
Notifications
You must be signed in to change notification settings - Fork 104
fix!: use Viewtype::File
for messages with invalid images, images of unknown size, huge images (#6825)
#6949
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
Conversation
8mp is too strict, just checked a random photo from iphone13 (already some years old) having 12mp, probably newer ones have more idea is to deal with incredibly large images, no image taken from a system camera (panorama aside) should fall in that category - in practise, they also never made problems, as mp scale to age of phone and ram, at least roughly EDIT: does this affect also recoding of oitgoing images? if not, at least "happy path" of sending images around would work. but even then, i would use a farer limit - chrrently we have none, so there is no need to be too strict |
I think it'd be better to use some percentage of the RAM size, otherwise photos taken by new devices can't be displayed on old ones. |
I agree with idea to increaste up to 32 megapixels. I think, up to ~50 megapixels should be OK. This limit does not affect sending pictures, there are other (much stricter) conversion rules applied, if i understand correctly. |
I increased the limit to 35 Mpx. Can increase it to 50 Mpx, but then need to regenerate the test data. CC @ivan-cx |
I don't have an opinion and don't feel the need to form one, I think there are enough ppl discussing already, and I trust that you are making a good decision :) |
Increased the limit to 50 Mpx. Some modern cameras have >= 40 Mpx. And let's merge this, even a high limit is better than no limit at all. Still need someone's approval because i made some changes. |
…unknown size, images > 50 Mpx (#6825) BREAKING CHANGE: messages with invalid images, images of unknown size, huge images, will have Viewtype::File After changing the logic of Viewtype selection, I had to fix 3 old tests that used invalid Base64 image data. Co-authored-by: iequidoo <117991069+iequidoo@users.noreply.github.com>
Fix #6825
Thanks to @ivan-cx. JSON-RPC tests pass, so this can be merged to
main
, but i want everyone to agree on the image size limit (~8.3 Mpx).