[camera] Android: Improve image streaming by creating a request suita… #2239
Conversation
mklim
left a comment
mklim
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Appreciate you finding and fixing this issue. If you don't have time to keep working on this PR, let us know and someone from the Flutter team will be happy to shepherd it forward from here.
This fix LG, but all PRs need to have automated tests before we can merge them into the tree, see Tree Hygiene #4.
Unfortunately testing this is going to be painful. The resulting behavior isn't something we can meaningfully verify. I think our only bet is to write an interaction test using JUnit for the Java code. Not ideal, but it will work. Then the test itself is going to be a little tricky to implement becausecameraDevice isn't exposed anywhere, and we'd basically want to put in a mock or spy cameraDevice and verify that it has createCaptureRequest called with the right template type when we call startPreviewWithImageStream on Camera.java. I don't mind adding a package private setTestCameraDevice method to Camera.java just to get the mock initialized in our test, but again that's not ideal. I think it would be possible to write a nicer test here with a broader refactor of the class and how it gets cameraDevice, but I don't think it would be trivial or necessarily worth inducing all that risk. So just the simple interaction test with a new setter is good for this PR.
Thanks again for the contribution and the fix. Let us know if you don't have time to keep working on this, and someone from our team will be happy to help land it.
| public void startPreviewWithImageStream(EventChannel imageStreamChannel) | ||
| throws CameraAccessException { | ||
| createCaptureSession(CameraDevice.TEMPLATE_STILL_CAPTURE, imageStreamReader.getSurface()); | ||
| createCaptureSession(CameraDevice.TEMPLATE_RECORD, imageStreamReader.getSurface()); |
There was a problem hiding this comment.
Do you think maybe it would make even more sense to use TEMPLATE_PREVIEW? That setting is optimized for a high FPS without much post processing, which I think makes sense here. RECORD is very similar but it has a "stable" FPS with more post processing.
There was a problem hiding this comment.
I've tested with both but using TEMPLATE_PREVIEW breaks trying to detect objects in a CameraImage using the ml kit on my test devices. With TEMPLATE_RECORD it both fixes the streaming issues and keeps the functionality intact. @mehmetf might have more insight in this as I know he looked in to improving streaming.
There was a problem hiding this comment.
Thanks for pinging me here. We forked this plugin for Google and have made a number of improvements. Glad to see similar efforts upstream:
TEMPLATE_RECORDis indeed what we use now for preview.- We also start a different thread for image stream handler (that we dispose when image stream is stopped).
- We drain the image queue while waiting for processing to finish. This allows preview to be smooth even if the stream does not deliver high fps (which is often unneeded).
Our code looks a bit like this:
public void startPreviewWithImageStream(
ImageReader imageStreamReader, StreamProcessor<Image> processor)
throws CameraAccessException {
imageStreamHandler = CameraUtils.startThread("imageStreamThread");
createCaptureSession(CameraDevice.TEMPLATE_RECORD, imageStreamReader.getSurface());
imageStreamReader.setOnImageAvailableListener(getImageStreamListener(processor), imageStreamHandler);
}
public void stopPreviewWithImageStream(ImageReader imageStreamReader)
throws CameraAccessException {
CameraUtils.stopThread(imageStreamHandler);
imageStreamReader.setOnImageAvailableListener(null, null);
startPreview();
}
private ImageReader.OnImageAvailableListener getImageStreamListener(
StreamProcessor<Image> processor) {
return reader -> {
Image img = reader.acquireLatestImage();
if (img == null) {
return;
}
if (openImage.getAndSet(true)) {
img.close();
return;
}
imageStreamHandler.post(
(Runnable)
() -> {
try {
Uninterruptibles.getUninterruptibly(processor.apply(img));
} catch (ExecutionException e) {
Log.e(TAG, "Exception from image processing", e);
} finally {
openImage.set(false);
img.close();
}
});
};
}There was a problem hiding this comment.
Thanks Mehmet, I hope some of those improvements can find its way to this plugin later. As of now, and regarding this PR, changing TEMPLATE_STILL_CAPTURE to TEMPLATE_RECORD singlehandedly fixes streaming on several devices. There's not much documentation around this in particular, but seems needed
…ble for video recording instead of still image capture.
PerLycke
force-pushed
the
streaming_fix
branch
from
b1cd904 to
a5a5552
Compare
|
@mklim I've added a basic test for streaming on Android. If I got something wrong, or if more sophisticated tests are needed, I will need to hand this over to someone in the Flutter team. |
mklim
left a comment
mklim
left a comment
There was a problem hiding this comment.
I've added a basic test for streaming on Android. If I got something wrong, or if more sophisticated tests are needed, I will need to hand this over to someone in the Flutter team.
This doesn't technically test the specific change in this PR, but I'm going to LGTM it because it tests the streaming feature overall and is an improvement over what we currently have. Thanks for the improvement and for adding tests!
flutter#2239) Existing behaviour creates a request which prioritizes image quality over frame rate and results in a choppy and / or freezing streaming session on many devices. Creating a request suitable for video recording means that a stable frame rate is used, and post-processing is set for recording quality. It noticeably improves streaming images on a variety of devices.
flutter#2239) Existing behaviour creates a request which prioritizes image quality over frame rate and results in a choppy and / or freezing streaming session on many devices. Creating a request suitable for video recording means that a stable frame rate is used, and post-processing is set for recording quality. It noticeably improves streaming images on a variety of devices.
|
Hi, did this make into latest stable version? We're still experiencing performance issues with CameraPreview freezing while streaming images with High resolution, and I thought this might fix this. |
|
@tigrenok00 It's included yes, but it won't enable streaming images on high resolution on all devices. It fixes so that you can use recommended resolution on all devices (afaik) without notable issues. Before this patch camera plugin was unusable on a variety of devices, even on lower resolutions. |
flutter#2239) Existing behaviour creates a request which prioritizes image quality over frame rate and results in a choppy and / or freezing streaming session on many devices. Creating a request suitable for video recording means that a stable frame rate is used, and post-processing is set for recording quality. It noticeably improves streaming images on a variety of devices.
5 participants
…ble for video recording instead of still image capture.
Description
Existing behaviour creates a request which prioritizes image quality over frame rate and results in a choppy and / or freezing streaming session on many devices. Creating a request suitable for video recording means that a stable frame rate is used, and post-processing is set for recording quality. It noticeably improves streaming images on a variety of devices.
Related Issues
Fixes flutter/flutter#43566
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?