Skip to content

Commit

Permalink
Make {Start,Stop}ImageStream synchronous
Browse files Browse the repository at this point in the history
The logic is synchronous, so there is no need to have the `@async`
annotation here.

Addresses:
  flutter#7067 (comment)
  • Loading branch information
liff committed Dec 5, 2024
1 parent 01cc152 commit 6481b07
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 161 deletions.
2 changes: 1 addition & 1 deletion packages/camera/camera_windows/lib/src/messages.g.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Autogenerated from Pigeon (v22.6.0), do not edit directly.
// Autogenerated from Pigeon (v22.6.2), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import, no_leading_underscores_for_local_identifiers

Expand Down
2 changes: 0 additions & 2 deletions packages/camera/camera_windows/pigeons/messages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,9 @@ abstract class CameraApi {
String stopVideoRecording(int cameraId);

/// Starts the image stream for the given camera.
@async
void startImageStream(int cameraId);

/// Stops the image stream for the given camera.
@async
void stopImageStream(int cameraId);

/// Starts the preview stream for the given camera.
Expand Down
2 changes: 0 additions & 2 deletions packages/camera/camera_windows/windows/camera.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ enum class PendingResultType {
kTakePicture,
kStartRecord,
kStopRecord,
kStartStream,
kStopStream,
kPausePreview,
kResumePreview,
};
Expand Down
48 changes: 16 additions & 32 deletions packages/camera/camera_windows/windows/camera_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -369,51 +369,35 @@ void CameraPlugin::StopVideoRecording(
}
}

