Skip to content

Feat/import subtitle from stream#221

Merged
mkdir700 merged 16 commits intodevfrom
feat/import-subtitle-from-stream
Oct 17, 2025
Merged

Feat/import subtitle from stream#221
mkdir700 merged 16 commits intodevfrom
feat/import-subtitle-from-stream

Conversation

@mkdir700
Copy link
Owner

@mkdir700 mkdir700 commented Oct 17, 2025

Summary by CodeRabbit

  • 新功能
    • 播放页新增嵌入字幕检测、轨道选择器与导入流程,支持从原始视频提取并导入字幕,含导入状态与 PGS 警告。
    • 增加本地字幕提取与探测服务及对应的清理机制(支持多种格式与超时)。
  • 体验与兼容性
    • 优化 FFmpeg/ffprobe 路径解析与异步文件存在性检查,提升鲁棒性与错误反馈。
  • 国际化
    • 大幅重构并补充多语言字幕相关文案,覆盖列表、选择器与导入流程。
  • 测试
    • 新增临时字幕文件清理相关单元测试及清理场景说明。

- Added new IPC channels for handling subtitle streams: Media_GetSubtitleStreams and Media_ExtractSubtitle
- Implemented SubtitleExtractorService to manage subtitle extraction
- Updated MediaParserService to include methods for retrieving subtitle stream information using ffprobe
- Integrated subtitle stream handling in ipc.ts with appropriate handlers
- Introduced SubtitleTrackSelector component for UI interaction with subtitle streams
- integrate SubtitleLibraryService to cache parsed subtitles
- add current video context to enable subtitle persistence
- log caching success and handle potential errors gracefully
- ensure subtitles are stored with associated video ID
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Caution

Review failed

The pull request is closed.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

新增嵌入式字幕探测与提取:扩展 IPC 枚举与主进程处理器,新增 SubtitleExtractorService 与 MediaParserService(ffprobe/ffmpeg 调用、临时文件管理),FFmpegService 异步检查/ffprobe 路径支持,前端新增字幕类型、选择器组件与多语言文案,退出时清理临时文件。

Changes

内聚组 / 文件(s) 变更摘要
IPC 枚举
packages/shared/IpcChannel.ts
IpcChannel 枚举新增 Media_GetSubtitleStreamsMedia_ExtractSubtitleSubtitleExtractor_CleanupTemp(字幕流相关通道)。
主进程 IPC 注册
src/main/ipc.ts
初始化 SubtitleExtractorService 并注册三个 IPC 处理器:Media_GetSubtitleStreams → mediaParserService.getSubtitleStreams(...);Media_ExtractSubtitle → subtitleExtractorService.extractSubtitle(...);SubtitleExtractor_CleanupTemp → subtitleExtractorService.cleanupTempFiles();改为使用异步 fs.promises.access 检查并记录存在性。
FFmpeg 服务
src/main/services/FFmpegService.ts
新增 getFFprobePath(); 将 fastCheckFFmpegExists 改为 async Promise;改用 PathConverter 和 fs.promises 异步文件检测及错误处理。
媒体解析(ffprobe)
src/main/services/MediaParserService.ts
新增 getSubtitleStreams、probeSubtitleStreams、parseFFprobeSubtitleStreams:通过 ffprobe 获取并解析字幕流(区分 text/image/PGS),含超时与错误处理,使用 PathConverter 与异步存在检查。
字幕提取服务
src/main/services/SubtitleExtractorService.ts
新增 SubtitleExtractorService,导出 extractSubtitle、cleanupTempFile、cleanupTempFiles;使用 ffmpeg 提取轨道至临时文件(30s 超时)、格式推断、临时文件验证与清理。新增 ExtractSubtitleOptions/ExtractSubtitleResult 类型。
退出时清理
src/main/index.ts
will-quit 处理器中动态导入并调用 SubtitleExtractorService.cleanupTempFiles(),记录成功/失败后继续关闭 DB 与日志。
前端类型
src/renderer/src/infrastructure/types/subtitle.ts
新增 SubtitleCodecType、SubtitleStream、SubtitleStreamsResponse(含 textStreams/imageStreams 分组)。
播放器页面集成
src/renderer/src/pages/player/PlayerPage.tsx
保存原始文件路径并新增 subtitleStreams 状态与原始路径引用;在加载后调用 Media_GetSubtitleStreams 检测嵌入式字幕,控制 SubtitleTrackSelector 的展示与忽略逻辑,并将结果传入 SubtitleListPanel。
字幕列表面板
src/renderer/src/pages/player/components/SubtitleListPanel.tsx
Props 增加 hasEmbeddedSubtitles?onOpenEmbeddedSubtitleSelector?;重构空状态为 OptionsGrid,新增嵌入式/外部/AI 导入选项,条件展示嵌入式入口。
字幕轨道选择器
src/renderer/src/pages/player/components/SubtitleTrackSelector.tsx
新增组件 SubtitleTrackSelector,列出 text 与 image/PGS 轨道;对 text 轨道调用 Media_ExtractSubtitle,读取临时文件并应用/持久化,含加载/成功/失败提示与 PGS 警告。
组件导出与微调
src/renderer/src/pages/player/components/index.ts
新增导出 SubtitleTrackSelector;移除 NavbarIcon 中一条 CSS 属性。
i18n 本地化
src/renderer/src/i18n/locales/*
在 en-us、ja-jp、ru-ru、zh-cn、zh-tw 中重组并扩展播放器/字幕相关翻译:新增 subtitleList、subtitleTrackSelector、导入选项、FFmpeg 提示与路径文案,迁移/调整键结构以支持新 UI。
前端钩子
src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts
移除 stableCurrentSubtitle memo,改为直接使用 currentSubtitleData,调整依赖与呈现逻辑。
并行处理工具
src/renderer/src/utils/ParallelVideoProcessor.ts
引入 PathConverter 将路径转换为本地路径,若转换无效则立即返回错误结果,移除旧转换辅助函数,保留日志与存在性检查。
测试
src/main/services/__tests__/SubtitleExtractorService.unit.test.ts
新增 SubtitleExtractorService 单元测试,覆盖临时文件创建/清理、cleanupTempFiles 的匹配/删除以及 cleanupTempFile 行为与日志断言。
集成测试调整
src/main/services/__tests__/FFmpegService.integration.test.ts
更新测试以 await 异步 fastCheckFFmpegExists,并 mock fs.promises.stat 场景(存在/不存在/目录/错误)。
文档
CLAUDE.md
新增关于测试中解除 fs mock 的说明、临时文件管理/清理策略及 SubtitleExtractorService 相关说明。

Sequence Diagram(s)

sequenceDiagram
    participant Renderer as Renderer (PlayerPage / Selector)
    participant IPC as ipcMain
    participant MediaParser as MediaParserService
    participant Extractor as SubtitleExtractorService
    participant FF as ffprobe / ffmpeg
    participant FS as Filesystem

    rect rgb(230,245,255)
    Note over Renderer,IPC: 嵌入式字幕探测
    Renderer->>IPC: invoke(Media_GetSubtitleStreams, originalFilePath)
    IPC->>MediaParser: getSubtitleStreams(originalFilePath)
    activate MediaParser
    MediaParser->>FF: ffprobe -show_streams -print_format json <file>
    FF-->>MediaParser: JSON streams
    MediaParser-->>IPC: SubtitleStreamsResponse (text/image 分组)
    IPC-->>Renderer: SubtitleStreamsResponse
    deactivate MediaParser
    end

    rect rgb(240,255,230)
    Note over Renderer,IPC: 字幕轨道提取
    Renderer->>IPC: invoke(Media_ExtractSubtitle, {videoPath, streamIndex, outputFormat?, subtitleCodec?})
    IPC->>Extractor: extractSubtitle(options)
    activate Extractor
    Extractor->>FF: ffmpeg -i <video> -map 0:<stream> -c:s <codec/convert> <out>
    FF-->>Extractor: exit/status
    Extractor->>FS: verify/read temp file
    Extractor-->>IPC: ExtractSubtitleResult { success, outputPath / error }
    IPC-->>Renderer: ExtractSubtitleResult
    deactivate Extractor
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 我是找字的小兔子,蹦进视频去听戏,
ffprobe 问清每一轨,ffmpeg 小锤敲出字迹,
临时文件藏口袋,退出门前轻轻掸,
选中导入一瞬间,播放器里放欢喜。

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed PR 标题 "Feat/import subtitle from stream" 清晰地表达了本次变更的核心功能——从视频文件的字幕流中导入字幕。根据原始总结,该 PR 确实新增了获取字幕流、提取字幕、清理临时文件等功能,并在前端实现了字幕选择器组件来支持这个导入流程。标题简洁明了,准确反映了主要变更内容,没有使用模糊用语或冗余信息。
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a85b241 and 7075f83.

📒 Files selected for processing (2)
  • src/main/ipc.ts (3 hunks)
  • src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (3 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

chatgpt-codex-connector[bot]

This comment was marked as spam.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 19

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/main/ipc.ts (1)

32-46: SubtitleExtractorService 临时文件需要在应用退出时清理

SubtitleExtractorService 在 generateTempSubtitlePath() 中生成的临时字幕文件(src/main/services/SubtitleExtractorService.ts 第 213-217 行)会存储在系统临时目录中,但目前缺乏应用退出时的集中清理机制。虽然 SubtitleExtractorService 有单文件清理方法 cleanupTempFile(filePath),但不存在类似 FFmpegDownloadService 的 cleanupTempFiles() 集中清理方法,且在应用生命周期中(如 before-quit)没有被调用。

建议

  • 在 SubtitleExtractorService 中添加 cleanupTempFiles() 方法,清理所有可能的临时字幕文件
  • 在应用关闭前(如 app.on('before-quit') 或现有清理流程中)调用该方法,参考 FFmpegService 的销毁逻辑(src/main/services/FFmpegService.ts 第 765-767 行)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx (2)

54-55: 重复调用 usePlayerEngine,可能造成不必要订阅

当前先调用一次 usePlayerEngine(),随后又解构调用一次。建议只调用一次后复用返回对象,减少重复订阅/渲染。

-  usePlayerEngine()
-  ...
-  const { orchestrator } = usePlayerEngine()
+  const engine = usePlayerEngine()
+  const { orchestrator } = engine

请确认该 Hook 的实现不会因多次调用造成重复订阅或副作用。

Also applies to: 61-63


29-40: 命名小问题:Pannel => Panel

接口名 SubtitleListPannelProps 存在拼写错误,建议更正为 SubtitleListPanelProps,避免未来搜索与一致性问题。

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2872648 and 629af64.

📒 Files selected for processing (15)
  • packages/shared/IpcChannel.ts (1 hunks)
  • src/main/ipc.ts (3 hunks)
  • src/main/services/FFmpegService.ts (1 hunks)
  • src/main/services/MediaParserService.ts (2 hunks)
  • src/main/services/SubtitleExtractorService.ts (1 hunks)
  • src/renderer/src/i18n/locales/en-us.json (3 hunks)
  • src/renderer/src/i18n/locales/ja-jp.json (2 hunks)
  • src/renderer/src/i18n/locales/ru-ru.json (2 hunks)
  • src/renderer/src/i18n/locales/zh-cn.json (2 hunks)
  • src/renderer/src/i18n/locales/zh-tw.json (2 hunks)
  • src/renderer/src/infrastructure/types/subtitle.ts (1 hunks)
  • src/renderer/src/pages/player/PlayerPage.tsx (7 hunks)
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx (6 hunks)
  • src/renderer/src/pages/player/components/SubtitleTrackSelector.tsx (1 hunks)
  • src/renderer/src/pages/player/components/index.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: 项目样式应使用 styled-components,v1 的 style 构建机制已废弃,不再引入或编写相关实现
定制 antd 组件样式时优先使用 styled-components(styled(Component) 包装)而非全局 SCSS/classNames,避免样式污染并保持架构一致性
优先使用 CSS 变量而不是硬编码样式值;主题相关属性在 styled-components 中使用 Ant Design CSS 变量(如 var(--ant-color-)、var(--ant-box-shadow-))
避免硬编码尺寸与时长,优先使用 useTheme() 的 token 或集中定义的 JS 样式变量(如 motionDurationMid、borderRadiusSM/MD 等)
在 styled-components 中采用“主题相关用 CSS 变量、设计系统常量用 JS 变量”的混合模式(如颜色/阴影用 CSS var,间距/圆角/字号/动效/ZIndex 用 JS 常量)
优先使用 antd 组件库;若已有可复用的 antd 组件则不要重复自定义实现
Zustand 必须在组件/Hook 顶层通过 selector 使用(useStore(selector)),禁止在 useMemo/useEffect 等内部调用 store Hook,避免 Hooks 顺序问题
避免使用返回对象的 Zustand selector(如 useStore(s => ({ a: s.a, b: s.b }))),应使用单字段选择器或配合 shallow 比较器避免不必要重渲染与更新循环
React 副作用与状态更新规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定(默认 TS + React 18)
Player 相关实现需遵循:Zustand selector 顶层调用,禁止在 useMemo/useEffect 中调用 store Hook;为 useSubtitleEngine 增加 subtitles 入参防御等(已在 SubtitleOverlay、SubtitleListPanel、usePlayerControls、useVideoEvents、VideoSurface、useSubtitleSync 落地)

Files:

  • src/renderer/src/pages/player/components/index.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/infrastructure/types/subtitle.ts
  • packages/shared/IpcChannel.ts
  • src/renderer/src/pages/player/components/SubtitleTrackSelector.tsx
  • src/main/services/SubtitleExtractorService.ts
  • src/main/services/FFmpegService.ts
  • src/main/services/MediaParserService.ts
  • src/renderer/src/pages/player/PlayerPage.tsx
  • src/main/ipc.ts
**/*.{ts,tsx,scss,css}

📄 CodeRabbit inference engine (CLAUDE.md)

布局优先使用 flex,除必要场景外避免将 grid 作为默认方案

Files:

  • src/renderer/src/pages/player/components/index.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/infrastructure/types/subtitle.ts
  • packages/shared/IpcChannel.ts
  • src/renderer/src/pages/player/components/SubtitleTrackSelector.tsx
  • src/main/services/SubtitleExtractorService.ts
  • src/main/services/FFmpegService.ts
  • src/main/services/MediaParserService.ts
  • src/renderer/src/pages/player/PlayerPage.tsx
  • src/main/ipc.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: 统一使用 loggerService 记录日志而不是 console
logger 使用约定:如 logger.error('msg', { error }),第二个参数必须是对象字面量({})承载上下文
任何组件或页面都不得写入 currentTime(播放器控制权由编排器统一管理)

Files:

  • src/renderer/src/pages/player/components/index.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/infrastructure/types/subtitle.ts
  • packages/shared/IpcChannel.ts
  • src/renderer/src/pages/player/components/SubtitleTrackSelector.tsx
  • src/main/services/SubtitleExtractorService.ts
  • src/main/services/FFmpegService.ts
  • src/main/services/MediaParserService.ts
  • src/renderer/src/pages/player/PlayerPage.tsx
  • src/main/ipc.ts
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

所有图标统一使用 lucide-react,不使用 emoji

Files:

  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/pages/player/components/SubtitleTrackSelector.tsx
  • src/renderer/src/pages/player/PlayerPage.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-16T15:20:02.493Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-16T15:20:02.493Z
Learning: Applies to **/*.{ts,tsx} : Player 相关实现需遵循:Zustand selector 顶层调用,禁止在 useMemo/useEffect 中调用 store Hook;为 useSubtitleEngine 增加 subtitles 入参防御等(已在 SubtitleOverlay、SubtitleListPanel、usePlayerControls、useVideoEvents、VideoSurface、useSubtitleSync 落地)

Applied to files:

  • src/renderer/src/pages/player/components/index.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/pages/player/components/SubtitleTrackSelector.tsx
  • src/renderer/src/pages/player/PlayerPage.tsx
🧬 Code graph analysis (6)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx (1)
src/renderer/src/infrastructure/styles/theme.ts (2)
  • SPACING (43-58)
  • BORDER_RADIUS (61-72)
src/renderer/src/pages/player/components/SubtitleTrackSelector.tsx (5)
src/renderer/src/services/Logger.ts (2)
  • loggerService (817-817)
  • error (422-424)
src/renderer/src/infrastructure/types/subtitle.ts (2)
  • SubtitleStreamsResponse (141-146)
  • SubtitleStream (129-138)
src/renderer/src/pages/player/state/player-context.ts (1)
  • useCurrentVideo (6-8)
src/renderer/src/services/subtitles/SubtitleReader.ts (1)
  • SubtitleReader (35-481)
src/renderer/src/infrastructure/styles/theme.ts (4)
  • SPACING (43-58)
  • BORDER_RADIUS (61-72)
  • FONT_SIZES (25-40)
  • FONT_WEIGHTS (11-22)
src/main/services/SubtitleExtractorService.ts (1)
src/renderer/src/services/Logger.ts (2)
  • loggerService (817-817)
  • error (422-424)
src/main/services/FFmpegService.ts (2)
src/main/services/FFmpegDownloadService.ts (1)
  • ffmpegDownloadService (1021-1021)
src/renderer/src/services/Logger.ts (1)
  • error (422-424)
src/main/services/MediaParserService.ts (2)
src/renderer/src/infrastructure/types/subtitle.ts (2)
  • SubtitleStreamsResponse (141-146)
  • SubtitleStream (129-138)
packages/shared/utils/PathConverter.ts (1)
  • PathConverter (17-174)
src/renderer/src/pages/player/PlayerPage.tsx (1)
src/renderer/src/infrastructure/types/subtitle.ts (1)
  • SubtitleStreamsResponse (141-146)
🪛 Biome (2.1.2)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx

[error] 203-203: The elements with this role can be changed to the following elements:

For examples and more information, see WAI-ARIA Roles

(lint/a11y/useSemanticElements)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: test (macos-latest, 20)
  • GitHub Check: test (windows-latest, 20)
  • GitHub Check: test (ubuntu-latest, 20)
🔇 Additional comments (9)
src/renderer/src/pages/player/components/index.ts (1)

12-12: 新增组件导出对齐功能改动,LGTM

与字幕轨选择流程一致,按约定通过 barrel 暴露即可。

src/renderer/src/i18n/locales/zh-tw.json (2)

125-126: 新增快捷键文案 LGTM

toggle_auto_pause 键新增无冲突。


5-68: 字幕文案键一致性已验证,无缺失,代码变更合格

已确认 zh-tw.json 与 zh-cn.json 在 player.subtitleList.optionsplayer.subtitleTrackSelector 两个命名空间下的键集合完全一致,不存在运行时缺失风险。

src/renderer/src/i18n/locales/zh-cn.json (1)

221-278: 字幕列表与轨道选择文案新增 LGTM(与新功能一致)

建议与 zh-tw/en 等语言包保持键集合一致,避免缺词。可复用上条评论脚本校验。

packages/shared/IpcChannel.ts (1)

138-141: 新增字幕相关 IPC 通道命名清晰,位置合理,LGTM

与现有命名风格与分组一致,可直接消费于主进程/渲染进程。

src/renderer/src/i18n/locales/ja-jp.json (1)

8-71: 文案键与组件调用对齐,复数占位与占位符使用正确,LGTM

新增的 player.subtitleList / player.subtitleTrackSelector 键齐全,placeholder 与组件调用匹配。

Also applies to: 166-166

src/renderer/src/pages/player/PlayerPage.tsx (1)

705-710: 渲染时机与参数传递看起来正确,LGTM

  • hasEmbeddedSubtitles 判定与弹窗开关逻辑清晰。
  • 为 Selector 传入原始文件路径与回调,契合提取流程。

请确认其它语言包已同步新增 subtitleTrackSelector 文案键,以避免运行时缺失。

Also applies to: 737-745

src/renderer/src/i18n/locales/ru-ru.json (1)

5-70: 俄语字幕相关文案结构完善,保持与新功能一致

新增 player.subtitleList 与 subtitleTrackSelector 结构清晰、键名与其它语言对齐。看起来可用。

请确认其它语言包(zh-cn/ja-jp/en-us 等)也都包含相同键集合,避免运行时缺失。

src/renderer/src/i18n/locales/en-us.json (1)

90-151: 新增字幕与 FFmpeg 相关文案结构清晰,对应 UI 功能完备

player.subtitleList 与 subtitleTrackSelector 的键值完整,提示与动作一致;FFmpeg path/prompt 文案也与设置页功能对齐。

请确认所有引用这些键的组件已替换硬编码文案(例如 SubtitleListPanel 空态选项)。

Also applies to: 122-151, 246-275

@mkdir700 mkdir700 added this to the 1.2.0 milestone Oct 17, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #222

coderabbitai bot added a commit that referenced this pull request Oct 17, 2025
Docstrings generation was requested by @mkdir700.

* #221 (comment)

The following files were modified:

* `src/main/ipc.ts`
* `src/renderer/src/pages/player/PlayerPage.tsx`
* `src/renderer/src/pages/player/components/SubtitleListPanel.tsx`
Docstrings generation was requested by @mkdir700.

* #221 (comment)

The following files were modified:

* `src/main/ipc.ts`
* `src/renderer/src/pages/player/PlayerPage.tsx`
* `src/renderer/src/pages/player/components/SubtitleListPanel.tsx`

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (6)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx (3)

203-206: A11y:使用语义元素

代替 role="complementary" 的 div

改为 styled.aside,并移除 role(aria-label 可保留)。
[ suggest_recommended_refactor ]

-  return (
-    <Container role="complementary" aria-label="caption-list">
+  return (
+    <Container aria-label="caption-list">
@@
-const Container = styled.div`
+const Container = styled.aside`

Biome a11y 规则提示相同问题。As per coding guidelines

Also applies to: 458-467


568-574: 避免魔法数:将 max-width: 480px 抽为样式常量

提取命名常量便于主题化与复用。
[ suggest_recommended_refactor ]

-  max-width: 480px;
+  max-width: ${PANEL_MAX_WIDTH}px;

在文件顶部添加:

const PANEL_MAX_WIDTH = 480

As per coding guidelines


213-253: 空状态中文硬编码需改为 i18n 资源;按钮文案同步本地化

请使用已有的语言包键值,避免硬编码文本。

-                <OptionContent>
-                  <OptionTitle>使用内嵌字幕</OptionTitle>
-                  <OptionDescription>视频文件包含字幕轨道,可直接导入</OptionDescription>
-                </OptionContent>
-                <Button type="primary" onClick={onOpenEmbeddedSubtitleSelector}>
-                  选择
-                </Button>
+                <OptionContent>
+                  <OptionTitle>{t('player.subtitleList.empty.options.embedded.title')}</OptionTitle>
+                  <OptionDescription>{t('player.subtitleList.empty.options.embedded.description')}</OptionDescription>
+                </OptionContent>
+                <Button type="primary" onClick={onOpenEmbeddedSubtitleSelector}>
+                  {t('player.subtitleList.empty.options.embedded.action')}
+                </Button>
@@
-              <OptionContent>
-                <OptionTitle>导入外挂字幕</OptionTitle>
-                <OptionDescription>从本地文件导入 SRT、VTT 等格式字幕</OptionDescription>
-              </OptionContent>
+              <OptionContent>
+                <OptionTitle>{t('player.subtitleList.empty.options.external.title')}</OptionTitle>
+                <OptionDescription>{t('player.subtitleList.empty.options.external.description')}</OptionDescription>
+              </OptionContent>
@@
-              <OptionContent>
-                <OptionTitle>AI 生成字幕</OptionTitle>
-                <OptionDescription>基于语音识别生成单词级字幕</OptionDescription>
-              </OptionContent>
-              <Button disabled>即将推出</Button>
+              <OptionContent>
+                <OptionTitle>{t('player.subtitleList.empty.options.ai.title')}</OptionTitle>
+                <OptionDescription>{t('player.subtitleList.empty.options.ai.description')}</OptionDescription>
+              </OptionContent>
+              <Button disabled>{t('player.subtitleList.empty.options.ai.action')}</Button>
src/main/ipc.ts (2)

685-689: Media_GetSubtitleStreams 缺少入参校验与错误兜底;建议最小化日志并统一返回空值

避免异常穿透渲染进程;校验空/无效路径与文件是否存在,失败时返回 null 并按规范记录日志上下文。

-  ipcMain.handle(IpcChannel.Media_GetSubtitleStreams, async (_, inputPath: string) => {
-    return await mediaParserService.getSubtitleStreams(inputPath)
-  })
+  ipcMain.handle(IpcChannel.Media_GetSubtitleStreams, async (_, inputPath: string) => {
+    if (typeof inputPath !== 'string' || inputPath.trim() === '') {
+      logger.warn('获取字幕流: 无效路径', { inputPath })
+      return null
+    }
+    try {
+      if (!fs.existsSync(inputPath)) {
+        logger.warn('获取字幕流: 文件不存在', { inputPath })
+        return null
+      }
+      const result = await mediaParserService.getSubtitleStreams(inputPath)
+      logger.info('获取字幕流成功', { inputPath, total: result?.streams?.length ?? 0 })
+      return result
+    } catch (error) {
+      logger.error('获取字幕流失败', { inputPath, error })
+      return null
+    }
+  })

按日志规范,第二个参数使用对象承载上下文。As per coding guidelines


690-708: Media_ExtractSubtitle 需强类型与参数校验;补充最小化日志

限制 outputFormat 联合类型,校验 streamIndex 与 videoPath 并记录上下文日志,避免脆弱调用。

+  type ExtractSubtitleOptions = {
+    videoPath: string
+    streamIndex: number
+    outputFormat?: 'srt' | 'ass' | 'vtt'
+    subtitleCodec?: string
+  }
   ipcMain.handle(
     IpcChannel.Media_ExtractSubtitle,
-    async (
-      _,
-      options: {
-        videoPath: string
-        streamIndex: number
-        outputFormat?: string
-        subtitleCodec?: string
-      }
-    ) => {
-      return await subtitleExtractorService.extractSubtitle({
-        videoPath: options.videoPath,
-        streamIndex: options.streamIndex,
-        outputFormat: (options.outputFormat as 'srt' | 'ass' | 'vtt') || 'srt',
-        subtitleCodec: options.subtitleCodec
-      })
-    }
+    async (_, options: ExtractSubtitleOptions) => {
+      if (!options?.videoPath || typeof options.videoPath !== 'string') {
+        throw new Error('videoPath 不能为空')
+      }
+      if (typeof options.streamIndex !== 'number' || options.streamIndex < 0) {
+        throw new Error('streamIndex 非法')
+      }
+      const outputFormat: 'srt' | 'ass' | 'vtt' = options.outputFormat ?? 'srt'
+      logger.info('提取内嵌字幕', {
+        videoPath: options.videoPath,
+        streamIndex: options.streamIndex,
+        outputFormat,
+        subtitleCodec: options.subtitleCodec
+      })
+      return await subtitleExtractorService.extractSubtitle({
+        videoPath: options.videoPath,
+        streamIndex: options.streamIndex,
+        outputFormat,
+        subtitleCodec: options.subtitleCodec
+      })
+    }
   )

按日志规范,第二个参数使用对象承载上下文。As per coding guidelines

src/renderer/src/pages/player/PlayerPage.tsx (1)

519-559: 字幕探测 effect:去除无关依赖,增加“过期结果”防护,并在无结果时清空缓存值

  • showMediaServerPrompt 会导致无谓重跑;移除。
  • 异步期间视频切换会写入过期结果;增加 cancelled/路径一致性校验。
  • 未命中时未清空 subtitleStreams,导致后续视频仍显示旧轨道。
  • “无字幕”日志缺少上下文对象。
   // 检测字幕轨道
   useEffect(() => {
-    if (!videoData || !originalFilePathRef.current || userDismissedEmbeddedSubtitles) {
+    if (!videoData || !originalFilePathRef.current || userDismissedEmbeddedSubtitles) {
       return
     }
 
-    const detectSubtitleStreams = async () => {
+    let cancelled = false
+    const detectionPath = originalFilePathRef.current
+    const detectSubtitleStreams = async () => {
       try {
-        // 使用原始文件路径检测字幕,而不是 HLS 播放源
-        const detectionPath = originalFilePathRef.current
         logger.info('🔍 开始检测字幕轨道', {
           detectionPath,
           playSource: videoData.src
         })
 
         const result = await window.electron.ipcRenderer.invoke(
           IpcChannel.Media_GetSubtitleStreams,
           detectionPath
         )
 
-        if (result && result.streams && result.streams.length > 0) {
+        // 过期结果保护:视频或路径已变化
+        if (cancelled || detectionPath !== originalFilePathRef.current) {
+          logger.debug('跳过过期的字幕探测结果', {
+            detectionPath,
+            currentPath: originalFilePathRef.current
+          })
+          return
+        }
+
+        if (result && result.streams && result.streams.length > 0) {
           logger.info('✅ 检测到字幕轨道', {
             total: result.streams.length,
             text: result.textStreams?.length || 0,
             image: result.imageStreams?.length || 0
           })
 
           setSubtitleStreams(result)
         } else {
-          logger.info('📄 此视频文件不含字幕轨道')
+          logger.info('📄 此视频文件不含字幕轨道', { detectionPath })
+          setSubtitleStreams(null)
         }
       } catch (error) {
         logger.warn('检测字幕轨道失败', {
-          error: error instanceof Error ? error.message : String(error)
+          error: error instanceof Error ? error.message : String(error),
+          detectionPath
         })
       }
     }
 
     detectSubtitleStreams()
-  }, [videoData, userDismissedEmbeddedSubtitles, showMediaServerPrompt])
+    return () => {
+      cancelled = true
+    }
+  }, [videoData?.id, userDismissedEmbeddedSubtitles])

此外,建议在视频切换时重置相关状态以避免“全局消音”:

  • 在组件内新增(可放置于 videoId 依赖的 useEffect 附近):
useEffect(() => {
  setUserDismissedEmbeddedSubtitles(false)
  setSubtitleStreams(null)
}, [videoId])

按日志规范,第二个参数使用对象承载上下文。As per coding guidelines

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 629af64 and 0da82d9.

📒 Files selected for processing (3)
  • src/main/ipc.ts (4 hunks)
  • src/renderer/src/pages/player/PlayerPage.tsx (9 hunks)
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx (6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: 项目样式应使用 styled-components,v1 的 style 构建机制已废弃,不再引入或编写相关实现
定制 antd 组件样式时优先使用 styled-components(styled(Component) 包装)而非全局 SCSS/classNames,避免样式污染并保持架构一致性
优先使用 CSS 变量而不是硬编码样式值;主题相关属性在 styled-components 中使用 Ant Design CSS 变量(如 var(--ant-color-)、var(--ant-box-shadow-))
避免硬编码尺寸与时长,优先使用 useTheme() 的 token 或集中定义的 JS 样式变量(如 motionDurationMid、borderRadiusSM/MD 等)
在 styled-components 中采用“主题相关用 CSS 变量、设计系统常量用 JS 变量”的混合模式(如颜色/阴影用 CSS var,间距/圆角/字号/动效/ZIndex 用 JS 常量)
优先使用 antd 组件库;若已有可复用的 antd 组件则不要重复自定义实现
Zustand 必须在组件/Hook 顶层通过 selector 使用(useStore(selector)),禁止在 useMemo/useEffect 等内部调用 store Hook,避免 Hooks 顺序问题
避免使用返回对象的 Zustand selector(如 useStore(s => ({ a: s.a, b: s.b }))),应使用单字段选择器或配合 shallow 比较器避免不必要重渲染与更新循环
React 副作用与状态更新规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定(默认 TS + React 18)
Player 相关实现需遵循:Zustand selector 顶层调用,禁止在 useMemo/useEffect 中调用 store Hook;为 useSubtitleEngine 增加 subtitles 入参防御等(已在 SubtitleOverlay、SubtitleListPanel、usePlayerControls、useVideoEvents、VideoSurface、useSubtitleSync 落地)

Files:

  • src/main/ipc.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/pages/player/PlayerPage.tsx
**/*.{ts,tsx,scss,css}

📄 CodeRabbit inference engine (CLAUDE.md)

布局优先使用 flex,除必要场景外避免将 grid 作为默认方案

Files:

  • src/main/ipc.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/pages/player/PlayerPage.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: 统一使用 loggerService 记录日志而不是 console
logger 使用约定:如 logger.error('msg', { error }),第二个参数必须是对象字面量({})承载上下文
任何组件或页面都不得写入 currentTime(播放器控制权由编排器统一管理)

Files:

  • src/main/ipc.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/pages/player/PlayerPage.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

所有图标统一使用 lucide-react,不使用 emoji

Files:

  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/pages/player/PlayerPage.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-16T15:20:02.493Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-16T15:20:02.493Z
Learning: Applies to **/*.{ts,tsx} : Player 相关实现需遵循:Zustand selector 顶层调用,禁止在 useMemo/useEffect 中调用 store Hook;为 useSubtitleEngine 增加 subtitles 入参防御等(已在 SubtitleOverlay、SubtitleListPanel、usePlayerControls、useVideoEvents、VideoSurface、useSubtitleSync 落地)

Applied to files:

  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/pages/player/PlayerPage.tsx
📚 Learning: 2025-10-16T15:20:02.493Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-16T15:20:02.493Z
Learning: Applies to **/*.{ts,tsx} : 避免硬编码尺寸与时长,优先使用 useTheme() 的 token 或集中定义的 JS 样式变量(如 motionDurationMid、borderRadiusSM/MD 等)

Applied to files:

  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
📚 Learning: 2025-10-16T15:20:02.493Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-16T15:20:02.493Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : logger 使用约定:如 logger.error('msg', { error }),第二个参数必须是对象字面量({})承载上下文

Applied to files:

  • src/renderer/src/pages/player/PlayerPage.tsx
🧬 Code graph analysis (2)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx (2)
src/renderer/src/infrastructure/types/subtitle.ts (1)
  • SubtitleItem (10-16)
src/renderer/src/infrastructure/styles/theme.ts (2)
  • SPACING (43-58)
  • BORDER_RADIUS (61-72)
src/renderer/src/pages/player/PlayerPage.tsx (1)
src/renderer/src/infrastructure/types/subtitle.ts (1)
  • SubtitleStreamsResponse (141-146)
🪛 Biome (2.1.2)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx

[error] 215-215: The elements with this role can be changed to the following elements:

For examples and more information, see WAI-ARIA Roles

(lint/a11y/useSemanticElements)

🪛 GitHub Check: test (ubuntu-latest, 20)
src/main/ipc.ts

[failure] 1134-1134:
Insert

src/renderer/src/pages/player/components/SubtitleListPanel.tsx

[failure] 838-838:
Insert

src/renderer/src/pages/player/PlayerPage.tsx

[failure] 971-971:
Insert

🔇 Additional comments (1)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx (1)

576-596: 硬编码动效时长与缓动建议改为主题常量

使用 ANIMATION_DURATION/EASING,保持全局一致性。
[ suggest_recommended_refactor ]

-  transition: all 0.2s ease;
+  transition: all ${ANIMATION_DURATION.FAST} ${EASING.STANDARD};

并在顶部引入:

-import { BORDER_RADIUS, SPACING } from '@renderer/infrastructure/styles/theme'
+import { BORDER_RADIUS, SPACING, ANIMATION_DURATION, EASING } from '@renderer/infrastructure/styles/theme'

As per coding guidelines

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
src/main/services/SubtitleExtractorService.ts (2)

113-172: 关键缺陷:提取策略不支持 codec 转换,会在多种场景失败

当前实现无条件使用 -c copy(Line 121),这在以下场景会失败:

  • mov_text → SRT:MP4 常用 mov_text codec,但 SRT 容器不兼容,必须转码为 srt codec
  • webvtt → VTT:需明确指定 -c:s webvtt 而非 copy
  • 图像型字幕(PGS/DVB/XSub):应在逻辑层直接拒绝,而非尝试提取

此外,缺少 FFmpeg 稳定性标志:

  • -y:覆盖已存在文件,避免交互提示阻塞
  • -loglevel error:减少冗余日志

建议修改(与历史评审一致):

   private async runFFmpegExtract(
     videoPath: string,
     streamIndex: number,
     outputPath: string,
     subtitleCodec?: string
   ): Promise<boolean> {
     return new Promise((resolve) => {
       const ffmpegPath = this.ffmpegService.getFFmpegPath()
-      const args = ['-i', videoPath, '-map', `0:${streamIndex}`, '-c', 'copy', outputPath]
+      
+      // 根据输出格式选择合适的编码器
+      const ext = path.extname(outputPath).replace('.', '').toLowerCase()
+      const encoderMap: Record<string, string> = {
+        srt: 'srt',
+        vtt: 'webvtt',
+        ass: 'ass'
+      }
+      const encoder = encoderMap[ext] ?? 'copy'
+      
+      const args = [
+        '-y',
+        '-loglevel', 'error',
+        '-i', videoPath,
+        '-map', `0:${streamIndex}`,
+        '-c:s', encoder,
+        outputPath
+      ]
       const fullCommand = `${ffmpegPath} ${args.map((arg) => (arg.includes(' ') ? `"${arg}"` : arg)).join(' ')}`

177-207: 图像型字幕应在逻辑层明确拒绝,而非映射到输出格式

Lines 186-188 将 hdmv_pgs_subtitledvb_subtitlexsub 等图像型字幕映射到 sup/sub 格式,但:

  1. 这些格式当前不支持导入(与 UI/i18n 提示一致)
  2. 即使提取成功,后续也无法正常使用,会误导用户

建议在 extractSubtitle 方法开始处(Line 53 之前)增加检测逻辑:

       options.outputFormat
     )
+    
+    // 图像型字幕当前不支持导入
+    const imageBasedCodecs = ['hdmv_pgs_subtitle', 'dvb_subtitle', 'xsub']
+    if (options.subtitleCodec && imageBasedCodecs.includes(options.subtitleCodec)) {
+      logger.warn('⚠️ 图像型字幕暂不支持导入', { 
+        subtitleCodec: options.subtitleCodec 
+      })
+      return {
+        success: false,
+        error: '图像型字幕(PGS/DVB/XSub)暂不支持导入'
+      }
+    }

     // 生成临时输出文件路径
     const outputPath = this.generateTempSubtitlePath(outputFormat)

同时,考虑从 getOutputFormatFromCodec 的映射表中移除 sup/sub 相关映射,避免误导:

     const codecToFormat: Record<string, string> = {
       subrip: 'srt',
       ass: 'ass',
       ssa: 'ass',
       webvtt: 'vtt',
       vtt: 'vtt',
-      mov_text: 'srt',
-      hdmv_pgs_subtitle: 'sup',
-      dvb_subtitle: 'sub',
-      xsub: 'sub'
+      mov_text: 'srt'
     }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0da82d9 and 09295a5.

📒 Files selected for processing (1)
  • src/main/services/SubtitleExtractorService.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: 项目样式应使用 styled-components,v1 的 style 构建机制已废弃,不再引入或编写相关实现
定制 antd 组件样式时优先使用 styled-components(styled(Component) 包装)而非全局 SCSS/classNames,避免样式污染并保持架构一致性
优先使用 CSS 变量而不是硬编码样式值;主题相关属性在 styled-components 中使用 Ant Design CSS 变量(如 var(--ant-color-)、var(--ant-box-shadow-))
避免硬编码尺寸与时长,优先使用 useTheme() 的 token 或集中定义的 JS 样式变量(如 motionDurationMid、borderRadiusSM/MD 等)
在 styled-components 中采用“主题相关用 CSS 变量、设计系统常量用 JS 变量”的混合模式(如颜色/阴影用 CSS var,间距/圆角/字号/动效/ZIndex 用 JS 常量)
优先使用 antd 组件库;若已有可复用的 antd 组件则不要重复自定义实现
Zustand 必须在组件/Hook 顶层通过 selector 使用(useStore(selector)),禁止在 useMemo/useEffect 等内部调用 store Hook,避免 Hooks 顺序问题
避免使用返回对象的 Zustand selector(如 useStore(s => ({ a: s.a, b: s.b }))),应使用单字段选择器或配合 shallow 比较器避免不必要重渲染与更新循环
React 副作用与状态更新规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定(默认 TS + React 18)
Player 相关实现需遵循:Zustand selector 顶层调用,禁止在 useMemo/useEffect 中调用 store Hook;为 useSubtitleEngine 增加 subtitles 入参防御等(已在 SubtitleOverlay、SubtitleListPanel、usePlayerControls、useVideoEvents、VideoSurface、useSubtitleSync 落地)

Files:

  • src/main/services/SubtitleExtractorService.ts
**/*.{ts,tsx,scss,css}

📄 CodeRabbit inference engine (CLAUDE.md)

布局优先使用 flex,除必要场景外避免将 grid 作为默认方案

Files:

  • src/main/services/SubtitleExtractorService.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: 统一使用 loggerService 记录日志而不是 console
logger 使用约定:如 logger.error('msg', { error }),第二个参数必须是对象字面量({})承载上下文
任何组件或页面都不得写入 currentTime(播放器控制权由编排器统一管理)

Files:

  • src/main/services/SubtitleExtractorService.ts
🧬 Code graph analysis (1)
src/main/services/SubtitleExtractorService.ts (1)
src/renderer/src/services/Logger.ts (2)
  • loggerService (817-817)
  • error (422-424)
🔇 Additional comments (4)
src/main/services/SubtitleExtractorService.ts (4)

1-10: 导入和日志初始化正确

符合编码规范,使用 loggerService 而非 console,日志上下文设置合理。


11-22: 接口定义清晰合理

类型定义明确,API 设计符合预期用途。


24-29: 构造函数实现合理

简洁清晰,依赖注入方式恰当。


223-239: 异步清理实现正确

已根据历史评审改用 fs.promises.unlink,非阻塞实现,ENOENT 错误处理恰当。

…itical paths

- Replace fs.existsSync with fs.promises.access in IPC handler
- Optimize MediaParserService by merging existsSync + statSync into single stat call
- Convert FFmpegService.fastCheckFFmpegExists to async method
- Replace all blocking file checks in async methods across services

This eliminates event loop blocking in hot paths and improves I/O performance.
Add path and videoId to logger.info when no subtitle streams detected,
improving log searchability and debugging capabilities.
- Remove duplicate convertFileUrlToLocalPath from MediaParserService
- Remove duplicate convertFileUrlToLocalPath from FFmpegService
- Remove convertFileUrlToLocalPathAsync from ParallelVideoProcessor
- Replace all calls with PathConverter.convertToLocalPath for consistency
- Add proper error handling for path conversion failures
- Remove ~100+ lines of duplicate code
…ettle

- Use this.ffmpegService.getFFprobePath() instead of inline new FFmpegService()
- Add -select_streams 's' to only return subtitle streams for better performance
- Implement settled flag to prevent double settle race conditions
- Change timeout handler to resolve(null) instead of reject for consistency
- Add settled checks in all handlers (timeout/close/error) before resolving/rejecting
- Ensure ffprobe process is killed on timeout with proper logging context
…e empty state

- Replace hardcoded Chinese text in SubtitleListPanel empty state options
- Update embedded subtitle option: title, description, and action button
- Update external subtitle option: title and description
- Update AI-generated subtitle option: title, description, and action button
- All i18n keys already exist in language packs (zh-cn, zh-tw, en-us, ja-jp, ru-ru)
…nctionality

- Add new IPC channel `subtitle-extractor:cleanup-temp` for cleaning up temporary subtitle files.
- Implement `cleanupTempFiles` method in `SubtitleExtractorService` to remove subtitle files matching the pattern from the system's temporary directory.
- Enhance logging for cleanup operations, including success and error messages.
- Add unit tests for `cleanupTempFiles` to ensure correct functionality and logging behavior.
- Add important notes on mocking `node:fs` and `node:fs/promises` in the global test setup.
- Include examples for unmocking these modules in test files for real filesystem operations.
- Introduce resource management and cleanup section detailing temporary file cleanup practices.
- Outline best practices for implementing `cleanupTempFiles()` and IPC channel usage for manual cleanup triggers.
- Specify naming conventions for temporary files to facilitate pattern matching and cleanup.
cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (1)

63-75: 移除冗余 useMemo:直接复用 currentSubtitleData 即可

stableCurrentSubtitle 只是复制了一次 currentSubtitleData,对渲染稳定性无增益,反而多了一层计算与对象分配。建议直接引用,简化依赖链并减少重渲染成本。

-  // === 缓存当前字幕数据以防止不必要的重新渲染 ===
-  const stableCurrentSubtitle = useMemo(() => {
-    if (!currentSubtitleData) return null
-
-    // 只有当内容实际变化时才返回新对象
-    return {
-      originalText: currentSubtitleData.originalText,
-      translatedText: currentSubtitleData.translatedText,
-      startTime: currentSubtitleData.startTime,
-      endTime: currentSubtitleData.endTime,
-      index: currentSubtitleData.index
-    }
-  }, [currentSubtitleData])
+  // 直接复用 currentSubtitleData,避免不必要的复制
+  const stableCurrentSubtitle = currentSubtitleData
src/renderer/src/pages/player/components/SubtitleListPanel.tsx (1)

66-76: 避免重复调用同一 Hook:usePlayerEngine() 仅调用一次

第 66 行对 usePlayerEngine() 的调用没有使用返回值;第 73 行再次调用获取 orchestrator。保留一次调用即可,减少副作用与订阅成本。

-  usePlayerEngine()
+  // 保留一次调用并解构需要的成员
@@
-  const { orchestrator } = usePlayerEngine()
+  const { orchestrator } = usePlayerEngine()

(直接删除第 66 行的空调用)

src/main/services/FFmpegService.ts (1)

185-228: fastCheckFFmpegExists 对 PATH 可执行文件易产生“假阴性”

stat('ffmpeg') 在仅 PATH 可达、无绝对路径场景会失败,易误判为不可用。建议:当 ffmpegPath 不含路径分隔符时,采用短超时 spawn('-version') 或调用平台 which/where 进行解析,结果仅用于“快速检查”缓存标记。

是否需要我补一个基于 spawn('-version') 的快速探测实现并加 1–2 个单元测试?

Also applies to: 230-282

♻️ Duplicate comments (12)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx (2)

205-206: A11y:使用语义元素

代替 role="complementary" 的 div

将容器从 styled.div 改为 styled.aside,并移除渲染处的 role="complementary"。此前已提示过,本次沿用相同建议。

-  <Container role="complementary" aria-label="caption-list">
+  <Container aria-label="caption-list">
@@
-    <Container role="complementary" aria-label="caption-list">
+    <Container aria-label="caption-list">
-const Container = styled.div`
+const Container = styled.aside`

Also applies to: 282-283, 464-474


574-581: 避免魔法数:将 max-width: 480px 抽为样式常量

延续既往建议,抽出命名常量,便于复用与主题适配。

 const OptionsGrid = styled.div`
   display: flex;
   flex-direction: column;
   gap: ${SPACING.MD}px;
   width: 100%;
-  max-width: 480px;
+  max-width: ${PANEL_MAX_WIDTH}px;
 `

在文件顶部合适位置新增:

const PANEL_MAX_WIDTH = 480
src/renderer/src/pages/player/PlayerPage.tsx (2)

971-974: 文件末尾缺少换行符(保持一致性)

为通过静态检查并保持统一格式,请在文件末尾补一个空行。

-`
+`
+

519-561: 字幕探测 effect 存在无关依赖与结果竞态,且未处理空结果清理

  • 依赖 showMediaServerPrompt 会导致弹窗显隐时重复探测;
  • 未做“过期结果”保护(视频切换或路径变化时可能覆盖新状态);
  • 当无字幕时未清空 subtitleStreams,遗留上一个视频的轨道信息。

应用以下补丁移除无关依赖、加入取消与路径校验,并在无结果时清空状态:

-  useEffect(() => {
+  useEffect(() => {
     if (!videoData || !originalFilePathRef.current || userDismissedEmbeddedSubtitles) {
       return
     }
 
     const detectSubtitleStreams = async () => {
+      let cancelled = false
       try {
         // 使用原始文件路径检测字幕,而不是 HLS 播放源
-        const detectionPath = originalFilePathRef.current
+        const detectionPath = originalFilePathRef.current
         logger.info('🔍 开始检测字幕轨道', {
           detectionPath,
           playSource: videoData.src
         })
 
         const result = await window.electron.ipcRenderer.invoke(
           IpcChannel.Media_GetSubtitleStreams,
           detectionPath
         )
 
-        if (result && result.streams && result.streams.length > 0) {
+        // 结果过期保护
+        if (cancelled || detectionPath !== originalFilePathRef.current) {
+          logger.debug('跳过过期的字幕探测结果', { detectionPath, currentPath: originalFilePathRef.current })
+          return
+        }
+
+        if (result && result.streams && result.streams.length > 0) {
           logger.info('✅ 检测到字幕轨道', {
             total: result.streams.length,
             text: result.textStreams?.length || 0,
             image: result.imageStreams?.length || 0
           })
 
           setSubtitleStreams(result)
         } else {
           logger.info('📄 此视频文件不含字幕轨道', {
             path: detectionPath,
             videoId
           })
+          setSubtitleStreams(null)
         }
       } catch (error) {
         logger.warn('检测字幕轨道失败', {
           error: error instanceof Error ? error.message : String(error)
         })
       }
     }
 
-    detectSubtitleStreams()
-  }, [videoData, userDismissedEmbeddedSubtitles, showMediaServerPrompt, videoId])
+    detectSubtitleStreams()
+    return () => {
+      // 取消本轮探测
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      let _ = (cancelled = true)
+    }
+  }, [videoData?.id, userDismissedEmbeddedSubtitles])

As per coding guidelines

src/main/ipc.ts (2)

685-689: Media_GetSubtitleStreams 缺少入参校验与最小化日志

此前已建议校验 inputPath 并输出结构化日志;当前实现仍直接透传,异常将冒泡到渲染进程。

建议按下列方式加防御并与全局约定保持一致(失败返回 null,记录 info/warn):

   ipcMain.handle(IpcChannel.Media_GetSubtitleStreams, async (_, inputPath: string) => {
-    return await mediaParserService.getSubtitleStreams(inputPath)
+    if (!inputPath || typeof inputPath !== 'string') {
+      logger.warn('获取字幕流入参非法', { inputPathType: typeof inputPath })
+      return null
+    }
+    logger.info('获取字幕流', { inputPath })
+    return await mediaParserService.getSubtitleStreams(inputPath)
   })

690-708: Media_ExtractSubtitle 需强类型与参数校验,补充日志上下文

options 为宽松结构且未校验必填项;streamIndex 边界与 outputFormat 白名单也未限制。

建议收紧类型并校验+日志:

+  type ExtractSubtitleOptions = {
+    videoPath: string
+    streamIndex: number
+    outputFormat?: 'srt' | 'ass' | 'vtt'
+    subtitleCodec?: string
+  }
   ipcMain.handle(
     IpcChannel.Media_ExtractSubtitle,
-    async (
-      _,
-      options: {
-        videoPath: string
-        streamIndex: number
-        outputFormat?: string
-        subtitleCodec?: string
-      }
-    ) => {
-      return await subtitleExtractorService.extractSubtitle({
+    async (_, options: ExtractSubtitleOptions) => {
+      if (!options?.videoPath) {
+        logger.warn('提取字幕入参缺少 videoPath')
+        return null
+      }
+      if (typeof options.streamIndex !== 'number' || options.streamIndex < 0) {
+        logger.warn('提取字幕入参 streamIndex 非法', { streamIndex: options.streamIndex })
+        return null
+      }
+      const outputFormat: 'srt' | 'ass' | 'vtt' = options.outputFormat ?? 'srt'
+      logger.info('提取内嵌字幕', {
+        videoPath: options.videoPath,
+        streamIndex: options.streamIndex,
+        outputFormat,
+        subtitleCodec: options.subtitleCodec
+      })
+      return await subtitleExtractorService.extractSubtitle({
         videoPath: options.videoPath,
         streamIndex: options.streamIndex,
-        outputFormat: (options.outputFormat as 'srt' | 'ass' | 'vtt') || 'srt',
+        outputFormat,
         subtitleCodec: options.subtitleCodec
       })
     }
   )
src/main/services/FFmpegService.ts (1)

172-183: FFprobe 路径获取应先做存在性判断并统一降级策略

避免以异常做控制流;建议先检查下载版是否存在,存在则返回;否则记录一次 warn 后统一降级到 'ffprobe'。

   public getFFprobePath(): string {
-    try {
-      return ffmpegDownloadService.getFFprobePath()
-    } catch (error) {
-      logger.warn('获取下载的 FFprobe 路径失败,使用系统 FFprobe', {
-        error: error instanceof Error ? error.message : String(error)
-      })
-      // 降级到系统 FFprobe
-      return 'ffprobe'
-    }
+    try {
+      if (ffmpegDownloadService.checkFFprobeExists()) {
+        const p = ffmpegDownloadService.getFFprobePath()
+        logger.info('使用动态下载的 FFprobe', { path: p })
+        return p
+      }
+      logger.warn('未检测到下载版 FFprobe,降级使用系统 FFprobe')
+    } catch (error) {
+      logger.warn('获取下载的 FFprobe 路径失败,降级使用系统 FFprobe', {
+        error: error instanceof Error ? error.message : String(error)
+      })
+    }
+    return 'ffprobe'
   }
src/main/services/MediaParserService.ts (2)

4-4: 主进程依赖 renderer 的 @types 存在边界耦合风险,建议迁移到 shared

MediaParserService 位于 main,却从 @types(指向 renderer 基础设施)导入;这会造成打包边界不清与循环依赖隐患。

将 SubtitleStream/SubtitleStreamsResponse(及 FFmpegVideoInfo)迁到 packages/shared/types,并将 @types 映射切到 shared;两端统一从 shared 引用。完成后跑类型检查与构建验证。


639-675: 收紧 codec 类型并更准确地区分图像型字幕;补充 dvd_subtitle

当前 codec as any 弱化了类型安全,isPGS 实际涵盖 dvb_subtitle/xsub 等图像型轨,命名易误导。

-  const pgsCodecs = ['hdmv_pgs_subtitle', 'dvb_subtitle', 'xsub']
+  // 图像型字幕编解码(PGS/VobSub/XSub)
+  const imageSubtitleCodecs = ['hdmv_pgs_subtitle', 'dvb_subtitle', 'xsub', 'dvd_subtitle']
@@
-  const codec = stream.codec_name || 'unknown'
-  const isPGS = pgsCodecs.includes(codec)
+  const rawCodec: string = stream.codec_name || 'unknown'
+  // 将 ffprobe 返回值映射到 SubtitleCodecType 白名单,未知兜底为 'unknown'
+  const allowed = new Set<SubtitleCodecType | 'unknown'>([
+    'srt','ass','ssa','webvtt','subrip','mov_text','hdmv_pgs_subtitle','dvb_subtitle','xsub','dvd_subtitle','unknown'
+  ])
+  const codec = (allowed.has(rawCodec as any) ? rawCodec : 'unknown') as SubtitleCodecType | 'unknown'
+  const isImageBased = imageSubtitleCodecs.includes(rawCodec)
@@
-    isPGS
+    isPGS: isImageBased // 为兼容现有 UI 字段,后续可重命名为 isImageBased

并在 UI 层添加注释说明:该标记表示“图像型字幕”,不只 PGS。
Based on learnings

src/main/services/SubtitleExtractorService.ts (3)

63-86: 在提取前拒绝图像型字幕,避免无效 FFmpeg 调用

当前代码对所有字幕编解码器都尝试提取,但图像型字幕(hdmv_pgs_subtitledvb_subtitlexsub)无法转换为文本格式。应在调用 FFmpeg 前进行检测并返回明确的不支持错误。

建议在 line 63 之前添加检测:

+      // 图像型字幕暂不支持(与 UI 提示保持一致)
+      const imageBasedCodecs = ['hdmv_pgs_subtitle', 'dvb_subtitle', 'xsub']
+      if (options.subtitleCodec && imageBasedCodecs.includes(options.subtitleCodec)) {
+        logger.warn('⚠️ 图像型字幕暂不支持导入', { subtitleCodec: options.subtitleCodec })
+        return {
+          success: false,
+          error: '图像型字幕(PGS/DVB/XSub)暂不支持导入'
+        }
+      }
+
       // 根据源字幕格式确定输出格式
       const outputFormat = this.getOutputFormatFromCodec(

125-184: 修复字幕提取策略:根据目标格式选择合适的编码器

当前使用 -c copy 对所有字幕流进行提取,但这在多种场景下会失败:

  • mov_text.srt 需要转码(-c:s srt
  • webvtt.vtt 在某些容器中需要重新封装
  • 缺少 -y(覆盖已存在文件)和 -loglevel error(减少日志噪音)标志

建议修改 line 133 的参数构建逻辑:

   private async runFFmpegExtract(
     videoPath: string,
     streamIndex: number,
     outputPath: string,
     subtitleCodec?: string
   ): Promise<boolean> {
     return new Promise((resolve) => {
       const ffmpegPath = this.ffmpegService.getFFmpegPath()
-      const args = ['-i', videoPath, '-map', `0:${streamIndex}`, '-c', 'copy', outputPath]
+      
+      // 根据输出格式选择合适的字幕编码器
+      const ext = path.extname(outputPath).replace('.', '').toLowerCase()
+      const encoderMap: Record<string, string> = {
+        srt: 'srt',
+        vtt: 'webvtt',
+        ass: 'ass'
+      }
+      const encoder = encoderMap[ext] || 'copy'
+      
+      const args = [
+        '-y',
+        '-loglevel', 'error',
+        '-i', videoPath,
+        '-map', `0:${streamIndex}`,
+        '-c:s', encoder,
+        outputPath
+      ]
       const fullCommand = `${ffmpegPath} ${args.map((arg) => (arg.includes(' ') ? `"${arg}"` : arg)).join(' ')}`

189-219: 考虑移除不支持的图像型字幕格式映射

Lines 198-200 映射了图像型字幕编解码器(hdmv_pgs_subtitledvb_subtitlexsub)到 sup/sub 格式,但当前不支持这些类型的提取。保留这些映射可能误导未来维护者认为已支持。

建议移除或明确注释:

   const codecToFormat: Record<string, string> = {
     subrip: 'srt',
     ass: 'ass',
     ssa: 'ass',
     webvtt: 'vtt',
     vtt: 'vtt',
-    mov_text: 'srt',
-    hdmv_pgs_subtitle: 'sup',
-    dvb_subtitle: 'sub',
-    xsub: 'sub'
+    mov_text: 'srt'
+    // 图像型字幕暂不支持:
+    // hdmv_pgs_subtitle: 'sup',
+    // dvb_subtitle: 'sub',
+    // xsub: 'sub'
   }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09295a5 and 398ea66.

📒 Files selected for processing (12)
  • CLAUDE.md (2 hunks)
  • packages/shared/IpcChannel.ts (1 hunks)
  • src/main/index.ts (1 hunks)
  • src/main/ipc.ts (3 hunks)
  • src/main/services/FFmpegService.ts (4 hunks)
  • src/main/services/MediaParserService.ts (4 hunks)
  • src/main/services/SubtitleExtractorService.ts (1 hunks)
  • src/main/services/__tests__/SubtitleExtractorService.unit.test.ts (1 hunks)
  • src/renderer/src/pages/player/PlayerPage.tsx (8 hunks)
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx (5 hunks)
  • src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (1 hunks)
  • src/renderer/src/utils/ParallelVideoProcessor.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: 项目样式应使用 styled-components,v1 的 style 构建机制已废弃,不再引入或编写相关实现
定制 antd 组件样式时优先使用 styled-components(styled(Component) 包装)而非全局 SCSS/classNames,避免样式污染并保持架构一致性
优先使用 CSS 变量而不是硬编码样式值;主题相关属性在 styled-components 中使用 Ant Design CSS 变量(如 var(--ant-color-)、var(--ant-box-shadow-))
避免硬编码尺寸与时长,优先使用 useTheme() 的 token 或集中定义的 JS 样式变量(如 motionDurationMid、borderRadiusSM/MD 等)
在 styled-components 中采用“主题相关用 CSS 变量、设计系统常量用 JS 变量”的混合模式(如颜色/阴影用 CSS var,间距/圆角/字号/动效/ZIndex 用 JS 常量)
优先使用 antd 组件库;若已有可复用的 antd 组件则不要重复自定义实现
Zustand 必须在组件/Hook 顶层通过 selector 使用(useStore(selector)),禁止在 useMemo/useEffect 等内部调用 store Hook,避免 Hooks 顺序问题
避免使用返回对象的 Zustand selector(如 useStore(s => ({ a: s.a, b: s.b }))),应使用单字段选择器或配合 shallow 比较器避免不必要重渲染与更新循环
React 副作用与状态更新规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定(默认 TS + React 18)
Player 相关实现需遵循:Zustand selector 顶层调用,禁止在 useMemo/useEffect 中调用 store Hook;为 useSubtitleEngine 增加 subtitles 入参防御等(已在 SubtitleOverlay、SubtitleListPanel、usePlayerControls、useVideoEvents、VideoSurface、useSubtitleSync 落地)

Files:

  • src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts
  • src/main/services/SubtitleExtractorService.ts
  • src/main/index.ts
  • packages/shared/IpcChannel.ts
  • src/main/services/__tests__/SubtitleExtractorService.unit.test.ts
  • src/main/ipc.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/main/services/MediaParserService.ts
  • src/renderer/src/pages/player/PlayerPage.tsx
  • src/renderer/src/utils/ParallelVideoProcessor.ts
  • src/main/services/FFmpegService.ts
**/*.{ts,tsx,scss,css}

📄 CodeRabbit inference engine (CLAUDE.md)

布局优先使用 flex,除必要场景外避免将 grid 作为默认方案

Files:

  • src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts
  • src/main/services/SubtitleExtractorService.ts
  • src/main/index.ts
  • packages/shared/IpcChannel.ts
  • src/main/services/__tests__/SubtitleExtractorService.unit.test.ts
  • src/main/ipc.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/main/services/MediaParserService.ts
  • src/renderer/src/pages/player/PlayerPage.tsx
  • src/renderer/src/utils/ParallelVideoProcessor.ts
  • src/main/services/FFmpegService.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: 统一使用 loggerService 记录日志而不是 console
logger 使用约定:如 logger.error('msg', { error }),第二个参数必须是对象字面量({})承载上下文
任何组件或页面都不得写入 currentTime(播放器控制权由编排器统一管理)

Files:

  • src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts
  • src/main/services/SubtitleExtractorService.ts
  • src/main/index.ts
  • packages/shared/IpcChannel.ts
  • src/main/services/__tests__/SubtitleExtractorService.unit.test.ts
  • src/main/ipc.ts
  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/main/services/MediaParserService.ts
  • src/renderer/src/pages/player/PlayerPage.tsx
  • src/renderer/src/utils/ParallelVideoProcessor.ts
  • src/main/services/FFmpegService.ts
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

所有图标统一使用 lucide-react,不使用 emoji

Files:

  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/pages/player/PlayerPage.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-16T15:20:02.493Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-16T15:20:02.493Z
Learning: Applies to **/*.{ts,tsx} : Player 相关实现需遵循:Zustand selector 顶层调用,禁止在 useMemo/useEffect 中调用 store Hook;为 useSubtitleEngine 增加 subtitles 入参防御等(已在 SubtitleOverlay、SubtitleListPanel、usePlayerControls、useVideoEvents、VideoSurface、useSubtitleSync 落地)

Applied to files:

  • src/renderer/src/pages/player/components/SubtitleListPanel.tsx
  • src/renderer/src/pages/player/PlayerPage.tsx
🧬 Code graph analysis (6)
src/main/services/SubtitleExtractorService.ts (1)
src/renderer/src/services/Logger.ts (2)
  • loggerService (817-817)
  • error (422-424)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx (2)
src/renderer/src/infrastructure/types/subtitle.ts (1)
  • SubtitleItem (10-16)
src/renderer/src/infrastructure/styles/theme.ts (2)
  • SPACING (43-58)
  • BORDER_RADIUS (61-72)
src/main/services/MediaParserService.ts (2)
packages/shared/utils/PathConverter.ts (1)
  • PathConverter (17-174)
src/renderer/src/infrastructure/types/subtitle.ts (2)
  • SubtitleStreamsResponse (141-146)
  • SubtitleStream (129-138)
src/renderer/src/pages/player/PlayerPage.tsx (1)
src/renderer/src/infrastructure/types/subtitle.ts (1)
  • SubtitleStreamsResponse (141-146)
src/renderer/src/utils/ParallelVideoProcessor.ts (1)
packages/shared/utils/PathConverter.ts (1)
  • PathConverter (17-174)
src/main/services/FFmpegService.ts (2)
src/main/services/FFmpegDownloadService.ts (1)
  • ffmpegDownloadService (1021-1021)
packages/shared/utils/PathConverter.ts (1)
  • PathConverter (17-174)
🪛 Biome (2.1.2)
src/renderer/src/pages/player/components/SubtitleListPanel.tsx

[error] 215-215: The elements with this role can be changed to the following elements:

For examples and more information, see WAI-ARIA Roles

(lint/a11y/useSemanticElements)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Cursor Bugbot
  • GitHub Check: test (windows-latest, 20)
  • GitHub Check: test (ubuntu-latest, 20)
  • GitHub Check: test (macos-latest, 20)
🔇 Additional comments (7)
src/renderer/src/utils/ParallelVideoProcessor.ts (2)

8-8: 引入 PathConverter 统一路径转换逻辑。

通过使用共享的 PathConverter 工具类替代内部实现,提升了代码复用性和一致性,且 PathConverter 提供了缓存机制以优化性能。


97-109: 路径转换逻辑改进,提前验证防止无效操作。

使用 PathConverter.convertToLocalPath() 实现了:

  • 同步转换适合当前场景(文件检查仍为异步)
  • 转换失败时立即返回错误,避免后续无意义的文件系统操作
  • 错误信息清晰且包含原始路径,便于调试
  • 与 PR 中其他服务(FFmpegService、MediaParserService)的路径处理方式保持一致
packages/shared/IpcChannel.ts (1)

138-142: LGTM:IPC 枚举扩展命名清晰、与既有分组一致

新增的字幕相关通道与现有命名风格/命名空间保持一致,可用性与可读性良好。

src/main/services/MediaParserService.ts (1)

546-566: ffprobe 调用收敛良好(选择字幕流 + 超时单次 settle)

已使用 this.ffmpegService.getFFprobePath()、-select_streams s,并通过 settled 标志避免 Promise 二次 settle,符合上轮建议。

Also applies to: 570-621

src/main/services/SubtitleExtractorService.ts (3)

1-29: LGTM!导入与接口定义规范

导入结构清晰,接口定义完整,logger 使用符合项目规范。


224-230: LGTM!临时文件路径生成合理

使用时间戳和随机字符串确保文件名唯一性,实现正确。


235-251: LGTM!异步文件清理实现正确

已正确使用 fs.promises.unlink,错误处理完善,符合之前评审的改进要求。

Comment on lines +97 to +107
## Resource Management & Cleanup

- **临时文件清理规范**:所有生成临时文件的服务都应实现集中清理机制,参考 `FFmpegDownloadService.cleanupTempFiles()` 和 `SubtitleExtractorService.cleanupTempFiles()` 的实现模式
- 临时文件清理的最佳实践:
1. 在服务中实现 `cleanupTempFiles()` 方法,扫描并删除符合特定模式的临时文件
2. 在 `src/main/index.ts` 的 `app.on('will-quit')` 事件中调用清理方法
3. 通过 IPC 通道暴露清理接口,允许渲染进程手动触发清理(如 `SubtitleExtractor_CleanupTemp`)
4. 使用正则表达式精确匹配临时文件模式,避免误删其他文件
5. 包含完整的错误处理和日志记录,跳过正在使用或无法删除的文件
- 临时文件命名规范:使用 `<prefix>_<timestamp>_<random>.<ext>` 格式(如 `subtitle_1234567890_abc123.srt`),便于模式匹配和清理

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

给出“临时文件”匹配的正则示例,降低实现偏差

当前只描述了命名约定,建议补充一个推荐正则模板,便于一致实现与审查。

示例补充:

^subtitle_\d{10}_[a-z0-9]{6}\.(srt|vtt|ass|ssa|sup)$

配合前缀可扩展:

^(subtitle|ffmpeg|probe)_\d{10}_[a-z0-9]{6}\.[a-z0-9]+$

并在文档中强调排除锁文件/进行中文件(如 *.part, *.lock)。

🤖 Prompt for AI Agents
In CLAUDE.md around lines 97 to 107, the doc describes temporary file naming but
lacks concrete regex examples; add recommended regex templates for common
prefixes (e.g. subtitle/ffmpeg/probe) and a more specific subtitle pattern, plus
guidance to exclude lock/partial files and other safe-guards; update the section
to show the two example regexes and a short note to explicitly exclude patterns
like *.part and *.lock and to prefer anchored regexes and lowercase hex for
deterministic matching.

Comment on lines 715 to 723
ipcMain.handle(IpcChannel.Fs_CheckFileExists, async (_, filePath: string) => {
try {
const exists = fs.existsSync(filePath)
logger.debug('检查文件存在性', { filePath, exists })
return exists
await fs.promises.access(filePath, fs.constants.F_OK)
logger.debug('检查文件存在性', { filePath, exists: true })
return true
} catch (error) {
logger.error('检查文件存在性时出错', { filePath, error })
logger.debug('检查文件存在性', { filePath, exists: false })
return false
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Fs_CheckFileExists 可提前过滤空路径以减少无意义 I/O

微优化:空字符串/非字符串直接返回 false 并打点一次 debug。

   ipcMain.handle(IpcChannel.Fs_CheckFileExists, async (_, filePath: string) => {
-    try {
+    if (!filePath || typeof filePath !== 'string') {
+      logger.debug('检查文件存在性', { filePath, exists: false, reason: 'invalid-arg' })
+      return false
+    }
+    try {
       await fs.promises.access(filePath, fs.constants.F_OK)
       logger.debug('检查文件存在性', { filePath, exists: true })
       return true

Comment on lines +208 to +210
// 保存原始文件路径用于字幕检测
originalFilePathRef.current = file.path

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

切换视频时应显式重置字幕探测相关状态

为避免上一个视频的探测结果残留,在开始加载新视频时重置 subtitleStreams 与 userDismissedEmbeddedSubtitles。

-      setWaitingForSessionReady(false)
+      setWaitingForSessionReady(false)
+      setSubtitleStreams(null)
+      setUserDismissedEmbeddedSubtitles(false)
@@
-        // 构造页面所需的视频数据(使用处理后的播放源)
+        // 构造页面所需的视频数据(使用处理后的播放源)
         const vd = {

Also applies to: 373-399

🤖 Prompt for AI Agents
In src/renderer/src/pages/player/PlayerPage.tsx around lines 208-210 (and
similarly at 373-399), when switching to a new video you must explicitly reset
subtitle detection state so previous results don't persist: clear
subtitleStreams and set userDismissedEmbeddedSubtitles back to its initial value
at the point you start loading the new file (immediately after assigning
originalFilePathRef.current = file.path), and also ensure the same reset occurs
in the alternative/new-load path around lines 373-399 so both load paths clear
subtitleStreams and userDismissedEmbeddedSubtitles before detection begins.

Comment on lines +700 to +705
<SubtitleListPanel
hasEmbeddedSubtitles={
subtitleStreams !== null && subtitleStreams.streams.length > 0
}
onOpenEmbeddedSubtitleSelector={() => setShowSubtitleTrackSelector(true)}
/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

UI 逻辑 OK,建议在 hasEmbeddedSubtitles 为 false 时禁用入口

当前通过 hasEmbeddedSubtitles 控制提示展示已合理。可进一步将按钮/入口置灰或隐藏,避免空弹窗。

Also applies to: 732-740

🤖 Prompt for AI Agents
In src/renderer/src/pages/player/PlayerPage.tsx around lines 700-705 (and also
apply same change at 732-740), the UI currently always exposes the
embedded-subtitle entry even when hasEmbeddedSubtitles is false; change the
rendering so the entry is disabled or hidden when subtitleStreams is null or has
zero streams — e.g. pass a disabled prop (or omit
onOpenEmbeddedSubtitleSelector) to SubtitleListPanel when hasEmbeddedSubtitles
is false, ensure the button is visually muted/gray and that the click handler is
a no-op when disabled to prevent opening an empty dialog.

…/await

- Update cleanupTempFiles method to be asynchronous, ensuring non-blocking file operations.
- Replace synchronous file reading and deletion with their asynchronous counterparts to prevent event loop blocking.
- Enhance error handling during file deletion and log the number of cleaned files.
- Update related IPC handlers and unit tests to accommodate the async changes, ensuring proper functionality and logging behavior.
…r filesystem checks

- Update FFmpegService integration tests to utilize async/await for filesystem operations, ensuring non-blocking behavior.
- Mock fs.promises.stat to simulate various scenarios, including existing files, non-existent files, and directories.
- Enhance error handling in tests to gracefully manage filesystem errors.
- Ensure all tests correctly await the results of fastCheckFFmpegExists, improving test reliability and performance.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (9)
src/main/ipc.ts (4)

685-689: Media_GetSubtitleStreams 缺少入参校验与最小化日志

与先前意见一致,建议校验 inputPath(非空字符串),失败返回 null 并记录一次 warn。

 ipcMain.handle(IpcChannel.Media_GetSubtitleStreams, async (_, inputPath: string) => {
-  return await mediaParserService.getSubtitleStreams(inputPath)
+  if (!inputPath || typeof inputPath !== 'string') {
+    logger.warn('获取字幕流参数非法', { inputPath })
+    return null
+  }
+  return await mediaParserService.getSubtitleStreams(inputPath)
 })

690-708: Media_ExtractSubtitle 需强类型与参数校验,统一默认值与日志

为稳定 IPC 面,收敛类型、校验 videoPath/streamIndex,并最小化记录上下文。

-  ipcMain.handle(
-    IpcChannel.Media_ExtractSubtitle,
-    async (
-      _,
-      options: {
-        videoPath: string
-        streamIndex: number
-        outputFormat?: string
-        subtitleCodec?: string
-      }
-    ) => {
-      return await subtitleExtractorService.extractSubtitle({
-        videoPath: options.videoPath,
-        streamIndex: options.streamIndex,
-        outputFormat: (options.outputFormat as 'srt' | 'ass' | 'vtt') || 'srt',
-        subtitleCodec: options.subtitleCodec
-      })
-    }
-  )
+  type ExtractSubtitleOptions = {
+    videoPath: string
+    streamIndex: number
+    outputFormat?: 'srt' | 'ass' | 'vtt'
+    subtitleCodec?: string
+  }
+  ipcMain.handle(IpcChannel.Media_ExtractSubtitle, async (_, options: ExtractSubtitleOptions) => {
+    if (!options?.videoPath) {
+      logger.warn('提取字幕参数缺失:videoPath')
+      throw new Error('videoPath 不能为空')
+    }
+    if (!Number.isInteger(options.streamIndex) || options.streamIndex < 0) {
+      logger.warn('提取字幕参数非法:streamIndex', { streamIndex: options?.streamIndex })
+      throw new Error('streamIndex 非法')
+    }
+    const outputFormat: 'srt' | 'ass' | 'vtt' = options.outputFormat ?? 'srt'
+    logger.info('提取内嵌字幕', {
+      videoPath: options.videoPath,
+      streamIndex: options.streamIndex,
+      outputFormat,
+      subtitleCodec: options.subtitleCodec
+    })
+    return await subtitleExtractorService.extractSubtitle({
+      videoPath: options.videoPath,
+      streamIndex: options.streamIndex,
+      outputFormat,
+      subtitleCodec: options.subtitleCodec
+    })
+  })

710-712: SubtitleExtractor_CleanupTemp 建议记录数量并稳定返回类型

返回计数并写一条 info,便于观测;错误时返回 0。

-ipcMain.handle(IpcChannel.SubtitleExtractor_CleanupTemp, async () => {
-  return await subtitleExtractorService.cleanupTempFiles()
-})
+ipcMain.handle(IpcChannel.SubtitleExtractor_CleanupTemp, async () => {
+  try {
+    const count = await subtitleExtractorService.cleanupTempFiles()
+    logger.info('触发字幕临时文件清理', { count })
+    return count
+  } catch (error) {
+    logger.error('字幕临时文件清理失败', { error })
+    return 0
+  }
+})

715-723: Fs_CheckFileExists 可提前过滤非法参数以减少无效 I/O

微优化:对空/非字符串参数直接返回 false,并打一条 debug。

 ipcMain.handle(IpcChannel.Fs_CheckFileExists, async (_, filePath: string) => {
-  try {
+  if (!filePath || typeof filePath !== 'string') {
+    logger.debug('检查文件存在性', { filePath, exists: false, reason: 'invalid-arg' })
+    return false
+  }
+  try {
     await fs.promises.access(filePath, fs.constants.F_OK)
     logger.debug('检查文件存在性', { filePath, exists: true })
     return true
   } catch (error) {
     logger.debug('检查文件存在性', { filePath, exists: false })
     return false
   }
 })
CLAUDE.md (2)

64-75: 补充 resetModules 步骤,确保加载真实 fs

在 vi.unmock 后加入 vi.resetModules(),并建议随后再导入依赖 fs 的模块。

 - **重要**:全局测试配置文件 `tests/setup.ts` 中 mock 了 `node:fs` 和 `node:fs/promises` 模块,这会影响需要真实文件系统操作的测试
 - 对于需要真实文件系统操作的测试(如文件清理、文件读写等),必须在测试文件开头使用 `vi.unmock('node:fs')` 和 `vi.unmock('node:fs/promises')` 来取消全局 mock
 - 示例:
 
   ```typescript
   // 在测试文件开头
   vi.unmock('node:fs')
   vi.unmock('node:fs/promises')
+  vi.resetModules()
 
   // 然后导入真实的 fs
   import * as fs from 'fs'

---

`97-107`: **加入“临时文件”推荐正则模板与排除项**

提供统一匹配规范,降低实现偏差。  

```diff
 - 临时文件命名规范:使用 `<prefix>_<timestamp>_<random>.<ext>` 格式(如 `subtitle_1234567890_abc123.srt`),便于模式匹配和清理
+ - 推荐正则(字幕文件):
+   - `^subtitle_\\d{10}_[a-z0-9]{6}\\.(srt|vtt|ass|ssa|sup)$`
+ - 可扩展前缀通用模板:
+   - `^(subtitle|ffmpeg|probe)_\\d{10}_[a-z0-9]{6}\\.[a-z0-9]+$`
+ - 注意排除:进行中的/锁文件(如 `*.part`, `*.lock`),并使用锚点避免误匹配。
src/main/services/SubtitleExtractorService.ts (3)

63-78: 关键问题:缺少图像型字幕拒绝逻辑

当前代码对所有字幕编解码器一视同仁,但图像型字幕(PGS/DVB/XSUB)无法转换为文本格式(SRT/VTT/ASS)。虽然 UI 层面已提示不支持,但服务层应明确拒绝,避免执行注定失败的提取操作。

建议在确定输出格式后立即添加验证:

       // 根据源字幕格式确定输出格式
       const outputFormat = this.getOutputFormatFromCodec(
         options.subtitleCodec,
         options.outputFormat
       )
+
+      // 拒绝图像型字幕(与 UI 提示保持一致)
+      const imageBasedCodecs = ['hdmv_pgs_subtitle', 'dvb_subtitle', 'xsub']
+      if (options.subtitleCodec && imageBasedCodecs.includes(options.subtitleCodec)) {
+        logger.error('❌ 图像型字幕暂不支持导入', { subtitleCodec: options.subtitleCodec })
+        return {
+          success: false,
+          error: '图像型字幕(PGS/DVB/XSUB)暂不支持导入'
+        }
+      }

       // 生成临时输出文件路径
       const outputPath = this.generateTempSubtitlePath(outputFormat)

125-184: 关键问题:无条件使用 -c copy 导致多种场景下提取失败

当前实现始终使用 -c copy,仅在源编解码器与目标容器兼容时有效。对于需要转码的场景(如 mov_text → SRT、webvtt → VTT),FFmpeg 会报错并导致提取失败。

应根据输出文件扩展名选择合适的字幕编码器:

   private async runFFmpegExtract(
     videoPath: string,
     streamIndex: number,
     outputPath: string,
     subtitleCodec?: string
   ): Promise<boolean> {
     return new Promise((resolve) => {
       const ffmpegPath = this.ffmpegService.getFFmpegPath()
-      const args = ['-i', videoPath, '-map', `0:${streamIndex}`, '-c', 'copy', outputPath]
+      
+      // 根据输出格式选择合适的字幕编码器
+      const ext = path.extname(outputPath).replace('.', '').toLowerCase()
+      const encoderMap: Record<string, string> = {
+        srt: 'srt',
+        vtt: 'webvtt',
+        ass: 'ass'
+      }
+      const encoder = encoderMap[ext] || 'copy'
+      
+      const args = [
+        '-y',                    // 覆盖已存在的输出文件
+        '-loglevel', 'error',    // 减少日志噪音
+        '-i', videoPath,
+        '-map', `0:${streamIndex}`,
+        '-c:s', encoder,         // 使用正确的字幕编码器
+        outputPath
+      ]
+      
       const fullCommand = `${ffmpegPath} ${args.map((arg) => (arg.includes(' ') ? `"${arg}"` : arg)).join(' ')}`

189-219: 建议移除图像型字幕的编解码器映射以避免误导

getOutputFormatFromCodec 方法将 hdmv_pgs_subtitledvb_subtitlexsub 映射到 sup/sub 扩展名,暗示服务支持这些格式。但由于图像型字幕实际无法导入(应在 extractSubtitle 中被拒绝),保留这些映射容易引起混淆。

建议移除不支持的格式映射:

   private getOutputFormatFromCodec(subtitleCodec?: string, fallbackFormat?: string): string {
     // 编解码器到文件格式的映射
     const codecToFormat: Record<string, string> = {
       subrip: 'srt',
       ass: 'ass',
       ssa: 'ass',
       webvtt: 'vtt',
       vtt: 'vtt',
-      mov_text: 'srt',
-      hdmv_pgs_subtitle: 'sup',
-      dvb_subtitle: 'sub',
-      xsub: 'sub'
+      mov_text: 'srt'
     }

如果未来计划支持图像型字幕,届时再添加这些映射。

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 398ea66 and 7d3d9a3.

📒 Files selected for processing (6)
  • CLAUDE.md (2 hunks)
  • src/main/index.ts (1 hunks)
  • src/main/ipc.ts (3 hunks)
  • src/main/services/SubtitleExtractorService.ts (1 hunks)
  • src/main/services/__tests__/FFmpegService.integration.test.ts (5 hunks)
  • src/main/services/__tests__/SubtitleExtractorService.unit.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: 项目样式应使用 styled-components,v1 的 style 构建机制已废弃,不再引入或编写相关实现
定制 antd 组件样式时优先使用 styled-components(styled(Component) 包装)而非全局 SCSS/classNames,避免样式污染并保持架构一致性
优先使用 CSS 变量而不是硬编码样式值;主题相关属性在 styled-components 中使用 Ant Design CSS 变量(如 var(--ant-color-)、var(--ant-box-shadow-))
避免硬编码尺寸与时长,优先使用 useTheme() 的 token 或集中定义的 JS 样式变量(如 motionDurationMid、borderRadiusSM/MD 等)
在 styled-components 中采用“主题相关用 CSS 变量、设计系统常量用 JS 变量”的混合模式(如颜色/阴影用 CSS var,间距/圆角/字号/动效/ZIndex 用 JS 常量)
优先使用 antd 组件库;若已有可复用的 antd 组件则不要重复自定义实现
Zustand 必须在组件/Hook 顶层通过 selector 使用(useStore(selector)),禁止在 useMemo/useEffect 等内部调用 store Hook,避免 Hooks 顺序问题
避免使用返回对象的 Zustand selector(如 useStore(s => ({ a: s.a, b: s.b }))),应使用单字段选择器或配合 shallow 比较器避免不必要重渲染与更新循环
React 副作用与状态更新规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定(默认 TS + React 18)
Player 相关实现需遵循:Zustand selector 顶层调用,禁止在 useMemo/useEffect 中调用 store Hook;为 useSubtitleEngine 增加 subtitles 入参防御等(已在 SubtitleOverlay、SubtitleListPanel、usePlayerControls、useVideoEvents、VideoSurface、useSubtitleSync 落地)

Files:

  • src/main/services/__tests__/FFmpegService.integration.test.ts
  • src/main/index.ts
  • src/main/services/__tests__/SubtitleExtractorService.unit.test.ts
  • src/main/services/SubtitleExtractorService.ts
  • src/main/ipc.ts
**/*.{ts,tsx,scss,css}

📄 CodeRabbit inference engine (CLAUDE.md)

布局优先使用 flex,除必要场景外避免将 grid 作为默认方案

Files:

  • src/main/services/__tests__/FFmpegService.integration.test.ts
  • src/main/index.ts
  • src/main/services/__tests__/SubtitleExtractorService.unit.test.ts
  • src/main/services/SubtitleExtractorService.ts
  • src/main/ipc.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: 统一使用 loggerService 记录日志而不是 console
logger 使用约定:如 logger.error('msg', { error }),第二个参数必须是对象字面量({})承载上下文
任何组件或页面都不得写入 currentTime(播放器控制权由编排器统一管理)

Files:

  • src/main/services/__tests__/FFmpegService.integration.test.ts
  • src/main/index.ts
  • src/main/services/__tests__/SubtitleExtractorService.unit.test.ts
  • src/main/services/SubtitleExtractorService.ts
  • src/main/ipc.ts
🧠 Learnings (1)
📚 Learning: 2025-10-16T15:20:02.493Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-16T15:20:02.493Z
Learning: 使用 vitest 作为测试框架

Applied to files:

  • CLAUDE.md
🧬 Code graph analysis (1)
src/main/services/SubtitleExtractorService.ts (1)
src/renderer/src/services/Logger.ts (2)
  • loggerService (817-817)
  • error (422-424)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: test (macos-latest, 20)
  • GitHub Check: test (windows-latest, 20)
  • GitHub Check: test (ubuntu-latest, 20)
🔇 Additional comments (5)
src/main/services/__tests__/FFmpegService.integration.test.ts (1)

100-119: 异步化与错误处理用例:通过

用例覆盖“存在/不存在/目录误判/异常”路径并统一 await fastCheckFFmpegExists(),断言合理。保持此风格。

Also applies to: 121-135, 137-156, 239-244, 248-260

src/main/ipc.ts (2)

32-32: 服务初始化位置 OK,但留意构造开销

模块级单例可保连接/缓存复用;若构造较重,可考虑惰性创建。

Also applies to: 45-46


48-57: 注释补充清晰

注册职责/副作用说明到位。

src/main/services/SubtitleExtractorService.ts (2)

235-251: 已正确实现异步文件清理

cleanupTempFile 方法现已使用 fs.promises.unlink,正确处理了 ENOENT 错误,避免阻塞事件循环。实现符合最佳实践。


257-301: 已正确实现异步批量清理并采用并行删除

cleanupTempFiles 方法现已完全异步化,使用 fs.promises.readdirfs.promises.unlink,并通过 Promise.all 并行执行删除操作。实现高效且符合最佳实践。

Comment on lines +185 to +193
// Cleanup temporary subtitle files
try {
const SubtitleExtractorService = (await import('./services/SubtitleExtractorService')).default
const subtitleExtractorService = new SubtitleExtractorService()
await subtitleExtractorService.cleanupTempFiles()
logger.info('Temporary subtitle files cleaned up')
} catch (error) {
logger.error('Error cleaning up temporary subtitle files:', { error })
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

已 await 清理:符合先前建议;可选增加超时护栏

will-quit 中已 await 清理,OK。为防极端阻塞退出,可包一层超时(如 2s)或在 before-quit 执行并设置兜底。当前改动可先合。

🤖 Prompt for AI Agents
In src/main/index.ts around lines 185 to 193, the awaited cleanup of temporary
subtitle files in the will-quit handler should be protected with a timeout to
avoid blocking app shutdown; wrap the await
subtitleExtractorService.cleanupTempFiles() in a cancellable/timeout promise
(e.g., race it against a 2000ms timeout) and if the timeout wins, log a warning
and proceed, ensuring the handler always resolves promptly; alternatively move
the cleanup to before-quit and keep a 2s fallback timeout to guarantee the
process exits.

Comment on lines +109 to +116
// Mock fs.promises.stat for async operation
const mockStat = vi.fn().mockResolvedValue({
isFile: () => true,
mode: 0o755,
size: 1024 * 1024
})
vi.mocked(fs.promises).stat = mockStat

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

补充 mock 清理,避免测试串扰

多处直接覆盖 fs.promises.stat。建议在套件末尾统一恢复 spy/mock,避免跨用例残留。最小改动如下:

@@
-  beforeEach(() => {
+  beforeEach(() => {
     vi.clearAllMocks()
@@
-    ffmpegService = new FFmpegService()
+    ffmpegService = new FFmpegService()
   })
+  afterEach(() => {
+    vi.restoreAllMocks()
+  })

如后续改为 vi.spyOn(fs.promises, 'stat') 则更易于管理恢复。

Also applies to: 125-132, 147-153, 228-235

Comment on lines +3 to +6
// 首先取消全局的 fs mock
vi.unmock('node:fs')
vi.unmock('node:fs/promises')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

unmock 后重置模块缓存,确保加载真实 fs 实现

为避免受 hoisted mock 影响,unmock 之后先 resetModules 再导入被测模块;并兼容 import 'fs' 场景。

 // 首先取消全局的 fs mock
-vi.unmock('node:fs')
-vi.unmock('node:fs/promises')
+vi.unmock('node:fs')
+vi.unmock('node:fs/promises')
+vi.unmock('fs')
+vi.unmock('fs/promises')
 
@@
 beforeAll(async () => {
-  // 动态导入服务
+  // 动态导入服务(确保使用真实 fs)
+  vi.resetModules()
   const module = await import('../SubtitleExtractorService')
   SubtitleExtractorService = module.default
 })

Also applies to: 31-35

🤖 Prompt for AI Agents
In src/main/services/__tests__/SubtitleExtractorService.unit.test.ts around
lines 3-6 (and similarly 31-35), after calling vi.unmock('node:fs') and
vi.unmock('node:fs/promises') you must call vi.resetModules() before importing
the module under test so the real fs implementation is loaded (this prevents
hoisted mocks from persisting); update the test to reset modules immediately
after unmocking and then require/import the tested module (ensuring tests also
cover the case where code uses import 'fs') so the real node:fs is used.

Comment on lines +37 to +43
beforeEach(() => {
service = new SubtitleExtractorService()
tempDir = os.tmpdir()
testFiles = []
vi.clearAllMocks()
})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

避免在系统临时根目录清理:隔离测试目录并重定向 os.tmpdir

当前在 os.tmpdir 根目录执行清理,存在误删真实临时文件风险。请用 mkdtemp 创建隔离子目录,并让服务使用该目录;afterEach 递归删除并恢复 spies。

 beforeEach(() => {
   service = new SubtitleExtractorService()
-  tempDir = os.tmpdir()
+  tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'SubtitleExtractorServiceTest-'))
+  vi.spyOn(os, 'tmpdir').mockReturnValue(tempDir)
   testFiles = []
   vi.clearAllMocks()
 })
 
 afterEach(() => {
-  // 清理测试创建的临时文件
-  for (const file of testFiles) {
-    try {
-      if (fs.existsSync(file)) {
-        fs.unlinkSync(file)
-      }
-    } catch (error) {
-      // 忽略清理错误
-    }
-  }
-  testFiles = []
+  // 清理测试目录并恢复 mocks
+  try {
+    fs.rmSync(tempDir, { recursive: true, force: true })
+  } catch {}
+  testFiles = []
+  vi.restoreAllMocks()
 })

Also applies to: 44-56

…me generation and update unit tests

- Modify the random string generation in generateTempSubtitlePath to ensure it only contains lowercase letters and numbers.
- Update unit tests to improve validation of log messages and regex patterns for subtitle filenames.
- Remove redundant test cases and streamline the cleanupTempFile tests for better clarity and efficiency.
…n count

- Update the cleanupTemp IPC handler to log the number of cleaned temporary subtitle files.
- Refactor the useSubtitleOverlay hook to remove unnecessary memoization and improve clarity by directly using currentSubtitleData.
- Ensure consistent handling of subtitle data across the component, enhancing performance and readability.
@mkdir700 mkdir700 merged commit 0e2e8f2 into dev Oct 17, 2025
4 of 5 checks passed
@mkdir700 mkdir700 deleted the feat/import-subtitle-from-stream branch October 17, 2025 11:46
mkdir700 added a commit that referenced this pull request Oct 17, 2025
* feat(media): add subtitle stream handling and extraction

- Added new IPC channels for handling subtitle streams: Media_GetSubtitleStreams and Media_ExtractSubtitle
- Implemented SubtitleExtractorService to manage subtitle extraction
- Updated MediaParserService to include methods for retrieving subtitle stream information using ffprobe
- Integrated subtitle stream handling in ipc.ts with appropriate handlers
- Introduced SubtitleTrackSelector component for UI interaction with subtitle streams

* feat(player): enhance subtitle import with caching

- integrate SubtitleLibraryService to cache parsed subtitles
- add current video context to enable subtitle persistence
- log caching success and handle potential errors gracefully
- ensure subtitles are stored with associated video ID

* 📝 Add docstrings to `feat/import-subtitle-from-stream` (#222)

Docstrings generation was requested by @mkdir700.

* #221 (comment)

The following files were modified:

* `src/main/ipc.ts`
* `src/renderer/src/pages/player/PlayerPage.tsx`
* `src/renderer/src/pages/player/components/SubtitleListPanel.tsx`

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* refactor: make subtitle temp cleanup async

* refactor: replace blocking fs.existsSync with async fs.promises.access in SubtitleExtractorService

* refactor: replace blocking fs.existsSync with async fs.promises in critical paths

- Replace fs.existsSync with fs.promises.access in IPC handler
- Optimize MediaParserService by merging existsSync + statSync into single stat call
- Convert FFmpegService.fastCheckFFmpegExists to async method
- Replace all blocking file checks in async methods across services

This eliminates event loop blocking in hot paths and improves I/O performance.

* fix: add context object to subtitle detection logger for searchability

Add path and videoId to logger.info when no subtitle streams detected,
improving log searchability and debugging capabilities.

* refactor: consolidate path conversion logic to use PathConverter

- Remove duplicate convertFileUrlToLocalPath from MediaParserService
- Remove duplicate convertFileUrlToLocalPath from FFmpegService
- Remove convertFileUrlToLocalPathAsync from ParallelVideoProcessor
- Replace all calls with PathConverter.convertToLocalPath for consistency
- Add proper error handling for path conversion failures
- Remove ~100+ lines of duplicate code

* refactor(MediaParser): fix ffprobe service usage and prevent double settle

- Use this.ffmpegService.getFFprobePath() instead of inline new FFmpegService()
- Add -select_streams 's' to only return subtitle streams for better performance
- Implement settled flag to prevent double settle race conditions
- Change timeout handler to resolve(null) instead of reject for consistency
- Add settled checks in all handlers (timeout/close/error) before resolving/rejecting
- Ensure ffprobe process is killed on timeout with proper logging context

* refactor(player): replace hardcoded Chinese text with i18n in subtitle empty state

- Replace hardcoded Chinese text in SubtitleListPanel empty state options
- Update embedded subtitle option: title, description, and action button
- Update external subtitle option: title and description
- Update AI-generated subtitle option: title, description, and action button
- All i18n keys already exist in language packs (zh-cn, zh-tw, en-us, ja-jp, ru-ru)

* feat(SubtitleExtractor): implement temporary subtitle file cleanup functionality

- Add new IPC channel `subtitle-extractor:cleanup-temp` for cleaning up temporary subtitle files.
- Implement `cleanupTempFiles` method in `SubtitleExtractorService` to remove subtitle files matching the pattern from the system's temporary directory.
- Enhance logging for cleanup operations, including success and error messages.
- Add unit tests for `cleanupTempFiles` to ensure correct functionality and logging behavior.

* docs: update CLAUDE.md with testing and resource management guidelines

- Add important notes on mocking `node:fs` and `node:fs/promises` in the global test setup.
- Include examples for unmocking these modules in test files for real filesystem operations.
- Introduce resource management and cleanup section detailing temporary file cleanup practices.
- Outline best practices for implementing `cleanupTempFiles()` and IPC channel usage for manual cleanup triggers.
- Specify naming conventions for temporary files to facilitate pattern matching and cleanup.

* fix(SubtitleExtractorService): refactor cleanupTempFiles to use async/await

- Update cleanupTempFiles method to be asynchronous, ensuring non-blocking file operations.
- Replace synchronous file reading and deletion with their asynchronous counterparts to prevent event loop blocking.
- Enhance error handling during file deletion and log the number of cleaned files.
- Update related IPC handlers and unit tests to accommodate the async changes, ensuring proper functionality and logging behavior.

* test(FFmpegService): refactor integration tests to use async/await for filesystem checks

- Update FFmpegService integration tests to utilize async/await for filesystem operations, ensuring non-blocking behavior.
- Mock fs.promises.stat to simulate various scenarios, including existing files, non-existent files, and directories.
- Enhance error handling in tests to gracefully manage filesystem errors.
- Ensure all tests correctly await the results of fastCheckFFmpegExists, improving test reliability and performance.

* refactor(SubtitleExtractorService): enhance temporary subtitle filename generation and update unit tests

- Modify the random string generation in generateTempSubtitlePath to ensure it only contains lowercase letters and numbers.
- Update unit tests to improve validation of log messages and regex patterns for subtitle filenames.
- Remove redundant test cases and streamline the cleanupTempFile tests for better clarity and efficiency.

* refactor(ipc): enhance cleanupTemp IPC handler with logging and return count

- Update the cleanupTemp IPC handler to log the number of cleaned temporary subtitle files.
- Refactor the useSubtitleOverlay hook to remove unnecessary memoization and improve clarity by directly using currentSubtitleData.
- Ensure consistent handling of subtitle data across the component, enhancing performance and readability.

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
github-actions bot pushed a commit that referenced this pull request Oct 17, 2025
# [1.2.0](v1.1.1...v1.2.0) (2025-10-17)

### Features

* add subtitle stream handling and extraction([#221](#221)) ([87d11bd](87d11bd)), closes [#222](#222)
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.

1 participant