[libcamera-devel] [PATCH 1/2] android: Make FRAME_DURATION key available in static metadata

[Hirokazu Honda]

Hi Paul,

On Wed, Jun 2, 2021 at 3:32 PM <paul.elder at ideasonboard.com> wrote:

> Hi Hiro,
>
> On Wed, Jun 02, 2021 at 02:01:24PM +0900, Hirokazu Honda wrote:
> > Hi Paul, thank you for the patch.
> >
> > On Wed, Jun 2, 2021 at 12:32 PM <paul.elder at ideasonboard.com> wrote:
> >
> >     On Wed, Jun 02, 2021 at 12:30:21PM +0900,
> paul.elder at ideasonboard.com
> >     wrote:
> >     > Hi Umang,
> >     >
> >     > On Tue, Jun 01, 2021 at 04:54:55PM +0530, Umang Jain wrote:
> >     > > Report ANDROID_SENSOR_FRAME_DURATION as an available key for CTS
> to
> >     >
> >     > s/available key/available result key/
> >     >
> >     > It's also a valid request key, and this patch doesn't add that, so
> I
> >     > think it should be specified (also in the subject).
> >
> >     By "specified" I mean that "result" should be specified.
> >
> >     As for the subject, s/key/result key/
> >
> >
> >     Paul
> >
> >     >
> >     > > read out the value of frame duration we set in
> >     CameraDevice::getResultMetadata().
> >     > > Failing to do so might fail the CTS test:
> >     > >  - android.hardware.camera2.cts.CaptureRequestTest#
> >     testNoiseReductionModeControl
> >     > >
> >     > > Fixes: 3beb1accac1d ("android: camera_device: Fix sensor frame
> >     duration")
> >     > > Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> >
> >
> > First, this code adds the missing request result key. So
> > Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> >
> > However, although this is not necessarily related to this change, I
> wonder if
> > those available keys should be there if and only if an entry with the
> key is
> > actually added.
>
> I think you're right. We might need to rethink where the keys and values
> come from :/
>
> I guess if we declare that we support some key then the application
> can expect that it's always present.
>
>
I wonder if we should dynamically construct the available keys upon
CameraMetadata::get() from keys that have been added.
We should look up the hard-code tables, if there is no smart way, to find
what each key is added to, ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS,
ANDROID_REQUEST_AVAILABLE_RESULT_KEYS or
ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS.

-Hiro

> > Could you run android.hardware.camera2.cts.CaptureResultTest while some
> entry
> > is dropped and the entry is in available keys?
>
>
> android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl
> fails complaining that the key that it expects to be present is not. The
> other tests don't seem to care.
>
>
> Paul
>
> >
> >     > > ---
> >     > >  src/android/camera_device.cpp | 1 +
> >     > >  1 file changed, 1 insertion(+)
> >     > >
> >     > > diff --git a/src/android/camera_device.cpp b/src/android/
> >     camera_device.cpp
> >     > > index fddc07ff..fe332ec3 100644
> >     > > --- a/src/android/camera_device.cpp
> >     > > +++ b/src/android/camera_device.cpp
> >     > > @@ -1422,6 +1422,7 @@ const camera_metadata_t
> >     *CameraDevice::getStaticMetadata()
> >     > >             ANDROID_REQUEST_PIPELINE_DEPTH,
> >     > >             ANDROID_SCALER_CROP_REGION,
> >     > >             ANDROID_SENSOR_EXPOSURE_TIME,
> >     > > +           ANDROID_SENSOR_FRAME_DURATION,
> >     > >             ANDROID_SENSOR_ROLLING_SHUTTER_SKEW,
> >     > >             ANDROID_SENSOR_TEST_PATTERN_MODE,
> >     > >             ANDROID_SENSOR_TIMESTAMP,
> >     > > --
> >     > > 2.31.1
> >     > >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210602/9b2def9c/attachment.htm>