Add CameraView by zhitaop · Pull Request #1758 · CommunityToolkit/Maui

zhitaop · 2024-03-17T23:15:51Z

Description of Change

As described in #259 (comment), this PR adds the initial implementation of the CameraView proposal on MacCatalyst, Windows, Android and iOS, and a corresponding demo page in the sample app . It's based on the fantastic work from the following PR/ branch #1080 and #1387.
Below is the summary of the proposed API design, it has been updated with feedback from #259 (comment)

public class CameraView
{
    public static readonly BindableProperty SelectedCameraProperty; // To be discussed: ActiveCameraProperty?
    public static readonly BindableProperty ZoomFactorProperty;
    public static readonly BindableProperty ResolutionProperty;
    public static readonly BindableProperty IsCameraBusyProperty;
    public static readonly BindableProperty IsAvailableProperty;
    public static readonly BindableProperty FlashModeProperty;
    public static readonly BindableProperty IsTorchOnProperty;

    public event EventHandler<MediaCapturedEventArgs> MediaCaptured;
    public event EventHandler<MediaCaptureFailedEventArgs> MediaCaptureFailed;

    public CameraInfo? SelectedCamera { get; set; } // To be discussed: ActiveCamera?
    public float ZoomFactor { get; set; }
    public Size CaptureResolution { get; set; } //  renamed from `Resolution`
    public bool IsCameraBusy { get; set; }
    public bool IsAvailable { get; set; }
    public bool IsTorchOn { get; set; }
    public CameraFlashMode FlashMode { get; set; }

    public void Shutter()
    public void Start()
    public void Stop()
}

public enum CameraFlashMode
{
    Off,
    On,
    Auto
}

public class CameraInfo
{
    public string Name { get; }
    public string DeviceId { get; }
    public CameraPosition Position { get; }
    public bool IsFlashSupported { get; } //To be discussed:  adding a list of capabilities like Auto Focus, etc?
    public float MinimumZoomFactor { get; }
    public float MaximumZoomFactor { get; }
    public ObservableCollection<Size> SupportedResolutions { get; } // renamed from `AvailableResolutions`. 
    //Observable makes it easier for binding as the resolution would only be updated after the camera view is initialized on Windows 
}

public enum CameraPosition // To be discussed: adding External value
{
    Unknown,
    Rear,
    Front
}

public class CameraProvider
{
    public ObservableCollection<CameraInfo> AvailableCameras { get; }
    public void RefreshAvailableCameras();
}

Linked Issues

Fixes [Proposal] CameraView minimal port #259
Closes Camera view initial work #1080
Closes Camera view initial work (iOS) #1387

zhitaop · 2024-03-18T12:51:16Z

@dotnet-policy-service agree

bijington

@zhitaop thank you so much for this PR! I have added my initial comments and most I don't think require much effort if they do need to be changed.

I plan to try and test some bits later this week. I would also love to look at possible macOS support which I am happy to contribute, I wouldn't let this hold up the PR completing though, worst case I can always open a follow up PR to add that in.

jfversluis · 2024-03-30T09:38:15Z

I added the Analyzers for the initialization code for you!

* Add exception handling when AvailableCameras is empty * Use expression body * Remove nullable in CameraInfo

bijington · 2024-04-15T08:40:25Z

@zhitaop Thanks for the responses. Sorry things have been chaotic here so I haven't responded or reviewed yet. I'll try and take a look this week

bijington

@zhitaop Thanks for your responses, I have added some further comments. I think the majority looks great and we could be close to getting this ready to merge. Let me know your thoughts and whether you need any assistance

…ommunityToolkitMaui into pr/1758

…ception`

… App

…ommunityToolkitMaui into pr/1758

TheCodeTraveler · 2024-06-05T23:55:43Z

Thanks @pictos - great review! I've incorporated your comments here: 0f85de2

I see a lot of exceptions that could be thrown on Manage
I went through every branch where we throw new CameraException(), and the only time we do that now when no cameras are available on the device is when the user does something that requires the camera. For example, when the dev calls StartCameraPreview() or UpdateCaptureResolution() (methods that require a camera), we'll throw CameraException.

This should ensure devs are able to catch the CameraException now.

TheCodeTraveler

Ok gang! I think this is ready to merge. 🚀

That being said, Pedro brought up some good comments in his review that we should discuss first! Let's chat about it in tomorrow's standup to get a group consensus before we click merge 🙌

zhitaop · 2024-06-06T16:45:31Z

@brminnick @bijington Thanks for all the great work! There has been definitely great progress and the code looks much more organized and consistent to the rest of the toolkit now.
After some testing on Windows using the sample app, I did notice a few issues that might worth looking into:

Due to the access level of CameraProvider , there is no way to access the AvaiableCameras other than CameraView.GetAvailableCameras. My original intent is for CameraProvider to be a public singleton class decoupled from CameraView so that it can be used to access the available CameraInfo without a CameraView instance. Both Android and iOS use a similar pattern in their native camera APIs.
The camera switcher and some of the original debug text (camera name, zoom level, etc) are missing on the sample camera view page
UpdateCameraInfo in CameraManage.Windows.cs is not needed anymore, since now CameraInfo is fully initialized in CameraProvider.Windows.cs and does not need to be updated.

I haven't got the chance to look into these issues myself, so just leave them here as discussion points before the merge!

…to Sample App `ICameraProvder` is public and can be accessed via Dependency Injection (or IPlatformApplication.Current.Services.GetRequiredService<ICameraProvider>()

TheCodeTraveler

I think we're good to go! Thanks @zhitaop!!!

FYI - We discussed your latest review in our standup on Thursday. Here's a quick rundown:

Due to the access level of CameraProvider , there is no way to access the AvaiableCameras other than CameraView.GetAvailableCameras.

The interface ICameraProvider is public and we add it to the IServiceCollection; it can be accessed by developers using Dependency Injection or IPlatformApplication.Current.Services.GetRequiredService<ICameraProvider>(). I've added an example of this to the Sample App in CameraViewViewModel.

The camera switcher and some of the original debug text (camera name, zoom level, etc) are missing on the sample camera view page

Yea, that was me being a bit lazy. I removed it because the AvailableCameras requires async code and after refactoring its async implementation, the value was null when the page first displayed. You're totally welcome to re-add it in a future PR, though! I was just being a bit lazy removing it and not re-implementing the UI with the async implementation, wanting to not delay this PR any further.

UpdateCameraInfo in CameraManage.Windows.cs is not needed anymore, since now CameraInfo is fully initialized in CameraProvider.Windows.cs and does not need to be updated.

Removed ✅

Changes implemented

CameraView and sample app initial commit

f1535fa

zhitaop changed the title ~~CameraView and sample app initial commit~~ CameraView initial work (Windows, Android, iOS) Mar 17, 2024

Merge branch 'main' into feature/zp/camera-view

092a381

Merge branch 'main' into feature/zp/camera-view

4c3adfd

TheCodeTraveler mentioned this pull request Mar 25, 2024

Camera view initial work #1080

Closed

Merge branch 'main' into feature/zp/camera-view

c8555c6

bijington requested changes Mar 27, 2024

View reviewed changes

jfversluis added 2 commits March 30, 2024 09:54

Merge branch 'main' into feature/zp/camera-view

3648cbf

Added analyzer projects for CameraView

c55a00e

jfversluis added the needs discussion Discuss it on the next Monthly standup label Mar 30, 2024

jfversluis and others added 2 commits April 4, 2024 11:20

Merge branch 'main' into feature/zp/camera-view

a3596d8

CameraView update

bc9e978

* Add exception handling when AvailableCameras is empty * Use expression body * Remove nullable in CameraInfo

bijington previously requested changes Apr 25, 2024

View reviewed changes

TheCodeTraveler removed the needs discussion Discuss it on the next Monthly standup label May 2, 2024

Merge branch 'main' into feature/zp/camera-view

c568fb1

bijington marked this pull request as ready for review May 2, 2024 19:28

bijington and others added 6 commits May 2, 2024 21:04

Remove CommunityToolkit.Mvvm

5d51b62

Merge branch 'feature/zp/camera-view' of https://github.com/zhitaop/C…

88ba354

…ommunityToolkitMaui into pr/1758

Add in the initial XML docs

b38a89a

Improve handler example

9b9d2d5

Add MacCatalyst Support, Make CameraInfo Immutable, Update Formatting

5b2d546

Update async/await

8e342fb

TheCodeTraveler changed the title ~~CameraView initial work (Windows, Android, iOS)~~ CameraView initial work May 5, 2024

TheCodeTraveler changed the title ~~CameraView initial work~~ Add CameraView May 5, 2024

TheCodeTraveler added 3 commits May 5, 2024 09:07

Reduce public API surface, Update Namespaces, Implement `CameraViewEx…

d2dec69

…ception`

Update ICameraView + CameraViewHandler for async/await, Update Sample…

6dd30e6

… App

donet format

9945bb5

pictos and others added 9 commits June 3, 2024 16:58

add alias for System.Math

0805502

use ArrayPool on CameraManager

7f22826

use ArrayPool on CameraManager.macios

b28ab7c

Merge branch 'main' into pr/1758

b9c82fe

Fix formatting

dcb4812

Remove Null Forgiving Operator

e68d299

Merge branch 'main' into feature/zp/camera-view

3a0ffc0

Incorporate Pedro's Review Comments

0f85de2

Merge branch 'feature/zp/camera-view' of https://github.com/zhitaop/C…

832b73f

…ommunityToolkitMaui into pr/1758

TheCodeTraveler added 2 commits June 5, 2024 17:07

CameraViewException -> CameraException

00fbb49

dotnet format

d69f062

TheCodeTraveler previously approved these changes Jun 6, 2024

View reviewed changes

TheCodeTraveler and others added 2 commits June 6, 2024 12:47

Merge branch 'main' into feature/zp/camera-view

3de0b7f

Code tidy up and make CameraProvider public

c9ee59c

bijington dismissed TheCodeTraveler’s stale review via c9ee59c June 6, 2024 20:30

bijington and others added 5 commits June 6, 2024 21:56

Remove the call to UpdateCameraInfo

32d86d5

Add remarks to CameraProvider.AvailableCameras

971aee7

Revert CameraProvider to Internal, Add ICameraProvider Example …

bba70c5

…to Sample App `ICameraProvder` is public and can be accessed via Dependency Injection (or IPlatformApplication.Current.Services.GetRequiredService<ICameraProvider>()

Revert CTS causing TaskCancellationException

0ea2c51

Inherit xml docs from interface

3bb8a43

TheCodeTraveler removed the needs discussion Discuss it on the next Monthly standup label Jun 8, 2024

Update ICameraProvider.shared.cs

a7650db

TheCodeTraveler approved these changes Jun 8, 2024

View reviewed changes

TheCodeTraveler enabled auto-merge (squash) June 8, 2024 16:09

TheCodeTraveler merged commit 8bc1f04 into CommunityToolkit:main Jun 8, 2024

zhitaop mentioned this pull request Jul 3, 2024

[BUG] CameraView IsTorchOn not working on Android #1992

Open

2 tasks

github-actions Bot locked and limited conversation to collaborators Nov 18, 2024

Conversation

zhitaop commented Mar 17, 2024 • edited by TheCodeTraveler Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Linked Issues

Uh oh!

zhitaop commented Mar 18, 2024

Uh oh!

bijington left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jfversluis commented Mar 30, 2024

Uh oh!

bijington commented Apr 15, 2024

Uh oh!

bijington left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TheCodeTraveler commented Jun 5, 2024

Uh oh!

TheCodeTraveler left a comment

Choose a reason for hiding this comment

Uh oh!

zhitaop commented Jun 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheCodeTraveler left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

zhitaop commented Mar 17, 2024 •

edited by TheCodeTraveler

Loading

zhitaop commented Jun 6, 2024 •

edited

Loading