Skip to content

Conversation

@dasyad00
Copy link
Contributor

@dasyad00 dasyad00 commented Oct 25, 2025

Implements MediaSettings::fps in camera_android_camerax for image preview, image streaming, and video recording.

  • Resolves #167719: implements the feature instead of adding documentation that it does not work.
  • Partially resolves #176148: only implements for camera_android_camerax, and uses the existing MediaSettings::fps instead of adding parameters to CameraImageStreamOptions

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

@dasyad00 dasyad00 force-pushed the androix_camerax/control_fps branch from 6707def to 3404a0b Compare October 25, 2025 13:40
@dasyad00 dasyad00 marked this pull request as ready for review October 25, 2025 13:40
@dasyad00 dasyad00 requested a review from camsim99 as a code owner October 25, 2025 13:40
Copy link
Contributor

@camsim99 camsim99 left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you so much for taking this on. This will be a huge improvement to the plugin!

I think we need to add a Dart test, but otherwise LGTM. Let's get a second review.


@RunWith(RobolectricTestRunner.class)
public class ImageAnalysisTest {
@SuppressWarnings({"unchecked", "rawtypes"})
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: where you added this annotation, can you leave a comment as to why this is safe?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add a test to make sure that if the fps media setting is passed, we actually set it as expected. I know these test are a bit daunting to write so let me know if you need help 😅

Just in case you need help getting started: The hardest part is mocking the pigeon calls. There are several other createCamera tests you can copy or modify to handle the mocks, though. But there are some helper methods at the top of the file that can help with that. I think you could end up doing something like

newCameraSelector:
({
LensFacing? requireLensFacing,
CameraInfo? cameraInfoForFilter,
// ignore: non_constant_identifier_names
BinaryMessenger? pigeon_binaryMessenger,
// ignore: non_constant_identifier_names
PigeonInstanceManager? pigeon_instanceManager,
}) {
if (cameraInfoForFilter == mockFrontCameraInfo) {
return mockFrontCameraSelector;
}
return mockBackCameraSelector;
},
where can check the fps and return a mock object then check that the use case (like camera.videoCapture) equals your mock object.

Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

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

LGTM pending unit tests and updating the lint-baseline.xml.

class ImageAnalysisProxyApi extends PigeonApiImageAnalysis {
static final long CLEAR_FINALIZED_WEAK_REFERENCES_INTERVAL_FOR_IMAGE_ANALYSIS = 1000;

@OptIn(markerClass = ExperimentalCamera2Interop.class)
Copy link
Contributor

Choose a reason for hiding this comment

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

This plugin updates [lint-baseline.xml] (https://github.com/flutter/packages/blob/main/packages/camera/camera_android_camerax/android/lint-baseline.xml) to handle this specific warning.

This should be updateable by following https://developer.android.com/studio/write/lint#snapshot in the example app. But this can be updated after getting both LGTMs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants