From 95eab61130271404cd12db176871b747ed2aac72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Wed, 14 Aug 2024 10:39:51 +0200 Subject: [PATCH 1/3] utils: Split image methods into a separate module --- po/POTFILES.in | 2 +- src/components/avatar/editable.rs | 2 +- src/components/avatar/image.rs | 2 +- src/components/media/content_viewer.rs | 2 +- .../room_details/edit_details_subpage.rs | 2 +- .../history_viewer/visual_media_item.rs | 2 +- .../room_history/message_row/visual_media.rs | 2 +- .../room_history/message_toolbar/mod.rs | 4 +- src/utils/{media.rs => media/image.rs} | 163 +----------------- src/utils/media/mod.rs | 161 +++++++++++++++++ 10 files changed, 175 insertions(+), 167 deletions(-) rename src/utils/{media.rs => media/image.rs} (70%) create mode 100644 src/utils/media/mod.rs diff --git a/po/POTFILES.in b/po/POTFILES.in index 37ab4919d..05f2b9b74 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -186,5 +186,5 @@ src/shortcuts.ui src/user_facing_error.rs src/utils/matrix/media_message.rs src/utils/matrix/mod.rs -src/utils/media.rs +src/utils/media/mod.rs src/window.ui diff --git a/src/components/avatar/editable.rs b/src/components/avatar/editable.rs index 4c00cd5fd..de48a6e56 100644 --- a/src/components/avatar/editable.rs +++ b/src/components/avatar/editable.rs @@ -14,7 +14,7 @@ use super::{AvatarData, AvatarImage}; use crate::{ components::{ActionButton, ActionState}, toast, - utils::{expression, media::load_image}, + utils::{expression, media::image::load_image}, }; /// The state of the editable avatar. diff --git a/src/components/avatar/image.rs b/src/components/avatar/image.rs index 6db264b76..b0cc0e2fe 100644 --- a/src/components/avatar/image.rs +++ b/src/components/avatar/image.rs @@ -11,7 +11,7 @@ use tracing::error; use crate::{ session::model::Session, spawn, spawn_tokio, - utils::{media::load_image, save_data_to_tmp_file}, + utils::{media::image::load_image, save_data_to_tmp_file}, }; /// The source of an avatar's URI. diff --git a/src/components/media/content_viewer.rs b/src/components/media/content_viewer.rs index a0d581bf0..ca740d42d 100644 --- a/src/components/media/content_viewer.rs +++ b/src/components/media/content_viewer.rs @@ -5,7 +5,7 @@ use gtk::{gdk, gio, glib, glib::clone, CompositeTemplate}; use tracing::warn; use super::{AnimatedImagePaintable, AudioPlayer, LocationViewer}; -use crate::{components::Spinner, spawn, utils::media::load_image}; +use crate::{components::Spinner, spawn, utils::media::image::load_image}; #[derive(Debug, Default, Clone, Copy)] pub enum ContentType { diff --git a/src/session/view/content/room_details/edit_details_subpage.rs b/src/session/view/content/room_details/edit_details_subpage.rs index f2b319818..666da5b5d 100644 --- a/src/session/view/content/room_details/edit_details_subpage.rs +++ b/src/session/view/content/room_details/edit_details_subpage.rs @@ -17,7 +17,7 @@ use crate::{ session::model::Room, spawn_tokio, toast, utils::{ - media::{load_file, ImageInfoLoader}, + media::{image::ImageInfoLoader, load_file}, template_callbacks::TemplateCallbacks, BoundObjectWeakRef, OngoingAsyncAction, }, diff --git a/src/session/view/content/room_details/history_viewer/visual_media_item.rs b/src/session/view/content/room_details/history_viewer/visual_media_item.rs index d93d6e867..d99d3498b 100644 --- a/src/session/view/content/room_details/history_viewer/visual_media_item.rs +++ b/src/session/view/content/room_details/history_viewer/visual_media_item.rs @@ -6,7 +6,7 @@ use tracing::warn; use super::{HistoryViewerEvent, VisualMediaHistoryViewer}; use crate::{ spawn, - utils::{add_activate_binding_action, matrix::VisualMediaMessage, media::load_image}, + utils::{add_activate_binding_action, matrix::VisualMediaMessage, media::image::load_image}, }; /// The default size requested by a thumbnail. diff --git a/src/session/view/content/room_history/message_row/visual_media.rs b/src/session/view/content/room_history/message_row/visual_media.rs index 5e24b3a9a..0d5ac743f 100644 --- a/src/session/view/content/room_history/message_row/visual_media.rs +++ b/src/session/view/content/room_history/message_row/visual_media.rs @@ -15,7 +15,7 @@ use crate::{ gettext_f, session::model::Session, spawn, - utils::{matrix::VisualMediaMessage, media::load_image, LoadingState}, + utils::{matrix::VisualMediaMessage, media::image::load_image, LoadingState}, }; const MAX_THUMBNAIL_WIDTH: i32 = 600; diff --git a/src/session/view/content/room_history/message_toolbar/mod.rs b/src/session/view/content/room_history/message_toolbar/mod.rs index db478ce70..44ab04c68 100644 --- a/src/session/view/content/room_history/message_toolbar/mod.rs +++ b/src/session/view/content/room_history/message_toolbar/mod.rs @@ -40,7 +40,9 @@ use crate::{ spawn, spawn_tokio, toast, utils::{ matrix::AT_ROOM, - media::{filename_for_mime, get_audio_info, get_video_info, load_file, ImageInfoLoader}, + media::{ + filename_for_mime, get_audio_info, get_video_info, image::ImageInfoLoader, load_file, + }, template_callbacks::TemplateCallbacks, Location, LocationError, TokioDrop, }, diff --git a/src/utils/media.rs b/src/utils/media/image.rs similarity index 70% rename from src/utils/media.rs rename to src/utils/media/image.rs index ec182b60e..5eeb1cd96 100644 --- a/src/utils/media.rs +++ b/src/utils/media/image.rs @@ -1,14 +1,10 @@ -//! Collection of methods for media files. +//! Collection of methods for images. -use std::{cell::Cell, str::FromStr, sync::Mutex}; +use std::str::FromStr; -use gettextrs::gettext; -use gtk::{gdk, gio, glib, prelude::*}; +use gtk::{gdk, gio, prelude::*}; use image::{ColorType, DynamicImage, ImageDecoder, ImageResult}; -use matrix_sdk::attachment::{ - BaseAudioInfo, BaseImageInfo, BaseThumbnailInfo, BaseVideoInfo, Thumbnail, -}; -use mime::Mime; +use matrix_sdk::attachment::{BaseImageInfo, BaseThumbnailInfo, Thumbnail}; use crate::{components::AnimatedImagePaintable, spawn_tokio, DISABLE_GLYCIN_SANDBOX}; @@ -35,93 +31,6 @@ const THUMBNAIL_MAX_FILESIZE_THRESHOLD: u32 = 1024 * 1024; /// assume it's worth it to generate a thumbnail. const THUMBNAIL_DIMENSIONS_THRESHOLD: u32 = 200; -/// Get a default filename for a mime type. -/// -/// Tries to guess the file extension, but it might not find it. -/// -/// If the mime type is unknown, it uses the name for `fallback`. The fallback -/// mime types that are recognized are `mime::IMAGE`, `mime::VIDEO` and -/// `mime::AUDIO`, other values will behave the same as `None`. -pub fn filename_for_mime(mime_type: Option<&str>, fallback: Option) -> String { - let (type_, extension) = - if let Some(mime) = mime_type.and_then(|m| m.parse::().ok()) { - let extension = - mime_guess::get_mime_extensions(&mime).map(|extensions| extensions[0].to_owned()); - - (Some(mime.type_().as_str().to_owned()), extension) - } else { - (fallback.map(|type_| type_.as_str().to_owned()), None) - }; - - let name = match type_.as_deref() { - // Translators: Default name for image files. - Some("image") => gettext("image"), - // Translators: Default name for video files. - Some("video") => gettext("video"), - // Translators: Default name for audio files. - Some("audio") => gettext("audio"), - // Translators: Default name for files. - _ => gettext("file"), - }; - - extension - .map(|extension| format!("{name}.{extension}")) - .unwrap_or(name) -} - -/// Information about a file -pub struct FileInfo { - /// The mime type of the file. - pub mime: Mime, - /// The name of the file. - pub filename: String, - /// The size of the file in bytes. - pub size: Option, -} - -/// Load a file and return its content and some information -pub async fn load_file(file: &gio::File) -> Result<(Vec, FileInfo), glib::Error> { - let attributes: &[&str] = &[ - gio::FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE, - gio::FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME, - gio::FILE_ATTRIBUTE_STANDARD_SIZE, - ]; - - // Read mime type. - let info = file - .query_info_future( - &attributes.join(","), - gio::FileQueryInfoFlags::NONE, - glib::Priority::DEFAULT, - ) - .await?; - - let mime = info - .content_type() - .and_then(|content_type| Mime::from_str(&content_type).ok()) - .unwrap_or(mime::APPLICATION_OCTET_STREAM); - - let filename = info.display_name().to_string(); - - let raw_size = info.size(); - let size = if raw_size >= 0 { - Some(raw_size as u32) - } else { - None - }; - - let (data, _) = file.load_contents_future().await?; - - Ok(( - data.into(), - FileInfo { - mime, - filename, - size, - }, - )) -} - /// Get an image reader for the given file. async fn image_reader(file: gio::File) -> Result, glycin::ErrorCtx> { let mut loader = glycin::Loader::new(file); @@ -415,67 +324,3 @@ impl From for BaseImageInfo { } } } - -async fn get_gstreamer_media_info(file: &gio::File) -> Option { - let timeout = gst::ClockTime::from_seconds(15); - let discoverer = gst_pbutils::Discoverer::new(timeout).ok()?; - - let (sender, receiver) = futures_channel::oneshot::channel(); - let sender = Mutex::new(Cell::new(Some(sender))); - discoverer.connect_discovered(move |_, info, _| { - if let Some(sender) = sender.lock().unwrap().take() { - sender.send(info.clone()).unwrap(); - } - }); - - discoverer.start(); - discoverer.discover_uri_async(&file.uri()).ok()?; - - let media_info = receiver.await.unwrap(); - discoverer.stop(); - - Some(media_info) -} - -pub async fn get_video_info(file: &gio::File) -> BaseVideoInfo { - let mut info = BaseVideoInfo { - duration: None, - width: None, - height: None, - size: None, - blurhash: None, - }; - - let media_info = match get_gstreamer_media_info(file).await { - Some(media_info) => media_info, - None => return info, - }; - - info.duration = media_info.duration().map(Into::into); - - if let Some(stream_info) = media_info - .video_streams() - .first() - .and_then(|s| s.downcast_ref::()) - { - info.width = Some(stream_info.width().into()); - info.height = Some(stream_info.height().into()); - } - - info -} - -pub async fn get_audio_info(file: &gio::File) -> BaseAudioInfo { - let mut info = BaseAudioInfo { - duration: None, - size: None, - }; - - let media_info = match get_gstreamer_media_info(file).await { - Some(media_info) => media_info, - None => return info, - }; - - info.duration = media_info.duration().map(Into::into); - info -} diff --git a/src/utils/media/mod.rs b/src/utils/media/mod.rs new file mode 100644 index 000000000..3ad8f5eb1 --- /dev/null +++ b/src/utils/media/mod.rs @@ -0,0 +1,161 @@ +//! Collection of methods for media. + +use std::{cell::Cell, str::FromStr, sync::Mutex}; + +use gettextrs::gettext; +use gtk::{gio, glib, prelude::*}; +use matrix_sdk::attachment::{BaseAudioInfo, BaseVideoInfo}; +use mime::Mime; + +pub mod image; + +/// Get a default filename for a mime type. +/// +/// Tries to guess the file extension, but it might not find it. +/// +/// If the mime type is unknown, it uses the name for `fallback`. The fallback +/// mime types that are recognized are `mime::IMAGE`, `mime::VIDEO` and +/// `mime::AUDIO`, other values will behave the same as `None`. +pub fn filename_for_mime(mime_type: Option<&str>, fallback: Option) -> String { + let (type_, extension) = + if let Some(mime) = mime_type.and_then(|m| m.parse::().ok()) { + let extension = + mime_guess::get_mime_extensions(&mime).map(|extensions| extensions[0].to_owned()); + + (Some(mime.type_().as_str().to_owned()), extension) + } else { + (fallback.map(|type_| type_.as_str().to_owned()), None) + }; + + let name = match type_.as_deref() { + // Translators: Default name for image files. + Some("image") => gettext("image"), + // Translators: Default name for video files. + Some("video") => gettext("video"), + // Translators: Default name for audio files. + Some("audio") => gettext("audio"), + // Translators: Default name for files. + _ => gettext("file"), + }; + + extension + .map(|extension| format!("{name}.{extension}")) + .unwrap_or(name) +} + +/// Information about a file +pub struct FileInfo { + /// The mime type of the file. + pub mime: Mime, + /// The name of the file. + pub filename: String, + /// The size of the file in bytes. + pub size: Option, +} + +/// Load a file and return its content and some information +pub async fn load_file(file: &gio::File) -> Result<(Vec, FileInfo), glib::Error> { + let attributes: &[&str] = &[ + gio::FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE, + gio::FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME, + gio::FILE_ATTRIBUTE_STANDARD_SIZE, + ]; + + // Read mime type. + let info = file + .query_info_future( + &attributes.join(","), + gio::FileQueryInfoFlags::NONE, + glib::Priority::DEFAULT, + ) + .await?; + + let mime = info + .content_type() + .and_then(|content_type| Mime::from_str(&content_type).ok()) + .unwrap_or(mime::APPLICATION_OCTET_STREAM); + + let filename = info.display_name().to_string(); + + let raw_size = info.size(); + let size = if raw_size >= 0 { + Some(raw_size as u32) + } else { + None + }; + + let (data, _) = file.load_contents_future().await?; + + Ok(( + data.into(), + FileInfo { + mime, + filename, + size, + }, + )) +} + +async fn get_gstreamer_media_info(file: &gio::File) -> Option { + let timeout = gst::ClockTime::from_seconds(15); + let discoverer = gst_pbutils::Discoverer::new(timeout).ok()?; + + let (sender, receiver) = futures_channel::oneshot::channel(); + let sender = Mutex::new(Cell::new(Some(sender))); + discoverer.connect_discovered(move |_, info, _| { + if let Some(sender) = sender.lock().unwrap().take() { + sender.send(info.clone()).unwrap(); + } + }); + + discoverer.start(); + discoverer.discover_uri_async(&file.uri()).ok()?; + + let media_info = receiver.await.unwrap(); + discoverer.stop(); + + Some(media_info) +} + +pub async fn get_video_info(file: &gio::File) -> BaseVideoInfo { + let mut info = BaseVideoInfo { + duration: None, + width: None, + height: None, + size: None, + blurhash: None, + }; + + let media_info = match get_gstreamer_media_info(file).await { + Some(media_info) => media_info, + None => return info, + }; + + info.duration = media_info.duration().map(Into::into); + + if let Some(stream_info) = media_info + .video_streams() + .first() + .and_then(|s| s.downcast_ref::()) + { + info.width = Some(stream_info.width().into()); + info.height = Some(stream_info.height().into()); + } + + info +} + +pub async fn get_audio_info(file: &gio::File) -> BaseAudioInfo { + let mut info = BaseAudioInfo { + duration: None, + size: None, + }; + + let media_info = match get_gstreamer_media_info(file).await { + Some(media_info) => media_info, + None => return info, + }; + + info.duration = media_info.duration().map(Into::into); + info +} -- GitLab From 598544e422cc602fa8e9464693c0b2177ae7b31e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Wed, 14 Aug 2024 12:49:54 +0200 Subject: [PATCH 2/3] media: Be smarter when downloading thumbnails Prefer the original if it's not too big. Also prefer thumbnails of the original rather than thumbnails of the thumbnail. --- .../history_viewer/visual_media_item.rs | 45 ++- .../room_history/message_row/visual_media.rs | 49 ++-- src/utils/matrix/media_message.rs | 91 ++++-- src/utils/media/image.rs | 263 +++++++++++++++++- 4 files changed, 365 insertions(+), 83 deletions(-) diff --git a/src/session/view/content/room_details/history_viewer/visual_media_item.rs b/src/session/view/content/room_details/history_viewer/visual_media_item.rs index d99d3498b..30e23640c 100644 --- a/src/session/view/content/room_details/history_viewer/visual_media_item.rs +++ b/src/session/view/content/room_details/history_viewer/visual_media_item.rs @@ -1,12 +1,15 @@ use gtk::{glib, glib::clone, prelude::*, subclass::prelude::*, CompositeTemplate}; -use matrix_sdk::media::MediaThumbnailSettings; use ruma::api::client::media::get_content_thumbnail::v3::Method; use tracing::warn; use super::{HistoryViewerEvent, VisualMediaHistoryViewer}; use crate::{ spawn, - utils::{add_activate_binding_action, matrix::VisualMediaMessage, media::image::load_image}, + utils::{ + add_activate_binding_action, + matrix::VisualMediaMessage, + media::image::{load_image, ThumbnailSettings}, + }, }; /// The default size requested by a thumbnail. @@ -158,32 +161,20 @@ mod imp { let client = session.client(); let scale_factor = self.obj().scale_factor(); - let settings = MediaThumbnailSettings::new( - Method::Scale, - ((THUMBNAIL_SIZE * scale_factor) as u32).into(), - ((THUMBNAIL_SIZE * scale_factor) as u32).into(), - ); - - let file = media_message - .thumbnail_tmp_file(&client, settings) - .await - .ok() - .flatten(); - - if file.is_none() && matches!(media_message, VisualMediaMessage::Video(_)) { - // No image to show for the video. - return; - } + let settings = ThumbnailSettings { + width: ((THUMBNAIL_SIZE * scale_factor) as u32), + height: ((THUMBNAIL_SIZE * scale_factor) as u32), + method: Method::Scale, + animated: false, + }; - let file = match file { - Some(file) => file, - None => match media_message.into_tmp_file(&client).await { - Ok(file) => file, - Err(error) => { - warn!("Could not retrieve media file: {error}"); - return; - } - }, + let file = match media_message.thumbnail_tmp_file(&client, settings).await { + Ok(Some(file)) => file, + Ok(None) => return, + Err(error) => { + warn!("Could not retrieve media file: {error}"); + return; + } }; match load_image(file).await { diff --git a/src/session/view/content/room_history/message_row/visual_media.rs b/src/session/view/content/room_history/message_row/visual_media.rs index 0d5ac743f..456b35ecb 100644 --- a/src/session/view/content/room_history/message_row/visual_media.rs +++ b/src/session/view/content/room_history/message_row/visual_media.rs @@ -5,7 +5,7 @@ use gtk::{ glib::{self, clone}, CompositeTemplate, }; -use matrix_sdk::{media::MediaThumbnailSettings, Client}; +use matrix_sdk::Client; use ruma::api::client::media::get_content_thumbnail::v3::Method; use tracing::warn; @@ -15,7 +15,11 @@ use crate::{ gettext_f, session::model::Session, spawn, - utils::{matrix::VisualMediaMessage, media::image::load_image, LoadingState}, + utils::{ + matrix::VisualMediaMessage, + media::image::{load_image, ThumbnailSettings}, + LoadingState, + }, }; const MAX_THUMBNAIL_WIDTH: i32 = 600; @@ -307,30 +311,23 @@ impl MessageVisualMedia { let filename = media_message.filename(); let scale_factor = self.scale_factor(); - let settings = MediaThumbnailSettings::new( - Method::Scale, - ((MAX_THUMBNAIL_WIDTH * scale_factor) as u32).into(), - ((MAX_THUMBNAIL_HEIGHT * scale_factor) as u32).into(), - ); - - let file = if let Some(file) = media_message - .thumbnail_tmp_file(client, settings) - .await - .ok() - .flatten() - { - file - } else { - match media_message.into_tmp_file(client).await { - Ok(file) => file, - Err(error) => { - warn!("Could not retrieve media file: {error}"); - imp.overlay_error - .set_tooltip_text(Some(&gettext("Could not retrieve media"))); - imp.set_state(LoadingState::Error); - - return; - } + let settings = ThumbnailSettings { + method: Method::Scale, + width: ((MAX_THUMBNAIL_WIDTH * scale_factor) as u32), + height: ((MAX_THUMBNAIL_HEIGHT * scale_factor) as u32), + animated: true, + }; + + let file = match media_message.thumbnail_tmp_file(client, settings).await { + Ok(Some(file)) => file, + Ok(None) => unreachable!("Only video messages do not have a fallback"), + Err(error) => { + warn!("Could not retrieve media file: {error}"); + imp.overlay_error + .set_tooltip_text(Some(&gettext("Could not retrieve media"))); + imp.set_state(LoadingState::Error); + + return; } }; diff --git a/src/utils/matrix/media_message.rs b/src/utils/matrix/media_message.rs index e77f43e00..6f645ead6 100644 --- a/src/utils/matrix/media_message.rs +++ b/src/utils/matrix/media_message.rs @@ -1,6 +1,6 @@ use gettextrs::gettext; use gtk::{gio, glib, prelude::*}; -use matrix_sdk::{media::MediaThumbnailSettings, Client}; +use matrix_sdk::Client; use ruma::{ events::{ room::message::{ @@ -13,7 +13,14 @@ use ruma::{ }; use tracing::{debug, error}; -use crate::{prelude::*, toast, utils::save_data_to_tmp_file}; +use crate::{ + prelude::*, + toast, + utils::{ + media::image::{ImageSource, ThumbnailDownloader, ThumbnailSettings}, + save_data_to_tmp_file, + }, +}; /// Get the filename of a media message. macro_rules! filename { @@ -264,53 +271,78 @@ impl VisualMediaMessage { /// settings. /// /// This might not return a thumbnail at the requested size, depending on - /// the homeserver. + /// the message and the homeserver. + /// + /// Returns `Ok(None)` if no thumbnail could be retrieved and no fallback + /// could be downloaded. This only applies to video messages. /// - /// Returns `Ok(None)` if no thumbnail could be retrieved. Returns an error - /// if something occurred while fetching the content. + /// Returns an error if something occurred while fetching the content. pub async fn thumbnail( &self, client: &Client, - settings: MediaThumbnailSettings, + settings: ThumbnailSettings, ) -> Result>, matrix_sdk::Error> { - let media = client.media(); - - macro_rules! thumbnail { - ($event_content:ident) => {{ - let event_content = $event_content.clone(); - $crate::spawn_tokio!(async move { - media.get_thumbnail(&event_content, settings, true).await - }) - .await - .unwrap() - }}; - } - - match self { + let downloader = match &self { Self::Image(c) => { - thumbnail!(c) + let image_info = c.info.as_deref(); + ThumbnailDownloader { + thumbnail: image_info.and_then(|i| { + i.thumbnail_source.as_ref().map(|s| ImageSource { + source: s.into(), + info: i.thumbnail_info.as_deref().map(Into::into), + }) + }), + original: Some(ImageSource { + source: (&c.source).into(), + info: image_info.map(Into::into), + }), + } } Self::Video(c) => { - thumbnail!(c) + let video_info = c.info.as_deref(); + ThumbnailDownloader { + thumbnail: video_info.and_then(|i| { + i.thumbnail_source.as_ref().map(|s| ImageSource { + source: s.into(), + info: i.thumbnail_info.as_deref().map(Into::into), + }) + }), + original: None, + } } Self::Sticker(c) => { - thumbnail!(c) + let image_info = &c.info; + ThumbnailDownloader { + thumbnail: image_info.thumbnail_source.as_ref().map(|s| ImageSource { + source: s.into(), + info: image_info.thumbnail_info.as_deref().map(Into::into), + }), + original: Some(ImageSource { + source: (&c.source).into(), + info: Some(image_info.into()), + }), + } } - } + }; + + downloader.download(client, settings).await } /// Fetch a thumbnail of the media with the given client and thumbnail /// settings and write it to a temporary file. /// /// This might not return a thumbnail at the requested size, depending on - /// the homeserver. + /// the message and the homeserver. /// - /// Returns `Ok(None)` if no thumbnail could be retrieved. Returns an error - /// if something occurred while fetching the content. + /// Returns `Ok(None)` if no thumbnail could be retrieved and no fallback + /// could be downloaded. This only applies to video messages. + /// + /// Returns an error if something occurred while fetching the content or + /// saving the content to a file. pub async fn thumbnail_tmp_file( &self, client: &Client, - settings: MediaThumbnailSettings, + settings: ThumbnailSettings, ) -> Result, MediaFileError> { let data = self.thumbnail(client, settings).await?; @@ -331,7 +363,8 @@ impl VisualMediaMessage { /// Fetch the content of the media with the given client and write it to a /// temporary file. /// - /// Returns an error if something occurred while fetching the content. + /// Returns an error if something occurred while fetching the content or + /// saving the content to a file. pub async fn into_tmp_file(self, client: &Client) -> Result { MediaMessage::from(self).into_tmp_file(client).await } diff --git a/src/utils/media/image.rs b/src/utils/media/image.rs index 5eeb1cd96..cc54cfa7d 100644 --- a/src/utils/media/image.rs +++ b/src/utils/media/image.rs @@ -4,7 +4,19 @@ use std::str::FromStr; use gtk::{gdk, gio, prelude::*}; use image::{ColorType, DynamicImage, ImageDecoder, ImageResult}; -use matrix_sdk::attachment::{BaseImageInfo, BaseThumbnailInfo, Thumbnail}; +use matrix_sdk::{ + attachment::{BaseImageInfo, BaseThumbnailInfo, Thumbnail}, + media::{MediaFormat, MediaRequest, MediaThumbnailSettings, MediaThumbnailSize}, + Client, +}; +use ruma::{ + api::client::media::get_content_thumbnail::v3::Method, + events::{ + room::{ImageInfo, MediaSource as CommonMediaSource, ThumbnailInfo}, + sticker::StickerMediaSource, + }, +}; +use tracing::warn; use crate::{components::AnimatedImagePaintable, spawn_tokio, DISABLE_GLYCIN_SANDBOX}; @@ -12,6 +24,8 @@ use crate::{components::AnimatedImagePaintable, spawn_tokio, DISABLE_GLYCIN_SAND const THUMBNAIL_DEFAULT_WIDTH: u32 = 800; /// The default height of a generated thumbnail. const THUMBNAIL_DEFAULT_HEIGHT: u32 = 600; +/// The content type of SVG. +const SVG_CONTENT_TYPE: &str = "image/svg+xml"; /// The content type of WebP. const WEBP_CONTENT_TYPE: &str = "image/webp"; /// The default WebP quality used for a generated thumbnail. @@ -324,3 +338,250 @@ impl From for BaseImageInfo { } } } + +/// An API to download a thumbnail for a media. +#[derive(Debug, Clone, Copy)] +pub struct ThumbnailDownloader<'a> { + /// The source of a thumbnail of the media. + pub thumbnail: Option>, + /// The source of the original image. + pub original: Option>, +} + +impl<'a> ThumbnailDownloader<'a> { + /// Download the thumbnail of the media. + /// + /// This might not return a thumbnail at the requested size, depending on + /// the sources and the homeserver. + /// + /// Returns `Ok(None)` if no thumbnail could be retrieved. Returns an error + /// if something occurred while fetching the content. + pub async fn download( + self, + client: &Client, + settings: ThumbnailSettings, + ) -> Result>, matrix_sdk::Error> { + // First, select which source we are going to download from. + let source = if let Some((original, thumbnail)) = self.original.zip(self.thumbnail) { + if !original.can_be_thumbnailed() + && (original.filesize_is_too_big() + || thumbnail.dimensions_are_too_big(settings.width, settings.height, false)) + { + // Use the thumbnail as source to save bandwidth. + thumbnail + } else { + original + } + } else if let Some(original) = self.original { + original + } else if let Some(thumbnail) = self.thumbnail { + thumbnail + } else { + return Ok(None); + }; + + if source.can_be_thumbnailed() + && (source.filesize_is_too_big() + || source.dimensions_are_too_big(settings.width, settings.height, true)) + { + // Try to get a thumbnail. + let media = client.media(); + let request = MediaRequest { + source: source.source.to_common_media_source(), + format: MediaFormat::Thumbnail(settings.into()), + }; + let handle = spawn_tokio!(async move { media.get_media_content(&request, true).await }); + + match handle.await.unwrap() { + Ok(data) => return Ok(Some(data)), + Err(error) => { + warn!("Could not retrieve media thumbnail: {error}"); + } + } + } + + // Fallback to downloading the full source. + let media = client.media(); + let request = MediaRequest { + source: source.source.to_common_media_source(), + format: MediaFormat::File, + }; + + let data = spawn_tokio!(async move { media.get_media_content(&request, true).await }) + .await + .unwrap()?; + + Ok(Some(data)) + } +} + +/// The source of an image. +#[derive(Debug, Clone, Copy)] +pub struct ImageSource<'a> { + /// The source of the image. + pub source: MediaSource<'a>, + /// Information about the image. + pub info: Option>, +} + +impl<'a> ImageSource<'a> { + /// Whether this source can be thumbnailed by the media repo. + /// + /// Returns `false` in these cases: + /// + /// - The image is encrypted, because it is not possible for the media repo + /// to make a thumbnail. + /// - The image uses the SVG format, because media repos usually do not + /// accept to create a thumbnail of those. + fn can_be_thumbnailed(&self) -> bool { + !self.source.is_encrypted() + && !self + .info + .is_some_and(|i| i.mimetype.is_some_and(|m| m == SVG_CONTENT_TYPE)) + } + + /// Whether the filesize of this source is too big. + /// + /// It means that it is worth it to download a thumbnail instead of the + /// original file, even if its dimensions are smaller than requested. + fn filesize_is_too_big(&self) -> bool { + self.info + .is_some_and(|i| i.size.is_some_and(|s| s > THUMBNAIL_MAX_FILESIZE_THRESHOLD)) + } + + /// Whether the dimensions of this source are too big for the given + /// requested dimensions. + fn dimensions_are_too_big( + &self, + requested_width: u32, + requested_height: u32, + increase_threshold: bool, + ) -> bool { + self.info.is_some_and(|i| { + i.width.is_some_and(|w| { + let threshold = if increase_threshold { + requested_width.saturating_add(THUMBNAIL_DIMENSIONS_THRESHOLD) + } else { + requested_width + }; + + w > threshold + }) || i.height.is_some_and(|h| { + let threshold = if increase_threshold { + requested_height.saturating_add(THUMBNAIL_DIMENSIONS_THRESHOLD) + } else { + requested_height + }; + + h > threshold + }) + }) + } +} + +/// The source of a media file. +#[derive(Debug, Clone, Copy)] +pub enum MediaSource<'a> { + /// A common media source. + Common(&'a CommonMediaSource), + /// The media source of a sticker. + Sticker(&'a StickerMediaSource), +} + +impl<'a> MediaSource<'a> { + /// Whether this source is encrypted. + fn is_encrypted(&self) -> bool { + match self { + Self::Common(source) => matches!(source, CommonMediaSource::Encrypted(_)), + Self::Sticker(source) => matches!(source, StickerMediaSource::Encrypted(_)), + } + } + + /// Get this source as a `CommonMediaSource`. + fn to_common_media_source(self) -> CommonMediaSource { + match self { + Self::Common(source) => source.clone(), + Self::Sticker(source) => source.clone().into(), + } + } +} + +impl<'a> From<&'a CommonMediaSource> for MediaSource<'a> { + fn from(value: &'a CommonMediaSource) -> Self { + Self::Common(value) + } +} + +impl<'a> From<&'a StickerMediaSource> for MediaSource<'a> { + fn from(value: &'a StickerMediaSource) -> Self { + Self::Sticker(value) + } +} + +/// Information about the source of an image. +#[derive(Debug, Clone, Copy, Default)] +pub struct ImageSourceInfo<'a> { + /// The width of the image. + width: Option, + /// The height of the image. + height: Option, + /// The MIME type of the image. + mimetype: Option<&'a str>, + /// The file size of the image. + size: Option, +} + +impl<'a> From<&'a ImageInfo> for ImageSourceInfo<'a> { + fn from(value: &'a ImageInfo) -> Self { + Self { + width: value.width.and_then(|u| u.try_into().ok()), + height: value.height.and_then(|u| u.try_into().ok()), + mimetype: value.mimetype.as_deref(), + size: value.size.and_then(|u| u.try_into().ok()), + } + } +} + +impl<'a> From<&'a ThumbnailInfo> for ImageSourceInfo<'a> { + fn from(value: &'a ThumbnailInfo) -> Self { + Self { + width: value.width.and_then(|u| u.try_into().ok()), + height: value.height.and_then(|u| u.try_into().ok()), + mimetype: value.mimetype.as_deref(), + size: value.size.and_then(|u| u.try_into().ok()), + } + } +} + +/// The settings for downloading a thumbnail. +#[derive(Debug, Clone)] +pub struct ThumbnailSettings { + /// The resquested width of the thumbnail. + pub width: u32, + /// The requested height of the thumbnail. + pub height: u32, + /// The method to use to resize the thumbnail. + pub method: Method, + /// Whether to request an animated thumbnail. + pub animated: bool, +} + +impl From for MediaThumbnailSettings { + fn from(value: ThumbnailSettings) -> Self { + let ThumbnailSettings { + width, + height, + method, + animated, + } = value; + + MediaThumbnailSettings { + size: MediaThumbnailSize { + method, + width: width.into(), + height: height.into(), + }, + animated, + } + } +} -- GitLab From f3ea74df838a7364f678e5e30174c05d451f7ad9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Sat, 17 Aug 2024 11:05:54 +0200 Subject: [PATCH 3/3] avatar-image: Be smarter when downloading thumbnail Use the same logic as other thumbnails, but prefer to get a thumbnail rather than the original when dimesions are unknown. Fixes SVG avatars not working. --- src/components/avatar/image.rs | 164 ++++++++++++------ src/session/model/remote_room.rs | 5 +- src/session/model/room/member.rs | 2 +- src/session/model/room/mod.rs | 114 ++++++++++-- src/session/model/session.rs | 7 +- src/session/model/user.rs | 5 +- .../view/account_settings/general_page/mod.rs | 29 ++-- .../view/content/explore/public_room.rs | 9 +- .../room_details/edit_details_subpage.rs | 10 +- .../history_viewer/visual_media_item.rs | 1 + .../room_history/message_row/visual_media.rs | 3 +- src/utils/matrix/media_message.rs | 76 ++++---- src/utils/matrix/mod.rs | 2 +- src/utils/media/image.rs | 143 +++++++++++---- 14 files changed, 382 insertions(+), 188 deletions(-) diff --git a/src/components/avatar/image.rs b/src/components/avatar/image.rs index b0cc0e2fe..99e1d8e73 100644 --- a/src/components/avatar/image.rs +++ b/src/components/avatar/image.rs @@ -1,17 +1,14 @@ use gtk::{gdk, glib, glib::clone, prelude::*, subclass::prelude::*}; -use matrix_sdk::{ - media::{MediaFormat, MediaRequest, MediaThumbnailSettings}, - ruma::{ - api::client::media::get_content_thumbnail::v3::Method, events::room::MediaSource, MxcUri, - OwnedMxcUri, - }, +use ruma::{ + api::client::media::get_content_thumbnail::v3::Method, events::room::avatar::ImageInfo, + OwnedMxcUri, }; use tracing::error; use crate::{ session::model::Session, - spawn, spawn_tokio, - utils::{media::image::load_image, save_data_to_tmp_file}, + spawn, + utils::media::image::{load_image, ImageSource, ThumbnailDownloader, ThumbnailSettings}, }; /// The source of an avatar's URI. @@ -27,7 +24,10 @@ pub enum AvatarUriSource { } mod imp { - use std::cell::{Cell, OnceCell, RefCell}; + use std::{ + cell::{Cell, OnceCell, RefCell}, + marker::PhantomData, + }; use super::*; @@ -42,9 +42,13 @@ mod imp { /// If this is `0`, no image will be loaded. #[property(get, set = Self::set_needed_size, explicit_notify, minimum = 0)] pub needed_size: Cell, - /// The Matrix URI of the `AvatarImage`. - #[property(get = Self::uri, set = Self::set_uri, explicit_notify, nullable, type = Option)] - pub uri: RefCell>, + /// The Matrix URI of the avatar. + pub(super) uri: RefCell>, + /// The Matrix URI of the `AvatarImage`, as a string. + #[property(get = Self::uri_string)] + uri_string: PhantomData>, + /// Information about the avatar. + pub(super) info: RefCell>>, /// The source of the avatar's URI. #[property(get, construct_only, builder(AvatarUriSource::default()))] pub uri_source: Cell, @@ -78,29 +82,37 @@ mod imp { } /// The Matrix URI of the `AvatarImage`. - fn uri(&self) -> Option { - self.uri.borrow().as_ref().map(ToString::to_string) + pub(super) fn uri(&self) -> Option { + self.uri.borrow().clone() } /// Set the Matrix URI of the `AvatarImage`. - fn set_uri(&self, uri: Option) { - let uri = uri.map(OwnedMxcUri::from); - + /// + /// Returns whether the URI changed. + pub(super) fn set_uri(&self, uri: Option) -> bool { if *self.uri.borrow() == uri { - return; + return false; } - let obj = self.obj(); - let has_uri = uri.is_some(); self.uri.replace(uri); + self.obj().notify_uri_string(); - if has_uri { - obj.load(); - } else { - self.set_paintable(None); - } + true + } + + /// The Matrix URI of the `AvatarImage`, as a string. + fn uri_string(&self) -> Option { + self.uri.borrow().as_ref().map(ToString::to_string) + } - obj.notify_uri(); + /// Information about the avatar. + pub(super) fn info(&self) -> Option> { + self.info.borrow().clone() + } + + /// Set information about the avatar. + pub(super) fn set_info(&self, info: Option>) { + self.info.replace(info); } /// Set the image content as a paintable @@ -117,60 +129,98 @@ glib::wrapper! { } impl AvatarImage { - /// Construct a new `AvatarImage` with the given session and Matrix URI. - pub fn new(session: &Session, uri: Option<&MxcUri>, uri_source: AvatarUriSource) -> Self { - glib::Object::builder() + /// Construct a new `AvatarImage` with the given session, Matrix URI and + /// avatar info. + pub fn new( + session: &Session, + uri_source: AvatarUriSource, + uri: Option, + info: Option>, + ) -> Self { + let obj = glib::Object::builder::() .property("session", session) - .property("uri", uri.map(|uri| uri.to_string())) .property("uri-source", uri_source) - .build() + .build(); + + obj.set_uri_and_info(uri, info); + obj } - /// Set the content of the image. - async fn set_image_data(&self, data: Vec) { - let Ok(file) = save_data_to_tmp_file(&data) else { - return; - }; + /// Set the Matrix URI and information of the avatar. + pub fn set_uri_and_info(&self, uri: Option, info: Option>) { + let imp = self.imp(); + + let changed = imp.set_uri(uri); + imp.set_info(info); + + if changed { + self.load(); + } + } - let paintable = load_image(file).await.ok(); - self.imp().set_paintable(paintable); + /// The Matrix URI of the avatar. + pub fn uri(&self) -> Option { + self.imp().uri() } + /// Information about the avatar. + pub fn info(&self) -> Option> { + self.imp().info() + } + + /// Load the image with the current settings. fn load(&self) { - // Don't do anything here if we don't need the avatar. if self.needed_size() == 0 { + // We do not need the avatar. + self.imp().set_paintable(None); return; } - let Some(uri) = self.imp().uri.borrow().clone() else { + let Some(uri) = self.uri() else { + // We do not have an avatar. + self.imp().set_paintable(None); return; }; - let client = self.session().client(); - let needed_size = self.needed_size(); - let request = MediaRequest { - source: MediaSource::Plain(uri), - format: MediaFormat::Thumbnail(MediaThumbnailSettings::new( - Method::Scale, - needed_size.into(), - needed_size.into(), - )), - }; - let handle = - spawn_tokio!(async move { client.media().get_media_content(&request, true).await }); - spawn!( glib::Priority::LOW, clone!( #[weak(rename_to = obj)] self, async move { - match handle.await.unwrap() { - Ok(data) => obj.set_image_data(data).await, - Err(error) => error!("Could not fetch avatar: {error}"), - }; + obj.load_inner(uri).await; } ) ); } + + async fn load_inner(&self, uri: OwnedMxcUri) { + let client = self.session().client(); + let info = self.info(); + let needed_size = self.needed_size(); + + let downloader = ThumbnailDownloader { + main: ImageSource { + source: (&uri).into(), + info: info.as_deref().map(Into::into), + }, + // Avatars are not encrypted so we should always get the thumbnail from the original. + alt: None, + }; + let settings = ThumbnailSettings { + width: needed_size, + height: needed_size, + method: Method::Crop, + animated: true, + prefer_thumbnail: true, + }; + + match downloader.download_to_file(&client, settings).await { + Ok(file) => { + let paintable = load_image(file).await.ok(); + self.imp().set_paintable(paintable); + } + Err(error) => error!("Could not fetch avatar: {error}"), + }; + } } diff --git a/src/session/model/remote_room.rs b/src/session/model/remote_room.rs index f2cdf4046..d654ab037 100644 --- a/src/session/model/remote_room.rs +++ b/src/session/model/remote_room.rs @@ -78,8 +78,9 @@ mod imp { self.obj().avatar_data().set_image(Some(AvatarImage::new( &session, - None, AvatarUriSource::Room, + None, + None, ))); } @@ -180,7 +181,7 @@ mod imp { self.set_joined_members_count(data.num_joined_members.try_into().unwrap_or(u32::MAX)); if let Some(image) = self.obj().avatar_data().image() { - image.set_uri(data.avatar_url.map(String::from)); + image.set_uri_and_info(data.avatar_url, None); } self.set_loading_state(LoadingState::Ready); diff --git a/src/session/model/room/member.rs b/src/session/model/room/member.rs index 709a3c3e2..e652b0eb7 100644 --- a/src/session/model/room/member.rs +++ b/src/session/model/room/member.rs @@ -200,7 +200,7 @@ impl Member { self.avatar_data() .image() .unwrap() - .set_uri(member.avatar_url().map(ToString::to_string)); + .set_uri_and_info(member.avatar_url().map(ToOwned::to_owned), None); self.set_power_level(member.power_level()); self.set_membership(member.membership().into()); } diff --git a/src/session/model/room/mod.rs b/src/session/model/room/mod.rs index 787f194fb..fadc9236d 100644 --- a/src/session/model/room/mod.rs +++ b/src/session/model/room/mod.rs @@ -9,7 +9,9 @@ use gtk::{ subclass::prelude::*, }; use matrix_sdk::{ - deserialized_responses::{AmbiguityChange, MemberEvent, SyncTimelineEvent}, + deserialized_responses::{ + AmbiguityChange, MemberEvent, SyncOrStrippedState, SyncTimelineEvent, + }, event_handler::EventHandlerDropGuard, room::Room as MatrixRoom, send_queue::RoomSendQueueUpdate, @@ -22,8 +24,8 @@ use ruma::{ receipt::{ReceiptEventContent, ReceiptType}, relation::Annotation, room::{ - encryption::SyncRoomEncryptionEvent, guest_access::GuestAccess, - history_visibility::HistoryVisibility, + avatar::RoomAvatarEventContent, encryption::SyncRoomEncryptionEvent, + guest_access::GuestAccess, history_visibility::HistoryVisibility, }, tag::{TagInfo, TagName}, typing::TypingEventContent, @@ -515,7 +517,6 @@ impl Room { imp.matrix_room.set(matrix_room).unwrap(); imp.update_topic(); - self.update_avatar(); self.load_predecessor(); self.load_tombstone(); self.load_category(); @@ -527,6 +528,17 @@ impl Room { self.update_guests_allowed(); self.update_history_visibility(); + spawn!( + glib::Priority::DEFAULT_IDLE, + clone!( + #[weak(rename_to = obj)] + self, + async move { + obj.update_avatar().await; + } + ) + ); + spawn!( glib::Priority::DEFAULT_IDLE, clone!( @@ -752,7 +764,17 @@ impl Room { self.imp().direct_member.replace(member); self.notify_direct_member(); - self.update_avatar(); + + spawn!( + glib::Priority::DEFAULT_IDLE, + clone!( + #[weak(rename_to = obj)] + self, + async move { + obj.update_avatar().await; + } + ) + ); } /// Load the other member of the room, if this room is a direct chat and @@ -1455,8 +1477,17 @@ impl Room { } )); } - AnySyncStateEvent::RoomAvatar(SyncStateEvent::Original(_)) => { - self.update_avatar(); + AnySyncStateEvent::RoomAvatar(_) => { + spawn!( + glib::Priority::DEFAULT_IDLE, + clone!( + #[weak(rename_to = obj)] + self, + async move { + obj.update_avatar().await; + } + ) + ); } AnySyncStateEvent::RoomName(_) => { self.notify_name(); @@ -2042,43 +2073,90 @@ impl Room { } /// Update the avatar for the room. - fn update_avatar(&self) { + async fn update_avatar(&self) { let Some(session) = self.session() else { return; }; let imp = self.imp(); let avatar_data = self.avatar_data(); - if let Some(avatar_url) = self.matrix_room().avatar_url() { + let matrix_room = self.matrix_room().clone(); + let handle = spawn_tokio!(async move { + matrix_room + .get_state_event_static::() + .await + }); + + let avatar_event = match handle.await.unwrap() { + Ok(Some(raw_event)) => match raw_event.deserialize() { + Ok(event) => Some(event), + Err(error) => { + warn!("Could not deserialize room avatar event: {error}"); + None + } + }, + Ok(None) => None, + Err(error) => { + warn!("Could not get room avatar event: {error}"); + None + } + }; + + let (avatar_url, avatar_info) = match avatar_event { + Some(event) => match event { + SyncOrStrippedState::Sync(event) => match event { + SyncStateEvent::Original(e) => (e.content.url, e.content.info), + SyncStateEvent::Redacted(_) => (None, None), + }, + SyncOrStrippedState::Stripped(event) => (event.content.url, event.content.info), + }, + None => (None, None), + }; + + if let Some(avatar_url) = avatar_url { imp.set_has_avatar(true); - let avatar_image = if let Some(avatar_image) = avatar_data + if let Some(avatar_image) = avatar_data .image() .filter(|i| i.uri_source() == AvatarUriSource::Room) { - avatar_image + avatar_image.set_uri_and_info(Some(avatar_url), avatar_info); } else { - let avatar_image = - AvatarImage::new(&session, Some(&avatar_url), AvatarUriSource::Room); + let avatar_image = AvatarImage::new( + &session, + AvatarUriSource::Room, + Some(avatar_url), + avatar_info, + ); + avatar_data.set_image(Some(avatar_image.clone())); - avatar_image - }; - avatar_image.set_uri(Some(avatar_url.to_string())); + } return; } imp.set_has_avatar(false); + // If we have a direct member, use their avatar. if let Some(direct_member) = self.direct_member() { avatar_data.set_image(direct_member.avatar_data().image()); } - if avatar_data.image().is_none() { + let avatar_image = avatar_data.image(); + + if let Some(avatar_image) = avatar_image + .as_ref() + .filter(|i| i.uri_source() == AvatarUriSource::Room) + { + // The room has no avatar, make sure we remove it. + avatar_image.set_uri_and_info(None, None); + } else if avatar_image.is_none() { + // We always need an avatar image, even if it is empty. avatar_data.set_image(Some(AvatarImage::new( &session, - None, AvatarUriSource::Room, + None, + None, ))) } } diff --git a/src/session/model/session.rs b/src/session/model/session.rs index 568d5720f..6d664c4e9 100644 --- a/src/session/model/session.rs +++ b/src/session/model/session.rs @@ -836,9 +836,10 @@ impl Session { let user = self.user(); if Some(user.display_name()) == profile.displayname - && user.avatar_data().image().is_some_and(|i| { - i.uri().as_deref() == profile.avatar_url.as_ref().map(|url| url.as_str()) - }) + && user + .avatar_data() + .image() + .is_some_and(|i| i.uri() == profile.avatar_url) { // Nothing to update. return; diff --git a/src/session/model/user.rs b/src/session/model/user.rs index ad56cd425..94613f36a 100644 --- a/src/session/model/user.rs +++ b/src/session/model/user.rs @@ -70,7 +70,7 @@ mod imp { self.parent_constructed(); let obj = self.obj(); - let avatar_image = AvatarImage::new(&obj.session(), None, AvatarUriSource::User); + let avatar_image = AvatarImage::new(&obj.session(), AvatarUriSource::User, None, None); obj.avatar_data().set_image(Some(avatar_image)); } @@ -352,7 +352,8 @@ pub trait UserExt: IsA { .avatar_data() .image() .unwrap() - .set_uri(uri.map(String::from)); + // User avatars never have information. + .set_uri_and_info(uri, None); } /// Get the `matrix.to` URI representation for this `User`. diff --git a/src/session/view/account_settings/general_page/mod.rs b/src/session/view/account_settings/general_page/mod.rs index d47424abc..bb6d60d46 100644 --- a/src/session/view/account_settings/general_page/mod.rs +++ b/src/session/view/account_settings/general_page/mod.rs @@ -5,6 +5,7 @@ use gtk::{ glib::{self, clone}, CompositeTemplate, }; +use ruma::OwnedMxcUri; use tracing::error; mod change_password_subpage; @@ -53,7 +54,7 @@ mod imp { pub user_id: TemplateChild, #[template_child] pub session_id: TemplateChild, - pub changing_avatar: RefCell>>, + pub changing_avatar: RefCell>>, pub changing_display_name: RefCell>>, pub avatar_uri_handler: RefCell>, pub display_name_handler: RefCell>, @@ -116,17 +117,17 @@ mod imp { self.session_id.set_subtitle(session.device_id().as_str()); let user = session.user(); - let avatar_uri_handler = - user.avatar_data() - .image() - .unwrap() - .connect_uri_notify(clone!( - #[weak] - obj, - move |avatar_image| { - obj.user_avatar_changed(avatar_image.uri()); - } - )); + let avatar_uri_handler = user + .avatar_data() + .image() + .unwrap() + .connect_uri_string_notify(clone!( + #[weak] + obj, + move |avatar_image| { + obj.user_avatar_changed(avatar_image.uri()); + } + )); self.avatar_uri_handler.replace(Some(avatar_uri_handler)); let display_name_handler = user.connect_display_name_notify(clone!( @@ -186,7 +187,7 @@ impl GeneralPage { } /// Update the view when the user's avatar changed. - fn user_avatar_changed(&self, uri: Option) { + fn user_avatar_changed(&self, uri: Option) { let imp = self.imp(); if let Some(action) = imp.changing_avatar.borrow().as_ref() { @@ -246,7 +247,7 @@ impl GeneralPage { } }; - let (action, weak_action) = OngoingAsyncAction::set(uri.to_string()); + let (action, weak_action) = OngoingAsyncAction::set(uri.clone()); imp.changing_avatar.replace(Some(action)); let uri_clone = uri.clone(); diff --git a/src/session/view/content/explore/public_room.rs b/src/session/view/content/explore/public_room.rs index 8bf439517..2288fd34e 100644 --- a/src/session/view/content/explore/public_room.rs +++ b/src/session/view/content/explore/public_room.rs @@ -51,7 +51,12 @@ mod imp { let obj = self.obj(); let avatar_data = if let Some(session) = obj.room_list().session() { - AvatarData::with_image(AvatarImage::new(&session, None, AvatarUriSource::Room)) + AvatarData::with_image(AvatarImage::new( + &session, + AvatarUriSource::Room, + None, + None, + )) } else { AvatarData::new() }; @@ -120,7 +125,7 @@ impl PublicRoom { self.avatar_data() .image() .unwrap() - .set_uri(room.avatar_url.clone().map(String::from)); + .set_uri_and_info(room.avatar_url.clone(), None); if let Some(room) = self.room_list().get(&room.room_id) { self.set_room(room); diff --git a/src/session/view/content/room_details/edit_details_subpage.rs b/src/session/view/content/room_details/edit_details_subpage.rs index 666da5b5d..138316da6 100644 --- a/src/session/view/content/room_details/edit_details_subpage.rs +++ b/src/session/view/content/room_details/edit_details_subpage.rs @@ -6,7 +6,7 @@ use gtk::{ CompositeTemplate, }; use matrix_sdk::RoomState; -use ruma::{assign, events::room::avatar::ImageInfo}; +use ruma::{assign, events::room::avatar::ImageInfo, OwnedMxcUri}; use tracing::error; use crate::{ @@ -51,7 +51,7 @@ mod imp { /// The presented room. #[property(get, set = Self::set_room, explicit_notify, nullable)] room: BoundObjectWeakRef, - changing_avatar: RefCell>>, + changing_avatar: RefCell>>, changing_name: RefCell>>, changing_topic: RefCell>>, expr_watch: RefCell>, @@ -97,7 +97,7 @@ mod imp { let avatar_data = room.avatar_data(); let expr_watch = AvatarData::this_expression("image") - .chain_property::("uri") + .chain_property::("uri-string") .watch( Some(&avatar_data), clone!( @@ -132,7 +132,7 @@ mod imp { } /// Handle when we receive an avatar URI change from the homeserver. - fn avatar_changed(&self, uri: Option) { + fn avatar_changed(&self, uri: Option) { if let Some(action) = self.changing_avatar.borrow().as_ref() { if uri.as_ref() != action.as_value() { // This is not the change we expected, maybe another device did a change too. @@ -207,7 +207,7 @@ mod imp { } }; - let (action, weak_action) = OngoingAsyncAction::set(uri.to_string()); + let (action, weak_action) = OngoingAsyncAction::set(uri.clone()); self.changing_avatar.replace(Some(action)); let matrix_room = matrix_room.clone(); diff --git a/src/session/view/content/room_details/history_viewer/visual_media_item.rs b/src/session/view/content/room_details/history_viewer/visual_media_item.rs index 30e23640c..b361eb7fd 100644 --- a/src/session/view/content/room_details/history_viewer/visual_media_item.rs +++ b/src/session/view/content/room_details/history_viewer/visual_media_item.rs @@ -166,6 +166,7 @@ mod imp { height: ((THUMBNAIL_SIZE * scale_factor) as u32), method: Method::Scale, animated: false, + prefer_thumbnail: false, }; let file = match media_message.thumbnail_tmp_file(&client, settings).await { diff --git a/src/session/view/content/room_history/message_row/visual_media.rs b/src/session/view/content/room_history/message_row/visual_media.rs index 456b35ecb..c13e67706 100644 --- a/src/session/view/content/room_history/message_row/visual_media.rs +++ b/src/session/view/content/room_history/message_row/visual_media.rs @@ -316,11 +316,12 @@ impl MessageVisualMedia { width: ((MAX_THUMBNAIL_WIDTH * scale_factor) as u32), height: ((MAX_THUMBNAIL_HEIGHT * scale_factor) as u32), animated: true, + prefer_thumbnail: false, }; let file = match media_message.thumbnail_tmp_file(client, settings).await { Ok(Some(file)) => file, - Ok(None) => unreachable!("Only video messages do not have a fallback"), + Ok(None) => unreachable!("Image messages should always have a fallback"), Err(error) => { warn!("Could not retrieve media file: {error}"); imp.overlay_error diff --git a/src/utils/matrix/media_message.rs b/src/utils/matrix/media_message.rs index 6f645ead6..7b6bdf854 100644 --- a/src/utils/matrix/media_message.rs +++ b/src/utils/matrix/media_message.rs @@ -268,7 +268,7 @@ impl VisualMediaMessage { } /// Fetch a thumbnail of the media with the given client and thumbnail - /// settings. + /// settings and write it to a temporary file. /// /// This might not return a thumbnail at the requested size, depending on /// the message and the homeserver. @@ -276,81 +276,63 @@ impl VisualMediaMessage { /// Returns `Ok(None)` if no thumbnail could be retrieved and no fallback /// could be downloaded. This only applies to video messages. /// - /// Returns an error if something occurred while fetching the content. - pub async fn thumbnail( + /// Returns an error if something occurred while fetching the content or + /// saving the content to a file. + pub async fn thumbnail_tmp_file( &self, client: &Client, settings: ThumbnailSettings, - ) -> Result>, matrix_sdk::Error> { + ) -> Result, MediaFileError> { let downloader = match &self { Self::Image(c) => { let image_info = c.info.as_deref(); ThumbnailDownloader { - thumbnail: image_info.and_then(|i| { + main: ImageSource { + source: (&c.source).into(), + info: image_info.map(Into::into), + }, + alt: image_info.and_then(|i| { i.thumbnail_source.as_ref().map(|s| ImageSource { source: s.into(), info: i.thumbnail_info.as_deref().map(Into::into), }) }), - original: Some(ImageSource { - source: (&c.source).into(), - info: image_info.map(Into::into), - }), } } Self::Video(c) => { - let video_info = c.info.as_deref(); + let Some(video_info) = c.info.as_deref() else { + return Ok(None); + }; + let Some(thumbnail_source) = video_info.thumbnail_source.as_ref() else { + return Ok(None); + }; + ThumbnailDownloader { - thumbnail: video_info.and_then(|i| { - i.thumbnail_source.as_ref().map(|s| ImageSource { - source: s.into(), - info: i.thumbnail_info.as_deref().map(Into::into), - }) - }), - original: None, + main: ImageSource { + source: thumbnail_source.into(), + info: video_info.thumbnail_info.as_deref().map(Into::into), + }, + alt: None, } } Self::Sticker(c) => { let image_info = &c.info; ThumbnailDownloader { - thumbnail: image_info.thumbnail_source.as_ref().map(|s| ImageSource { - source: s.into(), - info: image_info.thumbnail_info.as_deref().map(Into::into), - }), - original: Some(ImageSource { + main: ImageSource { source: (&c.source).into(), info: Some(image_info.into()), + }, + alt: image_info.thumbnail_source.as_ref().map(|s| ImageSource { + source: s.into(), + info: image_info.thumbnail_info.as_deref().map(Into::into), }), } } }; - downloader.download(client, settings).await - } - - /// Fetch a thumbnail of the media with the given client and thumbnail - /// settings and write it to a temporary file. - /// - /// This might not return a thumbnail at the requested size, depending on - /// the message and the homeserver. - /// - /// Returns `Ok(None)` if no thumbnail could be retrieved and no fallback - /// could be downloaded. This only applies to video messages. - /// - /// Returns an error if something occurred while fetching the content or - /// saving the content to a file. - pub async fn thumbnail_tmp_file( - &self, - client: &Client, - settings: ThumbnailSettings, - ) -> Result, MediaFileError> { - let data = self.thumbnail(client, settings).await?; - - let Some(data) = data else { - return Ok(None); - }; + let file = downloader.download_to_file(client, settings).await?; - Ok(Some(save_data_to_tmp_file(&data)?)) + Ok(Some(file)) } /// Fetch the content of the media with the given client. diff --git a/src/utils/matrix/mod.rs b/src/utils/matrix/mod.rs index 2eb90b67d..865c880fd 100644 --- a/src/utils/matrix/mod.rs +++ b/src/utils/matrix/mod.rs @@ -29,7 +29,7 @@ use thiserror::Error; mod media_message; -pub use self::media_message::{MediaMessage, VisualMediaMessage}; +pub use self::media_message::{MediaFileError, MediaMessage, VisualMediaMessage}; use crate::{ components::Pill, gettext_f, diff --git a/src/utils/media/image.rs b/src/utils/media/image.rs index cc54cfa7d..be79ceb7e 100644 --- a/src/utils/media/image.rs +++ b/src/utils/media/image.rs @@ -12,13 +12,22 @@ use matrix_sdk::{ use ruma::{ api::client::media::get_content_thumbnail::v3::Method, events::{ - room::{ImageInfo, MediaSource as CommonMediaSource, ThumbnailInfo}, + room::{ + avatar::ImageInfo as AvatarImageInfo, ImageInfo, MediaSource as CommonMediaSource, + ThumbnailInfo, + }, sticker::StickerMediaSource, }, + OwnedMxcUri, }; use tracing::warn; -use crate::{components::AnimatedImagePaintable, spawn_tokio, DISABLE_GLYCIN_SANDBOX}; +use crate::{ + components::AnimatedImagePaintable, + spawn_tokio, + utils::{matrix::MediaFileError, save_data_to_tmp_file}, + DISABLE_GLYCIN_SANDBOX, +}; /// The default width of a generated thumbnail. const THUMBNAIL_DEFAULT_WIDTH: u32 = 800; @@ -342,10 +351,14 @@ impl From for BaseImageInfo { /// An API to download a thumbnail for a media. #[derive(Debug, Clone, Copy)] pub struct ThumbnailDownloader<'a> { - /// The source of a thumbnail of the media. - pub thumbnail: Option>, - /// The source of the original image. - pub original: Option>, + /// The main source of the image. + /// + /// This should be the source with the best quality. + pub main: ImageSource<'a>, + /// An alternative source for the image. + /// + /// This should be a source with a lower quality. + pub alt: Option>, } impl<'a> ThumbnailDownloader<'a> { @@ -356,34 +369,31 @@ impl<'a> ThumbnailDownloader<'a> { /// /// Returns `Ok(None)` if no thumbnail could be retrieved. Returns an error /// if something occurred while fetching the content. - pub async fn download( + pub async fn download_to_file( self, client: &Client, settings: ThumbnailSettings, - ) -> Result>, matrix_sdk::Error> { + ) -> Result { // First, select which source we are going to download from. - let source = if let Some((original, thumbnail)) = self.original.zip(self.thumbnail) { - if !original.can_be_thumbnailed() - && (original.filesize_is_too_big() - || thumbnail.dimensions_are_too_big(settings.width, settings.height, false)) + let source = if let Some(alt) = self.alt { + if !self.main.can_be_thumbnailed() + && (self.main.filesize_is_too_big() + || alt.dimensions_are_too_big(settings.width, settings.height, false)) { - // Use the thumbnail as source to save bandwidth. - thumbnail + // Use the alternative as source to save bandwidth. + alt } else { - original + self.main } - } else if let Some(original) = self.original { - original - } else if let Some(thumbnail) = self.thumbnail { - thumbnail } else { - return Ok(None); + self.main }; - if source.can_be_thumbnailed() - && (source.filesize_is_too_big() - || source.dimensions_are_too_big(settings.width, settings.height, true)) - { + let data = if source.should_thumbnail( + settings.prefer_thumbnail, + settings.width, + settings.height, + ) { // Try to get a thumbnail. let media = client.media(); let request = MediaRequest { @@ -393,25 +403,32 @@ impl<'a> ThumbnailDownloader<'a> { let handle = spawn_tokio!(async move { media.get_media_content(&request, true).await }); match handle.await.unwrap() { - Ok(data) => return Ok(Some(data)), + Ok(data) => Some(data), Err(error) => { warn!("Could not retrieve media thumbnail: {error}"); + None } } - } + } else { + None + }; // Fallback to downloading the full source. - let media = client.media(); - let request = MediaRequest { - source: source.source.to_common_media_source(), - format: MediaFormat::File, - }; + let data = if let Some(data) = data { + data + } else { + let media = client.media(); + let request = MediaRequest { + source: source.source.to_common_media_source(), + format: MediaFormat::File, + }; - let data = spawn_tokio!(async move { media.get_media_content(&request, true).await }) - .await - .unwrap()?; + spawn_tokio!(async move { media.get_media_content(&request, true).await }) + .await + .unwrap()? + }; - Ok(Some(data)) + Ok(save_data_to_tmp_file(&data)?) } } @@ -425,6 +442,26 @@ pub struct ImageSource<'a> { } impl<'a> ImageSource<'a> { + /// Whether we should try to thumbnail this source for the given requested + /// dimensions. + fn should_thumbnail( + &self, + prefer_thumbnail: bool, + requested_width: u32, + requested_height: u32, + ) -> bool { + if !self.can_be_thumbnailed() { + return false; + } + + if prefer_thumbnail && !self.has_dimensions() { + return true; + } + + self.filesize_is_too_big() + || self.dimensions_are_too_big(requested_width, requested_height, true) + } + /// Whether this source can be thumbnailed by the media repo. /// /// Returns `false` in these cases: @@ -449,6 +486,12 @@ impl<'a> ImageSource<'a> { .is_some_and(|i| i.size.is_some_and(|s| s > THUMBNAIL_MAX_FILESIZE_THRESHOLD)) } + /// Whether we have the dimensions of this source. + fn has_dimensions(&self) -> bool { + self.info + .is_some_and(|i| i.width.is_some() && i.height.is_some()) + } + /// Whether the dimensions of this source are too big for the given /// requested dimensions. fn dimensions_are_too_big( @@ -486,6 +529,8 @@ pub enum MediaSource<'a> { Common(&'a CommonMediaSource), /// The media source of a sticker. Sticker(&'a StickerMediaSource), + /// An MXC URI. + Uri(&'a OwnedMxcUri), } impl<'a> MediaSource<'a> { @@ -494,6 +539,7 @@ impl<'a> MediaSource<'a> { match self { Self::Common(source) => matches!(source, CommonMediaSource::Encrypted(_)), Self::Sticker(source) => matches!(source, StickerMediaSource::Encrypted(_)), + Self::Uri(_) => false, } } @@ -502,6 +548,7 @@ impl<'a> MediaSource<'a> { match self { Self::Common(source) => source.clone(), Self::Sticker(source) => source.clone().into(), + Self::Uri(uri) => CommonMediaSource::Plain(uri.clone()), } } } @@ -518,6 +565,12 @@ impl<'a> From<&'a StickerMediaSource> for MediaSource<'a> { } } +impl<'a> From<&'a OwnedMxcUri> for MediaSource<'a> { + fn from(value: &'a OwnedMxcUri) -> Self { + Self::Uri(value) + } +} + /// Information about the source of an image. #[derive(Debug, Clone, Copy, Default)] pub struct ImageSourceInfo<'a> { @@ -553,6 +606,17 @@ impl<'a> From<&'a ThumbnailInfo> for ImageSourceInfo<'a> { } } +impl<'a> From<&'a AvatarImageInfo> for ImageSourceInfo<'a> { + fn from(value: &'a AvatarImageInfo) -> Self { + Self { + width: value.width.and_then(|u| u.try_into().ok()), + height: value.height.and_then(|u| u.try_into().ok()), + mimetype: value.mimetype.as_deref(), + size: value.size.and_then(|u| u.try_into().ok()), + } + } +} + /// The settings for downloading a thumbnail. #[derive(Debug, Clone)] pub struct ThumbnailSettings { @@ -564,6 +628,14 @@ pub struct ThumbnailSettings { pub method: Method, /// Whether to request an animated thumbnail. pub animated: bool, + /// Whether we should prefer to get a thumbnail if dimensions are unknown. + /// + /// This is particularly useful for avatars where we will prefer to save + /// bandwidth and memory usage as we download a lot of them and they might + /// appear several times on the screen. For media messages, we will on the + /// contrary prefer to download the original content to reduce the space + /// taken in the media cache. + pub prefer_thumbnail: bool, } impl From for MediaThumbnailSettings { @@ -573,6 +645,7 @@ impl From for MediaThumbnailSettings { height, method, animated, + .. } = value; MediaThumbnailSettings { -- GitLab