重构媒体控件相关集成#737
Conversation
Summary of ChangesHello @apoint123, 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! This pull request undertakes a significant refactoring effort to streamline how media controls are integrated across different platforms. By merging previously disparate native plugins into a single, cohesive module, the aim is to reduce complexity for frontend development and enhance maintainability. The changes introduce a unified interface for media interactions, abstracting away platform-specific details through conditional compilation, and enriching the data available for media presentation. Highlights
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
|
f6e5622 to
5dca670
Compare
|
/gemini review |
8ef3e54 to
4b4dfe0
Compare
|
/gemini review |
03c92ec to
a7d2f4f
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
这次重构做得非常出色,将多个原生插件统一为一个 external-media-integration 模块,极大地简化了代码结构和跨平台逻辑。通过 SystemMediaControls trait 抽象出平台差异,使得代码更清晰、更易于维护。文档和构建脚本的更新也非常及时和完善。
我在代码中发现了一些可以改进的地方,主要涉及 Linux 平台上的音量控制功能回归,以及 Discord RPC 实现中的一些细节。总体来说,这是一次高质量的重构,恭喜!
I am having trouble creating individual review comments. Click here to see my feedback.
electron/main/ipc/ipc-media.ts (252-261)
在这次重构中,media-update-volume 的 IPC 处理逻辑被移除了。之前的实现似乎允许通过 MPRIS D-Bus 接口控制 Linux 上的音量。
新的 external-media-integration 模块在构建 mpris_server::Player 时没有启用音量控制相关的能力(例如 can_control_volume),这可能会导致 Linux 用户失去通过系统媒体控件调节音量的功能。
如果这是一个无意的功能回归,建议在新的原生模块中重新实现对音量控制的支持。
native/external-media-integration/src/discord.rs (50)
TIMESTAMP_UPDATE_THRESHOLD_MS 设置为 100ms 可能太低了。Discord RPC 对 API 调用有速率限制,过于频繁地更新时间戳(尤其是在正常播放期间,进度更新可能会以几百毫秒的间隔发生)可能会导致被限流。
之前的实现中,这个阈值是 2 秒。建议将此阈值调高,例如 1500 或 2000 毫秒,以避免不必要的 API 调用并减少被限流的风险。
const TIMESTAMP_UPDATE_THRESHOLD_MS: i64 = 1500;
native/external-media-integration/src/discord.rs (377-398)
在 calc_playing_timestamps 函数中,当 current_time >= duration 时会返回 (0, 0)。然而,在 perform_update 函数中,调用方没有处理这个特殊的返回值。这会导致向 Discord 发送一个 start: 0, end: 0 的无效时间戳。
之前的代码实现中,对这种情况有专门的处理逻辑,会跳过时间戳的更新。建议在这里也加上类似的判断,当 calc_playing_timestamps 返回 (0, 0) 时,不为 Activity 设置时间戳。
例如,可以这样修改:
let (start, end) = Self::calc_playing_timestamps(data.current_time, duration);
if start == 0 && end == 0 {
debug!("当前时间超过总时长,跳过时间戳更新");
should_send = last_sent_end_timestamp.is_some();
if should_send {
warn!("没有有效时长,清除时间戳");
}
} else {
// 频繁调用 Discord RPC 接口会导致限流,所以在跳转发生时再更新时间戳
if let Some(last_end) = last_sent_end_timestamp {
let diff = (*last_end - end).abs();
if diff < TIMESTAMP_UPDATE_THRESHOLD_MS {
return true;
}
debug!(
diff_ms = diff,
threshold_ms = TIMESTAMP_UPDATE_THRESHOLD_MS,
"进度变更超过阈值,触发更新"
);
}
activity = activity.timestamps(Timestamps::new().start(start).end(end));
new_end_timestamp = Some(end);
should_send = true;
}这样可以避免发送无效的时间戳,并保持与之前逻辑的一致性。
69db441 to
4c4c9ef
Compare
4c4c9ef to
a0f5755
Compare
目前项目将不同平台的媒体控件拆分为了多个原生插件,导致前端需要进行大量判断什么平台应该加载什么插件、调用什么命令,此 PR 将所有媒体控件集中成为一个原生插件,通过条件编译指令实现不同平台的兼容性,前端无需任何判断。