Skip to content

feat(send_queue): report progress for media uploads #5008

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

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
b609637
feat(send_queue): add global setting for sending media progress updates
Johennes Jul 7, 2025
98ded6d
chore(send_queue): collect thumbnail-related metadata in a dedicated …
Johennes Jul 7, 2025
ed36540
feat(send_queue): cache thumbnail sizes to use them in progress repor…
Johennes Jul 7, 2025
84fa087
chore(send_queue): Make parent_is_thumbnail_upload available outside …
Johennes Jul 7, 2025
1c9436b
feat(send_queue): enable progress monitoring in RoomSendQueue::handle…
Johennes Jul 7, 2025
d2f47f3
feat(sdk): introduce AbstractProgress for tracking media progress in …
Johennes Jul 7, 2025
5c0bb6a
chore(send_queue): rename RoomSendQueueUpdate::UploadedMedia to Media…
Johennes Jul 7, 2025
e4e2796
feat(send_queue): communicate media upload progress in RoomSendQueueU…
Johennes Jul 7, 2025
6a82e29
feat(timeline): communicate media upload progress through EventSendSt…
Johennes Jul 7, 2025
5da557d
feat(ffi): expose media upload progress through EventSendState::NotSe…
Johennes Jul 7, 2025
7ce947b
fixup! feat(send_queue): add global setting for sending media progres…
Johennes Jul 9, 2025
80be2d2
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
f91be75
fixup! feat(timeline): communicate media upload progress through Even…
Johennes Jul 9, 2025
0f2eb69
fixup! feat(send_queue): enable progress monitoring in RoomSendQueue:…
Johennes Jul 9, 2025
1d1e9a6
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
08a805d
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
66827e1
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
5cb65e1
fixup! feat(timeline): communicate media upload progress through Even…
Johennes Jul 9, 2025
8ba3aa6
fixup! feat(ffi): expose media upload progress through EventSendState…
Johennes Jul 9, 2025
207fc5f
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
50ed24c
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
c7d1934
fixup! feat(timeline): communicate media upload progress through Even…
Johennes Jul 9, 2025
48ccad9
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
13631d5
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
50696e1
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
0e1baac
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 9, 2025
cf51ac9
fixup! feat(send_queue): cache thumbnail sizes to use them in progres…
Johennes Jul 9, 2025
97a553f
fixup! feat(send_queue): cache thumbnail sizes to use them in progres…
Johennes Jul 9, 2025
971dcda
fixup! feat(send_queue): cache thumbnail sizes to use them in progres…
Johennes Jul 10, 2025
ad963c7
fixup! feat(send_queue): communicate media upload progress in RoomSen…
Johennes Jul 10, 2025
7cba439
Merge branch 'main' into johannes/send-queue-media-progress
Johennes Jul 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions crates/matrix-sdk/src/send_queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ struct QueueThumbnailInfo {

/// The thumbnail's mime type.
content_type: Mime,

/// The thumbnail's file size in bytes.
file_size: usize,
}

/// A specific room's send queue ran into an error, and it has disabled itself.
Expand Down Expand Up @@ -964,6 +967,10 @@ struct QueueStorage {

/// To which room is this storage related.
room_id: OwnedRoomId,

/// In-memory mapping of media transaction IDs to thumbnail sizes for the
/// purpose of progress reporting.
thumbnail_size_cache: Arc<Mutex<HashMap<OwnedTransactionId, Vec<Option<usize>>>>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Could we explain in the code comment the oddities about the value:
    1. Why do we need to store multiple thumbnail sizes for a single media transaction? As far as I understand, the transaction id is for the media event (either a regular (single) media event or a gallery event)
    2. Why can it be None? Could we store a Vec<usize> instead, and filter out the cases where there's no thumbnails? (and not even insert into this map, in this case)
  2. Could we use a sync mutex here, aka matrix_sdk_common::locks::Mutex? we might not need the async part, if the lock is only held across sync portions of the code
  3. Out of curiosity, would it make sense to have this in send_queue::StoreLock, which already contains some information about the current request? (in the form of BeingSent)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Fixed in cf51ac9.
  2. Fixed in 97a553f.
  3. I will have to look into this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I addressed 3 with 971dcda and ad963c7. I had to switch back to the async Mutex though because that has lock_owned and I couldn't figure out the lifetimes without it. 🙈

}

impl QueueStorage {
Expand All @@ -975,7 +982,11 @@ impl QueueStorage {

/// Create a new queue for queuing requests to be sent later.
fn new(client: WeakClient, room: OwnedRoomId) -> Self {
Self { room_id: room, store: StoreLock { client, being_sent: Default::default() } }
Self {
room_id: room,
store: StoreLock { client, being_sent: Default::default() },
thumbnail_size_cache: Arc::new(Mutex::new(HashMap::new())),
}
}

/// Push a new event to be sent in the queue, with a default priority of 0.
Expand Down Expand Up @@ -1132,6 +1143,8 @@ impl QueueStorage {
warn!(txn_id = %transaction_id, "request marked as sent was missing from storage");
}

self.thumbnail_size_cache.lock().await.remove(transaction_id);

Ok(())
}

Expand Down Expand Up @@ -1172,6 +1185,8 @@ impl QueueStorage {
.remove_send_queue_request(&self.room_id, transaction_id)
.await?;

self.thumbnail_size_cache.lock().await.remove(transaction_id);

Ok(removed)
}

Expand Down Expand Up @@ -1234,6 +1249,8 @@ impl QueueStorage {
let client = guard.client()?;
let store = client.state_store();

let media_sizes = vec![thumbnail.as_ref().map(|t| t.file_size)];

let thumbnail_info = self
.push_thumbnail_and_media_uploads(
store,
Expand All @@ -1251,7 +1268,7 @@ impl QueueStorage {
.save_dependent_queued_request(
&self.room_id,
&upload_file_txn,
send_event_txn.into(),
send_event_txn.clone().into(),
created_at,
DependentQueuedRequestKind::FinishUpload {
local_echo: Box::new(event),
Expand All @@ -1261,6 +1278,8 @@ impl QueueStorage {
)
.await?;

self.thumbnail_size_cache.lock().await.insert(send_event_txn, media_sizes);

Ok(())
}

Expand All @@ -1281,6 +1300,7 @@ impl QueueStorage {
let store = client.state_store();

let mut finish_item_infos = Vec::with_capacity(item_queue_infos.len());
let mut media_sizes = Vec::with_capacity(item_queue_infos.len());

let Some((first, rest)) = item_queue_infos.split_first() else {
return Ok(());
Expand All @@ -1303,6 +1323,7 @@ impl QueueStorage {

finish_item_infos
.push(FinishGalleryItemInfo { file_upload: upload_file_txn.clone(), thumbnail_info });
media_sizes.push(thumbnail.as_ref().map(|t| t.file_size));

let mut last_upload_file_txn = upload_file_txn.clone();

Expand Down Expand Up @@ -1367,6 +1388,7 @@ impl QueueStorage {
file_upload: upload_file_txn.clone(),
thumbnail_info: thumbnail_info.cloned(),
});
media_sizes.push(thumbnail.as_ref().map(|t| t.file_size));

last_upload_file_txn = upload_file_txn.clone();
}
Expand All @@ -1377,7 +1399,7 @@ impl QueueStorage {
.save_dependent_queued_request(
&self.room_id,
&last_upload_file_txn,
send_event_txn.into(),
send_event_txn.clone().into(),
created_at,
DependentQueuedRequestKind::FinishGallery {
local_echo: Box::new(event),
Expand All @@ -1386,6 +1408,8 @@ impl QueueStorage {
)
.await?;

self.thumbnail_size_cache.lock().await.insert(send_event_txn, media_sizes);

Ok(())
}

Expand Down
2 changes: 2 additions & 0 deletions crates/matrix-sdk/src/send_queue/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ impl RoomSendQueue {
// Create the information required for filling the thumbnail section of the
// event.
let (data, content_type, thumbnail_info) = thumbnail.into_parts();
let file_size = data.len();

// Cache thumbnail in the cache store.
let thumbnail_media_request = Media::make_local_file_media_request(&txn);
Expand Down Expand Up @@ -472,6 +473,7 @@ impl RoomSendQueue {
},
media_request_parameters: thumbnail_media_request,
content_type,
file_size,
}),
})
} else {
Expand Down