feat(desktop): YouTube URL から通知音を作成 (#258)#259
Conversation
Settings > Notifications に "From YouTube" ボタンを追加し、 URL と開始時刻 / 長さ (最大30秒) を指定して カスタム通知音をクリップ生成できるようにする。 ローカルにインストール済みの yt-dlp と ffmpeg を使用する。 未インストール時は分かりやすいエラーで案内する。 Closes #258
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughYouTubeから通知音をインポートする機能を追加しました。フロントエンドダイアログで動画URLと時間範囲を入力し、バックエンドで Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI<br/>(RingtonesSettings)
participant Dialog as YouTubeImportDialog
participant TRPC as tRPC Router
participant YouTube as YouTube<br/>Service
participant FS as File<br/>System
UI->>Dialog: Open dialog
User->>Dialog: Enter URL, time range, name
Dialog->>Dialog: Validate input
User->>Dialog: Click Import
Dialog->>UI: Call onSubmit()
UI->>TRPC: importFromYouTube(url, startSeconds,<br/>durationSeconds, displayName?)
TRPC->>YouTube: importRingtoneFromYouTube(options)
YouTube->>YouTube: Validate URL & binaries
YouTube->>FS: Download audio segment<br/>(yt-dlp --download-sections)
FS-->>YouTube: Audio file
YouTube->>FS: Convert to MP3 (ffmpeg)
FS-->>YouTube: Converted MP3
YouTube->>FS: Import via custom ringtone
FS-->>YouTube: CustomRingtoneInfo
YouTube->>FS: Set display name (optional)
FS-->>YouTube: Updated metadata
YouTube-->>TRPC: Return ringtone info
TRPC-->>UI: Success response
UI->>Dialog: Close dialog
UI->>UI: Invalidate cache,<br/>switch to CUSTOM_RINGTONE_ID
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 複数ファイルにまたがる機能追加で、バックエンドのYouTubeダウンロード・変換ロジック(エラーハンドリング、外部プロセス実行、ファイル管理を含む)と、フロントエンドのフォーム検証・状態管理がそれぞれ独立した推論が必要。一部複雑なロジック密度があるが、コヒーシブなパターンで構成されている。 Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsx (1)
380-384:.catch(() => {})でエラーを握り潰す実装は不要で、ハンドラが同期的完了扱いになります。
onErrorでyoutubeErrorステートに反映する設計なので、mutateAsyncの.catchは不要です。さらに現状の.catch(() => {})はawaitの結果を正常完了にすり替えるため、onSubmitをawaitしている呼び出し側(将来の追加実装含め)で「成功した」ように見えてしまう危険があります。mutateに切り替えるか、.catchを外して呼び出し元に伝播させる方が安全です。♻️ 修正案
- onSubmit={async (input) => { - await importFromYouTube.mutateAsync(input).catch(() => { - // Error surfaced via youtubeError state. - }); - }} + onSubmit={(input) => { + importFromYouTube.mutate(input); + }}これに合わせて
YouTubeImportDialogのonSubmitの型もPromise<void>からvoidに変更するか、mutateAsyncのまま.catchを外すかを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsx` around lines 380 - 384, The onSubmit handler currently swallows errors by awaiting importFromYouTube.mutateAsync(...).catch(() => {}); remove that .catch so errors propagate (or switch to importFromYouTube.mutate to keep it synchronous), and update the YouTubeImportDialog onSubmit type to void if you choose mutate, or keep Promise<void> if you leave mutateAsync without .catch; ensure youtubeError is still set via the existing onError handler on the mutation (refer to importFromYouTube.mutateAsync, onSubmit, YouTubeImportDialog, and the youtubeError state).apps/desktop/src/main/lib/youtube-ringtone.ts (1)
181-189:displayName指定時にメタデータが二度書き込まれます。
importCustomRingtoneFromPath(outputPath)の内部で、ファイル名clip.mp3を元にしたsanitizeDisplayNameにより"clip"という表示名でメタデータが一旦書き込まれ、その直後setCustomRingtoneDisplayName(displayName)で上書きされます。機能上は問題ありませんが、ディスク書き込みが 1 回余計に発生し、呼び出し経路も追いにくくなります。importCustomRingtoneFromPathに任意のdisplayNameを渡せるオーバーロードを設けて一度で確定させる方がクリーンです(任意の refactor)。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/youtube-ringtone.ts` around lines 181 - 189, The code currently calls importCustomRingtoneFromPath(outputPath) which writes metadata using sanitizeDisplayName (e.g. "clip") and then immediately calls setCustomRingtoneDisplayName(displayName) to overwrite it, causing an extra disk write; change importCustomRingtoneFromPath to accept an optional displayName parameter (or add an overload) and apply the same trim/slice/sanitize rules there so the upstream call passes the user-specified displayName (options.displayName?.trim()) and you can remove the follow-up setCustomRingtoneDisplayName call; keep the existing sanitizeDisplayName, trimming and 80-char slice behavior inside importCustomRingtoneFromPath to preserve semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/lib/custom-ringtones.ts`:
- Around line 163-169: setCustomRingtoneDisplayName currently calls
writeCustomRingtoneMetadata which regenerates metadata and overwrites importedAt
with Date.now(); instead, change setCustomRingtoneDisplayName to read the
existing metadata via readCustomRingtoneMetadata(), update only the display name
(trim/limit to 80 or fallback to "Custom Audio"), preserve the existing
importedAt value, then write the merged metadata back using
writeCustomRingtoneMetadata (or a small helper that writes given metadata) so
importedAt is not changed; this prevents importCustomRingtoneFromPath /
importRingtoneFromYouTube flows or future name-only edits from accidentally
updating the import timestamp.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/components/YouTubeImportDialog/YouTubeImportDialog.tsx`:
- Around line 67-75: The seconds field (startSec) isn't clamped to 0–59 before
converting to total seconds, so startSeconds (computed in YouTubeImportDialog)
can be wrong when users type values like 120; update the computation to clamp
startSec to the 0–59 range (e.g. apply a clamp/normalize function to
clampNonNegativeInt(startSec) with an upper limit of 59) before multiplying
startMin by 60 and adding seconds, and apply the same 0–59 clamp wherever
startSec is used (also the similar block around lines 134–142) to ensure correct
minute-second conversion regardless of browser UI constraints.
---
Nitpick comments:
In `@apps/desktop/src/main/lib/youtube-ringtone.ts`:
- Around line 181-189: The code currently calls
importCustomRingtoneFromPath(outputPath) which writes metadata using
sanitizeDisplayName (e.g. "clip") and then immediately calls
setCustomRingtoneDisplayName(displayName) to overwrite it, causing an extra disk
write; change importCustomRingtoneFromPath to accept an optional displayName
parameter (or add an overload) and apply the same trim/slice/sanitize rules
there so the upstream call passes the user-specified displayName
(options.displayName?.trim()) and you can remove the follow-up
setCustomRingtoneDisplayName call; keep the existing sanitizeDisplayName,
trimming and 80-char slice behavior inside importCustomRingtoneFromPath to
preserve semantics.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsx`:
- Around line 380-384: The onSubmit handler currently swallows errors by
awaiting importFromYouTube.mutateAsync(...).catch(() => {}); remove that .catch
so errors propagate (or switch to importFromYouTube.mutate to keep it
synchronous), and update the YouTubeImportDialog onSubmit type to void if you
choose mutate, or keep Promise<void> if you leave mutateAsync without .catch;
ensure youtubeError is still set via the existing onError handler on the
mutation (refer to importFromYouTube.mutateAsync, onSubmit, YouTubeImportDialog,
and the youtubeError state).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20be24be-eef4-458d-8687-906c1aea6687
📒 Files selected for processing (6)
apps/desktop/src/lib/trpc/routers/ringtone/index.tsapps/desktop/src/main/lib/custom-ringtones.tsapps/desktop/src/main/lib/youtube-ringtone.tsapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/components/YouTubeImportDialog/YouTubeImportDialog.tsxapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/components/YouTubeImportDialog/index.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 088b2fa96c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 088b2fa96c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- yt-dlp/ffmpeg をログインシェルの PATH 経由で解決し、 Finder/Dock 起動時の Homebrew パス欠落を回避 - ffprobe も事前チェックして欠落時に明示エラー - yt-dlp の出力をテンプレート (.%(ext)s) で受け取り、 実際に生成されたファイルをディレクトリスキャンで取得 - setCustomRingtoneDisplayName 呼び出しで importedAt を 上書きしないように既存値を保持 - Start time の秒入力を 0-59 にクランプ
概要
Settings > Notifications に From YouTube ボタンを追加し、YouTube の URL を貼って区間を指定するとカスタム通知音として取り込めるようにします。Issue #258 の対応です。
実装内容
apps/desktop/src/main/lib/youtube-ringtone.tsを新規追加。yt-dlpの--download-sectionsで指定区間だけを mp3 として抽出し、既存のimportCustomRingtoneFromPathに渡してカスタム通知音として登録。apps/desktop/src/main/lib/custom-ringtones.tsにsetCustomRingtoneDisplayNameを追加し、ユーザーが任意で付けたラベルをメタデータに保存できるように。ringtone.importFromYouTubeを追加(URL / 開始秒 / 長さ秒 / 任意の表示名)。RingtonesSettings.tsxにFrom YouTubeボタンとモーダルを追加。YouTubeImportDialogを新規追加。URL / 開始時刻 (分・秒) / 長さ (最大30秒) / 表示名(任意)を入力。動作要件
yt-dlpとffmpegがインストールされていること(macOS ならbrew install yt-dlp ffmpeg)。テスト計画
Closes #258
Summary by CodeRabbit
リリースノート