Conversation
Summary of ChangesHello @MoYingJi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在为应用程序的外部媒体集成模块引入播放速率控制功能。通过建立新的 IPC 通信机制、扩展系统媒体事件模型,并为 Linux 和 macOS 提供平台特定的实现,它使得应用程序能够与操作系统的媒体控件无缝交互,从而允许用户通过系统级界面调整播放速率。这显著提升了用户体验,并增强了应用程序与外部媒体环境的兼容性。 Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This PR implements playback rate synchronization with native media controls, covering Linux and macOS platforms, and the code structure is clear. I mainly proposed two improvement suggestions:
- In the Linux implementation, it is recommended to extract the hardcoded playback rate range into constants to improve code maintainability.
- In the macOS implementation, currently only one-way rate updates from the application to the system are implemented. It is recommended to add handling for rate change events from the system UI to achieve complete bidirectional synchronization.
Apart from that, the overall implementation has successfully achieved its intended goals. Good job!
2d92a4b to
a6a80cb
Compare
|
/gemini review |
| .minimum_rate(0.2) | ||
| .maximum_rate(2.0) |
184c975 to
7b4cbfb
Compare
|
/gemini review |
| .minimum_rate(0.2) | ||
| .maximum_rate(2.0) |
| let rates = NSArray::from_retained_slice(&[ | ||
| NSNumber::new_f64(0.25), | ||
| NSNumber::new_f64(0.5), | ||
| NSNumber::new_f64(0.75), | ||
| NSNumber::new_f64(1.0), | ||
| NSNumber::new_f64(1.25), | ||
| NSNumber::new_f64(1.5), | ||
| NSNumber::new_f64(1.75), | ||
| NSNumber::new_f64(2.0), | ||
| ]); |
There was a problem hiding this comment.
建议将硬编码的支持播放速率列表提取为常量,以提高代码的可读性和可维护性。可以定义一个 f64 切片常量,然后在函数中动态构建 NSArray,这样更清晰。
// 可以在文件或模块级别定义
const SUPPORTED_RATES: &[f64] = &[0.25, 0.5, 0.75, 1.0, 1.25, 1.5, 1.75, 2.0];
// 在函数中使用
let rates_vec: Vec<_> = SUPPORTED_RATES
.iter()
.map(|&r| NSNumber::new_f64(r))
.collect();
let rates = NSArray::from_vec(rates_vec);
command.setSupportedPlaybackRates(&rates);NSArray::from_retained_slice 在这里不是最理想的,因为它需要一个 Retained<T> 的切片,导致了多行 NSNumber::new_f64 的调用。使用 from_vec 会更简洁。
|
@ITManCHINA 记得来测测 macOS 端喵 |
|
There was a problem hiding this comment.
Pull request overview
该 PR 为 EMI(External Media Integration)增加“播放速率(playback rate)”的读写能力,使应用可将当前倍速同步到系统媒体控件,并能接收系统媒体控件发起的倍速变更事件(Linux / macOS)。
Changes:
- 在渲染进程与主进程之间新增“播放速率更新”IPC 通道,并调用 EMI 的
updatePlaybackRate。 - Rust 侧扩展
SystemMediaControlstrait 与SystemMediaEvent,新增update_playback_rate与SetRate事件。 - Linux(MPRIS)与 macOS(MPRemoteCommand / NowPlayingInfo)实现倍速同步与倍速事件监听。
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/core/player/PlayerIpc.ts | 新增 sendMediaPlaybackRate,用于向主进程发送倍速更新 IPC。 |
| src/core/player/PlayerController.ts | 在 setRate 中同步更新系统媒体控件倍速。 |
| src/core/player/MediaSessionManager.ts | 初始化时同步初始倍速;监听 SetRate 事件并调用 player.setRate;新增 updatePlaybackRate 方法。 |
| electron/main/ipc/ipc-media.ts | 主进程新增 "media-update-playback-rate" 监听并调用 emi.updatePlaybackRate。 |
| native/external-media-integration/src/lib.rs | 暴露 N-API 方法 update_playback_rate 给 JS 调用。 |
| native/external-media-integration/src/model.rs | SystemMediaEventType 增加 SetRate,并在事件对象中增加 rate 字段。 |
| native/external-media-integration/src/sys_media/mod.rs | SystemMediaControls trait 增加 update_playback_rate。 |
| native/external-media-integration/src/sys_media/linux.rs | 基于 mpris-server 的 set_rate / connect_set_rate 实现倍速同步与事件派发,并设置 min/max rate。 |
| native/external-media-integration/src/sys_media/macos.rs | 增加 MPChangePlaybackRateCommand 处理与 NowPlayingInfo 的 PlaybackRate 更新。 |
| native/external-media-integration/src/sys_media/windows.rs | trait 补齐 update_playback_rate(当前未实现)。 |
| native/external-media-integration/index.d.ts | 更新 TS 声明:事件增加 rate,事件类型增加 SetRate,新增 updatePlaybackRate 导出。 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| audioManager.setRate(rate); | ||
|
|
||
| // 更新系统播放速率 | ||
| mediaSessionManager.updatePlaybackRate(rate); |
There was a problem hiding this comment.
这里会把 rate 直接同步到系统媒体控件;如果 rate 来自原生事件(SetRate)或外部调用,可能出现 NaN/Infinity/越界值,进而把非法值写入 statusStore、音频引擎和 EMI。建议在调用 audioManager.setRate / updatePlaybackRate 前对 rate 做 Number.isFinite 校验并按应用支持范围(例如 0.2~2.0)进行 clamp/round。
| unsafe { | ||
| command.setEnabled(true); | ||
| // 这里可以设置 supportedPlaybackRates,但如果不设置,系统可能会提供默认选项或允许任意值 | ||
| let rates = NSArray::from_retained_slice(&[ |
There was a problem hiding this comment.
应用内倍速下限看起来支持到 0.2x(例如 UI slider 的 min),但这里设置的 supportedPlaybackRates 从 0.25 开始,导致系统侧可选速率与应用内可设置范围不一致。建议把 0.2 加入列表,或在上层/本地实现中统一最小倍速为 0.25。
| let rates = NSArray::from_retained_slice(&[ | |
| let rates = NSArray::from_retained_slice(&[ | |
| NSNumber::new_f64(0.2), |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @description 通过外部媒体集成模块更新媒体控件的播放速率 | ||
| * @note 仅在 Electron 上有效 | ||
| * @param rate - 播放速率 (默认 1.0) | ||
| * @see {@link EmiModule.updatePlaybackRate 外部媒体集成模块的 `updatePlaybackRate` 方法} | ||
| */ | ||
| export const sendMediaPlaybackRate = (rate: number) => { | ||
| if (isElectron) window.electron.ipcRenderer.send("media-update-playback-rate", { rate }); | ||
| }; |
There was a problem hiding this comment.
JSDoc 写了 rate 有“默认 1.0”,但函数签名是必传参数且没有提供默认值。建议移除“默认 1.0”的描述,或把参数改为可选并在函数内部默认到 1.0,避免文档与行为不一致。
| let rate = args.RequestedPlaybackRate()?; | ||
| debug!(rate, "SMTC 请求更改播放速率"); | ||
| dispatch_event(SystemMediaEvent::set_rate(rate)); | ||
| } |
There was a problem hiding this comment.
TypedEventHandler 的闭包返回类型是 windows::core::Result<()>,但当前实现里在 if let Some(args) ... 之后没有返回 Ok(()),导致闭包在某些路径上返回 ()(无法编译)。请在闭包末尾补上 Ok(())(与其它 handler 保持一致)。
| } | |
| } | |
| Ok(()) |
| if (event.rate != null) { | ||
| player.setRate(event.rate); |
There was a problem hiding this comment.
SetRate 事件来自系统媒体控件,event.rate 可能是 NaN/Infinity 或超出播放器支持范围(注释里写的是 0.25-2.0)。建议在这里对 event.rate 做 Number.isFinite 校验并 clamp 到允许区间,再调用 player.setRate(...),避免把异常值写入 store/传给底层音频引擎。
| if (event.rate != null) { | |
| player.setRate(event.rate); | |
| if (event.rate != null && Number.isFinite(event.rate)) { | |
| const clampedRate = Math.min(2.0, Math.max(0.25, event.rate)); | |
| player.setRate(clampedRate); |
现在可以在 EMI 中读写 SPlayer 的播放速率,支持 Linux 和 macOS ## 变更 ### 通用 - 在 `SystemMediaControls` 下新增 `update_playback_rate` 函数 - 会在 `PlayerController` 的 `setRate` 中通过 Ipc 调用 `updatePlaybackRate` 更新播放速率 - 在 `SystemMediaEventType` 中新增 `SetRate` 事件类型,并在 `SystemMediaEvent` 中添加了 `rate` 字段 - 在 `MediaSessionManager` 中监听了 `SetRate` 事件,并调用 `PlayerController` 中的 `setRate` ### Linux 实现 - 通过 `mpris-server` 的 `set_rate` 和 `connect_set_rate` 来设置和监听 MPRIS 的 `Rate` 属性 - 设置了 `MinimumRate` 和 `MaximumRate` ### macOS 实现 AI 写的,我也不知道( ## 备注 此提交为 AI 生成后人工更正,Linux 实现在 KDE 下测试正常工作,macOS 实现也会找人测的
现在可以在 EMI 中读写 SPlayer 的播放速率
变更
通用
SystemMediaControls下新增update_playback_rate函数PlayerController的setRate中通过 Ipc 调用updatePlaybackRate更新播放速率SystemMediaEventType中新增SetRate事件类型,并在SystemMediaEvent中添加了rate字段MediaSessionManager中监听了SetRate事件,并调用PlayerController中的setRateLinux 实现
mpris-server的set_rate和connect_set_rate来设置和监听 MPRIS 的Rate属性MinimumRate和MaximumRatemacOS 实现
AI 写的,我也不知道(
参考:
备注
此提交为 AI 生成后人工更正,Linux 实现在 KDE 下测试正常工作,macOS 实现也会找人测的
已在 此处 执行 GitHub Actions,可以下载测试
未产生实际变更的自动生成代码均未包含在本提交中,(现在自动格式化排除了这些自动生成文件)native/external-media-integration/index.d.ts产生了实际变更,在提交前已格式化以匹配仓库中的文件