Skip to content

Conversation

@bonnieking
Copy link

No description provided.

Copilot AI review requested due to automatic review settings November 27, 2025 21:31
Copy link
Contributor

Copilot AI left a 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 adds command-line control over camera resolution by introducing an --resolution argument that allows users to specify custom width and height values for the video stream.

Key changes:

  • Modified start_stream() to accept configurable width and height parameters with backward-compatible defaults (854x480)
  • Added argparse-based command-line argument parsing for resolution specification in WIDTHxHEIGHT format
  • Updated the main execution flow to parse and pass resolution values to the streaming function

Comment on lines +164 to +165
try:
width, height = map(int, args.resolution.split('x'))
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolution parsing logic doesn't validate that the parsed width and height values are positive integers or within reasonable bounds. Negative values or zero could be passed to start_stream(), which may cause unexpected behavior in the camera command. Consider adding validation such as if width <= 0 or height <= 0: ... after parsing.

Copilot uses AI. Check for mistakes.
Copy link
Member

@sgbaird sgbaird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, these will need to be variables that's imported from my_secrets.py, similar to other variables and secrets. Since this will be going to YouTube, we might want to restrict it to the following options: 240p, 360p, 480p, 720p. It would also be good to allow people to rotate by 90 degrees (in addition to being able to flip vertically or horizontally) to have a portrait mode stream. However, default should be non-rotated. You can include the allowed options in the comments of my_secrets.py

remove all the CLI code

Copy link
Contributor

Copilot AI commented Nov 27, 2025

@sgbaird I've opened a new pull request, #539, to work on those changes. Once the pull request is ready, I'll request review from you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants