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

[paul.elder at ideasonboard.com]

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.

> 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
>     > >
>