-
Notifications
You must be signed in to change notification settings - Fork 8
Move resolution/rotation/frame rate/timestamp config to my_secrets.py, remove CLI code #539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: resolution
Are you sure you want to change the base?
Conversation
Co-authored-by: sgbaird <[email protected]>
Co-authored-by: sgbaird <[email protected]>
|
@copilot I think you can also add 144p and 1080p, but can make the note for 1080p that a Pi Zero 2W can't really handle that (720p at 15fps is pushing it, 480p at 15 fps is more comfortable for it, and recommendation to using a Pi 4B or Pi 5 if you want to increase resolution or frame rate past what the Zero 2W can handle). I also forgot, frame rate should be one of the variables that can be adjusted in my_secrets.py |
Co-authored-by: sgbaird <[email protected]>
Addressed in 0ad81a2:
|
|
@copilot Btw, I tested on the previous script (i.e., before my last message) and I'm getting: |
Co-authored-by: sgbaird <[email protected]>
Addressed in c2a71bd:
The non-monotonous DTS warning was likely a timing issue with the previous implementation. The current code should be more stable. |
Co-authored-by: sgbaird <[email protected]>
Fixed in e96f0af. The issue was the fixed |
|
@copilot timestamp overlay not functioning |
|
@copilot Aside: I'm also frequently getting warning messages like the following: I think these are new warnings based on recent changes, not something I normally saw over the last 3-12 months. |
Co-authored-by: sgbaird <[email protected]>
... Fixed in 496a2d7. The issue was incorrect character escaping in the ffmpeg drawtext filter. The filter string was causing ffmpeg to interpret part of the text parameter as a file path. Reorganized the filter parameter order and fixed the escaping so the text parameter with |
…nings Co-authored-by: sgbaird <[email protected]>
... Fixed in 29357c7. Added ffmpeg timestamp correction options:
These options should eliminate or significantly reduce the "Non-monotonous DTS" warnings that were appearing when using video filters (rotation/timestamp overlay) which require re-encoding. |
Co-authored-by: sgbaird <[email protected]>
|
@copilot Getting following with timestamp set to True. No timestamp is displayed on the YouTube video. Also noting that the stream seemed to go down more frequently, having to restart multiple times before the 8 hrs was up. Brief fly-by observation. Didn't look into it. |
…format Co-authored-by: sgbaird <[email protected]>
... Fixed in 61dcb3a. The error |
|
@copilot still no timestamp visible. |
…scaping issues Co-authored-by: sgbaird <[email protected]>
... Fixed in d359359. The issue is that ffmpeg's drawtext filter interprets colons ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the camera streaming configuration by moving resolution, rotation, frame rate, and timestamp settings from CLI arguments to the my_secrets.py configuration file. The changes enable portrait mode streaming, timestamp overlays, and improve the handling of different video configurations.
- Removes argparse CLI code and migrates all stream configuration to
my_secrets.py - Adds support for camera rotation (0/90/180/270 degrees) with proper dimension handling for portrait mode
- Implements combined video filter chain for rotation and timestamp overlays with ffmpeg re-encoding
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/ac_training_lab/picam/my_secrets_example.py | Adds new configuration settings for CAMERA_ROTATION, RESOLUTION, FRAME_RATE, and TIMESTAMP_OVERLAY with detailed documentation |
| src/ac_training_lab/picam/device.py | Removes CLI/argparse code, imports new settings from my_secrets, implements rotation logic and video filter chain, adds validation for all new settings |
| CHANGELOG.md | Documents the new configuration options and removal of CLI code |
| - Resolution setting in `my_secrets_example.py` for YouTube-compatible streaming (144p, 240p, 360p, 480p, 720p, 1080p). | ||
| - Camera rotation setting (0, 90, 180, 270 degrees) for portrait mode streaming in `my_secrets_example.py`. | ||
| - Frame rate setting in `my_secrets_example.py` for adjustable stream frame rate. | ||
| - Timestamp overlay setting in `my_secrets_example.py` to display date/time with seconds on video stream. |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changelog entry mentions "display date/time with seconds" but the actual format is YYYY-MM-DD_HH-MM-SS, which uses dashes instead of colons for the time portion. This is a technical detail but could be misleading.
Consider updating to be more precise: "Timestamp overlay setting in my_secrets_example.py to display date/time (format: YYYY-MM-DD_HH-MM-SS) on video stream."
| - Timestamp overlay setting in `my_secrets_example.py` to display date/time with seconds on video stream. | |
| - Timestamp overlay setting in `my_secrets_example.py` to display date/time (format: YYYY-MM-DD_HH-MM-SS) on video stream. |
| if timestamp_overlay: | ||
| # drawtext filter with white text, black background box, in top-left corner | ||
| # fontsize scales with video height for consistent appearance | ||
| fontsize = max(16, height // 20) |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timestamp filter's fontsize calculation uses the original height parameter, but when rotation is 90 or 270 degrees, this refers to the pre-rotation dimension. After rotation, the actual video height will be different. This could result in incorrectly sized text.
For portrait mode (90/270 rotation), the actual video height after rotation will be width, so the fontsize should be calculated based on the final rotated dimensions, not the input parameters.
Consider calculating fontsize based on the actual output dimensions:
# Calculate fontsize based on actual output dimensions after rotation
if rotation in (90, 270):
actual_height = width # After rotation, width becomes height
else:
actual_height = height
fontsize = max(16, actual_height // 20)| fontsize = max(16, height // 20) | |
| if rotation in (90, 270): | |
| actual_height = width | |
| else: | |
| actual_height = height | |
| fontsize = max(16, actual_height // 20) |
| video_filters.append("transpose=2") # 90 degrees counter-clockwise (270 clockwise) | ||
|
|
||
| # Add timestamp overlay if enabled | ||
| # Format: YYYY-MM-DD_HH-MM-SS (updates every second) |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment mentions "Format: YYYY-MM-DD_HH-MM-SS (updates every second)" but doesn't explain what "updates every second" means in the context of ffmpeg's localtime function. This could be clearer for maintainers.
Consider clarifying: "Format: YYYY-MM-DD_HH-MM-SS (ffmpeg's localtime evaluates per-frame, effectively updating once per second at typical framerates)"
| # Format: YYYY-MM-DD_HH-MM-SS (updates every second) | |
| # Format: YYYY-MM-DD_HH-MM-SS (ffmpeg's localtime evaluates per-frame, so the overlay updates once per second at typical framerates) |



--mode 1536:864sensor constraint-async 1and-vsync cfr) to fix non-monotonous DTS warnings✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.