BREAKING CHANGE: switching to sans-io-runtime #257
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
clippy found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Actionable comments outside the diff hunks (2)
README.md (2)
Line range hint
3-33: Add alternative text to images to improve accessibility.
Line range hint
98-98: Specify the language for the code block to improve readability and syntax highlighting.- ```bash + ```bash
| Our primary focus is currently on Goal 1. For details on the current phase plan, please refer to our [Roadmap](https://github.com/8xFF/atm0s-media-server/issues/69). | ||
|
|
||
| Goals 2 and 3 are in the research phase. For more information, you can visit this [Repository](https://github.com/giangndm/decentralized-p2p-streaming). | ||
| We are actively refactoring entiry media server and network stack with [sans-io-runtime](https://github.com/8xff/sans-io-runtime) for better performance. If you are looking for older version, please check in [legacy branch](https://github.com/8xFF/atm0s-media-server/tree/legacy). |
There was a problem hiding this comment.
Fix typo in the refactoring description.
- We are actively refactoring entiry media server and network stack with [sans-io-runtime](https://github.com/8xff/sans-io-runtime) for better performance.
+ We are actively refactoring the entire media server and network stack with [sans-io-runtime](https://github.com/8xff/sans-io-runtime) for better performance.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| We are actively refactoring entiry media server and network stack with [sans-io-runtime](https://github.com/8xff/sans-io-runtime) for better performance. If you are looking for older version, please check in [legacy branch](https://github.com/8xFF/atm0s-media-server/tree/legacy). | |
| We are actively refactoring the entire media server and network stack with [sans-io-runtime](https://github.com/8xff/sans-io-runtime) for better performance. If you are looking for older version, please check in [legacy branch](https://github.com/8xFF/atm0s-media-server/tree/legacy). |
packages/core/src/transport.rs
Outdated
| pub struct TransportSession(pub u64); | ||
| pub struct TrackId(pub u16); | ||
|
|
||
| pub enum TransportState {} |
There was a problem hiding this comment.
Define possible states in TransportState.
Currently, TransportState is an empty enum, which might be a placeholder. Defining actual states can clarify its usage and ensure it fulfills its intended role.
packages/core/src/transport.rs
Outdated
|
|
||
| pub enum TransportState {} | ||
|
|
||
| pub struct TransportStats {} |
There was a problem hiding this comment.
Define possible statistics in TransportStats.
Currently, TransportStats is an empty struct, which might be a placeholder. Defining actual statistics can clarify its usage and ensure it fulfills its intended role.
packages/core/src/transport.rs
Outdated
|
|
||
| pub struct TransportStats {} | ||
|
|
||
| pub enum TransportControlIn {} |
There was a problem hiding this comment.
Define possible control inputs in TransportControlIn.
Currently, TransportControlIn is an empty enum, which might be a placeholder. Defining actual control inputs can clarify its usage and ensure it fulfills its intended role.
packages/core/src/transport.rs
Outdated
|
|
||
| pub enum TransportControlIn {} | ||
|
|
||
| pub enum TransportControlOut {} |
There was a problem hiding this comment.
Define possible control outputs in TransportControlOut.
Currently, TransportControlOut is an empty enum, which might be a placeholder. Defining actual control outputs can clarify its usage and ensure it fulfills its intended role.
packages/core/src/endpoint.rs
Outdated
| } | ||
|
|
||
| impl<T: Transport> Endpoint<T> { | ||
| fn process_transport_output<'a>(&mut self, out: TransportOutput<'a>) -> Option<Output<'a>> { |
There was a problem hiding this comment.
Unused variable out in process_transport_output method.
The variable out is declared but not used in the process_transport_output method. Consider using it or removing it if it's unnecessary.
| pub struct SharedUdpPort<Task> { | ||
| backend_addr: Option<SocketAddr>, | ||
| task_remotes: HashMap<SocketAddr, Task>, | ||
| task_remotes_map: HashMap<Task, Vec<SocketAddr>>, | ||
| task_ufrags: HashMap<String, Task>, | ||
| task_ufrags_reverse: HashMap<Task, String>, | ||
| } |
There was a problem hiding this comment.
Unused fields in SharedUdpPort.
Several fields in the SharedUdpPort struct (backend_addr, task_remotes, task_remotes_map, task_ufrags, task_ufrags_reverse) are declared but never read. This could indicate dead code or an incomplete implementation. Consider removing them or implementing their functionality.
| impl<Task: Debug + Clone + Copy + Hash + PartialEq + Eq> SharedUdpPort<Task> { | ||
| pub fn set_backend_info(&mut self, addr: SocketAddr) { | ||
| self.backend_addr = Some(addr); | ||
| } | ||
|
|
||
| pub fn get_backend_addr(&self) -> Option<SocketAddr> { | ||
| self.backend_addr | ||
| } | ||
|
|
||
| pub fn add_ufrag(&mut self, ufrag: String, task: Task) { | ||
| log::info!("Add ufrag {} to task {:?}", ufrag, task); | ||
| self.task_ufrags.insert(ufrag.clone(), task); | ||
| self.task_ufrags_reverse.insert(task, ufrag); | ||
| } | ||
|
|
||
| pub fn remove_task(&mut self, task: Task) -> Option<()> { | ||
| let ufrag = self.task_ufrags_reverse.remove(&task)?; | ||
| log::info!("Remove task {:?} => ufrag {}", task, ufrag); | ||
| self.task_ufrags.remove(&ufrag)?; | ||
| let remotes = self.task_remotes_map.remove(&task)?; | ||
| for remote in remotes { | ||
| log::info!(" Remove remote {:?} => task {:?}", remote, task); | ||
| self.task_remotes.remove(&remote); | ||
| } | ||
| Some(()) | ||
| } | ||
|
|
||
| pub fn map_remote(&mut self, remote: SocketAddr, buf: &[u8]) -> Option<Task> { | ||
| if let Some(task) = self.task_remotes.get(&remote) { | ||
| return Some(*task); | ||
| } | ||
|
|
||
| let stun_username = Self::get_stun_username(buf)?; | ||
| log::warn!("Received a stun packet from an unknown remote: {:?}, username {}", remote, stun_username); | ||
| let task = self.task_ufrags.get(stun_username)?; | ||
| log::info!("Mapping remote {:?} to task {:?}", remote, task); | ||
| self.task_remotes.insert(remote, *task); | ||
| self.task_remotes_map.entry(*task).or_default().push(remote); | ||
| Some(*task) | ||
| } | ||
|
|
||
| fn get_stun_username(buf: &[u8]) -> Option<&str> { |
There was a problem hiding this comment.
Unused methods in SharedUdpPort.
Several methods in the SharedUdpPort struct (set_backend_info, get_backend_addr, add_ufrag, remove_task, map_remote, get_stun_username) are declared but never used. This could indicate dead code or an incomplete implementation. Consider removing them or implementing their functionality.
| impl Transport for TransportWebrtc { | ||
| fn on_tick<'a>(&mut self, now: Instant) -> Option<TransportOutput<'a>> { | ||
| let next_tick = self.next_tick?; | ||
| if next_tick > now { | ||
| return None; | ||
| } | ||
| self.rtc.handle_input(Str0mInput::Timeout(now)).ok()?; | ||
| self.pop_output(now) | ||
| } | ||
|
|
||
| fn on_input<'a>(&mut self, _now: Instant, _input: TransportInput<'a>) -> Option<TransportOutput<'a>> { | ||
| todo!() | ||
| } | ||
|
|
||
| fn pop_output<'a>(&mut self, _now: Instant) -> Option<TransportOutput<'a>> { | ||
| loop { | ||
| match self.rtc.poll_output().ok()? { | ||
| Str0mOutput::Timeout(instance) => { | ||
| self.next_tick = Some(instance); | ||
| } | ||
| Str0mOutput::Transmit(_) => { | ||
| //TODO convert transmit to transport output | ||
| } | ||
| Str0mOutput::Event(event) => match event { | ||
| Str0mEvent::Connected => todo!(), | ||
| Str0mEvent::IceConnectionStateChange(_) => todo!(), | ||
| Str0mEvent::MediaAdded(_) => todo!(), | ||
| Str0mEvent::MediaData(_) => todo!(), | ||
| Str0mEvent::MediaChanged(_) => todo!(), | ||
| Str0mEvent::ChannelOpen(_, _) => todo!(), | ||
| Str0mEvent::ChannelData(_) => todo!(), | ||
| Str0mEvent::ChannelClose(_) => todo!(), | ||
| Str0mEvent::PeerStats(_) => todo!(), | ||
| Str0mEvent::MediaIngressStats(_) => todo!(), | ||
| Str0mEvent::MediaEgressStats(_) => todo!(), | ||
| Str0mEvent::EgressBitrateEstimate(_) => todo!(), | ||
| Str0mEvent::KeyframeRequest(_) => todo!(), | ||
| Str0mEvent::StreamPaused(_) => todo!(), | ||
| Str0mEvent::RtpPacket(_) => todo!(), | ||
| _ => {} | ||
| }, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Implement functionality for transport methods.
The methods on_tick, on_input, pop_output use placeholders like todo!() and incomplete implementations. Complete these methods to ensure the transport functions as expected.
| @@ -0,0 +1,3 @@ | |||
| # Atm0s Media Server protocol | |||
|
|
|||
| This repository contains the protocol definition for inter-communication between the Atm0s Media Server and its clients. | |||
There was a problem hiding this comment.
Consider changing "inter-communication" to "intercommunication" for grammatical correctness.
- inter-communication
+ intercommunicationCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| This repository contains the protocol definition for inter-communication between the Atm0s Media Server and its clients. | |
| This repository contains the protocol definition for intercommunication between the Atm0s Media Server and its clients. |
| pub fn tasks(&self) -> usize { | ||
| self.sdn_worker.tasks() + self.sdn_worker.tasks() | ||
| } |
There was a problem hiding this comment.
Ensure correct task count calculation.
- self.sdn_worker.tasks() + self.sdn_worker.tasks()
+ self.sdn_worker.tasks() + self.media_cluster.tasks() + self.media_webrtc.tasks()The current implementation incorrectly sums the tasks of sdn_worker twice. It should include the tasks from media_cluster and media_webrtc to reflect the total number of tasks accurately.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| pub fn tasks(&self) -> usize { | |
| self.sdn_worker.tasks() + self.sdn_worker.tasks() | |
| } | |
| pub fn tasks(&self) -> usize { | |
| self.sdn_worker.tasks() + self.media_cluster.tasks() + self.media_webrtc.tasks() | |
| } |
| BackendIncoming::UdpPacket { slot: _, from, data } => { | ||
| let out = self.sdn_worker.on_event(now_ms, SdnWorkerInput::Net(NetInput::UdpPacket(from, data)))?; | ||
| Some(self.output_sdn(now, out)) | ||
| } | ||
| BackendIncoming::UdpListenResult { bind: _, result } => { | ||
| let (_addr, slot) = result.ok()?; | ||
| self.sdn_slot = slot; | ||
| None | ||
| } |
There was a problem hiding this comment.
Handle potential None case explicitly.
- let (_addr, slot) = result.ok()?;
+ if let Some((_addr, slot)) = result {
+ self.sdn_slot = slot;
+ } else {
+ // Handle error or log warning
+ }The current implementation uses result.ok()? which will return None from the function if result is Err. It's better to handle this case explicitly to ensure that appropriate actions (like logging or error handling) are taken when result is not Ok.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| BackendIncoming::UdpPacket { slot: _, from, data } => { | |
| let out = self.sdn_worker.on_event(now_ms, SdnWorkerInput::Net(NetInput::UdpPacket(from, data)))?; | |
| Some(self.output_sdn(now, out)) | |
| } | |
| BackendIncoming::UdpListenResult { bind: _, result } => { | |
| let (_addr, slot) = result.ok()?; | |
| self.sdn_slot = slot; | |
| None | |
| } | |
| BackendIncoming::UdpPacket { slot: _, from, data } => { | |
| let out = self.sdn_worker.on_event(now_ms, SdnWorkerInput::Net(NetInput::UdpPacket(from, data)))?; | |
| Some(self.output_sdn(now, out)) | |
| } | |
| BackendIncoming::UdpListenResult { bind: _, result } => { | |
| if let Some((_addr, slot)) = result { | |
| self.sdn_slot = slot; | |
| } else { | |
| // Handle error or log warning | |
| } | |
| None | |
| } |
| pub fn on_tick<'a>(&mut self, now: Instant) -> Option<Output<'a>> { | ||
| let s = &mut self.switcher; | ||
| while let Some(c) = s.looper_current(now) { | ||
| match c.try_into().ok()? { | ||
| TaskType::Sdn => { | ||
| let now_ms = self.timer.timestamp_ms(now); | ||
| if let Some(out) = s.looper_process(self.sdn_worker.on_tick(now_ms)) { | ||
| return Some(self.output_sdn(now, out)); | ||
| } | ||
| } | ||
| TaskType::MediaCluster => { | ||
| if let Some(out) = s.looper_process(self.media_cluster.on_tick(now)) { | ||
| return Some(self.output_cluster(now, out)); | ||
| } | ||
| } | ||
| TaskType::MediaWebrtc => { | ||
| if let Some(out) = s.looper_process(self.media_webrtc.on_tick(now)) { | ||
| return Some(self.output_webrtc(now, out)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| None | ||
| } |
There was a problem hiding this comment.
Refactor repeated code into a function.
Consider refactoring the repeated pattern of task processing in the on_tick method into a separate function. This will improve code readability and maintainability.
packages/media_runner/src/worker.rs
Outdated
| pub fn pop_output<'a>(&mut self, now: Instant) -> Option<Output<'a>> { | ||
| if let Some(out) = self.queue.pop_front() { | ||
| return Some(out); | ||
| } | ||
| let s = &mut self.switcher; | ||
| while let Some(c) = s.queue_current() { | ||
| match c.try_into().ok()? { | ||
| TaskType::Sdn => { | ||
| let now_ms = self.timer.timestamp_ms(now); | ||
| if let Some(out) = s.looper_process(self.sdn_worker.pop_output(now_ms)) { | ||
| return Some(self.output_sdn(now, out)); | ||
| } | ||
| } | ||
| TaskType::MediaCluster => { | ||
| if let Some(out) = s.looper_process(self.media_cluster.pop_output(now)) { | ||
| return Some(self.output_cluster(now, out)); | ||
| } | ||
| } | ||
| TaskType::MediaWebrtc => { | ||
| if let Some(out) = s.looper_process(self.media_webrtc.pop_output(now)) { | ||
| return Some(self.output_webrtc(now, out)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| None | ||
| } |
There was a problem hiding this comment.
Refactor repeated code into a function.
Similar to the on_tick method, the pop_output method also contains a repeated pattern of task processing. Consider refactoring this into a separate function to reduce code duplication and enhance clarity.
| pub fn shutdown<'a>(&mut self, now: Instant) -> Option<Output<'a>> { | ||
| let s = &mut self.switcher; | ||
| while let Some(c) = s.looper_current(now) { | ||
| match c.try_into().ok()? { | ||
| TaskType::Sdn => { | ||
| let now_ms = self.timer.timestamp_ms(now); | ||
| if let Some(out) = s.looper_process(self.sdn_worker.on_event(now_ms, SdnWorkerInput::ShutdownRequest)) { | ||
| return Some(self.output_sdn(now, out)); | ||
| } | ||
| } | ||
| TaskType::MediaCluster => { | ||
| if let Some(out) = s.looper_process(self.media_cluster.shutdown(now)) { | ||
| return Some(self.output_cluster(now, out)); | ||
| } | ||
| } | ||
| TaskType::MediaWebrtc => { | ||
| if let Some(out) = s.looper_process(self.media_webrtc.shutdown(now)) { | ||
| return Some(self.output_webrtc(now, out)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| None | ||
| } |
There was a problem hiding this comment.
Refactor repeated code into a function.
The shutdown method shares a similar structure with on_tick and pop_output. Refactoring the common task processing pattern into a separate function would greatly improve the maintainability and readability of the code.
| let stream = new MediaStream(); | ||
| document.querySelector("video").srcObject = stream; | ||
| pc.ontrack = (event) => { | ||
| stream.addTrack(event.track); |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has confirmed the presence of the ontrack event handler in whep.demo.js as described in the review. However, no specific test files for this script were found in the repository. This suggests that either the testing is handled in a more integrated environment not specific to whep.demo.js, or the testing coverage might be insufficient for this particular functionality.
It is recommended to:
- Ensure that comprehensive tests are developed and run to verify the new video track handling functionality.
- Review the overall testing strategy to include unit and integration tests that cover the changes made in
whep.demo.js.
Analysis chain
The modification to handle incoming video tracks by adding them to a new MediaStream object is a good practice, as it allows for more flexible management of media streams. Ensure that this change is tested thoroughly to confirm that it integrates well with the rest of the application.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Test the new video track handling functionality
echo "Ensure to run comprehensive tests on the new video track management."
Length of output: 146
Script:
#!/bin/bash
# Search for test files related to whep.demo.js
fd 'whep.demo.test.js' --exec cat {}
# Search for references to the ontrack event handler in the codebase
rg 'ontrack ='
Length of output: 111
packages/protocol/src/endpoint.rs
Outdated
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| todo!() | ||
| } |
There was a problem hiding this comment.
Implement the from_str method for ClusterConnId or mark it with a TODO comment for future implementation.
| // sendEncodings: [ | ||
| // { rid: "0", active: true, scaleResolutionDownBy: 2}, | ||
| // { rid: "1", active: true, scaleResolutionDownBy: 2}, | ||
| // { rid: "2", active: true }, | ||
| // ], |
There was a problem hiding this comment.
Document the reason for commenting out the sendEncodings configuration in the addTransceiver function or mark it as a temporary change if applicable.
packages/media_core/src/transport.rs
Outdated
| pub trait Transport { | ||
| fn on_tick<'a>(&mut self, now: Instant) -> Option<TransportOutput<'a>>; | ||
| fn on_input<'a>(&mut self, now: Instant, input: TransportInput<'a>) -> Option<TransportOutput<'a>>; | ||
| fn pop_output<'a>(&mut self, now: Instant) -> Option<TransportOutput<'a>>; | ||
| } |
There was a problem hiding this comment.
Complete the implementation of on_tick, on_input, and pop_output methods in the Transport trait or mark them with TODO comments for future implementation.
packages/media_core/src/endpoint.rs
Outdated
| pub fn on_tick<'a>(&mut self, now: Instant) -> Option<Output<'a>> { | ||
| let out = self.transport.on_tick(now)?; | ||
| self.process_transport_output(out) | ||
| } | ||
|
|
||
| pub fn on_event<'a>(&mut self, now: Instant, input: Input<'a>) -> Option<Output<'a>> { | ||
| let input = match input { | ||
| Input::Net(net) => TransportInput::Net(net), | ||
| _ => todo!(), | ||
| }; | ||
| let out = self.transport.on_input(now, input)?; | ||
| self.process_transport_output(out) | ||
| } | ||
|
|
||
| pub fn pop_output<'a>(&mut self, now: Instant) -> Option<Output<'a>> { | ||
| let out = self.transport.pop_output(now)?; | ||
| self.process_transport_output(out) | ||
| } | ||
|
|
||
| pub fn shutdown<'a>(&mut self, now: Instant) -> Option<Output<'a>> { | ||
| todo!() | ||
| } |
There was a problem hiding this comment.
Complete the implementation of on_tick, on_event, pop_output, and shutdown methods in the Endpoint structure or mark them with TODO comments for future implementation.
packages/protocol/src/endpoint.rs
Outdated
| impl FromStr for ClusterConnId { | ||
| type Err = String; | ||
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| todo!() | ||
| } |
There was a problem hiding this comment.
Complete the implementation of from_str for ClusterConnId or mark it with a TODO comment for future implementation.
| @@ -0,0 +1,57 @@ | |||
| # Media Server Core logic | |||
|
|
|||
| This module implement core logic of media server. It is responsible for: | |||
There was a problem hiding this comment.
Correct the verb form for better readability: "This module implements the core logic of the media server."
- This module implement core logic of media server.
+ This module implements the core logic of the media server.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| This module implement core logic of media server. It is responsible for: | |
| This module implements the core logic of the media server. It is responsible for: |
|
|
||
| ## Modules details | ||
|
|
||
| - Transport: interact with sdk or clients |
There was a problem hiding this comment.
Correct the spelling mistake: "SDK".
- interact with sdk or clients
+ interact with SDK or clientsCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| - Transport: interact with sdk or clients | |
| - Transport: interact with SDK or clients |
|
|
||
| - Transport: interact with sdk or clients | ||
| - Cluster: interact with atm0s-sdn, convert local action into atm0s-sdn APIs. | ||
| - Endpoint: interact with transport in/out and cluster in/out, it act as a bridge between transport and cluster. |
There was a problem hiding this comment.
Use the correct verb form: "acts".
- it act as a bridge between transport and cluster.
+ it acts as a bridge between transport and cluster.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| - Endpoint: interact with transport in/out and cluster in/out, it act as a bridge between transport and cluster. | |
| - Endpoint: interact with transport in/out and cluster in/out, it acts as a bridge between transport and cluster. |
| - RemoteTrack: track receiving media from client. | ||
| - LocalTrack: track sending data to client. | ||
|
|
||
| We design each module is very simple responsebility, and interact to each other with general event and control. We need design for avoiding bias into any protocol, instead we design it for room based interactive media server. |
There was a problem hiding this comment.
Replace "very simple responsibility" with "clearly defined responsibilities" for better clarity and professionalism.
- We design each module is very simple responsibility, and interact to each other with general event and control.
+ We design each module with clearly defined responsibilities, interacting with each other through general events and controls.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| We design each module is very simple responsebility, and interact to each other with general event and control. We need design for avoiding bias into any protocol, instead we design it for room based interactive media server. | |
| We design each module with clearly defined responsibilities, interacting with each other through general events and controls. We need design for avoiding bias into any protocol, instead we design it for room based interactive media server. |
packages/media_core/src/cluster.rs
Outdated
| PeerJoined(PeerId), | ||
| PeerLeaved(PeerId), | ||
| TrackStarted(PeerId, TrackName, TrackMeta), | ||
| TrackStoped(PeerId, TrackName), |
There was a problem hiding this comment.
Correct the typographical error in the TrackStopped event.
- TrackStoped
+ TrackStoppedCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| TrackStoped(PeerId, TrackName), | |
| TrackStopped(PeerId, TrackName), |
packages/media_core/src/cluster.rs
Outdated
| pub fn on_tick(&mut self, now: Instant) -> Option<Output<Owner>> { | ||
| todo!() | ||
| } | ||
|
|
||
| pub fn on_input(&mut self, now: Instant, input: Input<Owner>) -> Option<Output<Owner>> { | ||
| todo!() | ||
| } | ||
|
|
||
| pub fn pop_output(&mut self, now: Instant) -> Option<Output<Owner>> { | ||
| todo!() | ||
| } | ||
|
|
||
| pub fn shutdown<'a>(&mut self, now: Instant) -> Option<Output<Owner>> { | ||
| todo!() | ||
| } |
There was a problem hiding this comment.
Complete the implementation of on_tick, on_input, pop_output, and shutdown methods in MediaCluster or mark them with TODO comments for future implementation.
| pub fn on_tick<'a, ExtIn>(&mut self, now: Instant) -> Option<InternalOutput<'a, ExtIn>> { | ||
| None | ||
| } | ||
|
|
||
| pub fn pop_output<'a, ExtIn>(&mut self, now: Instant) -> Option<InternalOutput<'a, ExtIn>> { | ||
| None | ||
| } | ||
|
|
||
| pub fn shutdown<'a, ExtIn>(&mut self, now: Instant) -> Option<InternalOutput<'a, ExtIn>> { | ||
| None | ||
| } |
There was a problem hiding this comment.
Complete the implementation of on_tick, pop_output, and shutdown methods in EndpointInternal or mark them with TODO comments for future implementation.
| pub fn on_transport_event<'a, ExtIn, ExtOut>(&mut self, now: Instant, event: TransportEvent<'a, ExtOut>) -> Option<InternalOutput<'a, ExtIn>> { | ||
| match event { | ||
| TransportEvent::Net(out) => Some(InternalOutput::Net(out)), | ||
| TransportEvent::State(state) => self.on_transport_state_changed(now, state), | ||
| TransportEvent::RemoteTrack(track, event) => self.on_transport_remote_track(now, track, event), | ||
| TransportEvent::LocalTrack(track, event) => self.on_transport_local_track(now, track, event), | ||
| TransportEvent::Stats(stats) => self.on_transport_stats(now, stats), | ||
| TransportEvent::Control(control) => self.on_transport_control(now, control), | ||
| TransportEvent::Ext(_) => panic!("should not get here"), | ||
| } | ||
| } | ||
|
|
||
| fn on_transport_state_changed<'a, ExtIn>(&mut self, now: Instant, state: TransportState) -> Option<InternalOutput<'a, ExtIn>> { | ||
| match state { | ||
| TransportState::Connecting => todo!(), | ||
| TransportState::ConnectError(_) => todo!(), | ||
| TransportState::Connected => todo!(), | ||
| TransportState::Reconnecting => todo!(), | ||
| TransportState::Disconnected(_) => todo!(), | ||
| } | ||
| } | ||
|
|
||
| fn on_transport_remote_track<'a, ExtIn>(&mut self, now: Instant, track: RemoteTrackId, event: RemoteTrackEvent) -> Option<InternalOutput<'a, ExtIn>> { | ||
| match event { | ||
| RemoteTrackEvent::Started { name } => todo!(), | ||
| RemoteTrackEvent::Paused => todo!(), | ||
| RemoteTrackEvent::Media(_) => todo!(), | ||
| RemoteTrackEvent::Ended => todo!(), | ||
| } | ||
| } | ||
|
|
||
| fn on_transport_local_track<'a, ExtIn>(&mut self, now: Instant, track: LocalTrackId, event: LocalTrackEvent) -> Option<InternalOutput<'a, ExtIn>> { | ||
| match event { | ||
| LocalTrackEvent::Started { name } => todo!(), | ||
| LocalTrackEvent::Paused => todo!(), | ||
| LocalTrackEvent::RequestKeyFrame => todo!(), | ||
| LocalTrackEvent::Ended => todo!(), | ||
| } | ||
| } | ||
|
|
||
| fn on_transport_control<'a, ExtIn>(&mut self, now: Instant, control: ClientEndpointControl) -> Option<InternalOutput<'a, ExtIn>> { | ||
| todo!() | ||
| } | ||
|
|
||
| fn on_transport_stats<'a, ExtIn>(&mut self, now: Instant, stats: TransportStats) -> Option<InternalOutput<'a, ExtIn>> { | ||
| todo!() | ||
| } |
There was a problem hiding this comment.
Complete the implementation of event handling methods in EndpointInternal or mark them with TODO comments for future implementation.
| pub fn on_cluster_event<'a, ExtIn>(&mut self, now: Instant, event: ClusterEndpointEvent) -> Option<InternalOutput<'a, ExtIn>> { | ||
| match event { | ||
| ClusterEndpointEvent::PeerJoined(peer) => Some(InternalOutput::Transport(TransportControl::Event(ClientEndpointEvent::PeerJoined(peer)))), | ||
| ClusterEndpointEvent::PeerLeaved(peer) => Some(InternalOutput::Transport(TransportControl::Event(ClientEndpointEvent::PeerJoined(peer)))), | ||
| ClusterEndpointEvent::TrackStarted(peer, track, meta) => Some(InternalOutput::Transport(TransportControl::Event(ClientEndpointEvent::PeerTrackStarted(peer, track, meta)))), | ||
| ClusterEndpointEvent::TrackStoped(peer, track) => Some(InternalOutput::Transport(TransportControl::Event(ClientEndpointEvent::PeerTrackStopped(peer, track)))), | ||
| ClusterEndpointEvent::RemoteTrack(track, event) => self.on_cluster_remote_track(now, track, event), | ||
| ClusterEndpointEvent::LocalTrack(track, event) => self.on_cluster_local_track(now, track, event), | ||
| } | ||
| } | ||
|
|
||
| fn on_cluster_remote_track<'a, ExtIn>(&mut self, now: Instant, id: RemoteTrackId, event: ClusterRemoteTrackEvent) -> Option<InternalOutput<'a, ExtIn>> { | ||
| match event { | ||
| _ => todo!(), | ||
| } | ||
| } | ||
|
|
||
| fn on_cluster_local_track<'a, ExtIn>(&mut self, now: Instant, id: LocalTrackId, event: ClusterLocalTrackEvent) -> Option<InternalOutput<'a, ExtIn>> { | ||
| match event { | ||
| _ => todo!(), | ||
| } | ||
| } |
There was a problem hiding this comment.
Complete the implementation of cluster event handling methods in EndpointInternal or mark them with TODO comments for future implementation.
| None | ||
| } | ||
|
|
||
| //TODO try to get other tracks if avaiable |
There was a problem hiding this comment.
Correct the typo in the comment.
- //TODO try to get other tracks if avaiable
+ //TODO try to get other tracks if availableCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| //TODO try to get other tracks if avaiable | |
| //TODO try to get other tracks if available |
| pub fn on_cluster_event<'a>(&mut self, now: Instant, event: ClusterEndpointEvent) -> Option<InternalOutput> { | ||
| match event { | ||
| ClusterEndpointEvent::TrackStarted(peer, track, meta) => Some(InternalOutput::Event(EndpointEvent::PeerTrackStarted(peer, track, meta))), | ||
| ClusterEndpointEvent::TrackStoped(peer, track) => Some(InternalOutput::Event(EndpointEvent::PeerTrackStopped(peer, track))), |
There was a problem hiding this comment.
Correct the typo in the enum variant name.
- ClusterEndpointEvent::TrackStoped(peer, track) => Some(InternalOutput::Event(EndpointEvent::PeerTrackStopped(peer, track))),
+ ClusterEndpointEvent::TrackStopped(peer, track) => Some(InternalOutput::Event(EndpointEvent::PeerTrackStopped(peer, track))),Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ClusterEndpointEvent::TrackStoped(peer, track) => Some(InternalOutput::Event(EndpointEvent::PeerTrackStopped(peer, track))), | |
| ClusterEndpointEvent::TrackStopped(peer, track) => Some(InternalOutput::Event(EndpointEvent::PeerTrackStopped(peer, track))), |
| } else { | ||
| let (peer, name, _meta) = self.remote_tracks.remove(&track)?; | ||
| log::info!("[ClusterRoom {}] cluster: peer ({}) stopped track {}) => fire event to {:?}", self.room, peer, name, peers); | ||
| Some(Output::Endpoint(peers, ClusterEndpointEvent::TrackStoped(peer, name))) |
There was a problem hiding this comment.
Correct the typo in the enum variant name.
- Some(Output::Endpoint(peers, ClusterEndpointEvent::TrackStoped(peer, name)))
+ Some(Output::Endpoint(peers, ClusterEndpointEvent::TrackStopped(peer, name)))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| Some(Output::Endpoint(peers, ClusterEndpointEvent::TrackStoped(peer, name))) | |
| Some(Output::Endpoint(peers, ClusterEndpointEvent::TrackStopped(peer, name))) |
|
Important Auto Review SkippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 73 files out of 286 files are above the max files limit of 50. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
| #[derive(Debug, PartialEq, Eq, PartialOrd)] | ||
| pub struct F16u(u16); | ||
|
|
||
| impl Into<f32> for F16u { |
Check warning
Code scanning / clippy
an implementation of From is preferred since it gives you Into<_> for free where the reverse isn't true Warning
| #[derive(Debug, PartialEq, Eq, PartialOrd)] | ||
| pub struct F16i(i16); | ||
|
|
||
| impl Into<f32> for F16i { |
Check warning
Code scanning / clippy
an implementation of From is preferred since it gives you Into<_> for free where the reverse isn't true Warning
| Some(value) | ||
| } | ||
|
|
||
| pub fn len(&self) -> usize { |
Check warning
Code scanning / clippy
struct Small2dMap has a public len method, but no is_empty method Warning
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #257 +/- ##
==========================================
- Coverage 50.13% 1.81% -48.33%
==========================================
Files 156 36 -120
Lines 13971 2201 -11770
==========================================
- Hits 7005 40 -6965
+ Misses 6966 2161 -4805 ☔ View full report in Codecov by Sentry. |
This PR switched to use sans-io architecture with sans-io-runtime instead of async-runtime. The main reason is for performance and simplicity in data flow, everything will be sync now, make it easier for predicting time consume
Pull Request
Description
This PR changes the architecture of server with sans-io-runtime.
Related Issue
If this pull request is related to any issue, please mention it here.
Checklist
Screenshots
If applicable, add screenshots to help explain the changes made.
Additional Notes
Add any additional notes or context about the pull request here.
Summary by CodeRabbit
New Features
Refactor
Documentation
Bug Fixes