-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
: ThecreateFile
method determines file extensions based on the image format. The currentelse
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 overriddenULTRAHDR_FORMAT
property. Also, the newlyopen
base classCamera2UltraHDRCapture
and itsULTRAHDR_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 android/platform-samples#310 (review)