RedApparat / Fotoapparat

Making Camera for Android more friendly. 📸
Apache License 2.0
3.82k stars 405 forks source link

When not in continuousFocus mode, taking photo triggers focus #149

Closed Raenar4k closed 6 years ago

Raenar4k commented 6 years ago

What are you trying to achieve or the steps to reproduce?

When FA is initialized without continuous focus mode -> taking photo triggers focus. I've implemented manual focus on tap, so user can try to get focus manually

fotoapparat.focus().whenAvailable {
            Timber.tag(TAG).d("focus state is $it")
        }

Even if user has taps and camera gets focused, taking a photo still triggers camera to focus again, which may lose focus that user wanted.

How did you initialize FA?

        return Fotoapparat
                .with(context)
                .into(renderer)
                .previewScaleType(ScaleType.CENTER_CROP)
                .photoSize(AspectRatioSelectors.wideRatio(SizeSelectors.biggestSize()))
                .lensPosition(LensPositionSelectors.lensPosition(LensPosition.BACK))
                .focusMode(Selectors.firstAvailable(
                        FocusModeSelectors.autoFocus(),
                        FocusModeSelectors.fixed()
                ))
                .flash(FlashSelectors.off())
                .logger(Loggers.loggers(Logger { Timber.tag(CameraImpl.TAG).d(it) }))
                .cameraErrorCallback({ errorSubject.onNext(it) })
                .build()

What was the result you received?

When FA is not in continuousFocus mode, taking photo triggers focus, even if fotoapparat.focus() method was called and camera is focused.

What did you expect?

If camera was already focused manually, by focus() method, then taking photo would not trigger another round of focus- like in continuous focus mode.

Context:


That decision to do away with continuous focus itself is tied to other issues - 1) on some devices continuous focus stops working - camera stays unfocused -> which led me to adding manual focus 2) manual focus does not work when in continuousFocus mode -> so i've removed that mode altogether and now i'm using autoFocus and fixed mode

Should i file separate issues for them too?

jpribble commented 6 years ago

+1. It would be great if whether or not takePicture() triggered a refocus was configurable!

Diolor commented 6 years ago

I see. Okay we can delete that logic from there and if a developer wants a focus before taking a picture could use FA. autoFocus().takePicture(). The reason we had this was camera2, which has a cumbersome focus routine. By deleting this I am unsure about the stability of v2 (which is not actively developed either way). Few people here prefer use v2 but we will not develop it further for now. @dmitry-zaitsev further input?

Raenar4k commented 6 years ago

What about adding it as an option parameter to builder, like jpeg quality? I think getting rid of shouldFocus method and making user call autoFocus().takePicture()can be problem to existing users of the api.

Also it can be done as overload for .takePicture() that has something like boolean shouldFocus in it or as separate method (cant think of a short name for that one), allowing to take picture without focusing.

I can help with PR, waiting for feedback :)

Diolor commented 6 years ago

Ok, so plan for FA v2 is:

I didn't like to add an overload shouldFocus in takePicture as in my head adds more mixed responsibility in the actions than the above structure.

Any objections, ideas, input, something I miss?

Diolor commented 6 years ago

And @Raenar4k regarding why continuous focus fails, here you go: #159

sefaaycicek commented 6 years ago

Is there any improvement in beta5 version? We tried beta4 and some pictures are blurry.

Diolor commented 6 years ago

Not on this topic. You can better open a new issue with more details about your configuration, etc if you think there is something wrong..

PS: Stable 2.0.0 is also available.