Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/camera/camera/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.12.1

* Adds `setJpegImageQuality` for controlling JPEG compression quality.
* Updates minimum supported SDK version to Flutter 3.44/Dart 3.12.

## 0.12.0+2

* Fixes a crash where a `CameraController` could update its value after being disposed, throwing "A CameraController was used after being disposed".
Expand Down
23 changes: 23 additions & 0 deletions packages/camera/camera/lib/src/camera_controller.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ class CameraValue {
this.isPreviewPaused = false,
this.previewPauseOrientation,
this.videoStabilizationMode = VideoStabilizationMode.off,
// An initializing formal isn't possible here: named parameters can't be
// private, and `isRecordingPaused` is a derived getter, not a stored field.
// ignore: prefer_initializing_formals
}) : _isRecordingPaused = isRecordingPaused;
Comment on lines +54 to 57

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flyby comment while responding to CICD request: With private named parameters it can be, depends on what version of Dart this is on.


/// Creates a new camera controller state for an uninitialized controller.
Expand Down Expand Up @@ -969,6 +972,26 @@ class CameraController extends ValueNotifier<CameraValue> {
}
}

/// Sets the JPEG compression quality for still image capture.
///
/// This only applies to images captured in JPEG format.
/// The [quality] must be between 1 (lowest) and 100 (highest).
///
/// This is a best-effort setting: platforms that do not support controlling
/// the JPEG quality ignore it rather than throwing. See
/// https://github.com/flutter/flutter/issues/191790 for the current state of
/// platform support.
Future<void> setJpegImageQuality(int quality) async {
if (quality < 1 || quality > 100) {
throw ArgumentError.value(quality, 'quality', 'Must be between 1 and 100.');
}
try {
await CameraPlatform.instance.setJpegImageQuality(_cameraId, quality);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bparrishMines I just looked at the platform interface PR and the default implementation throws an unimplemented error, but there's no support query API. That's not a valid combination for new APIs, because there's no way for a client to know if it's safe to call this method.

Either the default needs to be a no-op, or there needs to be a support query that's plumbed through this layer, with clear docs about checking before calling this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with the no-op default. Opened #12123 to change it in the platform interface. Once that lands and publishes I'll bump the constraint here to ^2.13.1 and add a doc note that it's best-effort and ignored where it's not supported. Sounds good?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bparrishMines I just looked at the platform interface PR and the default implementation throws an unimplemented error, but there's no support query API. That's not a valid combination for new APIs, because there's no way for a client to know if it's safe to call this method.

I agree. I didn't want to add an API to query for it because I expected this to be a feature that could be implemented on each platform. (e.g. like adding an API query for takePicture). But I suppose this is the downside of not implementing it on all platforms initially.

Assuming we don't have plans to implement on windows or web in the near future, an unimplemented error or a noop are neither ideal solutions. UnimplementedError would be annoying for windows/web users, while a noop could potentially lead to confusion.

I would prefer that we go with the noop and create an issue to add support for web and windows. Then add the link to the issue in the app-facing plugin so that users can check when support has been added. Therefore the documentation won't cause confusion if we forget to update it after adding support.

} on PlatformException catch (e) {
throw CameraException(e.code, e.message);
}
}

/// Check whether the camera platform supports image streaming.
bool supportsImageStreaming() => CameraPlatform.instance.supportsImageStreaming();

Expand Down
12 changes: 6 additions & 6 deletions packages/camera/camera/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ description: A Flutter plugin for controlling the camera. Supports previewing
Dart.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.12.0+2
version: 0.12.1

environment:
sdk: ^3.10.0
flutter: ">=3.38.0"
sdk: ^3.12.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like yes!

flutter: ">=3.44.0"

flutter:
plugin:
Expand All @@ -21,9 +21,9 @@ flutter:
default_package: camera_web

dependencies:
camera_android_camerax: ^0.7.0
camera_avfoundation: ^0.10.0
camera_platform_interface: ^2.12.0
camera_android_camerax: ^0.7.4
camera_avfoundation: ^0.10.2
camera_platform_interface: ^2.13.1
camera_web: ^0.3.3
flutter:
sdk: flutter
Expand Down
3 changes: 3 additions & 0 deletions packages/camera/camera/test/camera_preview_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ class FakeController extends ValueNotifier<CameraValue> implements CameraControl
Future<Iterable<VideoStabilizationMode>> getSupportedVideoStabilizationModes() async =>
<VideoStabilizationMode>[];

@override
Future<void> setJpegImageQuality(int quality) async {}

@override
bool supportsImageStreaming() => true;
}
Expand Down
62 changes: 62 additions & 0 deletions packages/camera/camera/test/camera_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,64 @@ void main() {
);
});

test('setJpegImageQuality() calls CameraPlatform', () async {
final cameraController = CameraController(
const CameraDescription(
name: 'cam',
lensDirection: CameraLensDirection.back,
sensorOrientation: 90,
),
ResolutionPreset.max,
);
await cameraController.initialize();

await cameraController.setJpegImageQuality(50);

verify(CameraPlatform.instance.setJpegImageQuality(cameraController.cameraId, 50)).called(1);
});

test('setJpegImageQuality() throws CameraException on PlatformException', () async {
final cameraController = CameraController(
const CameraDescription(
name: 'cam',
lensDirection: CameraLensDirection.back,
sensorOrientation: 90,
),
ResolutionPreset.max,
);
await cameraController.initialize();

when(
CameraPlatform.instance.setJpegImageQuality(cameraController.cameraId, 50),
).thenThrow(PlatformException(code: 'TEST_ERROR', message: 'This is a test error message'));

expect(
cameraController.setJpegImageQuality(50),
throwsA(
isA<CameraException>().having(
(CameraException error) => error.description,
'TEST_ERROR',
'This is a test error message',
),
),
);
});

test('setJpegImageQuality() throws ArgumentError for invalid values', () async {
final cameraController = CameraController(
const CameraDescription(
name: 'cam',
lensDirection: CameraLensDirection.back,
sensorOrientation: 90,
),
ResolutionPreset.max,
);
await cameraController.initialize();

expect(() => cameraController.setJpegImageQuality(0), throwsA(isA<ArgumentError>()));
expect(() => cameraController.setJpegImageQuality(101), throwsA(isA<ArgumentError>()));
});

test('setExposureMode() calls $CameraPlatform', () async {
final cameraController = CameraController(
const CameraDescription(
Expand Down Expand Up @@ -3646,6 +3704,10 @@ class MockCameraPlatform extends Mock with MockPlatformInterfaceMixin implements
@override
Future<void> setVideoStabilizationMode(int cameraId, VideoStabilizationMode mode) async =>
super.noSuchMethod(Invocation.method(#setVideoStabilizationMode, <Object?>[cameraId, mode]));

@override
Future<void> setJpegImageQuality(int? cameraId, int? quality) async =>
super.noSuchMethod(Invocation.method(#setJpegImageQuality, <Object?>[cameraId, quality]));
}

class MockCameraDescription extends CameraDescription {
Expand Down