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

Ability to customize CameraParameters/CaptureRequest #21

Closed
hagabaka opened this issue May 11, 2017 · 15 comments
Closed

Ability to customize CameraParameters/CaptureRequest #21

hagabaka opened this issue May 11, 2017 · 15 comments
Labels
Milestone

Comments

@hagabaka
Copy link
Contributor

hagabaka commented May 11, 2017

Hi,

I'd like to configure the camera's ISO (sensor sensitivity), aperture, and exposure time settings. It looks like these may be possible in the old API with Camera.Parameters and in camera2 with CaptureRequest, with varying hardware support. This library doesn't seem to provide a way to access or inject them. Is it possible to provide this ability?

@Diolor Diolor added the feature label May 12, 2017
@dmitry-zaitsev
Copy link
Member

We indeed currently do not provide the possibility to change those parameters, but thanks for the suggestion. That would be the first thing which we'll do after fixing all the bugs.

@dmitry-zaitsev dmitry-zaitsev added this to the 1.1.0 milestone May 12, 2017
@hagabaka
Copy link
Contributor Author

hagabaka commented May 24, 2017

I need these features for a project in which I'm already using this library, and I would like to try to implement them. Could you provide some hints?

It looks like for v1, ParameterConverter implements converting io.fotoapparat.parameter.Parameters to android.Camera.Prameters. So Parameters and FotoapparatBuilder need to to allow specifying sensor sensitivity, aperture, and exposure time, and ParametersConverter needs to handle them.

For v2, it looks like CaptureRequestFactory is a class that will have control over the CaptureRequest (through CaptureRequestBuilder) and has access to Parameters (through ParametersProvider). So besides the same changes to Parameters, ParametersProvider needs to be updated to provide access to the new settings, and they need to be handled in CaptureRequestFactory and CaptureRequestBuilder.

Is that correct? Also what's the purpose of the above multiple classes used in v2?

@dmitry-zaitsev
Copy link
Member

@hagabaka if you would help to implement a feature, that would be appreciated. To simplify things, since Camera2 is still work-in-progress you can just throw UnsupportedOperationException. We'll take care of that.

As for Camera1, you almost got it right.

  • InitialParametersProvider is taking user preferences (which you specified in FotoapparatBuilder) and creates fotoapparat.Parameters out of them.
  • ParametersConverter is converting fotoapparat.Parameters to camera.Parameters
  • CapabilitiesFactory (from v1) creates Capabilities object which basically tells what kind of values camera supports (which preview sizes, what kind of auto focus, does it has flash, etc.)

Note, that FotoapparatBuilder accepts SelectorFunction instead of taking parameters directly. That is needed for two reasons:

  1. It allows users to write fail-safe code (see firstAvailable())
  2. Selection is executed later on a background thread, so we save some performance

Next thing to take into account, we never expose native camera classes in Fotoapparat or FotoapparatBuilder, since user should not care with which camera implementation he is working. That means each value provided by camera.Parameters should be mapped to our own type. Same as it is done with FocusMode or Flash.

If you have any other questions, feel free to ask! :)

@hagabaka
Copy link
Contributor Author

I started trying to add support for sensor sensitivity, and have encountered a few issues:

The only way to test for this capability with v1 seems to be explained here, which is to look for a device-dependent key like "iso-values" in Camera.Parameters#flatten(). This would give a list of accepted values. However my phone (Pixel XL) does not list any of the known keys names, and the string from device doesn't seem to contain any other information on this setting.

On v2 this can be queried using CameraCharacteristics#get(SENSOR_INFO_SENSITIVITY_RANGE), and my phone does report the range in the v2 API. However it's a range rather than a list as in v1, so I assume it would be difficult to use it with SelectorFunction. Can a Predicate be used somehow?

I'm under the impression that the finished API would look something like:

Fotoapparat
  .with(context)
  .sensorSensitivity(firstAvailable(
     sensorSensitivityValue(1000),
     highestSensorSensitivity(),
     defaultSensorSensitivity()
  ))

Does that make sense?

@dmitry-zaitsev
Copy link
Member

API Looks good. If I understood correctly this feature is available only on Camera2?

I assume it would be difficult to use it with SelectorFunction. Can a Predicate be used somehow?

I see the problem. Our SelectorFunction is not generic enough. I suggest we change it a bit so that it accepts input type as a generic parameter:

interface SelectorFunction<Input, Output> {

    Output select(Input input);

}

Then our existing selectors can be represented like that:

SelectorFunction<List<Size>, Size> biggestSize()

And your range could be represented like that (assuming you'll create Range class):

SelectorFunction<Range<Integer>, Integer> highestSensorSensitivity()

@hagabaka
Copy link
Contributor Author

hagabaka commented May 24, 2017

The feature depends on the device, but the ability to reliably query the capability is only on camera2. On my phone, the camera apparently supports it but only reports it on camera2. Still, it's known to work for some devices on camera1 as listed in the Stackoverflow answer.

I think changing SelectorFunction as you suggested will work. I'll try it out.

hagabaka pushed a commit to hagabaka/Fotoapparat that referenced this issue May 25, 2017
It has a Input and Output type, so that it's possible to have
SelectorFunctions based on input other than Collection.

Following advice in RedApparat#21 RedApparat#21 (comment)
hagabaka pushed a commit to hagabaka/Fotoapparat that referenced this issue May 25, 2017
Add querying and selection functions
WIP for RedApparat#21
@hagabaka
Copy link
Contributor Author

So far I think the approach of making SelectorFunction more generic works.

To recap, on v1 the sensor sensitivity information, if available, is reported as a list of numbers, while on v2 it is a range. I can't have functions overrides returning different SelectorFunction types with the same name and parameters, so I created a BaseSensorSensitivityCapability type, which is extended differently in v1 and v2, and used as input for selector functions.

I still need to add code and allow the user to use the new SelectorFunctions, and to handle them with the camera APIs.

hagabaka pushed a commit to hagabaka/Fotoapparat that referenced this issue May 25, 2017
Add querying and selection functions
WIP for RedApparat#21
@dmitry-zaitsev
Copy link
Member

dmitry-zaitsev commented May 25, 2017

How about instead of creating BaseSensorSensitivityCapability create an interface Range:

interface Range<T> {

    boolean contains(T value);

    T maxValue();

    T minValue();

}

Then you can implement ContinuousRange which would be used by Camera2 and DiscreeteRange for Camera1. I believe that would be cleaner since we still will be able to use SelectorFunction.

This is roughly the same idea as you had, but with a more generic name.

@hagabaka
Copy link
Contributor Author

OK. Should I put Range, ContinuousRange and DiscreetRange in io.fotoapparat.util, or a new package io.fotoapparat.range?

@dmitry-zaitsev
Copy link
Member

I think io.fotoapparat.parameter.range would be right place. @Diolor any thoughts?

@Diolor
Copy link
Member

Diolor commented May 26, 2017

I would go with the least abstract package. io.fotoapparat.parameter.range is concrete enough and follows same architecture as parameter.selector package

hagabaka pushed a commit to hagabaka/Fotoapparat that referenced this issue May 26, 2017
hagabaka pushed a commit to hagabaka/Fotoapparat that referenced this issue May 26, 2017
Add API for setting sensor sensitivity (FotoapparatBuilder#sensorSensitivity)
Handle conversion to CaptureRequest and Camera.Parameter
WIP RedApparat#21
@hagabaka
Copy link
Contributor Author

hagabaka commented May 26, 2017

So I think the sensor sensitivity/ISO part of this feature is done. It's hard to tell that it is working, but code using the added API is compiling, and debugger shows that the value is being set in Camera.Parameters or CaptureRequest.

The earlier change to SelectorFunction has broken some tests, and I haven't added tests for the added code. How can I run the tests in Android Studio?

@dmitry-zaitsev
Copy link
Member

@hagabaka sorry for the delay.

To run tests in Android Studio you can right click the test folder and click Run. Or, you can right click on each individual test and do the same.

Could you perhaps create a Pull Request with your changes so that we can review them and integrate into the project?

@hagabaka
Copy link
Contributor Author

hagabaka commented Jun 7, 2017

OK, I've fixed the tests and created a pull request.

@Diolor
Copy link
Member

Diolor commented Jun 11, 2017

Let's close this and track progress on separate issues : #38 , #39 , #40 since #35 got opened

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants