[libcamera-devel] [PATCH 1/2] android: Make FRAME_DURATION key available in static metadata
Hirokazu Honda
hiroh at chromium.org
Fri Jun 4 06:56:32 CEST 2021
Hi Paul,
On Wed, Jun 2, 2021 at 6:52 PM <paul.elder at ideasonboard.com> wrote:
> Hi Hiro,
>
> On Wed, Jun 02, 2021 at 04:57:58PM +0900, Hirokazu Honda wrote:
> > 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.
>
> The static metadata is constructed and returned way before any result
> metadata is ever returned though.
>
> I'm currently redesigning how we populate the static metadata based on
> Camera capabilities (eg. what libcamera controls the camera supports).
> It'll probably also connect into what we return in the result metadata,
> and what we accept in the request metadata. These both will of course
> affect the available keys that we report in the static metadata.
> I'll keep this in mind in the redesign.
>
>
Ack. I look forward to your redesign.
Thanks,
-Hiro
>
> Paul
>
> > 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/20210604/6d68806f/attachment-0001.htm>
More information about the libcamera-devel
mailing list