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

[Laurent Pinchart]

Hi Hiro,

On Wed, Jun 02, 2021 at 02:01:24PM +0900, Hirokazu Honda wrote:
> 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:
> > > 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/
> >
> > > > 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.

Generally speaking, we should only report available result keys that the
camera can provide, yes. In this particular case however, I think we
should make the frame duration mandatory for cameras to report, so we
can hardcode ANDROID_SENSOR_FRAME_DURATION.

> Could you run android.hardware.camera2.cts.CaptureResultTest while some
> entry is dropped and the entry is in available keys?
> 
> > > ---
> > > >  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,

-- 
Regards,

Laurent Pinchart