void CameraPlugin::StartImageStream(
int64_t camera_id,
std::function<void(std::optional<FlutterError> reply)> result) {
// check if request already exists
std::optional<FlutterError> CameraPlugin::StartImageStream(int64_t camera_id) {
Camera* camera = GetCameraByCameraId(camera_id);
if (!camera) {
return result(FlutterError("camera_error", "Camera not created"));
}
if (camera->HasPendingResultByType(PendingResultType::kStartStream)) {
return result(
FlutterError("camera_error", "Pending start stream request exists"));
return FlutterError("camera_error", "Camera not created");
}

if (!event_sink) {
return result(FlutterError("camera_error",
"Unable to make event channel from windows"));
return FlutterError("camera_error",
"Unable to make event channel from windows");
}

if (camera->AddPendingVoidResult(PendingResultType::kStartStream,
std::move(result))) {
CaptureController* cc = camera->GetCaptureController();
assert(cc);
cc->StartImageStream(std::move(event_sink));
}
CaptureController* cc = camera->GetCaptureController();
assert(cc);
cc->StartImageStream(std::move(event_sink));

return std::nullopt;
}

void CameraPlugin::StopImageStream(
int64_t camera_id,
std::function<void(std::optional<FlutterError> reply)> result) {
// check if request already exists
std::optional<FlutterError> CameraPlugin::StopImageStream(int64_t camera_id) {
Camera* camera = GetCameraByCameraId(camera_id);
if (!camera) {
return result(FlutterError("camera_error", "Camera not created"));
}
if (camera->HasPendingResultByType(PendingResultType::kStopStream)) {
return result(
FlutterError("camera_error", "Pending stop stream request exists"));
return FlutterError("camera_error", "Camera not created");
}

if (camera->AddPendingVoidResult(PendingResultType::kStopStream,
std::move(result))) {
CaptureController* cc = camera->GetCaptureController();
assert(cc);
cc->StopImageStream();
}
CaptureController* cc = camera->GetCaptureController();
assert(cc);
cc->StopImageStream();

return std::nullopt;
}

void CameraPlugin::TakePicture(
Expand Down
8 changes: 2 additions & 6 deletions packages/camera/camera_windows/windows/camera_plugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,8 @@ class CameraPlugin : public flutter::Plugin,
void StopVideoRecording(
int64_t camera_id,
std::function<void(ErrorOr<std::string> reply)> result) override;
void StartImageStream(
int64_t camera_id,
std::function<void(std::optional<FlutterError> reply)> result) override;
void StopImageStream(
int64_t camera_id,
std::function<void(std::optional<FlutterError> reply)> result) override;
std::optional<FlutterError> StartImageStream(int64_t camera_id) override;
std::optional<FlutterError> StopImageStream(int64_t camera_id) override;
void TakePicture(
int64_t camera_id,
std::function<void(ErrorOr<std::string> reply)> result) override;
Expand Down
40 changes: 19 additions & 21 deletions packages/camera/camera_windows/windows/messages.g.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Autogenerated from Pigeon (v22.6.0), do not edit directly.
// Autogenerated from Pigeon (v22.6.2), do not edit directly.
// See also: https://pub.dev/packages/pigeon

#undef _HAS_EXCEPTIONS
Expand Down Expand Up @@ -516,16 +516,15 @@ void CameraApi::SetUp(flutter::BinaryMessenger* binary_messenger,
return;
}
const int64_t camera_id_arg = encodable_camera_id_arg.LongValue();
api->StartImageStream(
camera_id_arg, [reply](std::optional<FlutterError>&& output) {
if (output.has_value()) {
reply(WrapError(output.value()));
return;
}
EncodableList wrapped;
wrapped.push_back(EncodableValue());
reply(EncodableValue(std::move(wrapped)));
});
std::optional<FlutterError> output =
api->StartImageStream(camera_id_arg);
if (output.has_value()) {
reply(WrapError(output.value()));
return;
}
EncodableList wrapped;
wrapped.push_back(EncodableValue());
reply(EncodableValue(std::move(wrapped)));
} catch (const std::exception& exception) {
reply(WrapError(exception.what()));
}
Expand All @@ -552,16 +551,15 @@ void CameraApi::SetUp(flutter::BinaryMessenger* binary_messenger,
return;
}
const int64_t camera_id_arg = encodable_camera_id_arg.LongValue();
api->StopImageStream(
camera_id_arg, [reply](std::optional<FlutterError>&& output) {
if (output.has_value()) {
reply(WrapError(output.value()));
return;
}
EncodableList wrapped;
wrapped.push_back(EncodableValue());
reply(EncodableValue(std::move(wrapped)));
});
std::optional<FlutterError> output =
api->StopImageStream(camera_id_arg);
if (output.has_value()) {
reply(WrapError(output.value()));
return;
}
EncodableList wrapped;
wrapped.push_back(EncodableValue());
reply(EncodableValue(std::move(wrapped)));
} catch (const std::exception& exception) {
reply(WrapError(exception.what()));
}
Expand Down
10 changes: 3 additions & 7 deletions packages/camera/camera_windows/windows/messages.g.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Autogenerated from Pigeon (v22.6.0), do not edit directly.
// Autogenerated from Pigeon (v22.6.2), do not edit directly.
// See also: https://pub.dev/packages/pigeon

#ifndef PIGEON_MESSAGES_G_H_
Expand Down Expand Up @@ -190,13 +190,9 @@ class CameraApi {
int64_t camera_id,
std::function<void(ErrorOr<std::string> reply)> result) = 0;
// Starts the image stream for the given camera.
virtual void StartImageStream(
int64_t camera_id,
std::function<void(std::optional<FlutterError> reply)> result) = 0;
virtual std::optional<FlutterError> StartImageStream(int64_t camera_id) = 0;
// Stops the image stream for the given camera.
virtual void StopImageStream(
int64_t camera_id,
std::function<void(std::optional<FlutterError> reply)> result) = 0;
virtual std::optional<FlutterError> StopImageStream(int64_t camera_id) = 0;
// Starts the preview stream for the given camera.
virtual void PausePreview(
int64_t camera_id,
Expand Down
100 changes: 10 additions & 90 deletions packages/camera/camera_windows/windows/test/camera_plugin_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ using ::testing::EndsWith;
using ::testing::Eq;
using ::testing::Pointee;
using ::testing::Return;
using ::testing::Optional;
using ::testing::Property;

void MockInitCamera(MockCamera* camera, bool success) {
EXPECT_CALL(*camera,
Expand Down Expand Up @@ -366,34 +368,14 @@ TEST(CameraPlugin, StartImageStreamHandlerCallsStartImageStream) {
return cam->camera_id_ == camera_id;
});

EXPECT_CALL(*camera,
HasPendingResultByType(Eq(PendingResultType::kStartStream)))
.Times(1)
.WillOnce(Return(false));

EXPECT_CALL(*camera,
AddPendingVoidResult(Eq(PendingResultType::kStartStream), _))
.Times(1)
.WillOnce([cam = camera.get()](
PendingResultType type,
std::function<void(std::optional<FlutterError>)> result) {
cam->pending_void_result_ = std::move(result);
return true;
});

EXPECT_CALL(*camera, GetCaptureController)
.Times(1)
.WillOnce([cam = camera.get()]() {
assert(cam->pending_void_result_);
return cam->capture_controller_.get();
});

EXPECT_CALL(*capture_controller, StartImageStream)
.Times(1)
.WillOnce([cam = camera.get()]() {
assert(cam->pending_void_result_);
return cam->pending_void_result_(std::nullopt);
});
.Times(1);

camera->camera_id_ = mock_camera_id;
camera->capture_controller_ = std::move(capture_controller);
Expand All @@ -402,24 +384,14 @@ TEST(CameraPlugin, StartImageStreamHandlerCallsStartImageStream) {
std::make_unique<MockBinaryMessenger>().get(),
std::make_unique<MockCameraFactory>());

// Add mocked camera to plugins camera list.
plugin.AddCamera(std::move(camera));

// Set the event sink to a mocked event sink.
auto mock_event_sink = std::make_unique<MockEventSink>();
plugin.SetEventSink(std::move(mock_event_sink));

bool result_called = false;
std::function<void(std::optional<FlutterError>)> start_image_stream_result =
[&result_called](std::optional<FlutterError> reply) {
EXPECT_FALSE(result_called); // Ensure only one reply call.
result_called = true;
EXPECT_FALSE(reply);
};

plugin.StartImageStream(mock_camera_id, std::move(start_image_stream_result));
// Add mocked camera to plugins camera list.
plugin.AddCamera(std::move(camera));

EXPECT_TRUE(result_called);
EXPECT_EQ(plugin.StartImageStream(mock_camera_id), std::nullopt);
}

TEST(CameraPlugin, StartImageStreamHandlerErrorOnInvalidCameraId) {
Expand Down Expand Up @@ -452,18 +424,7 @@ TEST(CameraPlugin, StartImageStreamHandlerErrorOnInvalidCameraId) {
// Add mocked camera to plugins camera list.
plugin.AddCamera(std::move(camera));

bool result_called = false;
std::function<void(std::optional<FlutterError>)> start_image_stream_result =
[&result_called](std::optional<FlutterError> reply) {
EXPECT_FALSE(result_called); // Ensure only one reply call.
result_called = true;
EXPECT_TRUE(reply);
};

plugin.StartImageStream(missing_camera_id,
std::move(start_image_stream_result));

EXPECT_TRUE(result_called);
EXPECT_THAT(plugin.StartImageStream(missing_camera_id), Optional(Property("code", &FlutterError::code, "camera_error")));
}

TEST(CameraPlugin, StopImageStreamHandlerCallsStopImageStream) {
Expand All @@ -481,34 +442,14 @@ TEST(CameraPlugin, StopImageStreamHandlerCallsStopImageStream) {
return cam->camera_id_ == camera_id;
});

EXPECT_CALL(*camera,
HasPendingResultByType(Eq(PendingResultType::kStopStream)))
.Times(1)
.WillOnce(Return(false));

EXPECT_CALL(*camera,
AddPendingVoidResult(Eq(PendingResultType::kStopStream), _))
.Times(1)
.WillOnce([cam = camera.get()](
PendingResultType type,
std::function<void(std::optional<FlutterError>)> result) {
cam->pending_void_result_ = std::move(result);
return true;
});

EXPECT_CALL(*camera, GetCaptureController)
.Times(1)
.WillOnce([cam = camera.get()]() {
assert(cam->pending_void_result_);
return cam->capture_controller_.get();
});

EXPECT_CALL(*capture_controller, StopImageStream)
.Times(1)
.WillOnce([cam = camera.get()]() {
assert(cam->pending_void_result_);
return cam->pending_void_result_(std::nullopt);
});
.Times(1);

camera->camera_id_ = mock_camera_id;
camera->capture_controller_ = std::move(capture_controller);
Expand All @@ -520,17 +461,7 @@ TEST(CameraPlugin, StopImageStreamHandlerCallsStopImageStream) {
// Add mocked camera to plugins camera list.
plugin.AddCamera(std::move(camera));

bool result_called = false;
std::function<void(std::optional<FlutterError>)> stop_image_stream_result =
[&result_called](std::optional<FlutterError> reply) {
EXPECT_FALSE(result_called); // Ensure only one reply call.
result_called = true;
EXPECT_FALSE(reply);
};

plugin.StopImageStream(mock_camera_id, std::move(stop_image_stream_result));

EXPECT_TRUE(result_called);
EXPECT_EQ(plugin.StopImageStream(mock_camera_id), std::nullopt);
}

TEST(CameraPlugin, StopImageStreamHandlerErrorOnInvalidCameraId) {
Expand Down Expand Up @@ -563,18 +494,7 @@ TEST(CameraPlugin, StopImageStreamHandlerErrorOnInvalidCameraId) {
// Add mocked camera to plugins camera list.
plugin.AddCamera(std::move(camera));

bool result_called = false;
std::function<void(std::optional<FlutterError>)> stop_image_stream_result =
[&result_called](std::optional<FlutterError> reply) {
EXPECT_FALSE(result_called); // Ensure only one reply call.
result_called = true;
EXPECT_TRUE(reply);
};

plugin.StopImageStream(missing_camera_id,
std::move(stop_image_stream_result));

EXPECT_TRUE(result_called);
EXPECT_THAT(plugin.StopImageStream(missing_camera_id), Optional(Property("code", &FlutterError::code, "camera_error")));
}

TEST(CameraPlugin, InitializeHandlerErrorOnInvalidCameraId) {
Expand Down

0 comments on commit 6481b07

Please sign in to comment.