-
Notifications
You must be signed in to change notification settings - Fork 1
Added feature to stream only the stream from front camera #2
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: master
Are you sure you want to change the base?
Conversation
|
|
||
| if (mode == 2 || mode == 3) { //stream rotation is enabled | ||
| SizeCalculator.updateMatrix(rotation, width, height, isPreview, isPortrait, MVPMatrix); | ||
| if(mode == 2 || mode == 3) { |
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.
Don't we want this logic here as well? Btw wrong formatting next to if
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.
Removed the if completely
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 guess we need those ifs, otherwise it may break logic for mode==0 and mode==1
Maybe you could leave previous logic as was and after updateMatrix call method like
if (!isPreview) {
SizeCalculator.applyStreamFlip(isFlipHorizontal, isFrilVertical) {
Matrix.setIdentityM(MVPMatrix, 0);
Matrix.scaleM(MVPMatrix, 0, flipStreamHorizontal ? -1 : 1, flipStreamVertical ? -1 : 1, 1f);
}
}
|
|
||
| if (mode == 2 || mode == 3) { //stream rotation is enabled | ||
| SizeCalculator.updateMatrix(rotation, width, height, isPreview, isPortrait, MVPMatrix); | ||
| if (mode == 2 || mode == 3 || flipStreamHorizontal || flipStreamVertical) { //stream rotation is enabled |
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.
Do we need those flips in the if check?
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 guess we need, because the whole flipping happens in updateMatrix method, which originally executes only when aspectRatioMode is 2 or 3. We're not changing those modes here, so additional condition is needed to execute this method.
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.
The other option would be to remove this if completely and move it to the updateMatrix method itself.
| public Camera1ApiManager(SurfaceTexture surfaceTexture, Context context, CameraSwitchCallback cameraSwitchCallback) { | ||
| this.surfaceTexture = surfaceTexture; | ||
| this.context = context; | ||
| this.cameraSwitchCallback = cameraSwitchCallback; |
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.
Do we need this in constructor? I guess it should be optional
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.
Removed
| private FaceDetectorCallback faceDetectorCallback; | ||
|
|
||
| public Camera1ApiManager(SurfaceView surfaceView, GetCameraData getCameraData) { | ||
| public Camera1ApiManager(SurfaceView surfaceView, GetCameraData getCameraData, CameraSwitchCallback cameraSwitchCallback) { |
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.
OpenGlViews works only for Camera2 api, so there is no need to add this callback here I guess?
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.
As discussed, it works for Camera1
| Matrix.scaleM(MVPMatrix, 0, scale.x, scale.y, 1f); | ||
| if (!isPreview && !isPortrait) rotation += 90; | ||
| Matrix.rotateM(MVPMatrix, 0, rotation, 0f, 0f, -1f); | ||
| if (!isPreview && isCameraFront) { |
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 think that user should decide whether he wants to flip front or back camera
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 think that isCameraFront param is not needed and user should decide whether to flip it.
So basically you don't need a callback for camera at all, or you could move it to Camera2ApiManager
so on theuser side it would be like
fun switchCamera {
try {
livestream.switchCamera()
} finally {
liveStream.glInterface.flipStreamVertical = livestream.isFrontCamera
}
}
or
livstream.cameraChangeListener() { isFrontCamera ->
liveStream.glInterface.flipStreamVertical = isFrontCamera
}
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.
Applied
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.
However, I'm not sure if putting the flip in finally block is a good idea. After all, it's executed after catch as well.
| private int encoderWidth, encoderHeight; | ||
| private boolean loadAA = false; | ||
| private int streamRotation; | ||
| private int flipStreamMode = 4; |
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.
what is it for?
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.
Can be removed.
| this.context = context; | ||
| } | ||
|
|
||
| public OffScreenGlThread(Context context, boolean isFlipStreamHorizontal, boolean isFlipStreamVertical) { |
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.
why is it in constructor?
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.
Well, I thought, that if for OpenGlView you can set isStreamFlipHorizontal and isStreamFlipVertical in xml attrubutes, the natural way for OffScreenGlThread, which is not in xml, would be to set them in constructor.
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.
But according to comment #2 (comment) I'll remove it.
|
|
||
| @Override | ||
| public void setIsFlipStreamVertical(boolean isStreamFlipVertical) { | ||
|
|
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.
don't we want this feature for this class?
|
|
||
| if (mode == 2 || mode == 3) { //stream rotation is enabled | ||
| SizeCalculator.updateMatrix(rotation, width, height, isPreview, isPortrait, MVPMatrix); | ||
| if(mode == 2 || mode == 3) { |
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 guess we need those ifs, otherwise it may break logic for mode==0 and mode==1
Maybe you could leave previous logic as was and after updateMatrix call method like
if (!isPreview) {
SizeCalculator.applyStreamFlip(isFlipHorizontal, isFrilVertical) {
Matrix.setIdentityM(MVPMatrix, 0);
Matrix.scaleM(MVPMatrix, 0, flipStreamHorizontal ? -1 : 1, flipStreamVertical ? -1 : 1, 1f);
}
}
f8d274d to
c8bd386
Compare
No description provided.