Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] 允许自定义倍速 #759

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

unvisual
Copy link

No description provided.

@Predidit
Copy link
Owner

这不是个好主意

我还没有见过添加了这个功能的视频软件,如果你想要特定倍速的话,可以单独添加到 constants.dart 中

此外过高的倍速可能会导致播放器出现未预期的行为

@unvisual
Copy link
Author

unvisual commented Feb 23, 2025

这不是个好主意

我还没有见过添加了这个功能的视频软件,如果你想要特定倍速的话,可以单独添加到 constants.dart 中

此外过高的倍速可能会导致播放器出现未预期的行为

其实pilipala就有,我是照着这个代码改的
主要想想弄个1.125倍速这种,更精细的倍速,每次调源码就比较麻烦了

@Predidit
Copy link
Owner

如果我们可以实现 pilipala 类似的单页而不是简单的 dialog 的话,我们可以合并这个PR,我们至少需要像 pilipala 那样提供已有倍速选项的预览和删除自定义选项的功能

除此之外,还有如下的问题

  1. 此实现没有阻止已有的预设倍速再次作为自定义倍速被添加到列表中

  2. 变量命名应使用驼峰命名法,所以我们应该有 playSpeedList 而不是 PlaySpeedList 这样的命名

  3. 我们为什么要修改 constants.dart 中已有常量的命名,还要移除它的 const 声明

  4. Dialog 应该使用 KazumiDialog 相关实现而不是 showDialog,隐藏 Dialog 应该调用 KazumiDialog 的 dismiss 方法

  5. 这个PR缺乏注释

非常抱歉我还没来得及实际测试这个 PR,我们可以先考虑上面的修改,完成之后我再测试后可能给出其他建议

@unvisual
Copy link
Author

如果我们可以实现 pilipala 类似的单页而不是简单的 dialog 的话,我们可以合并这个PR,我们至少需要像 pilipala 那样提供已有倍速选项的预览和删除自定义选项的功能

除此之外,还有如下的问题

  1. 此实现没有阻止已有的预设倍速再次作为自定义倍速被添加到列表中

  2. 变量命名应使用驼峰命名法,所以我们应该有 playSpeedList 而不是 PlaySpeedList 这样的命名

  3. 我们为什么要修改 constants.dart 中已有常量的命名,还要移除它的 const 声明

  4. Dialog 应该使用 KazumiDialog 相关实现而不是 showDialog,隐藏 Dialog 应该调用 KazumiDialog 的 dismiss 方法

  5. 这个PR缺乏注释

非常抱歉我还没来得及实际测试这个 PR,我们可以先考虑上面的修改,完成之后我再测试后可能给出其他建议

抱歉,我这个pr确实不是很成熟,中间很多东西没改好。我电脑上编译一直不通过,暂时没法测试。这里面可能还需要打磨,多谢建议

@unvisual unvisual closed this Feb 23, 2025
@unvisual unvisual reopened this Feb 23, 2025
@unvisual unvisual closed this Feb 23, 2025
@unvisual unvisual reopened this Feb 23, 2025
@unvisual unvisual marked this pull request as draft February 23, 2025 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants