Make minimum frame rate configurable in Python WebRTC/AVSM tests#72369
Make minimum frame rate configurable in Python WebRTC/AVSM tests#72369pidarped wants to merge 1 commit into
Conversation
- Add `--min-frame-rate` command-line argument to the test runner (default: 30). - Update `MatterTestConfig` to store `min_frame_rate`. - Update all Python test scripts and helper classes that had hardcoded `minFrameRate=15` to use `self.matter_test_config.min_frame_rate` instead. - This allows users to customize the minimum frame rate used during video stream allocation.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable minimum frame rate for video stream allocation across various test files, replacing the previously hardcoded value of 15 with a new min_frame_rate field in MatterTestConfig. The reviewer pointed out that changing the default value to 30 could break existing tests and suggested maintaining the default at 15 to ensure backward compatibility.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #72369 +/- ##
==========================================
- Coverage 55.57% 55.57% -0.01%
==========================================
Files 1630 1630
Lines 111220 111223 +3
Branches 13408 13409 +1
==========================================
Hits 61812 61812
- Misses 49408 49411 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| streamUsage=aStreamUsagePriorities[0], | ||
| videoCodec=aRateDistortionTradeOffPoints[0].codec, | ||
| minFrameRate=min(15, aVideoSensorParams.maxFPS), | ||
| minFrameRate=min(self.matter_test_config.min_frame_rate, aVideoSensorParams.maxFPS), |
There was a problem hiding this comment.
You need to also update the check where this value is read back from the allocated streams.
| config.endpoint = args.endpoint # This can be None, the get_endpoint function allows the tests to supply a default | ||
| config.restart_flag_file = args.restart_flag_file | ||
| config.debug = args.debug | ||
| config.min_frame_rate = args.min_frame_rate |
There was a problem hiding this comment.
How would a client normally get this information? If this needs to be provided manually, wouldn't that make it a trial and error game for any real client attempting to interact with a camera?
| help="The full path of the file to use to signal a restart to the app") | ||
| basic_group.add_argument('--debug', action="store_true", default=False, | ||
| help="Run the script in debug mode. This is needed to capture attribute dump at end of test modules if there are problems found during testing.") | ||
| basic_group.add_argument('--min-frame-rate', type=int, default=30, |
There was a problem hiding this comment.
Given that this only applies to camera tests, does this need to be a top-level argument? Why not use the int-arg input for this?
3 participants
Summary
--min-frame-ratecommand-line argument to the test runner (default: 30).MatterTestConfigto storemin_frame_rate.minFrameRate=15to useself.matter_test_config.min_frame_rateinstead.Related issues
Fixes #72290
Testing
ran test scripts using new cmd-line arg
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See:
Pull Request Guidelines