Add data stream progress events and file rendering#30
Conversation
Add logic to handle rendering data streams with names and/or mime types associated as a special "file" state. I am assuming that these will be relatively large so files are buffered in tempfiles rather than in memory. And then also, add a "save" button which can be used to save files to arbitrary locations for inspection.
| /// Streams every chunk of a byte reader into an anonymous temp file, returning the open | ||
| /// handle. The file is unlinked at creation (`tempfile`), so the OS reclaims it when the | ||
| /// handle is dropped or the process exits — including on a panic/abort, where `Drop` would | ||
| /// not run (the release profile sets `panic = "abort"`). | ||
| async fn stream_to_temp_file( | ||
| reader: &mut ByteStreamReader, | ||
| entry: &Arc<Mutex<TopicEntry>>, | ||
| n: u64, | ||
| total: Option<u64>, | ||
| ) -> Result<std::fs::File, String> { | ||
| use tokio::io::AsyncWriteExt; | ||
| let std_file = tempfile::tempfile_in(std::env::temp_dir()).map_err(|e| e.to_string())?; | ||
| let mut file = tokio::fs::File::from_std(std_file); | ||
| let mut processed = 0u64; | ||
| while let Some(chunk) = reader.next().await { | ||
| let chunk = chunk.map_err(|e| e.to_string())?; | ||
| file.write_all(&chunk).await.map_err(|e| e.to_string())?; | ||
| processed += chunk.len() as u64; | ||
| update_progress(entry, n, processed, total); | ||
| } | ||
| file.flush().await.map_err(|e| e.to_string())?; | ||
| Ok(file.into_std().await) | ||
| } |
There was a problem hiding this comment.
Note to any reviewers - this maybe could make sense as an extension to ByteStreamReader?
There was a problem hiding this comment.
Yep, sounds generally useful! Also note in the Swift implementation, I had the equivalent method use the temporary directory by default and return the path to the written file.
|
Logs flood the console for me. 
|
I see it comes from here: https://github.com/livekit/rust-sdks/pull/1192/changes#diff-b27b5fab061a986aea2fd0654f77f4640c712b4eec98f54b14c84fe61a1aced0R2366 The event is marked as deprecated, is this still there on purpose? |
|
Otherwise it looks good to me, works as expected. |
| image = "0.25" | ||
| log = "0.4" | ||
| parking_lot = { version = "0.12", features = ["deadlock_detection"] } | ||
| rfd = "0.15" |
There was a problem hiding this comment.
praise: Nice, didn't know about this crate!
There was a problem hiding this comment.
question: Is there a reason you didn't use the latest version (0.17.x)?
| Some(p) => format!("{} ({}%)", format_size(r.size), (p * 100.0).round() as i64), | ||
| None => format_size(r.size), | ||
| }; | ||
| let meta = format!( |
There was a problem hiding this comment.
issue(non-blocking): This is a relatively expensive operation in the render path (format is allocating).
| if r.is_file { | ||
| show_file_box(ui, service, entry, r); | ||
| } else if let Some(StreamContent::Preview(preview)) = &r.content { | ||
| ui.add(egui::Label::new(RichText::new(preview).monospace())); |
There was a problem hiding this comment.
suggestion(non-blocking): Consider how this will render for a long string and possibly use Label::wrap().
|
One other issue I noticed, but it was pre-existing so this might not be the right place to fix it. In |
3 participants
Warning
This change relies on the rust data streams v2 implementation. This cannot be merged until that is merged first. This is also why CI is failing.
Add logic to handle rendering data streams with names and/or mime types associated as a special "file" state. I am assuming that these will be relatively large so files are buffered in tempfiles rather than in memory. And then also, add a "save" button which can be used to save files to arbitrary locations for inspection.
Untitled.mov
I will say, design wise, I think this side panel is starting to break down a little bit now that there is a card rendered within a card. I can try some more stuff here but also I'm not an
eguiexpert so I would be interested in some ideas before investigating anything.