Skip to content

[camera_platform_interface] Make setJpegImageQuality default a no-op#12123

Merged
auto-submit[bot] merged 1 commit into
flutter:mainfrom
Bolling88:jpeg-quality-noop-default
Jul 9, 2026
Merged

[camera_platform_interface] Make setJpegImageQuality default a no-op#12123
auto-submit[bot] merged 1 commit into
flutter:mainfrom
Bolling88:jpeg-quality-noop-default

Conversation

@Bolling88

Copy link
Copy Markdown
Contributor

The default implementation of setJpegImageQuality threw UnimplementedError, but there is no support-query API for it, so a client has no way to know whether it is safe to call (raised in review of #11155). JPEG quality is a best-effort setting, so this changes the default to a no-op: platforms that support it override the method, and platforms that don't (e.g. web) simply ignore it.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request changes the default implementation of setJpegImageQuality in CameraPlatform from throwing an UnimplementedError to a no-op, allowing unsupported platforms to ignore the setting safely. The package version is bumped to 2.13.1, and corresponding tests and CHANGELOG are updated. Feedback suggests adding an assertion to enforce the valid quality range (1 to 100) and returning Future<void>.value() directly to avoid async overhead, along with updating the tests to verify this assertion.

Comment on lines +344 to 347
Future<void> setJpegImageQuality(int cameraId, int quality) async {
// No-op by default. Platforms that support setting the JPEG quality
// override this method.
}

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.

medium

The documentation states that quality must be between 1 and 100. Adding an assertion to enforce this contract in the default implementation ensures consistent behavior across all platforms during development, preventing silent bugs when testing on platforms that do not support JPEG quality control.

Additionally, since this is a synchronous no-op, we can return Future<void>.value() directly and avoid the async keyword overhead.

Suggested change
Future<void> setJpegImageQuality(int cameraId, int quality) async {
// No-op by default. Platforms that support setting the JPEG quality
// override this method.
}
Future<void> setJpegImageQuality(int cameraId, int quality) {
assert(quality >= 1 && quality <= 100);
return Future<void>.value();
}

Comment on lines +356 to 362
test('Default implementation of setJpegImageQuality() is a no-op', () {
// Arrange
final cameraPlatform = ExtendsCameraPlatform();

// Act & Assert
expect(() => cameraPlatform.setJpegImageQuality(1, 50), throwsUnimplementedError);
expect(cameraPlatform.setJpegImageQuality(1, 50), completes);
});

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.

medium

Update the test to verify that the default implementation completes successfully for valid quality values, and asserts on invalid quality values (e.g., < 1 or > 100).

    test('Default implementation of setJpegImageQuality() is a no-op and asserts on invalid quality', () {
      // Arrange
      final cameraPlatform = ExtendsCameraPlatform();

      // Act & Assert
      expect(cameraPlatform.setJpegImageQuality(1, 50), completes);
      expect(() => cameraPlatform.setJpegImageQuality(1, 0), throwsAssertionError);
      expect(() => cameraPlatform.setJpegImageQuality(1, 101), throwsAssertionError);
    });

@bparrishMines bparrishMines left a comment

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.

LGTM

@stuartmorgan-g for secondary review since you left the comment at #11155 (comment)

@stuartmorgan-g stuartmorgan-g left a comment

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.

LGTM

@stuartmorgan-g stuartmorgan-g added autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD labels Jul 9, 2026
@auto-submit auto-submit Bot merged commit 6fa759e into flutter:main Jul 9, 2026
87 checks passed
Renzo-Olivares pushed a commit to mvincentong/flutter that referenced this pull request Jul 9, 2026
…r#189194)

flutter/packages@52d84d6...20928d5

2026-07-09 44525804+sailendrabathi@users.noreply.github.com
[video_player_android] Implement backBufferDurationMs in ExoPlayer
(flutter/packages#12124)
2026-07-09 bolling.ludwig@gmail.com [camera_platform_interface] Make
setJpegImageQuality default a no-op (flutter/packages#12123)
2026-07-09 36081049+anirudhsharma392@users.noreply.github.com
[local_auth] Clarify getAvailableBiometrics documentation regarding iOS
permissions (flutter/packages#11901)
2026-07-09 stuartmorgan@google.com [ci] Add a third web Dart unit test
shard (2/2) (flutter/packages#12150)
2026-07-08 stuartmorgan@google.com [ci] Add a third web Dart unit test
shard (1/2) (flutter/packages#12138)
2026-07-08 43054281+camsim99@users.noreply.github.com Add pre-commit
hook to verify static analysis passes & format is correct
(flutter/packages#11941)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD p: camera

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants