Move Photo and Video Mode into plugins #16
Conversation
ISNIT0
commented
Jan 11, 2025
ISNIT0
commented
- Move photo and video mode logic and controls into plugins
- Implement video pause & snapshot functionality
- Improve dynamic use-case loading
julianharty
left a comment
julianharty
left a comment
There was a problem hiding this comment.
Approved subject to the comments for consideration.
| CameraCharacteristics.INFO_SUPPORTED_HARDWARE_LEVEL_LEGACY -> 1 | ||
| CameraCharacteristics.INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED -> 2 | ||
| CameraCharacteristics.INFO_SUPPORTED_HARDWARE_LEVEL_FULL -> 3 | ||
| CameraCharacteristics.INFO_SUPPORTED_HARDWARE_LEVEL_3 -> 3 |
There was a problem hiding this comment.
If this is intended to be identical to the previous use-case (_FULL) how about adding a comment to say this rather than simply setting it to the same value?
| val requiredUseCases: List<PluginUseCase> = | ||
| selectedModePlugin?.modeUseCases ?: emptyList() | ||
|
|
||
| // All use-cases (including not needed by selected plugin), but retaining the order from the selected plugin |
There was a problem hiding this comment.
v.minor suggested change to the comment to improve the grammar: "All use-cases (including those not needed by selected plugin), retaining the order of the selected plugin"
| } | ||
|
|
||
| val modeLabel: String? | ||
| get() = null |
There was a problem hiding this comment.
Are you planning to expand/extend this code later?
| get() = null | ||
|
|
||
| val renderModeControl: @Composable() (() -> Unit)? | ||
| get() = null |
There was a problem hiding this comment.
Ditto: are you planning to expand/extend this code later?
| previousMode: String, | ||
| nextMode: String | ||
| ): Unit { | ||
|
|
There was a problem hiding this comment.
What's this empty code for? Why is it needed?
| // Settings for the plugin | ||
| val settings: List<PluginSetting> | ||
| val settings: (viewModel: StoneCameraViewModel) -> List<PluginSetting> | ||
| get() = { emptyList() } |
There was a problem hiding this comment.
Should there be a TODO here?
| // Shutter button | ||
| Box( | ||
| modifier = Modifier | ||
| .size(60.dp) |
There was a problem hiding this comment.
Lots of hardcoded sizes... A trap for future maintenance.
| horizontalArrangement = Arrangement.SpaceEvenly, | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| // Camera Switcher Button |
There was a problem hiding this comment.
I'm very glad these comments exist, otherwise it'd be vary difficult to guess what the code does.
| if (isPaused) { | ||
| Icon( | ||
| imageVector = Icons.Filled.PlayArrow, | ||
| contentDescription = "Resume Recording", |
There was a problem hiding this comment.
Localisation to come, I hope.
