feat: jsonrpc: add late_file_message_mediasize
#6428
Conversation
Simon-Laux
force-pushed
the
simon/jsonrpc-add-late_file_message_mediasize
branch
from
96e65f9 to
ef8b98e
Compare
| // The new width and height to store in the message object. None if you don't want to change the dimensions. | ||
| pub wh: Option<(u32, u32)>, | ||
| // The new duration to store in the message object. None if you don't want to change it. | ||
| pub duration: Option<u32>, |
There was a problem hiding this comment.
What is the unit for duration, seconds, milliseconds?
| let duration = self | ||
| .duration | ||
| .unwrap_or(0) | ||
| .to_i32() |
There was a problem hiding this comment.
This converts everything to i32, but then function arguments are u32 everywhere.
There was a problem hiding this comment.
u32 is the right type obviously, but Core's Message::latefiling_mediasize() takes i32 args, probably should be fixed there
| /// Changes the message width, height or duration, and stores it into the database. | ||
| /// | ||
| /// Sometimes, the core cannot find out the width, the height or the duration | ||
| /// of an image, an audio or a video. |
There was a problem hiding this comment.
Why would this happen that image dimensions are wrong? If UI can determine the the image size, can it just display the image correctly, why does it need to store it back into the database and read back?
|
I don't understand the purpose of this PR. If the core is buggy and provides wrong dimensions, UI has to check and correct it, we can as well just drop the API to read these values and move the logic entirely in the UI instead of the UI telling the core what it should tell the UI. |
|
the idea of the api was to speed up things on subsequent loads on android, eg UI can show placeholder in correct aspect ratio, leading to less flickering and jumping, esp if there are lots of images/videos but that was 7 years ago, if desktop could live without the api up to now, there might be no need :) for android, we can check that at some point, if it is still needed, i would not remove the api just now |
No description provided.