Rust Rocket Backend / WebRTC H264 Camera Streaming #5
Conversation
TODO: Camera Controls Code Cleanup Comments
|
Thank you for your work, Mizael! A lot of my suggestions have to do with the formatting and redundancies. These can be detected with You can also configure your editor to use these automatically:
Please run these tools and ping me again when complete. Thanks again! |
megustaloscheetos
force-pushed
the
main
branch
from
9a7e098
to
5ea554e
Compare
|
This is very good! Just needs a few more (mostly documentation) changes. One more suggestion: |
| "{}x{} @{}fps", | ||
| self.width, self.height, self.frame_interval.denominator | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
As the Clippy lint suggests, you're better off using fmt::Display to get the ToString trait on a type. Most generic functions look for Display, not ToString, so this advice is generally useful.
There was a problem hiding this comment.
| "{}x{} @{}fps", | |
| self.width, self.height, self.frame_interval.denominator | |
| ) | |
| } | |
| } | |
| impl core::fmt::Display for CameraMode { | |
| fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { | |
| write!( | |
| f, | |
| "{}x{} @{}fps", | |
| self.width, self.height, self.frame_interval.denominator | |
| ) | |
| } | |
| } |
There was a problem hiding this comment.
Above is a typical conversion from ToString. The write! macro is pretty nifty here!
| } | ||
| } | ||
|
|
||
| // Handles communication between CameraThreadHandle(s) and WebRTC clients. |
There was a problem hiding this comment.
You may have intended to use a doc comment here:
| // Handles communication between CameraThreadHandle(s) and WebRTC clients. | |
| /// Handles communication between CameraThreadHandle(s) and WebRTC clients. |
| } | ||
|
|
||
| // IMPORTANT: Everything in here runs within the tokio runtime! | ||
| pub async fn add_client( |
There was a problem hiding this comment.
I appreciate the notice above. However, the exact meaning of a "client" might be unclear far into the future. Please consider adding doc comments to this method.
| "video".to_owned(), | ||
| "webrtc".to_owned(), |
There was a problem hiding this comment.
If you create multiple streams under the same IDs, what happens? I'm actually not sure, but I'm concerned that it may result in unexpected behavior. Please make sure the constant IDs are intentional.
Remember: it's very important that folks can understand this later on. May and June are going to be chock-full of debugging, so we want to get it out of the way now! :D
| camera_path, | ||
| manual_shutdown_needed, | ||
| cam_mode_tx, | ||
| current_mode: *modes.last().ok_or("No Last Mode")?, |
There was a problem hiding this comment.
Be careful with errors like this! These can lack context at competition.
In a future PR, I'll add tracing to the project to help better log the camera/server state. For now, you might want to add something like "Error creating camera thread: failed to initialize camera". (this also goes for the other usage ~20 lines up)
| return; | ||
| } | ||
|
|
||
| // Used to prevent having to constantly lock the c_tx_sink mutex. |
There was a problem hiding this comment.
This might be 2% more idiomatic? ;D
| // Used to prevent having to constantly lock the c_tx_sink mutex. | |
| while !c_manual_shutdown_needed.load(Ordering::SeqCst) { |
There was a problem hiding this comment.
lol not sure why it replaced the comment. it swaps out the loop {
For issue #4, a basic finished version of the Rust backend I have been working on. Notably, the backend streams H264 video to web browser clients through WebRTC. Of which, multiple clients can be handled at once without issue. The backend also includes a basic api to set the resolution and frame rate configuration at which the stream is sent (there are example api calls commented out in the react-app App.js file). But that should be all with regards to the actual backend! However, at the moment, this PR is mainly to add something that is usable at a basic capacity. Without a doubt, I do think that my code could be improved through review as I do use things such as boxed errors, however, this is something that I am looking for guidance with!