Skip to content

Conversation

@ruicraveiro
Copy link

Adds support for video stabilization to camera_platform_interface.

  • Adds getSupportedVideoStabilizationModes() and setVideoStabilizationMode() methods to CameraPlatform.

  • Adds VideoStabilizationMode enum to represent an abstraction of the available video stabilization modes, meant for Android and iOS, mapped as follows:

/// The possible video stabilization modes that can be capturing video.
enum VideoStabilizationMode {
  /// Video stabilization is disabled.
  off,

  /// Least stabilized video stabilization mode with the least latency.
  level1,

  /// More stabilized video with more latency.
  level2,

  /// Most stabilized video with the most latency.
  level3,
}

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

Address issue flutter/flutter#89525.
It is the camera_platform_interface sub-PR for #7108.

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2 3

Copy link

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

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 adds support for video stabilization to the camera_platform_interface. It introduces a VideoStabilizationMode enum, along with methods getSupportedVideoStabilizationModes and setVideoStabilizationMode in CameraPlatform. A helper method getFallbackVideoStabilizationMode is also added to handle unsupported modes. The changes are accompanied by corresponding tests. My review includes suggestions to improve code consistency by removing unnecessary async keywords and to fix a typo in the test descriptions.

/// Gets a list of video stabilization modes that are supported for the selected camera.
Future<Iterable<VideoStabilizationMode>> getSupportedVideoStabilizationModes(
int cameraId,
) async => <VideoStabilizationMode>[];

Choose a reason for hiding this comment

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

medium

The async keyword is not necessary here as you are returning a value directly. You can use Future.value() to make it more concise and slightly more performant.

Suggested change
) async => <VideoStabilizationMode>[];
) => Future.value(<VideoStabilizationMode>[]);

Comment on lines 293 to 295
) async {
throw UnimplementedError('setVideoStabilizationMode() is not implemented.');
}

Choose a reason for hiding this comment

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

medium

The async keyword is not necessary here since this method synchronously throws an error. You can remove it to be consistent with other unimplemented methods in this class, such as setZoomLevel.

Suggested change
) async {
throw UnimplementedError('setVideoStabilizationMode() is not implemented.');
}
) {
throw UnimplementedError('setVideoStabilizationMode() is not implemented.');
}

},
);

test('getFallbackViewStabilizationMode returns level2 for mode level3', () {

Choose a reason for hiding this comment

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

medium

There's a typo in the test description. It should be Video instead of View.

Suggested change
test('getFallbackViewStabilizationMode returns level2 for mode level3', () {
test('getFallbackVideoStabilizationMode returns level2 for mode level3', () {

expect(fallbackMode, VideoStabilizationMode.level2);
});

test('getFallbackViewStabilizationMode returns level1 for mode level2', () {

Choose a reason for hiding this comment

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

medium

There's a typo in the test description. It should be Video instead of View.

Suggested change
test('getFallbackViewStabilizationMode returns level1 for mode level2', () {
test('getFallbackVideoStabilizationMode returns level1 for mode level2', () {

expect(fallbackMode, VideoStabilizationMode.level1);
});

test('getFallbackViewStabilizationMode returns off for mode level1', () {

Choose a reason for hiding this comment

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

medium

There's a typo in the test description. It should be Video instead of View.

Suggested change
test('getFallbackViewStabilizationMode returns off for mode level1', () {
test('getFallbackVideoStabilizationMode returns off for mode level1', () {

expect(fallbackMode, VideoStabilizationMode.off);
});

test('getFallbackViewStabilizationMode returns null for mode off', () {

Choose a reason for hiding this comment

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

medium

There's a typo in the test description. It should be Video instead of View.

Suggested change
test('getFallbackViewStabilizationMode returns null for mode off', () {
test('getFallbackVideoStabilizationMode returns null for mode off', () {

- Adds getSupportedVideoStabilizationModes() and
  setVideoStabilizationMode() methods to CameraPlatform.

- Adds VideoStabilizationMode enum to represent an abstraction of the
  available video stabilization modes, meant for Android and iOS,
  mapped as follows:

  /// Video stabilization is disabled.
  off,

  /// Least stabilized video stabilization mode with the least latency.
  level1,

  /// More stabilized video with more latency.
  level2,

  /// Most stabilized video with the most latency.
  level3;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant