-
Notifications
You must be signed in to change notification settings - Fork 72
Implement camera switching #51
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
762a559
c42e65a
22d7372
7e5e25e
dd764a8
8f5c69a
4ff1eee
4aca0cd
c734a89
dfbd6ea
8a8f02d
328f777
6b44afb
8bf93a3
b0eb637
af1e57d
d0830fa
636425b
351b145
39cb4f2
9e5c5ed
69cb061
c6e1a86
35d7e48
bacfdab
546b9d0
fcb0c88
fb83485
7ce6669
7b16b04
f6791a3
cfc959c
735b62b
b6025f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,20 @@ class ScannerScreen extends StatefulWidget { | |
|
|
||
| class _ScannerScreenState extends State<ScannerScreen> { | ||
| final _torchIconState = ValueNotifier(false); | ||
| var _canChangeCamera = false; | ||
|
|
||
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| _checkCamera(); | ||
| } | ||
|
|
||
| Future<void> _checkCamera() async { | ||
|
||
| final canChangeCamera = await CameraController.instance.canChangeCamera(); | ||
| setState(() { | ||
| _canChangeCamera = canChangeCamera; | ||
|
||
| }); | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
|
|
@@ -32,25 +46,22 @@ class _ScannerScreenState extends State<ScannerScreen> { | |
| ValueListenableBuilder<bool>( | ||
| valueListenable: _torchIconState, | ||
| builder: (context, state, _) => IconButton( | ||
| icon: state | ||
| ? const Icon(Icons.flash_on) | ||
| : const Icon(Icons.flash_off), | ||
| icon: state ? const Icon(Icons.flash_on) : const Icon(Icons.flash_off), | ||
| onPressed: () async { | ||
| await CameraController.instance.toggleTorch(); | ||
| _torchIconState.value = | ||
| CameraController.instance.state.torchState; | ||
| _torchIconState.value = CameraController.instance.state.torchState; | ||
| }, | ||
| ), | ||
| ), | ||
| if (_canChangeCamera) | ||
| IconButton( | ||
| icon: const Icon(Icons.cameraswitch), | ||
| onPressed: CameraController.instance.toggleCamera, | ||
| ), | ||
| ], | ||
| ), | ||
| body: BarcodeCamera( | ||
| types: const [ | ||
| BarcodeType.ean8, | ||
| BarcodeType.ean13, | ||
| BarcodeType.code128, | ||
| BarcodeType.qr | ||
| ], | ||
| types: const [BarcodeType.ean8, BarcodeType.ean13, BarcodeType.code128, BarcodeType.qr], | ||
| resolution: Resolution.hd720, | ||
| framerate: Framerate.fps30, | ||
| mode: DetectionMode.pauseVideo, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,30 +88,50 @@ class BarcodeReader: NSObject { | |
| var captureDevice: AVCaptureDevice! | ||
| var captureSession: AVCaptureSession | ||
| let dataOutput: AVCaptureVideoDataOutput | ||
|
|
||
| var metadataOutput: AVCaptureMetadataOutput | ||
| let codeCallback: ([String]) -> Void | ||
| let detectionMode: DetectionMode | ||
| let position: AVCaptureDevice.Position | ||
|
|
||
| var position: AVCaptureDevice.Position | ||
| let detectionMode: DetectionMode | ||
| let framerate: Framerate | ||
| let resolution: Resolution | ||
| let codes: [String] | ||
|
|
||
| var torchActiveOnStop = false | ||
| var previewSize: CMVideoDimensions! | ||
|
|
||
| init(textureRegistry: FlutterTextureRegistry, | ||
| arguments: StartArgs, | ||
| codeCallback: @escaping ([String]) -> Void) throws { | ||
| self.textureRegistry = textureRegistry | ||
| arguments: StartArgs, | ||
| codeCallback: @escaping ([String]) -> Void) throws { | ||
| self.textureRegistry = textureRegistry | ||
| self.codeCallback = codeCallback | ||
| self.captureSession = AVCaptureSession() | ||
|
|
||
| self.captureSession = AVCaptureSession() | ||
| self.dataOutput = AVCaptureVideoDataOutput() | ||
| self.metadataOutput = AVCaptureMetadataOutput() | ||
| self.detectionMode = arguments.detectionMode | ||
|
|
||
| self.detectionMode = arguments.detectionMode | ||
| self.position = arguments.position | ||
| self.framerate = arguments.framerate | ||
| self.resolution = arguments.resolution | ||
| self.codes = arguments.codes | ||
|
|
||
| super.init() | ||
|
|
||
| captureDevice = AVCaptureDevice.default(.builtInWideAngleCamera, for: .video, position: position) | ||
| do { | ||
| try setupCaptureDevice(arguments) | ||
| } catch { | ||
| throw error | ||
| } | ||
| } | ||
|
||
|
|
||
| private func setupCaptureDevice(_ arguments: StartArgs) throws { | ||
| captureDevice = AVCaptureDevice.default(.builtInWideAngleCamera, for: .video, position: position) | ||
|
|
||
| guard captureDevice != nil else { | ||
| throw ReaderError.noInputDevice | ||
| } | ||
| guard captureDevice != nil else { | ||
| throw ReaderError.noInputDevice | ||
| } | ||
|
|
||
| do { | ||
| let input = try AVCaptureDeviceInput(device: captureDevice) | ||
|
|
@@ -126,25 +146,25 @@ class BarcodeReader: NSObject { | |
| captureSession.addOutput(dataOutput) | ||
| captureSession.addOutput(metadataOutput) | ||
|
|
||
| dataOutput.videoSettings = [kCVPixelBufferPixelFormatTypeKey as String: kCVPixelFormatType_32BGRA] | ||
| dataOutput.connection(with: .video)?.videoOrientation = .portrait | ||
| dataOutput.alwaysDiscardsLateVideoFrames = true | ||
| dataOutput.setSampleBufferDelegate(self, queue: DispatchQueue.global(qos: .default)) | ||
| dataOutput.videoSettings = [kCVPixelBufferPixelFormatTypeKey as String: kCVPixelFormatType_32BGRA] | ||
| dataOutput.connection(with: .video)?.videoOrientation = .portrait | ||
| dataOutput.alwaysDiscardsLateVideoFrames = true | ||
| dataOutput.setSampleBufferDelegate(self, queue: DispatchQueue.global(qos: .default)) | ||
|
|
||
| metadataOutput.setMetadataObjectsDelegate(self, queue: DispatchQueue.global(qos: .default)) | ||
| metadataOutput.metadataObjectTypes = arguments.codes.compactMap { avMetadataObjectTypes[$0] } | ||
| metadataOutput.setMetadataObjectsDelegate(self, queue: DispatchQueue.global(qos: .default)) | ||
| metadataOutput.metadataObjectTypes = arguments.codes.compactMap { avMetadataObjectTypes[$0] } | ||
|
|
||
| guard let optimalFormat = captureDevice.formats.first(where: { | ||
| let dimensions = CMVideoFormatDescriptionGetDimensions($0.formatDescription) | ||
| let mediaSubType = CMFormatDescriptionGetMediaSubType($0.formatDescription).toString() | ||
| guard let optimalFormat = captureDevice.formats.first(where: { | ||
| let dimensions = CMVideoFormatDescriptionGetDimensions($0.formatDescription) | ||
| let mediaSubType = CMFormatDescriptionGetMediaSubType($0.formatDescription).toString() | ||
|
|
||
| return $0.videoSupportedFrameRateRanges.first!.maxFrameRate >= arguments.framerate.doubleValue | ||
| && dimensions.height >= arguments.resolution.height | ||
| && dimensions.width >= arguments.resolution.width | ||
| && mediaSubType == "420f" // maybe 420v is also ok? Who knows... | ||
| }) else { | ||
| throw ReaderError.cameraNotSuitable(arguments.resolution, arguments.framerate) | ||
| } | ||
| return $0.videoSupportedFrameRateRanges.first!.maxFrameRate >= arguments.framerate.doubleValue | ||
| && dimensions.height >= arguments.resolution.height | ||
| && dimensions.width >= arguments.resolution.width | ||
| && mediaSubType == "420f" // maybe 420v is also ok? Who knows... | ||
| }) else { | ||
| throw ReaderError.cameraNotSuitable(arguments.resolution, arguments.framerate) | ||
| } | ||
|
|
||
| do { | ||
| try captureDevice.lockForConfiguration() | ||
|
|
@@ -158,8 +178,8 @@ class BarcodeReader: NSObject { | |
| throw ReaderError.configurationLockError(error) | ||
| } | ||
|
|
||
| previewSize = CMVideoFormatDescriptionGetDimensions(captureDevice.activeFormat.formatDescription) | ||
| } | ||
| previewSize = CMVideoFormatDescriptionGetDimensions(captureDevice.activeFormat.formatDescription) | ||
| } | ||
|
|
||
| func start(fromPause: Bool) throws { | ||
| guard captureDevice != nil else { return } | ||
|
|
@@ -237,9 +257,47 @@ class BarcodeReader: NSObject { | |
| try start(fromPause: true) | ||
| } | ||
| } | ||
|
|
||
| func changeCamera(type: String) { | ||
| position = type == "front" ? .front : .back | ||
| reloadCamera() | ||
| } | ||
|
|
||
| func toggleCamera() { | ||
| position = position.toggled() | ||
| reloadCamera() | ||
| } | ||
|
|
||
| private func reloadCamera() { | ||
| captureSession.stopRunning() | ||
|
|
||
| captureSession.outputs.forEach { captureSession.removeOutput($0) } | ||
| captureSession.inputs.forEach { captureSession.removeInput($0) } | ||
|
|
||
| do { | ||
| let arguments = StartArgs(position: position, detectionMode: detectionMode, framerate: framerate, resolution: resolution, codes: codes) | ||
| try setupCaptureDevice(arguments) | ||
| } catch { | ||
| print(error) | ||
| } | ||
|
|
||
| captureSession.startRunning() | ||
| } | ||
|
|
||
| } | ||
|
|
||
| private extension AVCaptureDevice.Position { | ||
|
|
||
| func toggled() -> AVCaptureDevice.Position { | ||
| switch self { | ||
| case .back: | ||
| return .front | ||
| default: | ||
| return .back | ||
| } | ||
| } | ||
| } | ||
|
|
||
| extension BarcodeReader: FlutterTexture { | ||
| func copyPixelBuffer() -> Unmanaged<CVPixelBuffer>? { | ||
| pixelBuffer == nil ? nil : .passRetained(pixelBuffer!) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good practice sake should declare type
boolThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Xazin , actually the Dart style guide encourages the use of var/final for local variables instead of declaring types::
https://dart.dev/guides/language/effective-dart/design#dont-redundantly-type-annotate-initialized-local-variables
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cannot be final, and I cannot find the place in the effective dart guidelines where it says to prefer
varover declaring it the known typebool. And it depends highly on how narrow the scope the variable is used in is. In my opinion this is not a narrow scope.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Internally we always use var, but as this is not our package, and I can't find any best practise references I've changed it to bool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Xazin sorry that my comment wasn't clear. I mentioned both var and final. Yes, this can't be final since you need to mutate the value, but it should be var. The part of the dart style guide that mentions this best practice is here:
https://dart.dev/guides/language/effective-dart/design#do-type-annotate-fields-and-top-level-variables-if-the-type-isnt-obvious
In this case the initializer makes the type obvious; it's initialized to false so it's obviously a bool and typing it out doesn't add any clarity.
So I believe the dart guide here recommends that this be
varrather thanbool.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but they also recommend doing this mostly in smaller functions. When in doubt always declare type, they even mention this themselves.
Although the subject is a bit contradictory even from their own documentation, I do agree if we're talking about functions of 10 lines or less, it makes no sense to declare type, but this is a local variable with a scope of 50+ lines.
Whether it adds clarity to type the variable, I can also agree with you that it's not because it has super much value, because the naming of the variable now correctly reflects what type it is as well. But with a larger scope and 1 character difference, I don't see a good enough reason to omit the type in this scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Xazin I hope you don't mind a continued discussion on this. It seems like such a small thing; but since we're talking about enforcing a convention I'm more interested in making sure we get the convention right than about this particular line of code. As as a contributor to this package I want to make sure my own contributions meet the package's standards.
Ultimately, it's neither right nor wrong to add the type annotation in this location. I think it should be fine if we do either; though I personally I take a minimalistic approach and prefer to leave them off when the annotation doesn't add value.
From the style guide section I shared it specifically says this:
And then as you pointed out it also says it's ok to add them even if it's obvious:
And so the use of the annotations requires some judgement. In addition to considering the size of the function I think it is also useful to consider the complexity of the expression the produces the type. Complex expressions can make the type more difficult for the reader to infer. consider these examples:
complex expression
In this first example you would have to mentally evaluate the expression before you know what type
secondListis. This would probably be fine in a small function where the logical flow of the program is easy to evaluate and follow. But in a bigger scope a type annotation would be useful in this spot to save the mental energy. Furthermore, a type annotation is helpful to avoid an accidentalList<dynamic>inference which is easy to do when mutating lists and the tools wouldn't necessarily help you see the issue.This would be much better with generic types and type annotations:
simple expression
In this second example, taken from the code in question,
falseis a reserved primitive type that is completely obvious. Adding a type annotation to a simple and unambiguous expressions like this can be avoided.In the end I think our opinion here is actually very close. You don't find a good enough reason to omit the type and I don't find a good enough reason to add the type. And so ultimately I think we should support either style; and in this case, leave this line how it is. A rule to always add or always omit isn't workable.