Fix CameraView crash when switching camera on Windows and other CameraView enhancements
Description of Change
-
Fixes #2559: In
CameraProvider.windows.cs, only one instance ofMediaCaptureis instantiated when iterating through thevideoCaptureSourceGroup. This causes only the firstmediaCapture.InitializeCameraForCameraView()to be successful. Wrong camera meta data from the first camera, such as the supported resolutions, are added to the subsequentCameraInfos.- Proposed solution: instantiate new
MediaCapturein each iteration and properly dispose it after use
- Proposed solution: instantiate new
-
Fixes #2519:
As per https://github.com/CommunityToolkit/Maui/issues/2519#issuecomment-2764251471:
CameraViewHandler.ConnectHandler doesn't actually check the result of the permission request
- Proposed solution: bubble-up a
PermissionExceptionand let consumers respond
- Proposed solution: bubble-up a
-
Also found that unnecessary calls to
CameraProvider.RefreshAvailableCameras()are scattered throughout the code, which is a time consuming operation especially on Windows. IMO this call is only necessary when theCameraProvideris initialized, or when consumer of the API knows that available cameras on device has been updated (e.g. external camera has been plugged in after the initialization)- Proposed solution: add
InitializeAsynctask inCameraProvider, which is automatically started to initialize the camera list, and can be awaited by the consumer. (This might need more consideration as it change the API ?) - Remove all other
RefreshAvailableCameras()calls throughoutCameraManager - Updated inline documentation for to better explain purposes of each function in
CameraProvider. This could potential be the solution to https://github.com/CommunityToolkit/Maui/issues/2272?
- Proposed solution: add
-
Also fixes a few issues in
CameraViewPageandCameraViewViewModel:- Add camera switcher on the camera view page
- Make
Camerasobservable in the view model, and make sure it's assigned after theCameraProvideris initialized so that camera list is not empty.
Linked Issues
- Fixes #2559
- Fixes #2519
PR Checklist
- [x] Has a linked Issue, and the Issue has been
approved(bug) orChampioned(feature/proposal) - [x] Has tests (if omitted, state reason in description)
- [x] Has samples (if omitted, state reason in description)
- [x] Rebased on top of
mainat time of PR - [x] Changes adhere to coding standard
- [ ] Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls
Additional information
Any guess when this will deploy to main? Hoping it fixes some issues I have with Windows camera.
Any guess when this will deploy to main? Hoping it fixes some issues I have with Windows camera.
Perhaps you could help us and test this branch on your machine to see if it helps?
Any guess when this will deploy to main? Hoping it fixes some issues I have with Windows camera.
Perhaps you could help us and test this branch on your machine to see if it helps?
I would love to help but don't know how do download the branch to test. Let me Google it.
I have tested the CommunityToolkitSampleApp using https://github.com/zhitaop/CommunityToolkitMaui/tree/fix/camera-provider-refresh branch on Surface Book 3 (Windows 10). The camera switcher is working as expected without any crashes.
Hi @bijington, could this PR be assigned for review soon? As mentioned in https://github.com/CommunityToolkit/Maui/pull/2634#issuecomment-3071604323, the branch has been tested that it does solve the original crash issue.
Hi @bijington, could this PR be assigned for review soon? As mentioned in https://github.com/CommunityToolkit/Maui/pull/2634#issuecomment-3071604323, the branch has been tested that it does solve the original crash issue.
I've added some comments I didn't have a chance to take a look on a laptop so I'll try and do a more in depth check later in the week
Sorry it's taken time to get to this. I'll try and sort it tomorrow or next week
I this it is same issue with recent code on Android
Hi @bijington, I've addressed some of your comments and provide explanation for others, could you please take a look again? Thanks!
Sorry things became really busy and then I went on vacation. I return at the weekend so I've added a reminder to take a look at this next week for you.
Thanks @zhitaop! This is a breaking change that requires the docs to be updated before we can merge the PR: https://github.com/MicrosoftDocs/CommunityToolkit/pulls
@bijington - could you work with @zhitaop on the updating the docs?
@TheCodeTraveler Thanks for the review and code update! I have raised a PR to update the documentation: https://github.com/MicrosoftDocs/CommunityToolkit/pull/593
Hi @bijington @TheCodeTraveler, is there any update to the status of this PR?
Hi Zhitao! Yes we discussed it in our standup this month.
We've decided not to keep the breaking changes to the API in this PR and to only keep the bug fixes. The new Initialize method doesn't add much value for developers.
I will be closing this PR and reopening it with just the bug fix for Windows.
@TheCodeTraveler Thanks for the update!
Just to clarify, in my understanding, adding the InitializeAsync() method wouldn’t be a breaking change - it’s a new API that doesn’t affect any existing behaviour. Existing code using RefreshAvailableCameras() will continue to work as before without modification, developers do not have to use the new method.
The intent for adding the new method was to give developers more flexibility, as both methods serve different use cases:
-
InitializeAsynconly performs a one-time discovery of cameras and populates theAvailableCamerasproperty. Subsequent calls will simply reuse the cached lists (a no-op). -
RefreshAvailableCamerasforces a refresh. It always re-runs camera discovery to ensure the list is up to date
Camera discovery can be expensive, especially on Windows (it needs to initialize a mediaCapture object for each camera). In addition, camera availability on a device rarely changes during an app’s lifecycle, so InitializeAsync() provides the option to ensure the list is only initialized once.
For example, a view model for a camera view page can call InitializeAsync to populate a list of cameras when the page opens for the first time. Subsequent openings of the page do not need to refresh the cameras, increasing performance.
public async Task InitializeViewModelAsync()
{
await cameraProvider.InitializeAsync(CancellationToken.None);
// alternatively, call camepraProvider.RefreshAvailableCameras() to ensure the list is always up to date
Cameras = cameraProvider.AvailableCameras ?? [];
}
That said, if the concern is it adds unnecessary confusion to the API usage without adding too much value, I’m happy to update the PR and remove the InitializeAsync() method to keep it focused on the bug fix.
Thanks @zhitaop! No worries - I've opened the new PR here and would love your review! https://github.com/CommunityToolkit/Maui/pull/2907