Maui icon indicating copy to clipboard operation
Maui copied to clipboard

Add CameraView

Open zhitaop opened this issue 1 year ago • 11 comments
trafficstars

Description of Change

As described in https://github.com/CommunityToolkit/Maui/issues/259#issuecomment-1984316965, 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 https://github.com/CommunityToolkit/Maui/pull/1080 and https://github.com/CommunityToolkit/Maui/pull/1387. Below is the summary of the proposed API design, it has been updated with feedback from https://github.com/CommunityToolkit/Maui/issues/259#issuecomment-1984316965

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 #259
  • Closes #1080
  • Closes #1387

zhitaop avatar Mar 17 '24 23:03 zhitaop

@dotnet-policy-service agree

zhitaop avatar Mar 18 '24 12:03 zhitaop

I added the Analyzers for the initialization code for you!

jfversluis avatar Mar 30 '24 09:03 jfversluis

@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 avatar Apr 15 '24 08:04 bijington

We're getting closer! I was able to get CameraView working on macOS. I've updated the title + description of this PR to include MacCatalyst 👍

image

The one thing I can't confirm is CameraView on Windows; I'm running Windows on a VM on my MacBook, and each time I run the app I get a CameraViewException telling me that it cannot find a camera. I'm unsure if it's my setup that is causing the problem, or if a refactor edit I made accidentally injected the bug. But, once we get Windows working and the Docs written, we're ready to ship!

TheCodeTraveler avatar May 05 '24 08:05 TheCodeTraveler

@bijington @brminnick Thanks a lot for the review and help with the support on macOS! Sorry I haven't got the chance to look at the comments earlier, but I guess most of them are addressed in the latest changes?

@brminnick Regarding the exception on Windows, recently we found a bug where the CameraView doesn't work on certain Windows machine. It could be fixed by changing the line https://github.com/CommunityToolkit/Maui/blob/45978391ff250dcd7055aa6891cf0c6d332ff6da/src/CommunityToolkit.Maui.CameraView/CameraManager.windows.cs#L129 to PhotoCaptureSource = PhotoCaptureSource.Auto.

I'm not sure if that's the same bug that you encountered, but please feel free to give a try.

zhitaop avatar May 05 '24 08:05 zhitaop

I just wanted to try out the new CameraView, but I think the new project CommunityToolkit.Maui.CameraView is either missing in the existing NuGet packages or creating a new NuGet package for that project is not there yet. Am I right?

MFinkBK avatar May 08 '24 13:05 MFinkBK

Yes the CameraView will ship in a new nuget package that doesn't exist yet. @jfversluis i assume this means we can ship a previous nuget for it right?

If not then you can test it out in the sample application for now

bijington avatar May 08 '24 14:05 bijington

If with previous you mean preview then yes 😄 I will have a look at getting this across the finish line in the upcoming week!

jfversluis avatar May 08 '24 17:05 jfversluis

Haha doh!

bijington avatar May 08 '24 18:05 bijington

@bijington @brminnick Thanks a lot for the review and help with the support on macOS! Sorry I haven't got the chance to look at the comments earlier, but I guess most of them are addressed in the latest changes?

@brminnick Regarding the exception on Windows, recently we found a bug where the CameraView doesn't work on certain Windows machine. It could be fixed by changing the line

https://github.com/CommunityToolkit/Maui/blob/45978391ff250dcd7055aa6891cf0c6d332ff6da/src/CommunityToolkit.Maui.CameraView/CameraManager.windows.cs#L129

to PhotoCaptureSource = PhotoCaptureSource.Auto. I'm not sure if that's the same bug that you encountered, but please feel free to give a try.

Thanks for the update. I wonder if there is value in just swapping to using Auto or allowing the developing to choose this through an options class when registering the CameraView plugin. Something like:

var builder = MauiApp.CreateBuilder();
    builder
    .UseMauiApp<App>()
    // Initialize the .NET MAUI Community Toolkit CameraView by adding the below line of code
    .UseMauiCommunityToolkitCameraView(options =>
    {
#if WINDOWS
        options.PhotoCaptureSource = PhotoCaptureSource.Auto;
#elif
    });

Thoughts?

bijington avatar May 13 '24 20:05 bijington

From reading this https://learn.microsoft.com/en-us/uwp/api/windows.media.capture.photocapturesource?view=winrt-18362 I think we should just go with Auto

bijington avatar May 13 '24 20:05 bijington

@MFinkBK there is now a CameraView NuGet package in the artifacts to try out! Please let us know how it goes! Do keep in mind that things are not finalized until they are merged so expect (breaking) changes to happen.

@bijington I disabled the Tizen target for now on CameraView because it was giving build errors. I'm fine with adding a dummy implementation (or @JoonghyunCho a real implementation...?) but we'll need to make sure it builds! :) uncomment the TargetFrameworks node for Tizen in the CameraView csproj to enable it again.

jfversluis avatar May 24 '24 12:05 jfversluis

not sure if Tizen can support CameraView APIs. Let me check in the coming week.

JoonghyunCho avatar May 25 '24 13:05 JoonghyunCho

I think we should put dummy implementation or set as not supported for Tizen... I don't see any way to test or verify the implementation... 😭

JoonghyunCho avatar May 27 '24 09:05 JoonghyunCho

omg 😮 I will add Tizen fix

JoonghyunCho avatar May 28 '24 10:05 JoonghyunCho

Looks like you made it builds now.

JoonghyunCho avatar May 28 '24 10:05 JoonghyunCho

Docs PR is at: https://github.com/MicrosoftDocs/CommunityToolkit/pull/428

bijington avatar May 28 '24 21:05 bijington

@jfversluis Thanks, the NuGet package is now there. What I will definitely need (but can be a follow-up issue and PR) is CameraPosition.External, e.g. for USB webcams on Windows.

MFinkBK avatar Jun 03 '24 09:06 MFinkBK

Thanks @pictos - great review! I've incorporated your comments here: https://github.com/CommunityToolkit/Maui/pull/1758/commits/0f85de2244c95678f6477b6a60a81781aa1f9cee

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 avatar Jun 05 '24 23:06 TheCodeTraveler

@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:

  1. 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.
  2. The camera switcher and some of the original debug text (camera name, zoom level, etc) are missing on the sample camera view page
  3. 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!

zhitaop avatar Jun 06 '24 16:06 zhitaop