android / android/architecture-samples
## Code Review
- Dominant language
- Kotlin
- Stars
- 45.8k
- Forks
- 11.9k
- PR merge metrics
- No merged PRs in 30d
Description
## Code Review
This pull request effectively introduces support for HEIC UltraHDR image capture. The approach of extending the existing `Camera2UltraHDRCapture` class is sound, and the necessary API changes (like using `CaptureRequest.JPEG_ORIENTATION` instead of EXIF manipulation for HEIC) are correctly implemented. The SDK version updates and new sample demo registration are also appropriate.
Overall, the changes are well-done and enhance the camera sample capabilities. I have one suggestion for improving the robustness of the file creation logic for future extensions.
### Summary of Findings
* **File Extension Handling in `createFile`**: The `createFile` method determines file extensions based on the image format. The current `else` case defaults to `.heic`, which is fine for this PR but could be problematic if other non-HEIC UltraHDR formats are supported in the future via subclassing. A more explicit handling of known formats and a clearer strategy for unknown ones would improve robustness. (Commented with medium severity)
* **Copyright Year**: The new file `samples/camera/camera2/src/main/java/com/example/platform/camera/imagecapture/Camera2HeicUltraHDRCapture.kt` has a copyright year of 2025. Typically, this is the year of creation (e.g., 2024) or matches the project's existing files (e.g., `Camera2UltraHDRCapture.kt` uses 2023). Please verify if 2025 is intentional or a typo. (Not commented due to review settings: low severity)
* **KDoc Documentation**: Consider adding KDoc comments to the new class `Camera2HeicUltraHDRCapture` and its overridden `ULTRAHDR_FORMAT` property. Also, the newly `open` base class `Camera2UltraHDRCapture` and its `ULTRAHDR_FORMAT` property could benefit from KDocs explaining their roles and extensibility. This would improve code clarity and maintainability. (Not commented due to review settings: low severity)
### Merge Readiness
The pull request is in good shape and introduces a valuable feature. I recommend addressing the medium-severity comment regarding file extension handling in `createFile` to enhance future maintainability. Once that's considered, the PR should be ready for merging. As an AI reviewer, I am not authorized to approve pull requests; please ensure further review and approval from authorized team members.
_Originally posted by @gemini-code-assist[bot] in https://github.com/android/platform-samples/pull/310#pullrequestreview-2901815087_
Contributor guide
Research direction
Start by reading PR #310 and the mentioned Camera2HeicUltraHDRCapture.kt and Camera2UltraHDRCapture files, especially createFile, ULTRAHDR_FORMAT, and their existing sample registration. Done means the review's file-extension, copyright, and KDoc concerns have been resolved or explicitly addressed, with the relevant camera sample checks passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- android, kotlin
- Domain
- mobile
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 20/100