<div dir="ltr"><div dir="ltr">Hi Paul,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 2, 2021 at 3:32 PM <<a href="mailto:paul.elder@ideasonboard.com">paul.elder@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Hiro,<br>
<br>
On Wed, Jun 02, 2021 at 02:01:24PM +0900, Hirokazu Honda wrote:<br>
> Hi Paul, thank you for the patch.<br>
> <br>
> On Wed, Jun 2, 2021 at 12:32 PM <<a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a>> wrote:<br>
> <br>
>     On Wed, Jun 02, 2021 at 12:30:21PM +0900, <a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a><br>
>     wrote:<br>
>     > Hi Umang,<br>
>     ><br>
>     > On Tue, Jun 01, 2021 at 04:54:55PM +0530, Umang Jain wrote:<br>
>     > > Report ANDROID_SENSOR_FRAME_DURATION as an available key for CTS to<br>
>     ><br>
>     > s/available key/available result key/<br>
>     ><br>
>     > It's also a valid request key, and this patch doesn't add that, so I<br>
>     > think it should be specified (also in the subject).<br>
> <br>
>     By "specified" I mean that "result" should be specified.<br>
> <br>
>     As for the subject, s/key/result key/<br>
> <br>
> <br>
>     Paul<br>
> <br>
>     ><br>
>     > > read out the value of frame duration we set in<br>
>     CameraDevice::getResultMetadata().<br>
>     > > Failing to do so might fail the CTS test:<br>
>     > >  - android.hardware.camera2.cts.CaptureRequestTest#<br>
>     testNoiseReductionModeControl<br>
>     > ><br>
>     > > Fixes: 3beb1accac1d ("android: camera_device: Fix sensor frame<br>
>     duration")<br>
>     > > Signed-off-by: Umang Jain <<a href="mailto:umang.jain@ideasonboard.com" target="_blank">umang.jain@ideasonboard.com</a>><br>
> <br>
> <br>
> First, this code adds the missing request result key. So<br>
> Reviewed-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org" target="_blank">hiroh@chromium.org</a>><br>
> <br>
> However, although this is not necessarily related to this change, I wonder if<br>
> those available keys should be there if and only if an entry with the key is<br>
> actually added.<br>
<br>
I think you're right. We might need to rethink where the keys and values<br>
come from :/<br>
<br>
I guess if we declare that we support some key then the application<br>
can expect that it's always present.<br>
<br></blockquote><div><br></div><div>I wonder if we should dynamically construct the available keys upon CameraMetadata::get() from keys that have been added.</div><div>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.</div><div><br></div><div>-Hiro</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Could you run android.hardware.camera2.cts.CaptureResultTest while some entry<br>
> is dropped and the entry is in available keys?<br>
<br>
android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl<br>
fails complaining that the key that it expects to be present is not. The<br>
other tests don't seem to care.<br>
<br>
<br>
Paul<br>
<br>
> <br>
>     > > ---<br>
>     > >  src/android/camera_device.cpp | 1 +<br>
>     > >  1 file changed, 1 insertion(+)<br>
>     > ><br>
>     > > diff --git a/src/android/camera_device.cpp b/src/android/<br>
>     camera_device.cpp<br>
>     > > index fddc07ff..fe332ec3 100644<br>
>     > > --- a/src/android/camera_device.cpp<br>
>     > > +++ b/src/android/camera_device.cpp<br>
>     > > @@ -1422,6 +1422,7 @@ const camera_metadata_t<br>
>     *CameraDevice::getStaticMetadata()<br>
>     > >             ANDROID_REQUEST_PIPELINE_DEPTH,<br>
>     > >             ANDROID_SCALER_CROP_REGION,<br>
>     > >             ANDROID_SENSOR_EXPOSURE_TIME,<br>
>     > > +           ANDROID_SENSOR_FRAME_DURATION,<br>
>     > >             ANDROID_SENSOR_ROLLING_SHUTTER_SKEW,<br>
>     > >             ANDROID_SENSOR_TEST_PATTERN_MODE,<br>
>     > >             ANDROID_SENSOR_TIMESTAMP,<br>
>     > > --<br>
>     > > 2.31.1<br>
>     > ><br>
> <br>
</blockquote></div></div>