Repository navigation
縦型切り抜き(REFRAME): 顔追跡クロップの基準凍結+実装(葉A/B/D/D2/E/G/G2/G3/H/H2/H3) - #54
Conversation
着手前に basis-reviewer へ2巡掛けて凍結した。 【固定素材の作り直し】旧fixtureは反証で崩れていた（RF-Rは等速直線のみ で決め打ちパンが通る、RF-Nは移動量とim閾値の余裕が無い、RF-Mは全30コマ 完全同一画像で絵の保持を原理的に測れない）。数値で作り直し、名前も G-EDITORの旧「素材R/N/M」（別実体・使用中）との衝突を避けRF-R/RF-N/RF-M とした。元画像のSHA-256照合と可逆書き出しの自己検査をtests/として作り pnpm testへ登録した。 【葉Dを分割】黒帯の存在(D)と絵の保持(D2)が1criteriaに同居していた原子性 違反を2葉に分割。 【葉Eを再設計】旧「切り抜き成功or黒帯」の二分岐は、切り抜き分岐を通ると 絵の保持を一度も測らない欠陥を持っていた。RF-Mへ動きを入れたうえで 「3人以上は常に黒帯へ落ちる」という単一の振る舞いに絞った。 「MAX_TRACKED_FACES=2」という具体的定数の凍結は、1巡目のbasis-reviewer 指摘（RF-Mが常に3人固定のためこの葉では上限の値そのものを反証できない） を受けて見送った。 【葉G/Hを新設】凍結素材が無音だったため切り抜きが音声を落としても全葉 緑のままだった穴を塞ぐ。音声の存在(G)と左右チャンネルの正しさ(H)。 葉Bのverifyに顔検出器の設定(cv2.FaceDetectorYN, SCORE_THRESHOLD=0.6, NMS_THRESHOLD=0.3)、葉D2のverifyに「各フレームごとに3.0以下」の判定 単位を明記した(いずれもbasis-reviewer 2巡目前の指摘)。 葉C・Fは実写素材調達(D-7)がマスター判断待ちのため引き続き対象外。 pnpm -r test 全緑。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNo27NkwpWAcFS1BkQ75B8
basis-reviewer round3指摘: 葉G/H(音声保持・左右チャンネル)がRF-R-A(追跡成功パス)と RF-N-A(黒帯フォールバックパス)という独立して落ちうる2つの受入事実を1criteriaのAND条件に 詰め込んでおり、AGENTS.mdの原子性規律に違反していた。G→G+G2、H→H+H2へ分割し、 親detailに音声素材仕様(RF-R-A/RF-N-A/RF-M-Aの周波数・長さ)を明記した。
basis-reviewer round4指摘: G2/H2はD/D2側フォールバック(顔0人・RF-N-A)のみを カバーしており、E側フォールバック(顔3人以上で追跡破綻・RF-M-A)は凍結済みなのに どの葉からも参照されていなかった(十分性の穴)。G3/H3を新設して埋め、親detailの 説明も実態に合わせて訂正した。
round1〜5のbasis-reviewerで反証を潰し終え、verdict: no_objectionを得た。 status を todo→doing に変え criteria/verify を凍結し、実装着手に進む。
G-EDIT-REFRAME-A/B/D/D2/E/G/G2/G3/H/H2/H3(凍結済みcriteria/verify)を実際に満たす 実装を追加した。criteria/verify の文言自体は変更していない。 - src/reframe.py: face_mosaic.py の create_detector/FaceTracker(SCORE_THRESHOLD=0.6, NMS_THRESHOLD=0.3, hold_frames=8を流用)で顔を検出・追跡し、保持後のトラック数が 1〜2人ならffmpegのcropフィルタ(フレーム番号nに応じたx位置式)で追従クロップ、 0人または3人以上ならrender-vertical.mjsと同じscale+padフィルタ(letterbox)へ 自動で切り替える。区間ごとに別ffmpeg呼び出しで書き出しconcatし、音声は入力から `-map 1:a? -c:a copy`で無変換コピーする。 - src/reframe_cli.py: `python3 src/reframe_cli.py <入力mp4> <出力mp4>` のCLIラッパー。 - tests/reframe-verify-check.py: 11 leafのverify手順(黒帯%・顔検出30/30・letterbox参照との 平均絶対差・音声の存在/長さ・FFT左右周波数)と、各対照実装(letterbox/decoy-pan/引き伸ばし/ 中央固定クロップ/真っ黒/1コマ目固着/常に重心追従/音声を落とす/チャンネル入替え/ モノラル混合)を実際に生成して不合格になることを、34件のPASS/FAILとして自動検証する。 package.json の test チェーン末尾に登録した。 pnpm -r test: 全体で674 PASS / 0 FAIL(video-shorts単体の新規追加ぶんは34 PASS / 0 FAIL)。 pnpm -r --if-present typecheck/lint/build: 対象パッケージ無し。pnpm audit --audit-level moderate: 脆弱性0件。node scripts/verify-roadmap-evidence.mjs: OK(roadmap.html未変更)。 docs/roadmap.htmlのstatus更新・pushはマスター確認後に別途行う(今回は含めない)。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNo27NkwpWAcFS1BkQ75B8
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesThe PR adds a face-tracked vertical reframing module and CLI. It crops videos with one or two tracked faces and uses letterbox fallback otherwise. It preserves audio, adds RF fixtures with stereo audio, runs fixture and output verification checks, and updates roadmap acceptance evidence. Vertical Reframing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant reframe
participant ffmpeg
participant InputAudio
CLI->>reframe: Submit input and output paths
reframe->>ffmpeg: Render crop or letterbox segments
reframe->>ffmpeg: Concatenate rendered segments
InputAudio->>ffmpeg: Supply optional input audio
ffmpeg-->>reframe: Return final video
reframe-->>CLI: Return diagnostics and status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
PR #54でCI(ci-green含む全チェック)緑を確認。independent-verifierによる 独立検証(criteria/verify本文どおりの測定・敵対的破壊テスト・独立再計算・ 再現性・オーバーフィット無しの確認)も完了。handoffを更新。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
video-shorts/package.json (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueテスト連結が長くなり、実行時間と切り分けが重くなります。
reframe-verify-check.pyはreframe()を6回実行し、多数の ffmpeg エンコードを行います。&&の直列連結では、最後尾に追加したこのテストへ到達するまでに全ての先行テストを通す必要があります。将来、test:fastとtest:mediaのようにスクリプトを分けると、開発時の反復が速くなります。CI では従来どおり全体を実行してください。現在の構成は既存スタイルに沿っているため、対応は任意です。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@video-shorts/package.json` at line 5, Optionally split the package.json test command into focused scripts such as test:fast and test:media, moving the expensive reframe-verify-check.py and related media tests into the media group while preserving a full test script that runs the complete suite for CI.video-shorts/src/reframe_cli.py (1)
37-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winモード判定のしきい値が実装と二重定義になっています。
reframe.pyはmax_tracked(既定MAX_TRACKED_FACES)で crop/letterbox を決め、その結果をinfo["segments"]のmodeとして返します。CLI 側は1 <= c <= 2を直接書いているため、MAX_TRACKED_FACESを変更すると表示が実際の処理と食い違います。返り値のsegmentsから集計すると、判定が1か所に集約されます。♻️ 提案する修正
- modes = ["crop" if 1 <= c <= 2 else "letterbox" for c in info["modes"]] - n_crop = modes.count("crop") - n_letterbox = modes.count("letterbox") + n_crop = sum(end - start for start, end, mode in info["segments"] if mode == "crop") + n_letterbox = sum(end - start for start, end, mode in info["segments"] if mode != "crop") + n_frames = n_crop + n_letterboxこの場合、Line 41 の
len(modes)はn_framesに置き換えてください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@video-shorts/src/reframe_cli.py` around lines 37 - 39, Update the mode-counting logic in the CLI to derive crop and letterbox counts from the `mode` values in `info["segments"]`, reusing the processing result from `reframe.py` instead of the hard-coded `1 <= c <= 2` threshold. Replace the related `len(modes)` usage with `n_frames` so the displayed counts use the reported frame total.video-shorts/tests/reframe-verify-check.py (1)
145-170: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value対照実装が
reframeの非公開関数に依存しています。
reframe._crop_x_exprとreframe._extract_segmentは非公開名です。対照実装が製品の内部関数を共有すると、その関数自体が壊れた場合に「本実装」と「対照」が同じ方向へ壊れ、対照が検出器として働きません。将来的には、公開ヘルパーとして切り出すか、対照側で独立に crop フィルタ文字列を組み立てることを検討してください。現状は検証の粒度上許容できる範囲のため、今回の対応は任意です。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@video-shorts/tests/reframe-verify-check.py` around lines 145 - 170, In the comparison implementations build_decoy_pan and build_always_centroid, remove reliance on reframe._crop_x_expr and reframe._extract_segment by independently constructing the crop filter expression and invoking the segment extraction path. Keep the decoy and always-centroid behaviors unchanged while ensuring they remain independent of reframe’s private helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@video-shorts/src/reframe.py`:
- Around line 80-90: Replace the full-list buffering in read_frames with
one-frame-at-a-time processing, moving face detection and FaceTracker updates
into the read loop used by analyze_tracks. Retain only the resulting analysis
records and input dimensions, and remove any callers’ dependence on a complete
decoded-frame list while preserving existing tracking behavior.
- Around line 139-163: The continuous crop flow using _crop_x_expr currently
creates one nested expression per frame, so long segments can exceed FFmpeg
limits. Update the segment-processing logic around _crop_x_expr to split
supported long crop segments into bounded frame chunks, rebase each chunk’s crop
x values so its expression starts at n=0, render each chunk separately, and
concatenate the chunk outputs while preserving frame order.
In `@video-shorts/tests/reframe-fixtures-check.py`:
- Around line 63-68: Update the face-count assertion in the fixture check to
require len(frames) == rf.FRAMES alongside all(c == want for c in counts).
Extend the failure detail to report the actual decoded frame count, while
preserving the existing count-set reporting.
In `@video-shorts/tests/reframe-verify-check.py`:
- Around line 96-101: Update per_frame_diffs to require equal-length frames_a
and frames_b before comparing frames, rather than allowing zip to truncate
silently; raise or assert on a frame-count mismatch so callers such as
diffs_black preserve the intended [:30] comparison and cannot pass with dropped
frames, while satisfying Ruff B905.
- Around line 377-382: Update the AUDIO_CASES verification loop around
ffprobe_audio and the streams[0]["duration"] access to read duration safely with
.get(). Treat a missing duration as a failed check while preserving the existing
duration tolerance validation and diagnostic output, so later cases continue to
report results instead of raising KeyError.
- Around line 401-412: Update channel_pcm to detect ffmpeg failures and handle
empty output consistently, then use the existing or newly added fmt_hz helper
for None-safe frequency formatting in all three diagnostic messages involving
fl/fr, fl_s/fr_s, and fl_m/fr_m. Preserve the current pass/fail conditions while
preventing formatting errors when dominant_freq returns None.
---
Nitpick comments:
In `@video-shorts/package.json`:
- Line 5: Optionally split the package.json test command into focused scripts
such as test:fast and test:media, moving the expensive reframe-verify-check.py
and related media tests into the media group while preserving a full test script
that runs the complete suite for CI.
In `@video-shorts/src/reframe_cli.py`:
- Around line 37-39: Update the mode-counting logic in the CLI to derive crop
and letterbox counts from the `mode` values in `info["segments"]`, reusing the
processing result from `reframe.py` instead of the hard-coded `1 <= c <= 2`
threshold. Replace the related `len(modes)` usage with `n_frames` so the
displayed counts use the reported frame total.
In `@video-shorts/tests/reframe-verify-check.py`:
- Around line 145-170: In the comparison implementations build_decoy_pan and
build_always_centroid, remove reliance on reframe._crop_x_expr and
reframe._extract_segment by independently constructing the crop filter
expression and invoking the segment extraction path. Keep the decoy and
always-centroid behaviors unchanged while ensuring they remain independent of
reframe’s private helpers.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b288b2e8-5f52-4a9f-9d8d-2cc8dfb4a823
📒 Files selected for processing (7)
docs/roadmap.htmlvideo-shorts/package.jsonvideo-shorts/src/reframe.pyvideo-shorts/src/reframe_cli.pyvideo-shorts/tests/reframe-fixtures-check.pyvideo-shorts/tests/reframe-verify-check.pyvideo-shorts/tests/reframe_fixtures.py
| def read_frames(path: str): | ||
| """全フレームを cv2 で読み込む（顔検出専用。書き出しは常に ffmpeg 側で行う）。""" | ||
| cap = cv2.VideoCapture(path) | ||
| frames = [] | ||
| while True: | ||
| ok, f = cap.read() | ||
| if not ok: | ||
| break | ||
| frames.append(f) | ||
| cap.release() | ||
| return frames |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Analyze frames without retaining the full video.
read_frames() stores every decoded BGR frame until analyze_tracks() finishes. A 1080p frame uses about 6 MiB. A multi-minute input can consume tens of GiB and terminate the process.
Read, detect, and update FaceTracker one frame at a time. Retain only the analysis records and input dimensions.
Also applies to: 207-215
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 81-81: Docstring contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF002)
[warning] 81-81: Docstring contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF002)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@video-shorts/src/reframe.py` around lines 80 - 90, Replace the full-list
buffering in read_frames with one-frame-at-a-time processing, moving face
detection and FaceTracker updates into the read loop used by analyze_tracks.
Retain only the resulting analysis records and input dimensions, and remove any
callers’ dependence on a complete decoded-frame list while preserving existing
tracking behavior.
| def _crop_x_expr(cx_values, in_w: int, crop_w: int) -> str: | ||
| """区間内の各フレーム(相対フレーム番号 n=0..)のクロップ左端xを求める ffmpeg 式を作る。 | ||
|
|
||
| ffmpeg の crop フィルタでは `n` はそのフィルタに入ってきたフレームの通し番号 | ||
| (0始まり)を指す。trim で区間を切り出したあとに crop を掛けるので、n は | ||
| その区間内で 0 から数え直される。 | ||
|
|
||
| 注意(未確定・backlog): この実装は区間内のフレーム数だけ if(eq(n,k),...) を | ||
| ネストするため、区間が数千フレームを超えるような長尺のクロップ区間では | ||
| フィルタ文字列が非常に長くなる。今回のスコープ(30フレームの検証素材)では | ||
| 問題にならないが、フルパイプライン統合時は区間をチャンクに分割するなどの | ||
| 対策が必要になりうる(今回のスコープ外)。 | ||
| """ | ||
| max_x = max(0, in_w - crop_w) | ||
| xs = [] | ||
| for cx in cx_values: | ||
| x = int(round(cx - crop_w / 2.0)) | ||
| x = max(0, min(max_x, x)) | ||
| xs.append(x) | ||
| if len(set(xs)) == 1: | ||
| return str(xs[0]) | ||
| expr = str(xs[-1]) | ||
| for n in range(len(xs) - 2, -1, -1): | ||
| expr = f"if(eq(n\\,{n})\\,{xs[n]}\\,{expr})" | ||
| return expr |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python - <<'PY'
for frames in (1800, 3600, 5400):
expr = "0"
for n in range(frames - 2, -1, -1):
expr = f"if(eq(n\\,{n})\\,0\\,{expr})"
print(f"{frames} frames: -vf crop expression is {len(expr)} bytes")
PY
getconf ARG_MAX 2>/dev/null || trueRepository: rahiseko-alt/ai-editer
Length of output: 305
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Files matching reframe.py:\n'
fd -a 'reframe\.py$' . || true
file="$(fd 'reframe\.py$' . | head -n 1)"
if [ -n "${file:-}" ]; then
printf '\nRelevant reframe.py outline:\n'
ast-grep outline "$file" --view compact || true
printf '\nLines 1-280:\n'
cat -n "$file" | sed -n '1,280p'
fi
printf '\nCrop-segment usage / crop expression construction searches:\n'
rg -n "_crop_x_expr|crop_x|crop|segment|trim|run_pipe|ffmpeg|ffprobe" "$file" || true
printf '\nRelevant occurrences across repo:\n'
rg -n "_crop_x_expr|crop_x|crop frame|segment" . -g '!work/**' -g '!output/**' || trueRepository: rahiseko-alt/ai-editer
Length of output: 40475
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo 'tests/reframe-verify-check.py crop-related lines:'
cat -n video-shorts/tests/reframe-verify-check.py | sed -n '120,190p'
echo
echo 'pipeline/mkdir/entrypoint references for reframe:'
rg -n "reframe|REFRAME|g-edit-reframe" video-shorts -g '!work/**' -g '!output/**' || trueRepository: rahiseko-alt/ai-editer
Length of output: 9520
🌐 Web query:
FFmpeg expression parser if eq nested expression depth maximum if(eq(n, ...) if(eq(n, ...)
💡 Result:
In FFmpeg's expression parser (implemented in libavutil/eval.c), the maximum depth allowed for nested expressions, including functions like eq or if, is 100 [1][2]. As of recent updates to the codebase, the parser explicitly checks the depth of the expression tree [3][4]. When constructing an expression, the parser calculates the depth of each node by taking the maximum depth of its children plus one [1][5]. If this depth exceeds the defined constant MAX_DEPTH (currently 100), the parser will reject the expression and return an error (AVERROR(EINVAL)) [3][5]. This mechanism was implemented to prevent stack exhaustion issues [4].
Citations:
- 1: https://ffmpeg.org/doxygen/trunk/eval_8c_source.html
- 2: https://roundup.ffmpeg.org/doxygen/trunk/eval_8c.html
- 3: BtbN/FFmpeg@ed5040e
- 4: https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg187832.html
- 5: https://github.com/FFmpeg/FFmpeg/blob/master/libavutil/eval.c
🌐 Web query:
FFmpeg libavutil eval.c MAX_DEPTH if expression parser
💡 Result:
In FFmpeg's libavutil/eval.c, MAX_DEPTH is a preprocessor macro defined as 100 that serves as a safety limit for the recursive depth of the expression tree [1][2][3]. To prevent stack exhaustion during the parsing and evaluation of expressions, FFmpeg tracks the depth of the AVExpr tree [4]. Specifically: 1. The AVExpr struct includes a depth field [5][4][6]. 2. During the construction of expression nodes (via make_eval_expr) and the parsing of primary expressions (via parse_primary), the depth of the tree is calculated and compared against MAX_DEPTH [1][2][5]. 3. If the calculated depth exceeds 100, the parser will fail and return an error (AVERROR(EINVAL) or NULL) to prevent creating an excessively deep tree that could cause a stack overflow during recursive evaluation [1][2][5][4]. This mechanism was introduced to explicitly check the depth of the AVExpr tree, complementing existing checks on the parser's own stack depth [5][4].
Citations:
- 1: https://github.com/FFmpeg/FFmpeg/blob/master/libavutil/eval.c
- 2: https://ffmpeg.org/doxygen/trunk/eval_8c_source.html
- 3: https://ffmpeg.org/doxygen/trunk/eval_8c.html
- 4: BtbN/FFmpeg@ed5040e
- 5: https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg187832.html
- 6: https://svn.ffmpeg.org/doxygen/trunk/structAVExpr.html
🌐 Web query:
FFmpeg libavutil eval.h MAX_DEPTH 100
💡 Result:
In FFmpeg, MAX_DEPTH is a preprocessor definition set to 100 within libavutil/eval.c [1][2]. It serves as an arbitrary limit designed to prevent unbounded recursion when parsing and evaluating mathematical expressions [3]. Introduced to improve security and stability, this constant restricts the maximum depth of the expression tree (AVExpr) that can be processed [4]. When the parser encounters an expression that would result in a tree depth exceeding this limit (e.g., deeply nested function calls or operations), it will trigger an error (typically returning AVERROR(EINVAL)) and fail to parse or evaluate the expression [4]. This check is applied both during the construction of the expression tree and when evaluating sub-expressions [4].
Citations:
- 1: https://ffmpeg.org/doxygen/trunk/eval_8c_source.html
- 2: https://ffmpeg.org/doxygen/trunk/eval_8c.html
- 3: https://fossies.org/dox/ffmpeg-8.1.1/eval_8c_source.html
- 4: BtbN/FFmpeg@ed5040e
Bound the dynamic crop expression size.
A continuous crop segment builds one nested if(eq(n,...),...) level for every frame. On supported inputs, a long crop segment can exceed FFmpeg's filter expression depth limit or the process argument limit before rendering starts.
If supported inputs can contain long crop segments, split crop segments into bounded frame chunks and rebase the crop x expression for each chunk, then concat the segment outputs.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 155-155: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@video-shorts/src/reframe.py` around lines 139 - 163, The continuous crop flow
using _crop_x_expr currently creates one nested expression per frame, so long
segments can exceed FFmpeg limits. Update the segment-processing logic around
_crop_x_expr to split supported long crop segments into bounded frame chunks,
rebase each chunk’s crop x values so its expression starts at n=0, render each
chunk separately, and concatenate the chunk outputs while preserving frame
order.
| counts = [len(detect_faces(f, det)) for f in frames] | ||
| check( | ||
| f"RF-{kind}: 全{rf.FRAMES}コマの顔検出件数が{want}件で揃っている", | ||
| all(c == want for c in counts), | ||
| f"実際={sorted(set(counts))}", | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Assert the expected frame count before accepting face counts.
all(c == want for c in counts) returns True for an empty list and also accepts a truncated fixture when every remaining frame has the expected count. video-shorts/tests/reframe_fixtures.py, Lines 300-310, only compares the decoded count with the generated count; it does not compare either count with rf.FRAMES. A 29-frame fixture can therefore pass while the message claims that all 30 frames passed.
Require len(frames) == rf.FRAMES in this assertion and include the actual frame count in the failure detail.
Proposed fix
check(
f"RF-{kind}: 全{rf.FRAMES}コマの顔検出件数が{want}件で揃っている",
- all(c == want for c in counts),
- f"実際={sorted(set(counts))}",
+ len(frames) == rf.FRAMES and all(c == want for c in counts),
+ f"実際のコマ数={len(frames)}, 顔件数={sorted(set(counts))}",
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| counts = [len(detect_faces(f, det)) for f in frames] | |
| check( | |
| f"RF-{kind}: 全{rf.FRAMES}コマの顔検出件数が{want}件で揃っている", | |
| all(c == want for c in counts), | |
| f"実際={sorted(set(counts))}", | |
| ) | |
| counts = [len(detect_faces(f, det)) for f in frames] | |
| check( | |
| f"RF-{kind}: 全{rf.FRAMES}コマの顔検出件数が{want}件で揃っている", | |
| len(frames) == rf.FRAMES and all(c == want for c in counts), | |
| f"実際のコマ数={len(frames)}, 顔件数={sorted(set(counts))}", | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@video-shorts/tests/reframe-fixtures-check.py` around lines 63 - 68, Update
the face-count assertion in the fixture check to require len(frames) ==
rf.FRAMES alongside all(c == want for c in counts). Extend the failure detail to
report the actual decoded frame count, while preserving the existing count-set
reporting.
| def mean_abs_diff(a, b): | ||
| return float(np.abs(a.astype(np.int16) - b.astype(np.int16)).mean()) | ||
|
|
||
|
|
||
| def per_frame_diffs(frames_a, frames_b): | ||
| return [mean_abs_diff(a, b) for a, b in zip(frames_a, frames_b)] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
フレーム数の不一致が黙って見逃されます。
zip は短い方に合わせて打ち切ります。実装がコマを落とした場合、per_frame_diffs は残ったコマだけを比較し、D2/E は PASS のままになります。コマ数の一致自体を受入事実として測ってください。Ruff の B905 も同じ箇所を指しています。
🛡️ 提案する修正
def per_frame_diffs(frames_a, frames_b):
+ if len(frames_a) != len(frames_b):
+ raise AssertionError(f"フレーム数が一致しません: {len(frames_a)} vs {len(frames_b)}")
- return [mean_abs_diff(a, b) for a, b in zip(frames_a, frames_b)]
+ return [mean_abs_diff(a, b) for a, b in zip(frames_a, frames_b, strict=True)]Line 335 の diffs_black は black_frames を [:30] に切っているため、この追加チェックでも成立します。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def mean_abs_diff(a, b): | |
| return float(np.abs(a.astype(np.int16) - b.astype(np.int16)).mean()) | |
| def per_frame_diffs(frames_a, frames_b): | |
| return [mean_abs_diff(a, b) for a, b in zip(frames_a, frames_b)] | |
| def mean_abs_diff(a, b): | |
| return float(np.abs(a.astype(np.int16) - b.astype(np.int16)).mean()) | |
| def per_frame_diffs(frames_a, frames_b): | |
| if len(frames_a) != len(frames_b): | |
| raise AssertionError(f"フレーム数が一致しません: {len(frames_a)} vs {len(frames_b)}") | |
| return [mean_abs_diff(a, b) for a, b in zip(frames_a, frames_b, strict=True)] |
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 101-101: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@video-shorts/tests/reframe-verify-check.py` around lines 96 - 101, Update
per_frame_diffs to require equal-length frames_a and frames_b before comparing
frames, rather than allowing zip to truncate silently; raise or assert on a
frame-count mismatch so callers such as diffs_black preserve the intended [:30]
comparison and cannot pass with dropped frames, while satisfying Ruff B905.
Source: Linters/SAST tools
| for leaf, kind, label in AUDIO_CASES: | ||
| out_path = p(f"{kind}-out.mp4") | ||
| streams = ffprobe_audio(out_path) | ||
| ok = len(streams) >= 1 and abs(float(streams[0]["duration"]) - 1.000) <= 0.05 | ||
| dur = streams[0]["duration"] if streams else "N/A" | ||
| check(f"{leaf}({label}): 音声ストリームが存在し長さ1.000秒±0.05秒", ok, f"streams={len(streams)} duration={dur}") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
duration が無いストリームで KeyError になります。
ffprobe の stream=duration は、コンテナや音声コーデックによっては出力されません。その場合 Line 380 は FAIL ではなく KeyError で異常終了し、G/G2/G3 の結果が出ません。.get() で受けて、欠落時は不合格として数えてください。
🛡️ 提案する修正
- ok = len(streams) >= 1 and abs(float(streams[0]["duration"]) - 1.000) <= 0.05
- dur = streams[0]["duration"] if streams else "N/A"
+ dur = streams[0].get("duration") if streams else None
+ ok = dur is not None and abs(float(dur) - 1.000) <= 0.05
+ dur = dur if dur is not None else "N/A"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for leaf, kind, label in AUDIO_CASES: | |
| out_path = p(f"{kind}-out.mp4") | |
| streams = ffprobe_audio(out_path) | |
| ok = len(streams) >= 1 and abs(float(streams[0]["duration"]) - 1.000) <= 0.05 | |
| dur = streams[0]["duration"] if streams else "N/A" | |
| check(f"{leaf}({label}): 音声ストリームが存在し長さ1.000秒±0.05秒", ok, f"streams={len(streams)} duration={dur}") | |
| for leaf, kind, label in AUDIO_CASES: | |
| out_path = p(f"{kind}-out.mp4") | |
| streams = ffprobe_audio(out_path) | |
| dur = streams[0].get("duration") if streams else None | |
| ok = dur is not None and abs(float(dur) - 1.000) <= 0.05 | |
| dur = dur if dur is not None else "N/A" | |
| check(f"{leaf}({label}): 音声ストリームが存在し長さ1.000秒±0.05秒", ok, f"streams={len(streams)} duration={dur}") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@video-shorts/tests/reframe-verify-check.py` around lines 377 - 382, Update
the AUDIO_CASES verification loop around ffprobe_audio and the
streams[0]["duration"] access to read duration safely with .get(). Treat a
missing duration as a failed check while preserving the existing duration
tolerance validation and diagnostic output, so later cases continue to report
results instead of raising KeyError.
| fl = dominant_freq(left) | ||
| fr = dominant_freq(right) | ||
| ok = fl is not None and fr is not None and abs(fl - 440) <= 5 and abs(fr - 880) <= 5 | ||
| check(f"{leaf}({label}): 左440Hz±5Hz・右880Hz±5Hzの両方を満たす", ok, f"左={fl:.2f}Hz 右={fr:.2f}Hz") | ||
|
|
||
| # 対照1: 左右チャンネルを入れ替えた実装 | ||
| swapped = p(f"{kind}-swapped.mp4") | ||
| run_ffmpeg(["-i", out_path, "-c:v", "copy", "-af", "pan=stereo|c0=c1|c1=c0", "-c:a", "aac", "-b:a", "128k", swapped]) | ||
| fl_s = dominant_freq(channel_pcm(swapped, 0)) | ||
| fr_s = dominant_freq(channel_pcm(swapped, 1)) | ||
| swap_ok = fl_s is not None and fr_s is not None and abs(fl_s - 440) <= 5 and abs(fr_s - 880) <= 5 | ||
| check(f"{leaf}対照(左右入替え): 不合格になる", not swap_ok, f"左={fl_s:.2f}Hz 右={fr_s:.2f}Hz") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
支配周波数が None のとき、書式化で TypeError になります。
dominant_freq は、サンプルが0個のとき None を返します。ok の判定は None を考慮していますが、Line 404 の f"左={fl:.2f}Hz" は None を書式化できず TypeError で落ちます。同じ問題は Line 412 の fl_s/fr_s と Line 424 の fl_m/fr_m にもあります。根本原因は channel_pcm(Line 126-133)が ffmpeg の終了コードを確認せず、失敗時に空配列を返す点です。書式化を None 安全にし、channel_pcm で失敗を検出してください。
🛡️ 提案する修正
+def fmt_hz(v):
+ return "N/A" if v is None else f"{v:.2f}Hz" def channel_pcm(path, channel_index, sr=48000):
args = [
"ffmpeg", "-v", "error", "-i", path,
"-af", f"pan=mono|c0=c{channel_index}",
"-f", "f32le", "-ar", str(sr), "-",
]
- out = subprocess.run(args, capture_output=True).stdout
- return np.frombuffer(out, dtype=np.float32)
+ proc = subprocess.run(args, capture_output=True)
+ if proc.returncode != 0:
+ raise RuntimeError(f"音声抽出に失敗しました: {path}\n{proc.stderr[-2000:].decode(errors='replace')}")
+ return np.frombuffer(proc.stdout, dtype=np.float32)書式化側は3か所とも fmt_hz を使ってください。
- check(f"{leaf}({label}): 左440Hz±5Hz・右880Hz±5Hzの両方を満たす", ok, f"左={fl:.2f}Hz 右={fr:.2f}Hz")
+ check(f"{leaf}({label}): 左440Hz±5Hz・右880Hz±5Hzの両方を満たす", ok, f"左={fmt_hz(fl)} 右={fmt_hz(fr)}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@video-shorts/tests/reframe-verify-check.py` around lines 401 - 412, Update
channel_pcm to detect ffmpeg failures and handle empty output consistently, then
use the existing or newly added fmt_hz helper for None-safe frequency formatting
in all three diagnostic messages involving fl/fr, fl_s/fr_s, and fl_m/fr_m.
Preserve the current pass/fail conditions while preventing formatting errors
when dominant_freq returns None.
マスターの終了指示によりCodeRabbit指摘7件の修正エージェントをTaskStopで 中断した。未検証の変更はcommitせずgit stashへ退避(次セッションでpop後に 実測値の非回帰を確認して再開)。詳細をdocs/failures.mdに記録し、 meta.handoff/meta.nextを次セッション向けに更新した。
2 participants
概要
docs/roadmap.htmlのG-EDIT-REFRAME（縦型切り抜き）サブツリーについて、基準凍結から実装・独立検証までを行った。変更内容
docs/roadmap.html: G-EDIT-REFRAMEの基準凍結（A/B/D/E是正、D2/G/G2/G3/H/H2/H3新設、status: todo→doing）video-shorts/src/reframe.py（新規）: 顔検出・追跡による縦型クロップ本体。face_mosaic.pyのYuNet検出器(SCORE_THRESHOLD=0.6, NMS_THRESHOLD=0.3)とFaceTracker(hold_frames=8)を再利用し、保持後のトラック数が1〜2人ならffmpegのcropフィルタで追従クロップ、0人または3人以上ならrender-vertical.mjsと同じletterbox(scale+pad)方式へ自動切替え。音声は-map 1:a? -c:a copyで無変換コピー。video-shorts/src/reframe_cli.py（新規）: CLIラッパー。video-shorts/tests/reframe-verify-check.py（新規）: 11 leafのverify手順と対照実装を実際に実行する自動テスト（34 PASS/0 FAIL）。package.jsonのtestチェーンに登録。video-shorts/tests/reframe_fixtures.py/reframe-fixtures-check.py: 検証素材(RF-R/RF-N/RF-M、音声付き版RF-R-A/RF-N-A/RF-M-A)の生成・自己検査。テスト計画
pnpm --filter video-shorts test（674 PASS / 0 FAIL、うち新規34件含む）node scripts/verify-roadmap-evidence.mjsBASE_REF=origin/main node scripts/verify-criteria-freeze.mjsGenerated by Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation