feat: no spawn ffmpeg #737
Merged
+170
−178
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
审阅者指南 by Sourcery这个拉取请求使用 @ffmpeg.wasm/main 库替换了 node:child_process spawn 的使用，以处理音频转换。这一变更消除了对外部 ffmpeg 可执行文件的依赖，提高了可移植性并简化了构建过程。 音频文件转换的序列图sequenceDiagram
participant C as Client
participant FS as FFmpegService
participant FF as FFmpeg.wasm
participant FSystem as FileSystem
C->>FS: convert(filePath, pcmPath, logger)
activate FS
FS->>FSystem: Read input file
FSystem-->>FS: Input file data
FS->>FF: Write file to virtual FS
FS->>FF: Run FFmpeg conversion
FF-->>FS: Conversion complete
FS->>FF: Read converted file
FF-->>FS: Converted data
FS->>FSystem: Write PCM file
FS-->>C: Return Buffer
deactivate FS
新 FFmpegService 的类图classDiagram
class FFmpegService {
-ffmpegRef: FFmpeg
+constructor(ffmpegRef: FFmpeg)
+extractThumbnail(videoPath: string, thumbnailPath: string): Promise~void~
+convertFile(inputFile: string, outputFile: string, format: string): Promise~void~
+convert(filePath: string, pcmPath: string, logger: LogWrapper): Promise~Buffer~
}
note for FFmpegService "替换直接 ffmpeg spawn 调用
为 @ffmpeg.wasm/main 实现"
文件级别变更
提示和命令与 Sourcery 交互
自定义您的体验访问您的仪表板以：
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request replaces the usage of the node:child_process spawn with the @ffmpeg.wasm/main library to handle audio conversions. This change eliminates the need to rely on an external ffmpeg executable, improving portability and simplifying the build process. Sequence diagram for audio file conversionsequenceDiagram
participant C as Client
participant FS as FFmpegService
participant FF as FFmpeg.wasm
participant FSystem as FileSystem
C->>FS: convert(filePath, pcmPath, logger)
activate FS
FS->>FSystem: Read input file
FSystem-->>FS: Input file data
FS->>FF: Write file to virtual FS
FS->>FF: Run FFmpeg conversion
FF-->>FS: Conversion complete
FS->>FF: Read converted file
FF-->>FS: Converted data
FS->>FSystem: Write PCM file
FS-->>C: Return Buffer
deactivate FS
Class diagram for the new FFmpegServiceclassDiagram
class FFmpegService {
-ffmpegRef: FFmpeg
+constructor(ffmpegRef: FFmpeg)
+extractThumbnail(videoPath: string, thumbnailPath: string): Promise~void~
+convertFile(inputFile: string, outputFile: string, format: string): Promise~void~
+convert(filePath: string, pcmPath: string, logger: LogWrapper): Promise~Buffer~
}
note for FFmpegService "Replaces direct ffmpeg spawn calls
with @ffmpeg.wasm/main implementation"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿 @MliKiowa - 我已经审查了您的更改并发现了一些需要解决的问题。
阻塞性问题：
- FFmpeg 退出代码检查被反转，导致操作在成功时失败，反之亦然（链接）
整体评论：
- FFmpegService 中的退出代码检查被反转 - 目前在成功时（代码 === 0）抛出错误，在失败时成功。需要通过将条件更改为
if (code !== 0)来修复。 - 从 NCoreInitShell 中删除 extractThumbnail 的调试测试代码 - 这似乎是不应该出现在生产环境中的开发测试代码。
以下是我在审查期间查看的内容
- 🔴 一般性问题：1 个阻塞性问题，1 个其他问题
- 🟢 安全性：一切看起来都很好
- 🟢 测试：一切看起来都很好
- 🟢 复杂性：一切看起来都很好
- 🟢 文档：一切看起来都很好
帮助我变得更有用！请在每条评论上点击 👍 或 👎，我将使用反馈来改进您的评论。
Original comment in English
Hey @MliKiowa - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- The FFmpeg exit code check is inverted, causing operations to fail when they succeed and vice versa (link)
Overall Comments:
- The exit code checks in FFmpegService are inverted - currently throwing errors on success (code === 0) and succeeding on failure. This needs to be fixed by changing the condition to
if (code !== 0). - Remove the debug test code for extractThumbnail from NCoreInitShell - this appears to be development testing code that shouldn't be in production.
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 1 other issue
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
使用 @ffmpeg.wasm/main 和 @ffmpeg.wasm/core-mt 替换原生 FFmpeg 调用，用于音频和视频处理。
新特性：
测试：
Original summary in English
Summary by Sourcery
Replace native FFmpeg calls with @ffmpeg.wasm/main and @ffmpeg.wasm/core-mt for audio and video processing.
New Features:
Tests: