Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Crash on iOS devices with half image cropping #1582

Closed
wants to merge 3 commits into from
Closed
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
191 changes: 164 additions & 27 deletions packages/smooth_app/lib/pages/scan/camera_image_cropper.dart
Original file line number Diff line number Diff line change
@@ -1,68 +1,156 @@
import 'dart:io';
import 'dart:typed_data';

import 'package:camera/camera.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:google_ml_barcode_scanner/google_ml_barcode_scanner.dart';
import 'package:smooth_app/pages/scan/abstract_camera_image_getter.dart';
import 'package:typed_data/typed_buffers.dart';

/// Camera Image Cropper, in order to limit the barcode scan computations.
///
/// Note: on iOS, we can only crop the size, as the coordinates are hardcoded.
/// [width01] and [height01] are thus ignored on this platform.
Copy link
Contributor

Choose a reason for hiding this comment

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

You mean "[left01], [top01]"?

///
/// Use CameraController with imageFormatGroup: ImageFormatGroup.yuv420
/// [left01], [top01], [width01] and [height01] are values between 0 and 1
/// that delimit the cropping area.
/// For instance:
/// * left01: 0, top01: 0, width01: 1, height01: .2 delimit the top 20% banner
/// * left01: .5, top01: .5, width01: .5, height01: ..5 the bottom right rect
class CameraImageCropper extends AbstractCameraImageGetter {
CameraImageCropper(
abstract class CameraImageCropper extends AbstractCameraImageGetter {
factory CameraImageCropper(
final CameraImage cameraImage,
final CameraDescription cameraDescription, {
required double left01,
required double top01,
required double width01,
required double height01,
}) {
if (Platform.isIOS) {
return _CameraImageCropperImplIOS(
cameraImage,
cameraDescription,
width01: width01,
height01: height01,
);
} else {
return _CameraImageCropperImplDefault(
cameraImage,
cameraDescription,
left01: left01,
top01: top01,
width01: width01,
height01: height01,
);
}
}

CameraImageCropper._(
final CameraImage cameraImage,
final CameraDescription cameraDescription, {
required this.left01,
required this.top01,
required this.width01,
required this.height01,
}) : super(cameraImage, cameraDescription) {
}) : assert(width01 > 0 && width01 <= 1),
assert(height01 > 0 && height01 <= 1),
super(
cameraImage,
cameraDescription,
) {
_computeCropParameters();
}

final double left01;
final double top01;
final double width01;
final double height01;
late int _left;
late int _top;

late int _width;
late int _height;

void _computeCropParameters() {
assert(width01 > 0 && width01 <= 1);
assert(height01 > 0 && height01 <= 1);
if (orientation == 0) {
_width = _computeWidth();
_height = _computeHeight();
} else if (orientation == 90) {
_width = _computeWidth();
_height = _computeHeight();
} else {
throw Exception('Orientation $orientation not dealt with for the moment');
}
}

int _computeWidth() {
if (orientation == 0) {
return _getEven(fullWidth * width01);
} else if (orientation == 90) {
return _getEven(fullWidth * height01);
}

throw Exception('Orientation $orientation not dealt with for the moment');
}

int _computeHeight() {
if (orientation == 0) {
return _getEven(fullHeight * height01);
} else if (orientation == 90) {
return _getEven(fullHeight * width01);
}

throw Exception('Orientation $orientation not dealt with for the moment');
}

int _getEven(final double value) => 2 * (value ~/ 2);

int get fullWidth => cameraImage.width;

int get fullHeight => cameraImage.height;

int get orientation => cameraDescription.sensorOrientation;

@override
Size getSize() => Size(
_width.toDouble(),
_height.toDouble(),
);
}

class _CameraImageCropperImplDefault extends CameraImageCropper {
_CameraImageCropperImplDefault(
final CameraImage cameraImage,
final CameraDescription cameraDescription, {
required this.left01,
required this.top01,
required double width01,
required double height01,
}) : super._(
cameraImage,
cameraDescription,
width01: width01,
height01: height01,
);

final double left01;
final double top01;
late int _left;
late int _top;

@override
void _computeCropParameters() {
super._computeCropParameters();
assert(left01 >= 0 && left01 < 1);
assert(top01 >= 0 && top01 < 1);
assert(left01 + width01 <= 1);
assert(top01 + height01 <= 1);

final int fullWidth = cameraImage.width;
final int fullHeight = cameraImage.height;
final int orientation = cameraDescription.sensorOrientation;

int _getEven(final double value) => 2 * (value ~/ 2);

if (orientation == 0) {
_width = _getEven(fullWidth * width01);
_height = _getEven(fullHeight * height01);
_left = _getEven(fullWidth * left01);
_top = _getEven(fullHeight * top01);
return;
}
if (orientation == 90) {
_width = _getEven(fullWidth * height01);
_height = _getEven(fullHeight * width01);
} else if (orientation == 90) {
_left = _getEven(fullWidth * top01);
_top = _getEven(fullHeight * left01);
return;
} else {
throw Exception('Orientation $orientation not dealt with for the moment');
}
throw Exception('Orientation $orientation not dealt with for the moment');
}

// cf. https://en.wikipedia.org/wiki/YUV#Y′UV420p_(and_Y′V12_or_YV12)_to_RGB888_conversion
Expand All @@ -73,7 +161,10 @@ class CameraImageCropper extends AbstractCameraImageGetter {
};

@override
Size getSize() => Size(_width.toDouble(), _height.toDouble());
Size getSize() => Size(
_width.toDouble(),
_height.toDouble(),
);

@override
Uint8List getBytes() {
Expand Down Expand Up @@ -125,3 +216,49 @@ class CameraImageCropper extends AbstractCameraImageGetter {
return planeData;
}
}

/// On iOS, coordinates are hardcoded in the native code (0, 0)
/// [https://github.com/danjodanjo/google_ml_barcode_scanner/blob/master/ios/Classes/MLKVisionImage%2BFlutterPlugin.m#L136]
///
/// The only crop we can do is to shrink the width and/or the height
class _CameraImageCropperImplIOS extends CameraImageCropper {
_CameraImageCropperImplIOS(
final CameraImage cameraImage,
final CameraDescription cameraDescription, {
required double width01,
required double height01,
}) : assert(width01 > 0 && width01 <= 1),
assert(height01 > 0 && height01 <= 1),
super._(
cameraImage,
cameraDescription,
width01: width01,
height01: height01,
);

// Same implementation as [CameraImageFullGetter]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit surprised: what's the added value if you copy the whole set of planes?
We would be better off with CameraImageFullGetter, wouldn't we?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Basically it's a CameraImageFullGetter with a custom size.
Since this class still an "ImageCropper", using a mixin may be a solution

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically it's a CameraImageFullGetter with a custom size.

That's what I read, but I have serious doubts about copying a full size while saying it's half size. It's not worth the complication anyway as there's no performance gain.

Since this class still an "ImageCropper", using a mixin may be a solution

I'm not familiar with mixin, but from what I read that's too complex for what we want. Assuming your idea of "full copy but half getSize" is correct, you could just extend CameraImageFullGetter and override getSize.
In the end we don't need a CameraImageCropper anyway, we need an AbstractCameraImageGetter.

@override
Uint8List getBytes() {
final WriteBuffer allBytes = WriteBuffer();
for (final Plane plane in cameraImage.planes) {
allBytes.putUint8List(plane.bytes);
}
return allBytes.done().buffer.asUint8List();
}

// Same implementation as [CameraImageFullGetter]
@override
List<InputImagePlaneMetadata> getPlaneMetaData() {
final List<InputImagePlaneMetadata> planeData = <InputImagePlaneMetadata>[];
for (final Plane plane in cameraImage.planes) {
planeData.add(
InputImagePlaneMetadata(
bytesPerRow: plane.bytesPerRow,
height: plane.height,
width: plane.width,
),
);
}
return planeData;
}
}