Repository navigation
M-3: 隠しそこねを見つけて直せるようにし、処理を高速化する - #30
Conversation
【M-3】 - M-3-A review_frames/write_review_images: 顔を見失って直前位置で埋めた区間を 確認対象として拾い出し、原寸の静止画として書き出す。全編を目視させるのは 非現実的なので、機械が既に把握している「危ない場面」だけを人に見せる - M-3-B apply_manual_masks: 時刻と位置を指定して後から隠しを足す。自動処理の 取りこぼしを作り直さずに塞ぐ経路。手で足した隠しで実際に顔が検出されなくなる ことまでテストで確認する（位置を書いただけで終わらせない） 【処理速度】 検出を2パス化した。1周目で数フレームおきに検出して各トラックの位置を出し、 2周目で全フレームへ焼く。当初は単一パスで実装したが、最初の区間には「次の検出」が 未来にあって補間できず、枠が直前位置に取り残されて素顔が出たため改めた。 検出だけは縮小した画で行う。**出力の解像度・画質は一切変えていない** （顔以外の画素が1つも書き換わらないことをテストで確認している）。 縮小による唯一の劣化は「小さく写った顔の取りこぼし」で、1080p素材での実測は: 検出幅1920 -> 顔高さ20pxまで検出 / 60秒の検出に111.8秒 検出幅1280 -> 25pxまで / 55.5秒 検出幅 960 -> 30pxまで / 30.3秒 ← 既定 検出幅 640 -> 45pxまで / 13.4秒 検出幅 480 -> 65pxまで / 8.3秒 検出幅 320 -> 95pxまで / 4.2秒 既定の960は、1080pで顔高さ30px(画面高の約2.8%＝かなり遠くの通行人相当)まで拾う。 間引きの影響も実測した。顔高さ245pxで動きの速さを変えると、3/8/15px per frame では 間引き1〜5のいずれでも素顔は0/40コマ。30px per frame(1秒で画面をほぼ横断)で 間引き5のときだけ1/40コマ。実素材は15px per frame 以下に収まるため既定3で取りこぼしは出ない。 これらの実測表は face_mosaic.py の定数コメントに根拠として残した。 テストは29件->39件。出力画質が落ちていないことの検証項目も追加した。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg
📝 WalkthroughWalkthroughThe face mosaic pipeline now supports configurable detection scaling and frame intervals, followed by full-resolution rendering with track interpolation. The change also adds review-image export, manual masks, related tests, and updated roadmap evidence. ChangesFace mosaic workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant mosaic_frames
participant Detector
participant Tracker
participant Renderer
mosaic_frames->>Detector: detect selected scaled frames
Detector->>Tracker: provide restored face boxes
Tracker->>mosaic_frames: return track assignments
mosaic_frames->>Renderer: interpolate tracks across full-resolution frames
Renderer-->>mosaic_frames: return mosaiced frames
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
M-3-A/B の criteria は tests/face-mosaic-check.py で機械判定できるため、 CI(quality ジョブ)を審判とし、その run URL を evidence に書いた。 evidence: https://github.com/rahiseko-alt/ai-editer/actions/runs/30878151890 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
video-shorts/src/face_mosaic.py (1)
307-339: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject non-positive
detect_everyvalues.Line 339 accepts a negative value.
range()then produces no keyframes. The second pass returns unmasked frames.Validate
detect_every >= 1before detection starts.Proposed fix
def mosaic_frames(frames, hold_frames: int = HOLD_FRAMES_DEFAULT, ratio: float = BLOCK_RATIO_DEFAULT, detector=None, block_for=None, people=None, ratio_for=None, recognizer=None, detect_every: int = DETECT_EVERY_DEFAULT, detect_width: int = DETECT_WIDTH_DEFAULT): + if detect_every < 1: + raise ValueError("detect_every must be at least 1") + tracker = FaceTracker(hold_frames=hold_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/face_mosaic.py` around lines 307 - 339, Validate detect_every at the start of the frame-processing function, before the detection loop and before any frames can be returned, and reject values less than 1. Preserve the existing processing behavior for valid positive intervals.
🤖 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/face_mosaic.py`:
- Around line 379-382: Update the frame-processing logic around interpolate and
tracker.held_frames to record every tail frame when the total frame count is not
divisible by detect_every and the reused keyframe box a is non-empty. Preserve
existing interpolation behavior, and add a test covering non-divisible frame
counts to verify those stale-position frames appear in review_frames().
- Around line 414-423: Update write_review_images to check the boolean result
returned by cv2.imwrite; raise OSError when writing fails, and append path to
written only after a successful write.
In `@video-shorts/tests/face-mosaic-check.py`:
- Around line 500-507: Update the M-3-B test around apply_manual_masks to use
fixed non-uniform content and verify that both boundary frames, manual[4] and
manual[6], differ from their pre-mask regions at BOX. Retain the
outside-interval assertions and ensure the checks still confirm frames 0 and 9
remain unchanged.
- Around line 530-538: Update the face-preservation check around mask_free to
build a Boolean mask covering the full frame, exclude the detected face box
using fx, fy, fw2, and fh2 (including the existing padding if intended), and
compare every remaining pixel between out_q[0] and frames[0]. Preserve the
assertion that all pixels outside the face region remain unchanged.
---
Outside diff comments:
In `@video-shorts/src/face_mosaic.py`:
- Around line 307-339: Validate detect_every at the start of the
frame-processing function, before the detection loop and before any frames can
be returned, and reject values less than 1. Preserve the existing processing
behavior for valid positive intervals.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: efa20305-c571-4dbc-b1a7-bd826bab4364
📒 Files selected for processing (3)
docs/roadmap.htmlvideo-shorts/src/face_mosaic.pyvideo-shorts/tests/face-mosaic-check.py
| k0 = (i // detect_every) * detect_every | ||
| a = keys.get(k0, {}) | ||
| b = keys.get(k0 + detect_every) | ||
| draw = interpolate(a, b, (i - k0) / detect_every) if b else a |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Record held tail frames for review.
When the frame count is not divisible by detect_every, Line 382 applies the last keyframe box to tail frames. Those frames are not added to tracker.held_frames. review_frames() therefore omits frames that use a stale position and can contain an uncovered face.
Record each tail frame that reuses a non-empty a box. Add a non-divisible-frame test for this path.
Proposed fix
k0 = (i // detect_every) * detect_every
a = keys.get(k0, {})
b = keys.get(k0 + detect_every)
- draw = interpolate(a, b, (i - k0) / detect_every) if b else a
+ if b is None:
+ draw = a
+ if a and i > k0:
+ tracker.held_frames.append(i)
+ else:
+ draw = interpolate(a, b, (i - k0) / detect_every)🤖 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/face_mosaic.py` around lines 379 - 382, Update the
frame-processing logic around interpolate and tracker.held_frames to record
every tail frame when the total frame count is not divisible by detect_every and
the reused keyframe box a is non-empty. Preserve existing interpolation
behavior, and add a test covering non-divisible frame counts to verify those
stale-position frames appear in review_frames().
| def write_review_images(frames, indices, out_dir, prefix="review"): | ||
| """確認用の静止画を書き出す（M-3-A）。書き出したパスを返す。""" | ||
| os.makedirs(out_dir, exist_ok=True) | ||
| written = [] | ||
| for i in indices: | ||
| if not (0 <= i < len(frames)): | ||
| continue | ||
| path = os.path.join(out_dir, f"{prefix}-{i:06d}.png") | ||
| cv2.imwrite(path, frames[i]) | ||
| written.append(path) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import os
import tempfile
import cv2
import numpy as np
with tempfile.TemporaryDirectory() as tmp:
target = os.path.join(tmp, "missing", "review.png")
result = cv2.imwrite(target, np.zeros((2, 2, 3), dtype=np.uint8))
print(f"cv2={cv2.__version__}, result={result}, exists={os.path.exists(target)}")
PYRepository: rahiseko-alt/ai-editer
Length of output: 276
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target function ---'
sed -n '390,435p' video-shorts/src/face_mosaic.py
printf '%s\n' '--- call sites ---'
rg -n -C 4 'write_review_images\(' video-shorts
printf '%s\n' '--- related export/error handling ---'
rg -n -C 3 'cv2\.imwrite|review|M-3-A' video-shorts/src video-shorts/tests 2>/dev/null || trueRepository: rahiseko-alt/ai-editer
Length of output: 10787
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
root = Path("video-shorts")
target = root / "src" / "face_mosaic.py"
tree = ast.parse(target.read_text(), filename=str(target))
for path in sorted(root.rglob("*.py")):
try:
module = ast.parse(path.read_text(), filename=str(path))
except SyntaxError as exc:
print(f"syntax-error {path}: {exc}")
continue
for node in ast.walk(module):
if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) and node.func.id == "write_review_images":
print(f"call {path}:{node.lineno}")
if isinstance(node, ast.ImportFrom) and node.module == "face_mosaic":
names = {a.name for a in node.names}
if "write_review_images" in names:
print(f"import {path}:{node.lineno}")
print("target-definition-count", sum(
isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == "write_review_images"
for node in ast.walk(tree)
))
PY
printf '%s\n' '--- module entry points and review workflow references ---'
rg -n -C 3 'face_mosaic|write_review_images|review_frames|_review_tmp|M-3-A' video-shorts --glob '!tests/face-mosaic-check.py'Repository: rahiseko-alt/ai-editer
Length of output: 5904
🌐 Web query:
OpenCV official documentation cv::imwrite returns false if image cannot be written
💡 Result:
Yes, the official OpenCV documentation and source code confirm that the cv::imwrite function returns a boolean value indicating whether the image was successfully written to the specified file [1]. Specifically, it returns true if the image is successfully written and false otherwise [1]. If the function fails to write the image—for example, due to an invalid file path, insufficient write permissions, or an unsupported file format—it will return false [2]. It is recommended to check this return value to ensure the operation succeeded, as imwrite may fail silently in certain scenarios (such as when the target directory does not exist) rather than throwing an exception [2]. In addition to checking the return value, it is also good practice to wrap the call in a try-catch block to handle potential cv::Exception occurrences [3][2].
Citations:
- 1: https://github.com/opencv/opencv/blob/335abd23/modules/imgcodecs/include/opencv2/imgcodecs.hpp
- 2: https://stackoverflow.com/questions/24624995/imwrite-does-not-create-image
- 3: imwrite return value documentation opencv/opencv#25309
Check the result of cv2.imwrite. If it returns False, raise OSError before adding path to written. Otherwise, callers receive a path for an image that was not written.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 415-415: Docstring contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF002)
[warning] 415-415: 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/face_mosaic.py` around lines 414 - 423, Update
write_review_images to check the boolean result returned by cv2.imwrite; raise
OSError when writing fails, and append path to written only after a successful
write.
| apply_manual_masks(manual, [{"start": 4, "end": 6, "box": BOX}]) | ||
| check( | ||
| "M-3-B: 指定した区間の指定位置に、後から隠しを足せる", | ||
| not np.array_equal(before_manual, manual[5][region]), | ||
| ) | ||
| check( | ||
| "M-3-B: 指定した区間の外は変わらない", | ||
| np.array_equal(manual[0], frames[0]) and np.array_equal(manual[9], frames[9]), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test both inclusive interval boundaries.
Line 500 masks frames 4 through 6. The assertions only inspect frame 5. A regression that excludes start or end will still pass.
Use fixed non-uniform test content and assert that frames 4 and 6 change at the specified box.
🤖 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/face-mosaic-check.py` around lines 500 - 507, Update the
M-3-B test around apply_manual_masks to use fixed non-uniform content and verify
that both boundary frames, manual[4] and manual[6], differ from their pre-mask
regions at BOX. Retain the outside-interval assertions and ensure the checks
still confirm frames 0 and 9 remain unchanged.
| # 顔の外側は1画素も書き換わっていないこと（＝背景が再圧縮・再サンプルされていない） | ||
| face_box = detect_faces(frames[0], det)[0] | ||
| fx, fy, fw2, fh2 = [int(v) for v in face_box] | ||
| pad = int(fw2 * 0.6) | ||
| mask_free = (slice(0, max(0, fy - pad)), slice(None)) | ||
| check( | ||
| "顔以外の領域は1画素も書き換わらない（背景の画質が落ちない）", | ||
| np.array_equal(out_q[0][mask_free], frames[0][mask_free]), | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Check every pixel outside the face box.
mask_free contains only rows above the face. Changes beside or below the face will pass this test, despite the assertion text and roadmap claim that no non-face pixel changes.
Build a Boolean mask for the complete frame, exclude the face box, and compare all remaining pixels.
Proposed fix
- mask_free = (slice(0, max(0, fy - pad)), slice(None))
+ outside_face = np.ones(frames[0].shape[:2], dtype=bool)
+ outside_face[fy : fy + fh2, fx : fx + fw2] = False
check(
"顔以外の領域は1画素も書き換わらない（背景の画質が落ちない）",
- np.array_equal(out_q[0][mask_free], frames[0][mask_free]),
+ np.array_equal(out_q[0][outside_face], frames[0][outside_face]),
)📝 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.
| # 顔の外側は1画素も書き換わっていないこと（＝背景が再圧縮・再サンプルされていない） | |
| face_box = detect_faces(frames[0], det)[0] | |
| fx, fy, fw2, fh2 = [int(v) for v in face_box] | |
| pad = int(fw2 * 0.6) | |
| mask_free = (slice(0, max(0, fy - pad)), slice(None)) | |
| check( | |
| "顔以外の領域は1画素も書き換わらない（背景の画質が落ちない）", | |
| np.array_equal(out_q[0][mask_free], frames[0][mask_free]), | |
| ) | |
| # 顔の外側は1画素も書き換わっていないこと（＝背景が再圧縮・再サンプルされていない） | |
| face_box = detect_faces(frames[0], det)[0] | |
| fx, fy, fw2, fh2 = [int(v) for v in face_box] | |
| pad = int(fw2 * 0.6) | |
| outside_face = np.ones(frames[0].shape[:2], dtype=bool) | |
| outside_face[fy : fy + fh2, fx : fx + fw2] = False | |
| check( | |
| "顔以外の領域は1画素も書き換わらない（背景の画質が落ちない）", | |
| np.array_equal(out_q[0][outside_face], frames[0][outside_face]), | |
| ) |
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 530-530: Comment contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
[warning] 530-530: Comment contains ambiguous ＝ (FULLWIDTH EQUALS SIGN). Did you mean = (EQUALS SIGN)?
(RUF003)
[warning] 530-530: Comment contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
[warning] 536-536: String contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF001)
[warning] 536-536: String contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF001)
🤖 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/face-mosaic-check.py` around lines 530 - 538, Update the
face-preservation check around mask_free to build a Boolean mask covering the
full frame, exclude the detected face box using fx, fy, fw2, and fh2 (including
the existing padding if intended), and compare every remaining pixel between
out_q[0] and frames[0]. Preserve the assertion that all pixels outside the face
region remain unchanged.
* M-3: レビュー指摘5件を是正する（保護に穴が開く2件を含む） PR #30 のレビュー指摘が、マージ前に反映しきれず main に入ったため追いかけて是正する。 【保護に穴が開いていた2件】 - detect_every に0以下を渡すと検出が1回も走らず、モザイクの掛かっていない フレームがそのまま返っていた。1未満は理由を示して止めるようにした - フレーム数が detect_every で割り切れないときの末尾コマは、次の検出が無いため 直前の位置を流用する＝枠が古い。ここが確認対象(held_frames)に入っておらず、 素顔が残っていても人に示されないままだった。記録するようにした 【黙って失敗していた1件】 - cv2.imwrite は失敗しても例外を出さず False を返す。書けていないパスを 「書き出した」として返していたため、確認したつもりの取りこぼしが生まれうる。 失敗時は OSError にした 【検証が主張を裏付けていなかった2件】 - 手動指定の区間テストが中央のコマしか見ておらず、開始か終了が抜けても緑になった。 両端を含むことを両端で確かめるようにした - 「顔以外は1画素も書き換わらない」の検証が顔より上の行しか見ておらず、左右・下側が 書き換わっても緑になった。フレーム全体から実際にモザイクを掛けた枠だけを除いて 全画素を比較するようにした。なお除外範囲は原寸で検出し直した枠ではなく 「パイプラインが実際に使った枠」から作る（縮小画で検出するため座標が僅かに違い、 別物を除外すると検証にならない） いずれも「黙って保護が外れる」種類の穴なので、回帰テストを3件足した。 テストは 39件 -> 44件。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg * roadmap: PR #31 の handoff を更新し、失敗2件を failures.md へ追記 roadmap-required が落ちたため。AGENTS.md の「全PRは docs/roadmap.html を必ず 更新する（例外なし）」を、是正PRでは不要と暗黙に読み替えていた。 あわせて本セッションの失敗2件を docs/failures.md へ append した。 (1) レビュー指摘の反映前にPRがマージされ保護に穴が開いたまま main に入った (2) コード修正だけのPRで roadmap-required が落ちた Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
何をしたか
ロードマップ
M-3（隠しそこねを見つけて直せる）の2葉と、M-5-Bに向けた処理速度の改善です。review_frames/write_review_images— 顔を見失って直前位置で埋めた区間を拾い出し、原寸の静止画として書き出すapply_manual_masks— 時刻と位置を指定して隠しを足す全編を目視させるのは非現実的なので、機械が既に把握している「危ない場面」だけを人に見せる作りです。手で足した隠しで実際に顔が検出されなくなることまでテストしています（位置を書いただけで終わらせない）。
出力画質は落としていません
検出を2パス化し、検出だけ縮小した画で行うようにしました。モザイクは原寸フレームに焼いています。これをテストで固定しました。
ただし別種の劣化が1つあります（実測値）
検出用の縮小は小さく写った顔を取りこぼします。1080p素材での実測:
既定の960は、1080pで顔高さ30px（画面高の約2.8%＝かなり遠くの通行人相当）まで拾います。より小さい顔まで隠したい素材では
detect_widthを上げれば原寸まで戻せます。検出の間引きも測りました。顔の動きが 3 / 8 / 15 px per frame では間引き1〜5のいずれでも素顔は 0/40コマ。30px per frame（1秒で画面をほぼ横断）で間引き5のときだけ 1/40コマ。実素材は15px per frame 以下に収まるため、既定の間引き3で取りこぼしは出ません。
両方の実測表は
face_mosaic.pyの定数コメントに根拠として残しています。実装中に踏んだ問題（解消済み）
検証
テストは 29件 → 39件（全PASS）。ワークスペース全体も緑です。
M-3-A/M-3-Bのstatusはまだdoingです。CI が緑になってから run URL を evidence にしてdoneにします。Generated by Claude Code
Summary by CodeRabbit