[camera_windows] Improve several error handling scenarios (#6149)

Author: loic-sharma

packages/camera/camera_windows/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.1.0+5
+
+* Fixes bugs in in error handling.
+
 ## 0.1.0+4
 
 * Allows retrying camera initialization after error.

packages/camera/camera_windows/lib/camera_windows.dart

@@ -22,7 +22,7 @@ class CameraWindows extends CameraPlatform {
   final MethodChannel pluginChannel =
       const MethodChannel('plugins.flutter.io/camera_windows');
 
-  /// Camera specific method channels to allow comminicating with specific cameras.
+  /// Camera specific method channels to allow communicating with specific cameras.
   final Map<int, MethodChannel> _cameraChannels = <int, MethodChannel>{};
 
   /// The controller that broadcasts events coming from handleCameraMethodCall

packages/camera/camera_windows/pubspec.yaml

@@ -2,7 +2,7 @@ name: camera_windows
 description: A Flutter plugin for getting information about and controlling the camera on Windows.
 repository: https://github.com/flutter/plugins/tree/main/packages/camera/camera_windows
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
-version: 0.1.0+4
+version: 0.1.0+5
 
 environment:
   sdk: ">=2.12.0 <3.0.0"

packages/camera/camera_windows/windows/camera.cpp

@@ -85,9 +85,9 @@ bool CameraImpl::HasPendingResultByType(PendingResultType type) const {
 }
 
 void CameraImpl::SendErrorForPendingResults(const std::string& error_code,
-                                            const std::string& descripion) {
+                                            const std::string& description) {
   for (const auto& pending_result : pending_results_) {
-    pending_result.second->Error(error_code, descripion);
+    pending_result.second->Error(error_code, description);
   }
   pending_results_.clear();
 }

packages/camera/camera_windows/windows/camera.h

@@ -139,9 +139,9 @@ class CameraImpl : public Camera {
   // error ID and description. Pending results are cleared in the process.
   //
   // error_code: A string error code describing the error.
-  // error_message: A user-readable error message (optional).
+  // description: A user-readable error message (optional).
   void SendErrorForPendingResults(const std::string& error_code,
-                                  const std::string& descripion);
+                                  const std::string& description);
 
   // Called when camera is disposed.
   // Sends camera closing message to the cameras method channel.

packages/camera/camera_windows/windows/capture_controller.cpp

@@ -237,14 +237,16 @@ HRESULT CaptureControllerImpl::CreateCaptureEngine() {
     return hr;
   }
 
+  // Check MF_CAPTURE_ENGINE_INITIALIZED event handling
+  // for response process.
   hr = capture_engine_->Initialize(capture_engine_callback_handler_.Get(),
                                    attributes.Get(), audio_source_.Get(),
                                    video_source_.Get());
   return hr;
 }
 
 void CaptureControllerImpl::ResetCaptureController() {
-  if (record_handler_) {
+  if (record_handler_ && record_handler_->CanStop()) {
     if (record_handler_->IsContinuousRecording()) {
       StopRecord();
     } else if (record_handler_->IsTimedRecording()) {
@@ -391,7 +393,7 @@ uint32_t CaptureControllerImpl::GetMaxPreviewHeight() const {
   }
 }
 
-// Finds best mediat type for given source stream index and max height;
+// Finds best media type for given source stream index and max height;
 bool FindBestMediaType(DWORD source_stream_index, IMFCaptureSource* source,
                        IMFMediaType** target_media_type, uint32_t max_height,
                        uint32_t* target_frame_width,
@@ -533,8 +535,6 @@ void CaptureControllerImpl::StopRecord() {
   // Check MF_CAPTURE_ENGINE_RECORD_STOPPED event handling for response
   // process.
   if (!record_handler_->StopRecord(capture_engine_.Get())) {
-    // Destroy record handler on error cases to make sure state is resetted.
-    record_handler_ = nullptr;
     return OnRecordStopped(false, "Failed to stop video recording");
   }
 }
@@ -578,6 +578,8 @@ void CaptureControllerImpl::StartPreview() {
   texture_handler_->UpdateTextureSize(preview_frame_width_,
                                       preview_frame_height_);
 
+  // TODO(loic-sharma): This does not handle duplicate calls properly.
+  // See: https://github.com/flutter/flutter/issues/108404
   if (!preview_handler_) {
     preview_handler_ = std::make_unique<PreviewHandler>();
   } else if (preview_handler_->IsInitialized()) {
@@ -605,7 +607,7 @@ void CaptureControllerImpl::StartPreview() {
 void CaptureControllerImpl::StopPreview() {
   assert(capture_engine_);
 
-  if (!IsInitialized() && !preview_handler_) {
+  if (!IsInitialized() || !preview_handler_) {
     return;
   }
 
@@ -619,7 +621,7 @@ void CaptureControllerImpl::StopPreview() {
 void CaptureControllerImpl::PausePreview() {
   assert(capture_controller_listener_);
 
-  if (!preview_handler_ && !preview_handler_->IsInitialized()) {
+  if (!preview_handler_ || !preview_handler_->IsInitialized()) {
     return capture_controller_listener_->OnPausePreviewFailed(
         "Preview not started");
   }
@@ -638,7 +640,7 @@ void CaptureControllerImpl::PausePreview() {
 void CaptureControllerImpl::ResumePreview() {
   assert(capture_controller_listener_);
 
-  if (!preview_handler_ && !preview_handler_->IsInitialized()) {
+  if (!preview_handler_ || !preview_handler_->IsInitialized()) {
     return capture_controller_listener_->OnResumePreviewFailed(
         "Preview not started");
   }
@@ -722,6 +724,13 @@ void CaptureControllerImpl::OnPicture(bool success, const std::string& error) {
 void CaptureControllerImpl::OnCaptureEngineInitialized(
     bool success, const std::string& error) {
   if (capture_controller_listener_) {
+    if (!success) {
+      capture_controller_listener_->OnCreateCaptureEngineFailed(
+          "Failed to initialize capture engine");
+      ResetCaptureController();
+      return;
+    }
+
     // Create texture handler and register new texture.
     texture_handler_ = std::make_unique<TextureHandler>(texture_registrar_);
 
@@ -848,7 +857,7 @@ void CaptureControllerImpl::UpdateCaptureTime(uint64_t capture_time_us) {
   }
 
   if (preview_handler_ && preview_handler_->IsStarting()) {
-    // Informs that first frame is captured succeffully and preview has
+    // Informs that first frame is captured successfully and preview has
     // started.
     OnPreviewStarted(true, "");
   }

packages/camera/camera_windows/windows/photo_handler.h

@@ -51,7 +51,7 @@ class PhotoHandler {
   bool TakePhoto(const std::string& file_path, IMFCaptureEngine* capture_engine,
                  IMFMediaType* base_media_type);
 
-  // Set the photo handler recording state to: kIdel.
+  // Set the photo handler recording state to: kIdle.
   void OnPhotoTaken();
 
   // Returns true if photo state is kIdle.

packages/camera/camera_windows/windows/preview_handler.h

@@ -75,7 +75,7 @@ class PreviewHandler {
 
   // Returns true if preview state is running or paused.
   bool IsInitialized() const {
-    return preview_state_ == PreviewState::kRunning &&
+    return preview_state_ == PreviewState::kRunning ||
            preview_state_ == PreviewState::kPaused;
   }
 

packages/camera/camera_windows/windows/record_handler.cpp

@@ -238,6 +238,7 @@ void RecordHandler::OnRecordStopped() {
     recording_duration_us_ = 0;
     max_video_duration_ms_ = -1;
     recording_state_ = RecordState::kNotStarted;
+    type_ = RecordingType::kNone;
   }
 }
 

packages/camera/camera_windows/windows/record_handler.h

@@ -16,6 +16,8 @@ namespace camera_windows {
 using Microsoft::WRL::ComPtr;
 
 enum class RecordingType {
+  // Camera is not recording.
+  kNone,
   // Recording continues until it is stopped with a separate stop command.
   kContinuous,
   // Recording stops automatically after requested record time is passed.
@@ -109,7 +111,7 @@ class RecordHandler {
   uint64_t recording_duration_us_ = 0;
   std::string file_path_;
   RecordState recording_state_ = RecordState::kNotStarted;
-  RecordingType type_;
+  RecordingType type_ = RecordingType::kNone;
   ComPtr<IMFCaptureRecordSink> record_sink_;
 };