Skip to content

Conversation

@bselwe
Copy link

@bselwe bselwe commented Jul 8, 2021

Description

  • Added web camera options used to specify audio and video constraints. These options are going to be used when accessing a user media device with getUserMedia.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@bselwe bselwe added the enhancement New feature or request label Jul 8, 2021
@bselwe bselwe requested a review from felangel July 8, 2021 15:53
@bselwe bselwe self-assigned this Jul 8, 2021
@bselwe bselwe requested a review from ditman July 9, 2021 09:38
Copy link

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM, My only specific concern is that the FacingMode constraint definition is a little bit too complex (compared to the VideoSize constraint, which also has an "ideal" value).

Also: why is the VideoConstraints toJson async? It doesn't seem to need to be!

One last thing about naming of classes in this file, I'd add the "Constraint" surname to those that don't have it already: FacingModeConstraint and VideoSizeConstraint, that way it's very clear that we're creating CameraOptions from BlahBlahConstraint objects.

@bselwe bselwe requested a review from ditman July 12, 2021 14:47
Copy link

@ditman ditman left a comment

Choose a reason for hiding this comment

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

This LGTM! Powerful enough to express whatever we need, and easily extensible. 🚀

@bselwe
Copy link
Author

bselwe commented Jul 19, 2021

Closing, moved to flutter/plugins#4164.

@bselwe bselwe closed this Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants