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

feat: handle more facing directions #1328

Merged

Conversation

navaronbracke
Copy link
Collaborator

This PR adds the remaining facing modes to the CameraFacing enum.
It also passes the actual CameraFacing value from the camera to the MobileScannerState / surface producer delegate, instead of the requested facing direction. That should help prevent a wrong sign for the orientation correction.

Part of #867
Work towards fixing #1319


// On MacOS, even though the facing mode is supported, it is not reported.
// Use the label for FaceTime cameras to detect the user facing webcam.
if (videoTrack.label.contains('FaceTime')) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an additional workaround for the mirroring on MacOS. For example Google Meet does this for you.
This is also an issue for the native (non-web) MacOS preview. But I might fix that later.

extension NullableMediaStreamTrackCapabilities on MediaStreamTrack {
/// The `getCapabilities` function is not supported on Firefox.
@JS('getCapabilities')
external JSFunction? get getCapabilitiesNullable;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now unused, replaced by the nullable getter below

@@ -434,10 +434,18 @@ public class MobileScannerPlugin: NSObject, FlutterPlugin, FlutterStreamHandler,
let answer: [String : Any?]

if let device = self.device {
let cameraDirection: Int? = switch(device.position) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fun fact: MacOS reports .unspecified for the front facing FaceTime webcam. Probably because there is only 1 camera?

@@ -1,7 +1,12 @@
## NEXT

**BREAKING CHANGES:**

* The initial state of the `MobileScannerState` camera facing direction is changed to `CameraFacing.unknown`.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was done to keep the actual camera state in sync with the value

@@ -126,31 +131,23 @@ class MobileScannerWeb extends MobileScannerPlatform {
HTMLVideoElement videoElement,
MediaStream videoStream,
) {
final List<MediaStreamTrack> tracks = videoStream.getVideoTracks().toDart;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was actually duplicated, so I cleaned it up.

_textureId = textureId;

if (defaultTargetPlatform == TargetPlatform.android) {
_surfaceProducerDelegate =
AndroidSurfaceProducerDelegate.fromConfiguration(
startResult,
startOptions.cameraDirection,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To stay consistent with the camera state, we should take the camera direction from the start result

/// Whether the video feed is paused
bool? get paused =>
throw UnimplementedError('paused has not been implemented.');
/// Whether the video feed is paused.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Formatting, missing period & adding the videoStream getter


if (facingModes == null || facingModes.toDart.isEmpty) {
if (settings.facingModeNullable == null) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can just check the nullable facingMode directly in the settings. The capabilities check here is redundant.

@@ -8,6 +9,31 @@ final class MediaTrackConstraintsDelegate {
/// Constructs a [MediaTrackConstraintsDelegate] instance.
const MediaTrackConstraintsDelegate();

/// Get the camera direction from the given [videoStream].
CameraFacing getCameraDirection(MediaStream? videoStream) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved this conversion code into the delegate, so that we can reuse it and that it stays in one place.

Copy link
Owner

@juliansteenbakker juliansteenbakker left a comment

Choose a reason for hiding this comment

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

Thanks for your work! Looks good to me.

@juliansteenbakker juliansteenbakker merged commit b862ab6 into juliansteenbakker:develop Feb 17, 2025
5 checks passed
@navaronbracke navaronbracke deleted the more_facing_directions branch February 17, 2025 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants