feat: channel pub-sub feature and tests. cluster integration test #262
Conversation
WalkthroughWalkthroughThe update enhances the media core package by refining data types, introducing new enums, and restructuring logic for better performance and scalability. Changes include type adjustments for bitrate fields, new identifier generation functions, and improved feedback handling mechanisms. This update aims for more robust and efficient media clustering functionalities. Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (7)
Additional Context UsedLanguageTool (74)
GitHub Check Runs (1)
Additional comments not posted (16)
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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| subscribers.push((owner, track)); | ||
| if subscribers.len() == 1 { | ||
| self.subscribers.insert((owner, track), (channel_id, target_peer, target_track)); | ||
| let channel_container = self.channels.entry(channel_id).or_insert(Default::default()); |
Check warning
Code scanning / clippy
use of or_insert to construct default value Warning
| if let Some(sum_fb) = &mut sum_fb { | ||
| *sum_fb = *sum_fb + *fb; | ||
| } else { | ||
| sum_fb = Some(fb.clone()); |
Check warning
Code scanning / clippy
using clone on type Feedback which implements the Copy trait Warning
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #262 +/- ##
===========================================
+ Coverage 23.60% 36.15% +12.55%
===========================================
Files 36 37 +1
Lines 2779 3059 +280
===========================================
+ Hits 656 1106 +450
+ Misses 2123 1953 -170 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Out of diff range and nitpick comments (1)
packages/media_core/src/cluster/room/channel_sub.rs (1)
26-30: Consider adjusting the comment forKEYFRAME_FEEDBACK_INTERVALto correctly reflect the interval of 1000 ms instead of 100 ms.
| subscribers.push((owner, track)); | ||
| if subscribers.len() == 1 { | ||
| self.subscribers.insert((owner, track), (channel_id, target_peer, target_track)); | ||
| let channel_container = self.channels.entry(channel_id).or_insert(Default::default()); |
There was a problem hiding this comment.
Use of or_insert to construct default value: Consider using or_insert_with to lazily evaluate the default constructor only when needed.
- let channel_container = self.channels.entry(channel_id).or_insert(Default::default());
+ let channel_container = self.channels.entry(channel_id).or_insert_with(Default::default);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.
| let channel_container = self.channels.entry(channel_id).or_insert(Default::default()); | |
| let channel_container = self.channels.entry(channel_id).or_insert_with(Default::default); |
| if let Some(sum_fb) = &mut sum_fb { | ||
| *sum_fb = *sum_fb + *fb; | ||
| } else { | ||
| sum_fb = Some(fb.clone()); |
There was a problem hiding this comment.
Avoid using clone on type Feedback which implements the Copy trait. Use direct assignment instead.
- sum_fb = Some(fb.clone());
+ sum_fb = Some(*fb);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.
| sum_fb = Some(fb.clone()); | |
| sum_fb = Some(*fb); |
| //clean if if timeout | ||
| channel_container | ||
| .bitrate_fbs | ||
| .retain(|_, (ts, _)| now.duration_since(*ts).as_millis() < BITRATE_FEEDBACK_TIMEOUT as u128); | ||
|
|
There was a problem hiding this comment.
Ensure that the feedback cleanup logic correctly handles the timeout. The current implementation might lead to premature removal of feedback entries due to the comparison of milliseconds and seconds.
- .retain(|_, (ts, _)| now.duration_since(*ts).as_millis() < BITRATE_FEEDBACK_TIMEOUT as u128);
+ .retain(|_, (ts, _)| now.duration_since(*ts).as_secs() < BITRATE_FEEDBACK_TIMEOUT as u64 / 1000);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.
| //clean if if timeout | |
| channel_container | |
| .bitrate_fbs | |
| .retain(|_, (ts, _)| now.duration_since(*ts).as_millis() < BITRATE_FEEDBACK_TIMEOUT as u128); | |
| //clean if if timeout | |
| channel_container | |
| .bitrate_fbs | |
| .retain(|_, (ts, _)| now.duration_since(*ts).as_secs() < BITRATE_FEEDBACK_TIMEOUT as u64 / 1000); |
…FF#262) * feat: channel pub-sub feature and tests. cluster integration test * chore: fix typos
Pull Request
Description
This PR implement logic for pubsub channel with feedback: Bitrate Limit and Key Frame request. This also added more tests
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
destroyedflag.