feat: add webcam-to-ASCII pipeline#26
Open
mikedotexe wants to merge 5 commits into
Open
Conversation
Add camera_ascii.py for capturing webcam frames and rendering them as colored ANSI art via RASCII. Supports live mode, mirror, auto terminal width, block/emoji charsets, and background color fill. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Reviewer's GuideAdds a new Python CLI script Sequence diagram for live webcam-to-ASCII pipelinesequenceDiagram
actor User
participant camera_ascii as camera_ascii_py
participant OpenCV as OpenCV_cv2
participant FS as Filesystem
participant RASCII as RASCII_binary
participant Term as Terminal
User->>camera_ascii: run python camera_ascii.py --live
camera_ascii->>OpenCV: VideoCapture(camera_index)
OpenCV-->>camera_ascii: camera_handle
loop warmup_frames
camera_ascii->>OpenCV: read()
OpenCV-->>camera_ascii: frame
end
camera_ascii->>Term: hide cursor and clear screen
loop live_mode
camera_ascii->>OpenCV: read()
OpenCV-->>camera_ascii: frame
alt mirror_enabled
camera_ascii->>OpenCV: flip(frame, horizontal)
OpenCV-->>camera_ascii: mirrored_frame
else mirror_disabled
camera_ascii-->>camera_ascii: use original frame
end
camera_ascii->>FS: write frame to temp_jpg
FS-->>camera_ascii: temp_path
camera_ascii->>RASCII: rascii temp_path -w width -c/-b/-i/-C
RASCII-->>camera_ascii: ansi_art
camera_ascii->>Term: cursor home, write ansi_art, clear below
camera_ascii-->>camera_ascii: sleep to maintain fps
end
User-->>camera_ascii: Ctrl+C
camera_ascii->>OpenCV: release()
camera_ascii->>FS: delete temp_jpg
camera_ascii->>Term: show cursor
Flowchart for camera_ascii CLI control flowflowchart TD
A[start] --> B["Parse command line arguments"]
B --> C{Image_path_provided}
C -->|yes| D["Render static image via RASCII"]
D --> E[end]
C -->|no| F["Open webcam with OpenCV VideoCapture"]
F --> G{Camera_opened}
G -->|no| H["Print error and exit"]
H --> E
G -->|yes| I["Warmup loop: read 30 frames for auto_exposure"]
I --> J{Live_mode_enabled}
J -->|no| K["Single_shot: grab frame (optional mirror)"]
K --> L{Save_path_provided}
L -->|yes| M["Write frame to save_path"]
L -->|no| N["Skip saving"]
M --> O["Write frame to temp_file"]
N --> O
O --> P["Call RASCII with width_color_background_invert_charset"]
P --> Q["Print ANSI output once"]
Q --> R["Release camera, delete temp_file"]
R --> E
J -->|yes| S["Live_loop"]
S --> T["Grab frame from webcam"]
T --> U{Mirror_enabled}
U -->|yes| V["Flip frame horizontally"]
U -->|no| W["Use original frame"]
V --> X["Write frame to temp_file"]
W --> X
X --> Y["Call RASCII to render frame"]
Y --> Z["Move cursor home, print ANSI, clear below"]
Z --> AA["Sleep to maintain target fps"]
AA --> AB{Interrupted_by_Ctrl_C}
AB -->|no| T
AB -->|yes| R
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}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
renderfunction swallows RASCII failures by printing to stderr and returning an empty string; consider either propagating the error (raising or exiting) or returning a structured error so callers can handle failure explicitly, especially in live mode where silent failures could be confusing. - The hard-coded
./target/release/rasciipath inrendermakes the script fragile when run from other working directories; consider accepting the binary path as a CLI flag or resolving it relative to the script location.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `render` function swallows RASCII failures by printing to stderr and returning an empty string; consider either propagating the error (raising or exiting) or returning a structured error so callers can handle failure explicitly, especially in live mode where silent failures could be confusing.
- The hard-coded `./target/release/rascii` path in `render` makes the script fragile when run from other working directories; consider accepting the binary path as a CLI flag or resolving it relative to the script location.
## Individual Comments
### Comment 1
<location path="camera_ascii.py" line_range="78-79" />
<code_context>
+ help="Disable horizontal flip (webcam mirrors by default)")
+ parser.add_argument("--live", action="store_true",
+ help="Continuous live view (Ctrl+C to stop)")
+ parser.add_argument("--fps", type=float, default=4,
+ help="Target frames per second in live mode (default: 4)")
+ args = parser.parse_args()
+
</code_context>
<issue_to_address>
**issue:** Validate that --fps is strictly positive to avoid ZeroDivisionError and confusing behavior.
`fps` is passed to `_live_loop`, where `interval = 1.0 / fps` will raise on `--fps 0`, and negative values will cause incorrect timing. Please enforce `fps > 0` (via a custom type or explicit check) and fail fast with a clear error message for invalid values.
</issue_to_address>
### Comment 2
<location path="camera_ascii.py" line_range="40" />
<code_context>
+ cmd.append("-i")
+ if charset:
+ cmd.extend(["-C", charset])
+ result = subprocess.run(cmd, capture_output=True, text=True)
+ if result.returncode != 0:
+ print(f"RASCII error: {result.stderr}", file=sys.stderr)
</code_context>
<issue_to_address>
**issue:** Handle missing RASCII binary (FileNotFoundError) to provide a clearer error path.
If `./target/release/rascii` is missing or not executable, `subprocess.run` will raise `FileNotFoundError` before you can inspect `returncode`, causing an uncaught exception. Consider wrapping the call in `try/except FileNotFoundError` and emitting a clear message (e.g. "rascii executable not found") with a defined fallback (such as returning an empty string or exiting) for a controlled failure, especially in live mode.
</issue_to_address>
### Comment 3
<location path="camera_ascii.py" line_range="101-103" />
<code_context>
+ print(f"Error: could not open camera {args.camera}", file=sys.stderr)
+ sys.exit(1)
+
+ # Warmup: let auto-exposure settle
+ for _ in range(30):
+ cap.read()
+
+ mirror = not args.no_mirror
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider checking the return value during the warmup loop to avoid masking camera failures.
Here the warmup loop drops the `ret` flag from `cap.read()`, whereas `_grab` treats a failed read as fatal. If the camera can’t provide frames, this loop will just spin and only fail later. Please add the same check as in `_grab` (or otherwise break/abort on failure) so we fail fast and keep behavior consistent.
Suggested implementation:
```python
# Warmup: let auto-exposure settle
# Use the same grab logic as elsewhere so we fail fast on camera read errors
for _ in range(30):
_grab(cap)
```
This change assumes there is a `_grab(cap)` helper in the same module that:
1. Calls `cap.read()`
2. Checks the return flag
3. Exits or raises on failure
If `_grab` has a different signature or side effects (e.g., returns a frame that must be used), adjust the call accordingly, for example:
- If `_grab` requires additional parameters, pass them in the warmup loop as well.
- If `_grab` returns a frame and the function enforces usage, you may need to bind the return value (e.g., `frame = _grab(cap)`), even if the frame is discarded afterwards.
If `_grab` does not yet exist, you should instead:
1. Factor out the existing camera-read-and-check logic from wherever it currently is into a `_grab` function.
2. Call `_grab` both in the warmup loop and where the read previously occurred so behavior stays consistent.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+78
to
+79
| parser.add_argument("--fps", type=float, default=4, | ||
| help="Target frames per second in live mode (default: 4)") |
Contributor
There was a problem hiding this comment.
issue: Validate that --fps is strictly positive to avoid ZeroDivisionError and confusing behavior.
fps is passed to _live_loop, where interval = 1.0 / fps will raise on --fps 0, and negative values will cause incorrect timing. Please enforce fps > 0 (via a custom type or explicit check) and fail fast with a clear error message for invalid values.
| cmd.append("-i") | ||
| if charset: | ||
| cmd.extend(["-C", charset]) | ||
| result = subprocess.run(cmd, capture_output=True, text=True) |
Contributor
There was a problem hiding this comment.
issue: Handle missing RASCII binary (FileNotFoundError) to provide a clearer error path.
If ./target/release/rascii is missing or not executable, subprocess.run will raise FileNotFoundError before you can inspect returncode, causing an uncaught exception. Consider wrapping the call in try/except FileNotFoundError and emitting a clear message (e.g. "rascii executable not found") with a defined fallback (such as returning an empty string or exiting) for a controlled failure, especially in live mode.
Comment on lines
+101
to
+103
| # Warmup: let auto-exposure settle | ||
| for _ in range(30): | ||
| cap.read() |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Consider checking the return value during the warmup loop to avoid masking camera failures.
Here the warmup loop drops the ret flag from cap.read(), whereas _grab treats a failed read as fatal. If the camera can’t provide frames, this loop will just spin and only fail later. Please add the same check as in _grab (or otherwise break/abort on failure) so we fail fast and keep behavior consistent.
Suggested implementation:
# Warmup: let auto-exposure settle
# Use the same grab logic as elsewhere so we fail fast on camera read errors
for _ in range(30):
_grab(cap)This change assumes there is a _grab(cap) helper in the same module that:
- Calls
cap.read() - Checks the return flag
- Exits or raises on failure
If _grab has a different signature or side effects (e.g., returns a frame that must be used), adjust the call accordingly, for example:
- If
_grabrequires additional parameters, pass them in the warmup loop as well. - If
_grabreturns a frame and the function enforces usage, you may need to bind the return value (e.g.,frame = _grab(cap)), even if the frame is discarded afterwards.
If _grab does not yet exist, you should instead:
- Factor out the existing camera-read-and-check logic from wherever it currently is into a
_grabfunction. - Call
_grabboth in the warmup loop and where the read previously occurred so behavior stays consistent.
Replace Python+OpenCV pipeline with native Rust webcam capture via nokhwa. Frames go directly from camera to DynamicImage in memory — no temp files, no process spawning, no JPEG roundtrip. Library-quality camera module (rascii_art::camera) with: - CameraSource: init, warmup, frame capture with proper error types - LiveRenderer: decoupled terminal loop accepting any frame source - CameraError enum with Display/Error/source chain - Feature-gated behind "camera" (default on), opt out with --no-default-features Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rascii_art now compiles to wasm32-wasip1 with --no-default-features. Feature tiers: default (camera+terminal+nokhwa+crossterm), terminal (crossterm only), none (pure rendering, WASM-safe). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace unmaintained ansi_term with owo-colors 4.3 (zero-alloc, maintained) - Move clap + unicode-segmentation behind optional `cli` feature - Binary uses required-features = ["cli"] so library consumers skip it - Change default features from ["camera"] to ["terminal"] — library users get renderer + terminal-width detection, not nokhwa - Deduplicate image_renderer.rs (extract resolve_dimensions, style_for_pixel) - Add doc comments to all public camera and core API types Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Owner
|
That looks really cool, although it has performance problems on my laptop, so I guess this pr needs an optimization. But I'm busy these days, I'd sadly not able to help to optimize it, maybe you can help? |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Add camera_ascii.py for capturing webcam frames and rendering them as colored ANSI art via RASCII. Supports live mode, mirror, auto terminal width, block/emoji charsets, and background color fill.
Summary by Sourcery
Add a Python-based webcam-to-ASCII pipeline that streams camera frames through the existing RASCII binary to render colored ANSI art in the terminal.
New Features:
Documentation: